[U-Boot] [PATCH] nitrogen6x: Fix the PAD settings for the ECSPI chipselect

From: Fabio Estevam fabio.estevam@freescale.com
ECSPI chipselect (MX6_PAD_EIM_D19__GPIO3_IO19) is used with GPIO functionality, so it does not make sense to set its pad as SPI pin.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com --- Eric,
This is untested.
board/boundary/nitrogen6x/nitrogen6x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index d9c05b0..f2492e4 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -331,7 +331,7 @@ int board_mmc_init(bd_t *bis) #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */ - MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(SPI_PAD_CTRL), + MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(NO_PAD_CTRL), MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),

Thanks Fabio,
On 04/11/2014 01:43 PM, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
ECSPI chipselect (MX6_PAD_EIM_D19__GPIO3_IO19) is used with GPIO functionality, so it does not make sense to set its pad as SPI pin.
You're right, although most of the SPI pad settings won't have any effect as soon as the pad is configured as a GPIO output: http://git.denx.de/?p=u-boot.git;a=blob;f=board/boundary/nitrogen6x/nitrogen...
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Eric,
This is untested.
board/boundary/nitrogen6x/nitrogen6x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index d9c05b0..f2492e4 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -331,7 +331,7 @@ int board_mmc_init(bd_t *bis) #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(SPI_PAD_CTRL),
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(NO_PAD_CTRL), MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
I'll test this out ASAP, but I have to ask: what made you look at this pin's settings.
It seems that the only bit which could affect things is dropping the drive strength setting, and 40 ohms is the power-on default.
Regards,
Eric

Hi Eric,
On Sat, Apr 12, 2014 at 12:04 PM, Eric Nelson eric.nelson@boundarydevices.com wrote:
I'll test this out ASAP, but I have to ask: what made you look at this pin's settings.
I was debugging a SPI issue on mx6sl evk, so that's why I looked at it for comparison.
Regards,
Fabio Estevam

Hi Fabio,
On 04/11/2014 01:43 PM, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
ECSPI chipselect (MX6_PAD_EIM_D19__GPIO3_IO19) is used with GPIO functionality, so it does not make sense to set its pad as SPI pin.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Eric,
This is untested.
board/boundary/nitrogen6x/nitrogen6x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index d9c05b0..f2492e4 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -331,7 +331,7 @@ int board_mmc_init(bd_t *bis) #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(SPI_PAD_CTRL),
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(NO_PAD_CTRL), MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
Tested-by: Eric Nelson eric.nelson@boundarydevices.com Acked-by: Eric Nelson eric.nelson@boundarydevices.com

On 4/12/2014 3:54 PM, Eric Nelson wrote:
Hi Fabio,
On 04/11/2014 01:43 PM, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
ECSPI chipselect (MX6_PAD_EIM_D19__GPIO3_IO19) is used with GPIO functionality, so it does not make sense to set its pad as SPI pin.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Eric,
This is untested.
board/boundary/nitrogen6x/nitrogen6x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index d9c05b0..f2492e4 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -331,7 +331,7 @@ int board_mmc_init(bd_t *bis) #ifdef CONFIG_MXC_SPI iomux_v3_cfg_t const ecspi1_pads[] = { /* SS1 */
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(SPI_PAD_CTRL),
- MX6_PAD_EIM_D19__GPIO3_IO19 | MUX_PAD_CTRL(NO_PAD_CTRL), MX6_PAD_EIM_D17__ECSPI1_MISO | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D18__ECSPI1_MOSI | MUX_PAD_CTRL(SPI_PAD_CTRL), MX6_PAD_EIM_D16__ECSPI1_SCLK | MUX_PAD_CTRL(SPI_PAD_CTRL),
Tested-by: Eric Nelson eric.nelson@boundarydevices.com Acked-by: Eric Nelson eric.nelson@boundarydevices.com
NAK. Please don't use NO_PAD_CTRL. What is wrong with SPI_PAD_CTRL. Your commit message doesn't say. It is an SPI pin (even if used as a GPIO,) so why doesn't it make sense.
I think your fixing a problem that doesn't exist.
Troy

On Sun, Apr 13, 2014 at 7:08 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
NAK. Please don't use NO_PAD_CTRL. What is wrong with SPI_PAD_CTRL. Your commit message doesn't say. It is an SPI pin (even if used as a GPIO,) so why doesn't it make sense.
SPI_PAD_CTRL should be used by the pads that have SPI functionality.
This is not the case for the MX6_PAD_EIM_D19__GPIO3_IO19, which is a GPIO, so why SPI_PAD_CTRL?
If we follow your argument then the enet_pads1 array is incorrect and we should change all of them to ENET_PAD_CTRL instead.
iomux_v3_cfg_t const enet_pads1[] = { MX6_PAD_ENET_MDIO__ENET_MDIO | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_ENET_MDC__ENET_MDC | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TXC__RGMII_TXC | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TD0__RGMII_TD0 | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TD1__RGMII_TD1 | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TD2__RGMII_TD2 | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TD3__RGMII_TD3 | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_RGMII_TX_CTL__RGMII_TX_CTL | MUX_PAD_CTRL(ENET_PAD_CTRL), MX6_PAD_ENET_REF_CLK__ENET_TX_CLK | MUX_PAD_CTRL(ENET_PAD_CTRL), /* pin 35 - 1 (PHY_AD2) on reset */ MX6_PAD_RGMII_RXC__GPIO6_IO30 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 32 - 1 - (MODE0) all */ MX6_PAD_RGMII_RD0__GPIO6_IO25 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 31 - 1 - (MODE1) all */ MX6_PAD_RGMII_RD1__GPIO6_IO27 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 28 - 1 - (MODE2) all */ MX6_PAD_RGMII_RD2__GPIO6_IO28 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 27 - 1 - (MODE3) all */ MX6_PAD_RGMII_RD3__GPIO6_IO29 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 33 - 1 - (CLK125_EN) 125Mhz clockout enabled */ MX6_PAD_RGMII_RX_CTL__GPIO6_IO24 | MUX_PAD_CTRL(NO_PAD_CTRL), /* pin 42 PHY nRST */ MX6_PAD_EIM_D23__GPIO3_IO23 | MUX_PAD_CTRL(NO_PAD_CTRL), MX6_PAD_ENET_RXD0__GPIO1_IO27 | MUX_PAD_CTRL(NO_PAD_CTRL), };

On 4/13/2014 4:01 PM, Fabio Estevam wrote:
On Sun, Apr 13, 2014 at 7:08 PM, Troy Kisky troy.kisky@boundarydevices.com wrote:
NAK. Please don't use NO_PAD_CTRL. What is wrong with SPI_PAD_CTRL. Your commit message doesn't say. It is an SPI pin (even if used as a GPIO,) so why doesn't it make sense.
SPI_PAD_CTRL should be used by the pads that have SPI functionality.
This is not the case for the MX6_PAD_EIM_D19__GPIO3_IO19, which is a GPIO, so why SPI_PAD_CTRL?
If we follow your argument then the enet_pads1 array is incorrect and we should change all of them to ENET_PAD_CTRL instead.
I would ack that patch. I do believe that all NO_PAD_CTRL should be replaced with whatever the register actually contains currently. A "nop" patch that just makes things explicit.
Would you have a problem with that patch?
For your particular example of enet, I see no reason that the pad settings should change when switching the mux from ENET to gpio.
Btw, I do appreciate your looking at this board file. Sorry, if I sounded rude.
Troy

On 11/04/2014 22:43, Fabio Estevam wrote:
From: Fabio Estevam fabio.estevam@freescale.com
ECSPI chipselect (MX6_PAD_EIM_D19__GPIO3_IO19) is used with GPIO functionality, so it does not make sense to set its pad as SPI pin.
Signed-off-by: Fabio Estevam fabio.estevam@freescale.com
Near Eric's Acked and Tested-by, I see Troy agree on this patch, too.
Applied to u-boot-imx, thanks !
Best regards, Stefano Babic
participants (4)
-
Eric Nelson
-
Fabio Estevam
-
Stefano Babic
-
Troy Kisky