| Message ID | 20180929234553.31019-2-vagrant@debian.org |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | [U-Boot,1/5] mmc: sunxi: add support for automatic delay calibration | expand |
On 9/30/18 12:45 AM, Vagrant Cascadian wrote: > From: Vasily Khoruzhick <anarsoul@gmail.com> > > A64 supports automatic delay calibration and Linux driver uses it > instead of hardcoded delays. Add support for it to u-boot driver. So technically that should be derived from the node's compatible string, like we do in Linux. But I see that we are not there yet in U-Boot. Meanwhile I don't think you should introduce a new Kconfig option, you could keep the hacky U-Boot style and just add an #if defined(CONFIG_MACH_SUN50I) || defined(CONFIG_MACH_SUN50I_H6) to the two places instead. A better way would be to introduce a "can_calibrate" member to struct sunxi_mmc_priv, then set this once in sunxi_mmc_init(), guarded by the two MACH symbols as above. This would allow an easy transition to being DT driven later and would prevent us from forgetting about this. Cheers, Andre. > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > Signed-off-by: Vagrant Cascadian <vagrant@debian.org> > --- > > arch/arm/include/asm/arch-sunxi/mmc.h | 6 +++++- > arch/arm/mach-sunxi/Kconfig | 1 + > drivers/mmc/Kconfig | 4 ++++ > drivers/mmc/sunxi_mmc.c | 20 +++++++++++++++++++- > 4 files changed, 29 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/mmc.h b/arch/arm/include/asm/arch-sunxi/mmc.h > index d98c53faaa..f2deafddd2 100644 > --- a/arch/arm/include/asm/arch-sunxi/mmc.h > +++ b/arch/arm/include/asm/arch-sunxi/mmc.h > @@ -46,7 +46,9 @@ struct sunxi_mmc { > u32 cbda; /* 0x94 */ > u32 res2[26]; > #if defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_MACH_SUN50I_H6) > - u32 res3[64]; > + u32 res3[17]; > + u32 samp_dl; > + u32 res4[46]; > #endif > u32 fifo; /* 0x100 / 0x200 FIFO access address */ > }; > @@ -130,5 +132,7 @@ struct sunxi_mmc { > #define SUNXI_MMC_COMMON_CLK_GATE (1 << 16) > #define SUNXI_MMC_COMMON_RESET (1 << 18) > > +#define SUNXI_MMC_CAL_DL_SW_EN (0x1 << 7) > + > struct mmc *sunxi_mmc_init(int sdc_no); > #endif /* _SUNXI_MMC_H */ > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index 686f38fec4..ae77ee9e8e 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -271,6 +271,7 @@ config MACH_SUN50I > bool "sun50i (Allwinner A64)" > select ARM64 > select DM_I2C > + select MMC_SUNXI_SUPPORTS_CALIBRATION > select PHY_SUN4I_USB > select SUNXI_DE2 > select SUNXI_GEN_SUN6I > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig > index 0a0d4aaf6c..fb8f6697d4 100644 > --- a/drivers/mmc/Kconfig > +++ b/drivers/mmc/Kconfig > @@ -569,6 +569,10 @@ config MMC_SUNXI_HAS_NEW_MODE > bool > depends on MMC_SUNXI > > +config MMC_SUNXI_SUPPORTS_CALIBRATION > + bool > + depends on MMC_SUNXI > + > config GENERIC_ATMEL_MCI > bool "Atmel Multimedia Card Interface support" > depends on DM_MMC && BLK && ARCH_AT91 > diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c > index 39f15eb423..7b064b482c 100644 > --- a/drivers/mmc/sunxi_mmc.c > +++ b/drivers/mmc/sunxi_mmc.c > @@ -99,11 +99,15 @@ static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz) > { > unsigned int pll, pll_hz, div, n, oclk_dly, sclk_dly; > bool new_mode = false; > + bool calibrate = false; > u32 val = 0; > > if (IS_ENABLED(CONFIG_MMC_SUNXI_HAS_NEW_MODE) && (priv->mmc_no == 2)) > new_mode = true; > > + if (IS_ENABLED(CONFIG_MMC_SUNXI_SUPPORTS_CALIBRATION)) > + calibrate = true; > + > /* > * The MMC clock has an extra /2 post-divider when operating in the new > * mode. > @@ -174,7 +178,11 @@ static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz) > val = CCM_MMC_CTRL_MODE_SEL_NEW; > setbits_le32(&priv->reg->ntsr, SUNXI_MMC_NTSR_MODE_SEL_NEW); > #endif > - } else { > + } else if (!calibrate) { > + /* > + * Use hardcoded delay values if controller doesn't support > + * calibration > + */ > val = CCM_MMC_CTRL_OCLK_DLY(oclk_dly) | > CCM_MMC_CTRL_SCLK_DLY(sclk_dly); > } > @@ -228,6 +236,16 @@ static int mmc_config_clock(struct sunxi_mmc_priv *priv, struct mmc *mmc) > rval &= ~SUNXI_MMC_CLK_DIVIDER_MASK; > writel(rval, &priv->reg->clkcr); > > +#ifdef CONFIG_MMC_SUNXI_SUPPORTS_CALIBRATION > + /* A64 supports calibration of delays on MMC controller and we > + * have to set delay of zero before starting calibration. > + * Allwinner BSP driver sets a delay only in the case of > + * using HS400 which is not supported by mainline U-Boot or > + * Linux at the moment > + */ > + writel(SUNXI_MMC_CAL_DL_SW_EN, &priv->reg->samp_dl); > +#endif > + > /* Re-enable Clock */ > rval |= SUNXI_MMC_CLK_ENABLE; > writel(rval, &priv->reg->clkcr); >
On Sun, Sep 30, 2018 at 04:16:36PM +0100, André Przywara wrote: > On 9/30/18 12:45 AM, Vagrant Cascadian wrote: > > From: Vasily Khoruzhick <anarsoul@gmail.com> > > > > A64 supports automatic delay calibration and Linux driver uses it > > instead of hardcoded delays. Add support for it to u-boot driver. > > So technically that should be derived from the node's compatible string, > like we do in Linux. But I see that we are not there yet in U-Boot. > Meanwhile I don't think you should introduce a new Kconfig option, you > could keep the hacky U-Boot style and just add an > #if defined(CONFIG_MACH_SUN50I) || defined(CONFIG_MACH_SUN50I_H6) > to the two places instead. IIRC, the calibration is only needed for the eMMC though, so we'd need to check that against the MMC number too. Maxime
On Mon, 1 Oct 2018 10:09:55 +0200 Maxime Ripard <maxime.ripard@bootlin.com> wrote: > On Sun, Sep 30, 2018 at 04:16:36PM +0100, André Przywara wrote: > > On 9/30/18 12:45 AM, Vagrant Cascadian wrote: > > > From: Vasily Khoruzhick <anarsoul@gmail.com> > > > > > > A64 supports automatic delay calibration and Linux driver uses it > > > instead of hardcoded delays. Add support for it to u-boot > > > driver. > > > > So technically that should be derived from the node's compatible > > string, like we do in Linux. But I see that we are not there yet in > > U-Boot. Meanwhile I don't think you should introduce a new Kconfig > > option, you could keep the hacky U-Boot style and just add an > > #if defined(CONFIG_MACH_SUN50I) || defined(CONFIG_MACH_SUN50I_H6) > > to the two places instead. > > IIRC, the calibration is only needed for the eMMC though, so we'd need > to check that against the MMC number too. Is that so? I was looking at the Linux driver, and that one sets can_calibrate for both the "normal" and eMMC A64 compatible strings. Isn't the difference between the two the new timing mode, which the eMMC doesn't have? This looks to be covered in U-Boot, though in a similar hacky way. Cheers, Andre.
On Mon, Oct 01, 2018 at 09:48:37AM +0100, Andre Przywara wrote: > On Mon, 1 Oct 2018 10:09:55 +0200 > Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > On Sun, Sep 30, 2018 at 04:16:36PM +0100, André Przywara wrote: > > > On 9/30/18 12:45 AM, Vagrant Cascadian wrote: > > > > From: Vasily Khoruzhick <anarsoul@gmail.com> > > > > > > > > A64 supports automatic delay calibration and Linux driver uses it > > > > instead of hardcoded delays. Add support for it to u-boot > > > > driver. > > > > > > So technically that should be derived from the node's compatible > > > string, like we do in Linux. But I see that we are not there yet in > > > U-Boot. Meanwhile I don't think you should introduce a new Kconfig > > > option, you could keep the hacky U-Boot style and just add an > > > #if defined(CONFIG_MACH_SUN50I) || defined(CONFIG_MACH_SUN50I_H6) > > > to the two places instead. > > > > IIRC, the calibration is only needed for the eMMC though, so we'd need > > to check that against the MMC number too. > > Is that so? I was looking at the Linux driver, and that one sets > can_calibrate for both the "normal" and eMMC A64 compatible strings. > Isn't the difference between the two the new timing mode, which the > eMMC doesn't have? This looks to be covered in U-Boot, though in a > similar hacky way. You're right, sorry for the noise... Maxime
diff --git a/arch/arm/include/asm/arch-sunxi/mmc.h b/arch/arm/include/asm/arch-sunxi/mmc.h index d98c53faaa..f2deafddd2 100644 --- a/arch/arm/include/asm/arch-sunxi/mmc.h +++ b/arch/arm/include/asm/arch-sunxi/mmc.h @@ -46,7 +46,9 @@ struct sunxi_mmc { u32 cbda; /* 0x94 */ u32 res2[26]; #if defined(CONFIG_SUNXI_GEN_SUN6I) || defined(CONFIG_MACH_SUN50I_H6) - u32 res3[64]; + u32 res3[17]; + u32 samp_dl; + u32 res4[46]; #endif u32 fifo; /* 0x100 / 0x200 FIFO access address */ }; @@ -130,5 +132,7 @@ struct sunxi_mmc { #define SUNXI_MMC_COMMON_CLK_GATE (1 << 16) #define SUNXI_MMC_COMMON_RESET (1 << 18) +#define SUNXI_MMC_CAL_DL_SW_EN (0x1 << 7) + struct mmc *sunxi_mmc_init(int sdc_no); #endif /* _SUNXI_MMC_H */ diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 686f38fec4..ae77ee9e8e 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -271,6 +271,7 @@ config MACH_SUN50I bool "sun50i (Allwinner A64)" select ARM64 select DM_I2C + select MMC_SUNXI_SUPPORTS_CALIBRATION select PHY_SUN4I_USB select SUNXI_DE2 select SUNXI_GEN_SUN6I diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig index 0a0d4aaf6c..fb8f6697d4 100644 --- a/drivers/mmc/Kconfig +++ b/drivers/mmc/Kconfig @@ -569,6 +569,10 @@ config MMC_SUNXI_HAS_NEW_MODE bool depends on MMC_SUNXI +config MMC_SUNXI_SUPPORTS_CALIBRATION + bool + depends on MMC_SUNXI + config GENERIC_ATMEL_MCI bool "Atmel Multimedia Card Interface support" depends on DM_MMC && BLK && ARCH_AT91 diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c index 39f15eb423..7b064b482c 100644 --- a/drivers/mmc/sunxi_mmc.c +++ b/drivers/mmc/sunxi_mmc.c @@ -99,11 +99,15 @@ static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz) { unsigned int pll, pll_hz, div, n, oclk_dly, sclk_dly; bool new_mode = false; + bool calibrate = false; u32 val = 0; if (IS_ENABLED(CONFIG_MMC_SUNXI_HAS_NEW_MODE) && (priv->mmc_no == 2)) new_mode = true; + if (IS_ENABLED(CONFIG_MMC_SUNXI_SUPPORTS_CALIBRATION)) + calibrate = true; + /* * The MMC clock has an extra /2 post-divider when operating in the new * mode. @@ -174,7 +178,11 @@ static int mmc_set_mod_clk(struct sunxi_mmc_priv *priv, unsigned int hz) val = CCM_MMC_CTRL_MODE_SEL_NEW; setbits_le32(&priv->reg->ntsr, SUNXI_MMC_NTSR_MODE_SEL_NEW); #endif - } else { + } else if (!calibrate) { + /* + * Use hardcoded delay values if controller doesn't support + * calibration + */ val = CCM_MMC_CTRL_OCLK_DLY(oclk_dly) | CCM_MMC_CTRL_SCLK_DLY(sclk_dly); } @@ -228,6 +236,16 @@ static int mmc_config_clock(struct sunxi_mmc_priv *priv, struct mmc *mmc) rval &= ~SUNXI_MMC_CLK_DIVIDER_MASK; writel(rval, &priv->reg->clkcr); +#ifdef CONFIG_MMC_SUNXI_SUPPORTS_CALIBRATION + /* A64 supports calibration of delays on MMC controller and we + * have to set delay of zero before starting calibration. + * Allwinner BSP driver sets a delay only in the case of + * using HS400 which is not supported by mainline U-Boot or + * Linux at the moment + */ + writel(SUNXI_MMC_CAL_DL_SW_EN, &priv->reg->samp_dl); +#endif + /* Re-enable Clock */ rval |= SUNXI_MMC_CLK_ENABLE; writel(rval, &priv->reg->clkcr);