ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

PATCH New Eric Nelson 6 participants 17 messages Dec 4, 2015 – Dec 9, 2015 MMC, SD and SDIO

Share & tools

Eric Nelson

author

2015-12-04 17:32 UTC #0 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.

Guard against their usage by hiding the bit mask macros on those processors.

These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.

Signed-off-by Eric Nelson <[email protected]>

---
 drivers/mmc/fsl_esdhc.c | 15 +++++++++++++--
 include/fsl_esdhc.h     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

drivers/mmc/fsl_esdhc.c +13 -2

index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
 	clk = (pre_div << 8) | (div << 4);
 
+#ifdef CONFIG_FSL_USDHC
+	esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
 	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
 	udelay(10000);
 
-	clk = SYSCTL_PEREN | SYSCTL_CKEN;
+#ifdef CONFIG_FSL_USDHC
+	esdhc_clrbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
+	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
+#endif
 
-	esdhc_setbits32(&regs->sysctl, clk);
 }
 
 #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK

@@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
 	esdhc_write32(&regs->scr, 0x00000040);
 #endif
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
+#endif
 
 	/* Set the initial clock speed */
 	mmc_set_clock(mmc, 400000);

@@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 	/* First reset the eSDHC controller */
 	esdhc_reset(regs);
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
 				| SYSCTL_IPGEN | SYSCTL_CKEN);
+#endif
 
 	writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
 	memset(&cfg->cfg, 0, sizeof(cfg->cfg));

include/fsl_esdhc.h +2 -0

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
 #define SYSCTL_CLOCK_MASK	0x0000fff0
+#if !defined(CONFIG_MX6)
 #define SYSCTL_CKEN		0x00000008
 #define SYSCTL_PEREN		0x00000004
 #define SYSCTL_HCKEN		0x00000002
 #define SYSCTL_IPGEN		0x00000001
+#endif
 #define SYSCTL_RSTA		0x01000000
 #define SYSCTL_RSTC		0x02000000
 #define SYSCTL_RSTD		0x04000000

Signature · 1 line

2.6.2

Eric Nelson

author

2015-12-04 17:38 UTC #1 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Quoted 10 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

Quoted 6 lines

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Signed-off-by Eric Nelson <[email protected]>

I forgot to add an in-reply-to header.

http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651

Eric Nelson

author

2015-12-04 17:43 UTC #2 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Quoted 16 lines · Eric Nelson

Quoted 15 lines · Eric Nelson
The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.

Quoted 7 lines

Quoted 1 line
Signed-off-by Eric Nelson <[email protected]>
I forgot to add an in-reply-to header.
http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651

Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648" issue (with or without a code change) for a couple of hours now.

Can you give this a spin?

It seems unlikely to address the issue unless what we're seeing is a side effect of a glitch while switching clocks.

Fabio Estevam

2015-12-04 18:24 UTC #3 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Quoted 4 lines · Eric Nelson

Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648" issue (with or without a code change) for a couple of hours now.
Can you give this a spin?

Sure, just gave it a try and the 'mmc erase' issue still happens.

I think your patch is correct anyway, so:

Tested-by Fabio Estevam <[email protected]>

Thanks

Hector Palacios

2015-12-04 18:25 UTC #4 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Hi Eric,

Quoted 25 lines · Eric Nelson

Quoted 16 lines · Eric Nelson
Quoted 15 lines · Eric Nelson
The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Quoted 7 lines
Quoted 1 line
Signed-off-by Eric Nelson <[email protected]>
I forgot to add an in-reply-to header.
http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651

Quoted 7 lines

Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648" issue (with or without a code change) for a couple of hours now.
Can you give this a spin?
It seems unlikely to address the issue unless what we're seeing is a side effect of a glitch while switching clocks.

As Fabio, I can reproduce this 100% of the times.
The patch does not fix it, though.

--
Hector Palacios

Eric Nelson

2015-12-04 18:33 UTC #5 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Hi Fabio and Hector,

Quoted 25 lines · Eric Nelson

Quoted 16 lines · Eric Nelson
Quoted 15 lines · Eric Nelson
The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Quoted 7 lines
Quoted 1 line
Signed-off-by Eric Nelson <[email protected]>
I forgot to add an in-reply-to header.
http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651

Quoted 9 lines

Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648" issue (with or without a code change) for a couple of hours now.
Can you give this a spin?
It seems unlikely to address the issue unless what we're seeing is a side effect of a glitch while switching clocks.

I switched back to a v2014.10 release and am able to reproduce the issue at will.

The sysctl patch had no effect, but adding an #ifndef around the ENGcm03648 block allows things to proceed.

+#ifndef CONFIG_FSL_USDHC

        /* Workaround for ESDHC errata ENGcm03648 */
        if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {

...

Since the need to poll the DAT0 line is only documented in the errata for the i.MX35, I wonder if this isn't the right thing to do.

The CC bit of irqstat does indicate that the command completed and without error (I'm seeing values of 1 in irqstat).

From what I can tell, the linux kernel doesn't do this test and doesn't appear to have any trouble.

What code base are you running against (u-boot-imx/master)?

What do you see if you do the same?

I had been testing against v2015.10 and the board I'm testing isn't upstream, so I can't easily bisect.

Please advise,

Eric

Hector Palacios

2015-12-04 18:51 UTC #6 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Hi Eric,

Quoted 52 lines · Eric Nelson

Hi Fabio and Hector,
Quoted 25 lines · Eric Nelson
Quoted 16 lines · Eric Nelson
Quoted 15 lines · Eric Nelson
The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Quoted 7 lines
Quoted 1 line
Signed-off-by Eric Nelson <[email protected]>
I forgot to add an in-reply-to header.
http://lists.denx.de/pipermail/u-boot/2015-December/thread.html#236651
Quoted 9 lines
Fabio, I haven't been able to reproduce the "mmc erase/ENGcm03648" issue (with or without a code change) for a couple of hours now.
Can you give this a spin?
It seems unlikely to address the issue unless what we're seeing is a side effect of a glitch while switching clocks.
I switched back to a v2014.10 release and am able to reproduce the issue at will.
The sysctl patch had no effect, but adding an #ifndef around the ENGcm03648 block allows things to proceed.
+#ifndef CONFIG_FSL_USDHC
        /* Workaround for ESDHC errata ENGcm03648 */
        if (!data && (cmd->resp_type & MMC_RSP_BUSY)) {
...
Since the need to poll the DAT0 line is only documented in the errata for the i.MX35, I wonder if this isn't the right thing to do.

Quoted 7 lines

The CC bit of irqstat does indicate that the command completed and without error (I'm seeing values of 1 in irqstat).
From what I can tell, the linux kernel doesn't do this test and doesn't appear to have any trouble.
What code base are you running against (u-boot-imx/master)?

I'm running v2015.04 on a non-upstream platform.

Quoted 1 line

What do you see if you do the same?

The command takes a while and it is erasing all blocks.
I still get a timeout error at the end and a zero number of sectors, though.

=> mmc erase 441000 10000

MMC erase: dev # 0, block # 4460544, count 65536 ... Timeout waiting card ready 0 blocks erased: ERROR

but all blocks were erased correctly.

Signature · 1 line

H?ctor Palacios

Eric Nelson

author

2015-12-04 19:24 UTC #7 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Thanks Hector,

Quoted 2 lines · Hector Palacios

Quoted 1 line · Eric Nelson
On 12/04/2015 10:43 AM, Eric Nelson wrote:

...

Quoted 7 lines

Quoted 4 lines
From what I can tell, the linux kernel doesn't do this test and doesn't appear to have any trouble.
What code base are you running against (u-boot-imx/master)?
I'm running v2015.04 on a non-upstream platform.

Thanks.

Quoted 10 lines

Quoted 1 line
What do you see if you do the same?
The command takes a while and it is erasing all blocks.
I still get a timeout error at the end and a zero number of sectors, though.
=> mmc erase 441000 10000
MMC erase: dev # 0, block # 4460544, count 65536 ... Timeout waiting card ready 0 blocks erased: ERROR

Erasing 64K blocks should take a while, but I'm not sure why you're seeing things die at the end.

U-Boot > time mmc erase 441000 10000
MMC erase: dev # 0, block # 4460544, count 65536 ... 65536 blocks erased: OK
time: 28.704 seconds

I did some more testing to see if the prsstat register is accurately reflecting the state of dat0 by setting the SION bit in the mux ctrl register for my DAT0 (SD4_DAT0 on an i.MX6Q).

I found that the pin is reading as low through the GPIO2_PSR register (bit 8):

U-Boot > mmc erase 4000 1000
MMC erase: dev # 0, block # 16384, count 4096 ... Timeout waiting for DAT0 to go high!
irqstat 0x00000001
GPIO2_PSR(020a0008) == 0xc80000d6
...

The pin shows up as high when idle, reflecting the internal pullup:

U-Boot > md.l 0x020a0008 1
020a0008: c80001d6 ....

I've tested with stronger pull-ups without success, and testing with a longer timeout indicates that this isn't a slew rate problem.

Michael Trimarchi

2015-12-04 17:40 UTC #8 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Quoted 26 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.
From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Signed-off-by Eric Nelson <[email protected]>
---
 drivers/mmc/fsl_esdhc.c | 15 +++++++++++++--
 include/fsl_esdhc.h     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)
drivers/mmc/fsl_esdhc.c +0 -0
index c5054d6..1ccc576 100644

drivers/mmc/fsl_esdhc.c static void set_sysctl(struct mmc *mmc, uint clock) · line 510

Quoted 9 lines

drivers/mmc/fsl_esdhc.c +4 -0

@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
        clk = (pre_div << 8) | (div << 4);
 
+#ifdef CONFIG_FSL_USDHC
+       esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
        esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif

I really prefer is_usdhc()

drivers/mmc/fsl_esdhc.c static void set_sysctl(struct mmc *mmc, uint clock) · line 520

Quoted 11 lines

drivers/mmc/fsl_esdhc.c +5 -1

        esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
        udelay(10000);
 
-       clk = SYSCTL_PEREN | SYSCTL_CKEN;
+#ifdef CONFIG_FSL_USDHC
+       esdhc_clrbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
+       esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
+#endif

same here

Michael

Quoted 45 lines

drivers/mmc/fsl_esdhc.c +4 -1

-       esdhc_setbits32(&regs->sysctl, clk);
 }
 
 #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK

@@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
        esdhc_write32(&regs->scr, 0x00000040);
 #endif
 
+#ifndef CONFIG_FSL_USDHC
        esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
+#endif
 
        /* Set the initial clock speed */
        mmc_set_clock(mmc, 400000);

@@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
        /* First reset the eSDHC controller */
        esdhc_reset(regs);
 
+#ifndef CONFIG_FSL_USDHC
        esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
                                | SYSCTL_IPGEN | SYSCTL_CKEN);
+#endif
 
        writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
        memset(&cfg->cfg, 0, sizeof(cfg->cfg));

include/fsl_esdhc.h +2 -1

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA           0x08000000
 #define SYSCTL_TIMEOUT_MASK    0x000f0000
 #define SYSCTL_CLOCK_MASK      0x0000fff0
+#if !defined(CONFIG_MX6)
 #define SYSCTL_CKEN            0x00000008
 #define SYSCTL_PEREN           0x00000004
 #define SYSCTL_HCKEN           0x00000002
 #define SYSCTL_IPGEN           0x00000001
+#endif
 #define SYSCTL_RSTA            0x01000000
 #define SYSCTL_RSTC            0x02000000
 #define SYSCTL_RSTD            0x04000000
--

2.6.2

Signature · 4 lines

| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

Eric Nelson

author

2015-12-04 17:49 UTC #9 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Hi Michael,

Quoted 1 line · Michael Trimarchi

On Fri, Dec 4, 2015 at 6:32 PM, Eric Nelson <[email protected]> wrote:

...

Quoted 8 lines

drivers/mmc/fsl_esdhc.c +0 -0

index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
        clk = (pre_div << 8) | (div << 4);

Quoted 9 lines

drivers/mmc/fsl_esdhc.c static void set_sysctl(struct mmc *mmc, uint clock) · line 510
Quoted 6 lines
drivers/mmc/fsl_esdhc.c +4 -0
+#ifdef CONFIG_FSL_USDHC
+       esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
        esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
I really prefer is_usdhc()

Am I overlooking something?

I'm not seeing any such animal, and using a run-time test would prevent the compiler from catching the use of the bits below:

...

Quoted 6 lines

include/fsl_esdhc.h +0 -0

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA           0x08000000

Quoted 8 lines

include/fsl_esdhc.h line 33

Quoted 8 lines

include/fsl_esdhc.h +2 -0

 #define SYSCTL_TIMEOUT_MASK    0x000f0000
 #define SYSCTL_CLOCK_MASK      0x0000fff0
+#if !defined(CONFIG_MX6)
 #define SYSCTL_CKEN            0x00000008
 #define SYSCTL_PEREN           0x00000004
 #define SYSCTL_HCKEN           0x00000002
 #define SYSCTL_IPGEN           0x00000001
+#endif

Please advise,

Eric

Michael Trimarchi

2015-12-04 17:51 UTC #10 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Quoted 24 lines · Eric Nelson

Hi Michael,
Quoted 1 line · Michael Trimarchi
On Fri, Dec 4, 2015 at 6:32 PM, Eric Nelson <[email protected]> wrote:
...
Quoted 8 lines
Quoted 8 lines
drivers/mmc/fsl_esdhc.c +0 -0
index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
        clk = (pre_div << 8) | (div << 4);
 
Quoted 9 lines
drivers/mmc/fsl_esdhc.c static void set_sysctl(struct mmc *mmc, uint clock) · line 510
Quoted 6 lines
drivers/mmc/fsl_esdhc.c +4 -0
+#ifdef CONFIG_FSL_USDHC
+       esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
        esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
I really prefer is_usdhc()

Quoted 5 lines

Am I overlooking something?
I'm not seeing any such animal, and using a run-time test would prevent the compiler from catching the use of the bits below:

If you create a static inline with your define compiler should be optimized but I think that result will be much clean

Michael

Quoted 21 lines

...

Quoted 6 lines

include/fsl_esdhc.h +0 -0

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA           0x08000000

Quoted 8 lines

include/fsl_esdhc.h line 33

Quoted 8 lines

include/fsl_esdhc.h +2 -0

 #define SYSCTL_TIMEOUT_MASK    0x000f0000
 #define SYSCTL_CLOCK_MASK      0x0000fff0
+#if !defined(CONFIG_MX6)
 #define SYSCTL_CKEN            0x00000008
 #define SYSCTL_PEREN           0x00000004
 #define SYSCTL_HCKEN           0x00000002
 #define SYSCTL_IPGEN           0x00000001
+#endif

Please advise,

Eric

Signature · 4 lines

| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder                                      Cruquiuskade 47 |
| +31(0)851119172                                 Amsterdam 1018 AM NL |
|                  [`as] http://www.amarulasolutions.com               |

Hector Palacios

2015-12-04 18:39 UTC #11 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Hi,

Quoted 76 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.

Guard against their usage by hiding the bit mask macros on those processors.

These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.

Signed-off-by Eric Nelson <[email protected]>

---
 drivers/mmc/fsl_esdhc.c | 15 +++++++++++++--
 include/fsl_esdhc.h     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

drivers/mmc/fsl_esdhc.c +13 -2

index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
 	clk = (pre_div << 8) | (div << 4);
 
+#ifdef CONFIG_FSL_USDHC
+	esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
 	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
 	udelay(10000);
 
-	clk = SYSCTL_PEREN | SYSCTL_CKEN;
+#ifdef CONFIG_FSL_USDHC
+	esdhc_clrbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
+	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
+#endif
 
-	esdhc_setbits32(&regs->sysctl, clk);
 }
 
 #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK

@@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
 	esdhc_write32(&regs->scr, 0x00000040);
 #endif
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
+#endif
 
 	/* Set the initial clock speed */
 	mmc_set_clock(mmc, 400000);

@@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 	/* First reset the eSDHC controller */
 	esdhc_reset(regs);
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
 				| SYSCTL_IPGEN | SYSCTL_CKEN);
+#endif
 
 	writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
 	memset(&cfg->cfg, 0, sizeof(cfg->cfg));

include/fsl_esdhc.h +0 -0

index aa1b4cf..a4b87ce 100644

include/fsl_esdhc.h line 28

Quoted 4 lines

include/fsl_esdhc.h +1 -0

@@ -25,10 +25,12 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
 #define SYSCTL_CLOCK_MASK	0x0000fff0
+#if !defined(CONFIG_MX6)

Per your commit message should this be
#if (!defined(CONFIG_MX6) && !defined(CONFIG_MX7))

include/fsl_esdhc.h line 36

Quoted 9 lines

include/fsl_esdhc.h +1 -0

 #define SYSCTL_CKEN		0x00000008
 #define SYSCTL_PEREN		0x00000004
 #define SYSCTL_HCKEN		0x00000002
 #define SYSCTL_IPGEN		0x00000001
+#endif
 #define SYSCTL_RSTA		0x01000000
 #define SYSCTL_RSTC		0x02000000
 #define SYSCTL_RSTD		0x04000000

--
Hector Palacios

Eric Nelson

author

2015-12-04 18:53 UTC #12 lore raw

PATCH 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Thanks Hector,

Quoted 10 lines · Hector Palacios

Hi,
Quoted 7 lines · Eric Nelson
The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.

...

Quoted 4 lines

Quoted 4 lines
include/fsl_esdhc.h +0 -0
index aa1b4cf..a4b87ce 100644

Quoted 9 lines

include/fsl_esdhc.h line 28
Quoted 5 lines
include/fsl_esdhc.h +1 -0
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
 #define SYSCTL_CLOCK_MASK	0x0000fff0
+#if !defined(CONFIG_MX6)
Per your commit message should this be
#if (!defined(CONFIG_MX6) && !defined(CONFIG_MX7))

Good catch.

Will fix in a V2.

Eric Nelson

author

2015-12-04 19:32 UTC #13 lore raw

PATCH v2 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Hector Palacios

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.

Guard against their usage by hiding the bit mask macros on those processors.

These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.

Signed-off-by Eric Nelson <[email protected]>

---
V2 uses CONFIG_FSL_USDHC instead of CONFIG_MX6 in fsl_esdhc.h

 drivers/mmc/fsl_esdhc.c | 15 +++++++++++++--
 include/fsl_esdhc.h     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

drivers/mmc/fsl_esdhc.c +13 -2

index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
 	clk = (pre_div << 8) | (div << 4);
 
+#ifdef CONFIG_FSL_USDHC
+	esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
 	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
 	udelay(10000);
 
-	clk = SYSCTL_PEREN | SYSCTL_CKEN;
+#ifdef CONFIG_FSL_USDHC
+	esdhc_clrbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
+	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
+#endif
 
-	esdhc_setbits32(&regs->sysctl, clk);
 }
 
 #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK

@@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
 	esdhc_write32(&regs->scr, 0x00000040);
 #endif
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
+#endif
 
 	/* Set the initial clock speed */
 	mmc_set_clock(mmc, 400000);

@@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 	/* First reset the eSDHC controller */
 	esdhc_reset(regs);
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
 				| SYSCTL_IPGEN | SYSCTL_CKEN);
+#endif
 
 	writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
 	memset(&cfg->cfg, 0, sizeof(cfg->cfg));

include/fsl_esdhc.h +2 -0

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000
 #define SYSCTL_CLOCK_MASK	0x0000fff0
+#if !defined(CONFIG_FSL_USDHC)
 #define SYSCTL_CKEN		0x00000008
 #define SYSCTL_PEREN		0x00000004
 #define SYSCTL_HCKEN		0x00000002
 #define SYSCTL_IPGEN		0x00000001
+#endif
 #define SYSCTL_RSTA		0x01000000
 #define SYSCTL_RSTC		0x02000000
 #define SYSCTL_RSTD		0x04000000

Signature · 1 line

2.6.2

Fabio Estevam

2015-12-04 19:41 UTC #14 lore raw

PATCH v2 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

Quoted 10 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

Quoted 6 lines

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Signed-off-by Eric Nelson <[email protected]>

Reviewed-by Fabio Estevam <[email protected]>

Stefano Babic

2015-12-07 14:05 UTC #15 lore raw

PATCH v2 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Quoted 80 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.

Guard against their usage by hiding the bit mask macros on those processors.

These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.

Signed-off-by Eric Nelson <[email protected]>

---
V2 uses CONFIG_FSL_USDHC instead of CONFIG_MX6 in fsl_esdhc.h

 drivers/mmc/fsl_esdhc.c | 15 +++++++++++++--
 include/fsl_esdhc.h     |  2 ++
 2 files changed, 15 insertions(+), 2 deletions(-)

drivers/mmc/fsl_esdhc.c +13 -2

index c5054d6..1ccc576 100644
@@ -502,15 +502,22 @@ static void set_sysctl(struct mmc *mmc, uint clock)
 
 	clk = (pre_div << 8) | (div << 4);
 
+#ifdef CONFIG_FSL_USDHC
+	esdhc_setbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
 	esdhc_clrbits32(&regs->sysctl, SYSCTL_CKEN);
+#endif
 
 	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_CLOCK_MASK, clk);
 
 	udelay(10000);
 
-	clk = SYSCTL_PEREN | SYSCTL_CKEN;
+#ifdef CONFIG_FSL_USDHC
+	esdhc_clrbits32(&regs->sysctl, SYSCTL_RSTA);
+#else
+	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
+#endif
 
-	esdhc_setbits32(&regs->sysctl, clk);
 }
 
 #ifdef CONFIG_FSL_ESDHC_USE_PERIPHERAL_CLK

@@ -585,7 +592,9 @@ static int esdhc_init(struct mmc *mmc)
 	esdhc_write32(&regs->scr, 0x00000040);
 #endif
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_HCKEN | SYSCTL_IPGEN);
+#endif
 
 	/* Set the initial clock speed */
 	mmc_set_clock(mmc, 400000);

@@ -657,8 +666,10 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 	/* First reset the eSDHC controller */
 	esdhc_reset(regs);
 
+#ifndef CONFIG_FSL_USDHC
 	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
 				| SYSCTL_IPGEN | SYSCTL_CKEN);
+#endif
 
 	writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
 	memset(&cfg->cfg, 0, sizeof(cfg->cfg));

include/fsl_esdhc.h +0 -0

index aa1b4cf..a4b87ce 100644
@@ -25,10 +25,12 @@
 #define SYSCTL_INITA		0x08000000
 #define SYSCTL_TIMEOUT_MASK	0x000f0000

include/fsl_esdhc.h line 36

Quoted 10 lines

include/fsl_esdhc.h +2 -0

 #define SYSCTL_CLOCK_MASK	0x0000fff0
+#if !defined(CONFIG_FSL_USDHC)
 #define SYSCTL_CKEN		0x00000008
 #define SYSCTL_PEREN		0x00000004
 #define SYSCTL_HCKEN		0x00000002
 #define SYSCTL_IPGEN		0x00000001
+#endif
 #define SYSCTL_RSTA		0x01000000
 #define SYSCTL_RSTC		0x02000000
 #define SYSCTL_RSTD		0x04000000

Reviewed-by Stefano Babic <[email protected]>

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

Hector Palacios

2015-12-09 10:04 UTC #16 lore raw

PATCH v2 1/1 ARM: imx: fsl_esdhc: fix usage of low 4 bits of sysctl register

In reply to Eric Nelson

Quoted 10 lines · Eric Nelson

The low four bits of the SYSCTL register are reserved on the USDHC controller on i.MX6 and i.MX7 processors, but are used for clocking operations on earlier models.
Guard against their usage by hiding the bit mask macros on those processors.
These bits are used to prevent glitches when changing clocks on i.MX35 et al. Use the RSTA bit instead for i.MX6 and i.MX7.

Quoted 6 lines

From the i.MX6DQ RM:
To prevent possible glitch on the card clock, clear the FRC_SDCLK_ON bit when changing clock divisor value(SDCLKFS or DVS in System Control Register) or setting RSTA bit.
Signed-off-by Eric Nelson <[email protected]>

Reviewed-by Hector Palacios <[email protected]>