mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

PATCH New Benoît Thébaudeau 3 participants 5 messages Jan 14, 2018 – Jan 15, 2018 MMC, SD and SDIO

Share & tools

Benoît Thébaudeau

author

2018-01-14 23:46 UTC #0 lore raw

PATCH 1/1 mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

Commit 4f425280fa71 ("mmc: fsl_esdhc: Allow all supported prescaler values") made it possible to set SYSCTL.SDCLKFS to 0 in SDR mode on i.MX, thus bypassing the SD clock frequency prescaler, in order to be able to get higher SD clock frequencies in some contexts. However, that commit missed the fact that this value is illegal on the eSDHCv3 instance of the i.MX53. This seems to be the only exception on i.MX, this value being legal even for the eSDHCv2 instances of the i.MX53.

Fix this issue by changing the minimum prescaler value for the single instance of the i.MX53 eSDHCv3 controller.

Signed-off-by Benoît Thébaudeau <[email protected]>

---
 drivers/mmc/fsl_esdhc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/mmc/fsl_esdhc.c +6 -1

index 499d622c6d..90425e8a30 100644
@@ -528,14 +528,19 @@ out:
 
 static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 {
+	struct fsl_esdhc *regs = priv->esdhc_regs;
 	int div = 1;
 #ifdef ARCH_MXC
+#ifdef CONFIG_MX53
+	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
+	int pre_div = regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1;
+#else
 	int pre_div = 1;
+#endif
 #else
 	int pre_div = 2;
 #endif
 	int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
-	struct fsl_esdhc *regs = priv->esdhc_regs;
 	int sdhc_clk = priv->sdhc_clk;
 	uint clk;

Signature · 1 line

2.14.1

Fabio Estevam

2018-01-15 00:29 UTC #1 lore raw

PATCH 1/1 mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

+ Adding Jaehoon, who is the U-Boot MMC maintainer.

Quoted 8 lines · Benoît Thébaudeau

Commit 4f425280fa71 ("mmc: fsl_esdhc: Allow all supported prescaler values") made it possible to set SYSCTL.SDCLKFS to 0 in SDR mode on i.MX, thus bypassing the SD clock frequency prescaler, in order to be able to get higher SD clock frequencies in some contexts. However, that commit missed the fact that this value is illegal on the eSDHCv3 instance of the i.MX53. This seems to be the only exception on i.MX, this value being legal even for the eSDHCv2 instances of the i.MX53.

Quoted 4 lines

Fix this issue by changing the minimum prescaler value for the single instance of the i.MX53 eSDHCv3 controller.
Signed-off-by Benoît Thébaudeau <[email protected]>

Thanks for your fix in U-Boot and in the kernel:

Reviewed-by Fabio Estevam <[email protected]>

Quoted 36 lines

---
 drivers/mmc/fsl_esdhc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

drivers/mmc/fsl_esdhc.c +6 -2

index 499d622c6d..90425e8a30 100644
@@ -528,14 +528,19 @@ out:
 
 static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 {
+       struct fsl_esdhc *regs = priv->esdhc_regs;
        int div = 1;
 #ifdef ARCH_MXC
+#ifdef CONFIG_MX53
+       /* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
+       int pre_div = regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1;
+#else
        int pre_div = 1;
+#endif
 #else
        int pre_div = 2;
 #endif
        int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
-       struct fsl_esdhc *regs = priv->esdhc_regs;
        int sdhc_clk = priv->sdhc_clk;
        uint clk;
 
--

2.14.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot

Stefano Babic

2018-01-15 10:59 UTC #2 lore raw

PATCH 1/1 mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

In reply to Benoît Thébaudeau

Hi Benoît,

Quoted 21 lines · Benoît Thébaudeau

Commit 4f425280fa71 ("mmc: fsl_esdhc: Allow all supported prescaler values") made it possible to set SYSCTL.SDCLKFS to 0 in SDR mode on i.MX, thus bypassing the SD clock frequency prescaler, in order to be able to get higher SD clock frequencies in some contexts. However, that commit missed the fact that this value is illegal on the eSDHCv3 instance of the i.MX53. This seems to be the only exception on i.MX, this value being legal even for the eSDHCv2 instances of the i.MX53.
Fix this issue by changing the minimum prescaler value for the single instance of the i.MX53 eSDHCv3 controller.
Signed-off-by Benoît Thébaudeau <[email protected]>
---
 drivers/mmc/fsl_esdhc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
drivers/mmc/fsl_esdhc.c +0 -0
index 499d622c6d..90425e8a30 100644

drivers/mmc/fsl_esdhc.c out: · line 536

Quoted 9 lines

drivers/mmc/fsl_esdhc.c +4 -0

@@ -528,14 +528,19 @@ out:
 
 static void set_sysctl(struct fsl_esdhc_priv *priv, struct mmc *mmc, uint clock)
 {
+	struct fsl_esdhc *regs = priv->esdhc_regs;
 	int div = 1;
 #ifdef ARCH_MXC
+#ifdef CONFIG_MX53
+	/* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
+	int pre_div = regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1;

It is surely a question of taste - but is it not cleared to surround the expression with parenthesis ? (regs == .... )

drivers/mmc/fsl_esdhc.c out: · line 546

Quoted 12 lines

drivers/mmc/fsl_esdhc.c +2 -1

+#else
 	int pre_div = 1;
+#endif
 #else
 	int pre_div = 2;
 #endif
 	int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
-	struct fsl_esdhc *regs = priv->esdhc_regs;
 	int sdhc_clk = priv->sdhc_clk;
 	uint clk;

Best regards,
Stefano

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
=====================================================================

Benoît Thébaudeau

author

2018-01-15 21:11 UTC #3 lore raw

PATCH 1/1 mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

Hi Stefano,

Quoted 5 lines · Stefano Babic

Quoted 1 line · Benoît Thébaudeau
+ int pre_div = regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1;
It is surely a question of taste - but is it not cleared to surround the expression with parenthesis ? (regs == .... )

I can send a v2 for that if you want. Which of the following do you prefer (personally, I'd say the last one)?

int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1); int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;

Note that this patch can be tested with the 2nd MMC on i.MX53 Loco if anyone has this board. Wladimir?

Best regards,
Benoît

Stefano Babic

2018-01-15 21:27 UTC #4 lore raw

PATCH 1/1 mmc: fsl_esdhc: Fix i.MX53 eSDHCv3 clock

Hi Benoît,

Quoted 6 lines · Benoît Thébaudeau

Hi Stefano,
Quoted 3 lines · Stefano Babic
Quoted 1 line · Benoît Thébaudeau
+ int pre_div = regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1;

Quoted 9 lines

Quoted 2 lines
It is surely a question of taste - but is it not cleared to surround the expression with parenthesis ? (regs == .... )
I can send a v2 for that if you want. Which of the following do you prefer (personally, I'd say the last one)?
int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR ? 2 : 1); int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;

I agree for last one - but as I said, it is just a question of taste.

Quoted 2 lines

Note that this patch can be tested with the 2nd MMC on i.MX53 Loco if anyone has this board. Wladimir?

Best regards,
Stefano

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
=====================================================================