Skip to contentSkip to conversation

nitrogen6x: Fix the PAD settings for the ECSPI chipselect

PATCH New Fabio Estevam 4 participants 8 messages Apr 11, 2014 – Apr 28, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

From: Fabio Estevam <[email protected]>

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.

---
Eric,

This is untested.

 board/boundary/nitrogen6x/nitrogen6x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
board/boundary/nitrogen6x/nitrogen6x.c +1 -1
index d9c05b0..f2492e4 100644
@@ -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),
Signature · 1 line
1.8.3.2
#1 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

Thanks Fabio,

Quoted 5 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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/nitrogen6x.c;h=d9c05b07bfae83466513735cbec95c7f949de7a5;hb=HEAD#l47
Quoted 15 lines

---
Eric,

This is untested.

  board/boundary/nitrogen6x/nitrogen6x.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
board/boundary/nitrogen6x/nitrogen6x.c +0 -0
index d9c05b0..f2492e4 100644

board/boundary/nitrogen6x/nitrogen6x.c int board_mmc_init(bd_t *bis) · line 337

Quoted 8 lines
board/boundary/nitrogen6x/nitrogen6x.c +1 -1
@@ -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

#2 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

Hi Eric,

Quoted 2 lines · Eric Nelson

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

#3 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

In reply to Fabio Estevam

Hi Fabio,

Quoted 20 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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.

---
Eric,

This is untested.

  board/boundary/nitrogen6x/nitrogen6x.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
board/boundary/nitrogen6x/nitrogen6x.c +0 -0
index d9c05b0..f2492e4 100644

board/boundary/nitrogen6x/nitrogen6x.c int board_mmc_init(bd_t *bis) · line 337

Quoted 8 lines
board/boundary/nitrogen6x/nitrogen6x.c +1 -1
@@ -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),
#4 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

Quoted 26 lines · Eric Nelson

Hi Fabio,

Quoted 23 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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.

---
Eric,

This is untested.

  board/boundary/nitrogen6x/nitrogen6x.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
board/boundary/nitrogen6x/nitrogen6x.c +0 -1
index d9c05b0..f2492e4 100644
@@ -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),
Quoted 8 lines

board/boundary/nitrogen6x/nitrogen6x.c int board_mmc_init(bd_t *bis) · line 337

Quoted 5 lines
board/boundary/nitrogen6x/nitrogen6x.c +1 -0
+    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),

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

#5 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

Quoted 4 lines · Troy Kisky

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),
};
#6 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

Quoted 8 lines · Fabio Estevam
Quoted 4 lines · Troy Kisky

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.

Quoted 7 lines

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

#7 lore raw

PATCH 1/1 nitrogen6x: Fix the PAD settings for the ECSPI chipselect

In reply to Fabio Estevam
Quoted 7 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

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.

---

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

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================