Re: [PATCH v5 3/3] rockchip: sdhci: Add HS400 Enhanced Strobe support for RK3568

[Kever Yang <kever.yang@rock-chips.com>]

On 2022/3/16 01:46, Alper Nebi Yasak wrote:
> On RK3568, a register bit must be set to enable Enhanced Strobe.
> However, it appears that the address of this register may differ from
> vendor to vendor and should be read from the underlying MMC IP. Let the
> Rockchip SDHCI driver read this address and set the relevant bit when
> Enhanced Strobe configuration is requested.
>
> The IP uses a custom mode select value (0x7) for HS400, use that instead
> of the common but non-standard SDHCI_CTRL_HS400 value (0x5). Also add
> some necessary DLL_STRBIN and DLL_TXCLK configuration for HS400.
>
> Additionally, a bit signifying that the connected hardware is an eMMC
> chip must be set to enable Data Strobe for HS400 and HS400ES modes. Also
> make the driver set this bit as appropriate.
>
> This is partly ported from Linux's Synopsys DWC MSHC driver which
> happens to be the underlying IP. (drivers/mmc/host/sdhci-of-dwcmshc.c in
> Linux tree).
>
> Co-developed-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>


Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever

> ---
> This is a fixed version I received off-list from Yifeng. I didn't modify
> the diff, but added a paragraph in the commit message mentioning their
> changes and adjusted the signoffs in the end.
>
> Didn't add the Reviewed-by tag due to changes.
>
> Changes in v5:
> - Incorporate RK3568 HS400ES fixes from Yifeng Zhao:
>    - Use DWCMSHC_CTRL_HS400 = 0x7, instead of SDHCI_CTRL_HS400 = 0x5
>    - Configure DWCMSHC_CARD_IS_EMMC in rk3568_sdhci_set_ios_post()
>    - Configure DLL_STRBIN and DLL_TXCLK for HS400.
>
> Changes in v3:
> - Set DWCMSHC_CARD_IS_EMMC bit in rk3568_emmc_phy_init()
>
> Changes in v2:
> - Rename rk3568_set_enhanced_strobe -> rk3568_sdhci_set_enhanced_strobe
> - Let set_enhanced_strobe() unset the ES bit if mode is not HS400_ES
>
>   drivers/mmc/rockchip_sdhci.c | 64 ++++++++++++++++++++++++++++++++----
>   1 file changed, 58 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index f4d5a59036a2..f3f9d83ba36f 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -22,6 +22,8 @@
>   #include <asm/arch-rockchip/clock.h>
>   #include <asm/arch-rockchip/hardware.h>
>   
> +/* DWCMSHC specific Mode Select value */
> +#define DWCMSHC_CTRL_HS400		0x7
>   /* 400KHz is max freq for card ID etc. Use that as min */
>   #define EMMC_MIN_FREQ	400000
>   #define KHz	(1000)
> @@ -45,6 +47,14 @@
>   #define ARASAN_VENDOR_REGISTER		0x78
>   #define ARASAN_VENDOR_ENHANCED_STROBE	BIT(0)
>   
> +/* DWC IP vendor area 1 pointer */
> +#define DWCMSHC_P_VENDOR_AREA1		0xe8
> +#define DWCMSHC_AREA1_MASK		GENMASK(11, 0)
> +/* Offset inside the vendor area 1 */
> +#define DWCMSHC_EMMC_CONTROL		0x2c
> +#define DWCMSHC_CARD_IS_EMMC		BIT(0)
> +#define DWCMSHC_ENHANCED_STROBE		BIT(8)
> +
>   /* Rockchip specific Registers */
>   #define DWCMSHC_EMMC_DLL_CTRL		0x800
>   #define DWCMSHC_EMMC_DLL_CTRL_RESET	BIT(1)
> @@ -60,8 +70,14 @@
>   #define DWCMSHC_EMMC_DLL_INC_VALUE	2
>   #define DWCMSHC_EMMC_DLL_INC		8
>   #define DWCMSHC_EMMC_DLL_DLYENA		BIT(27)
> -#define DLL_TXCLK_TAPNUM_DEFAULT	0x10
> -#define DLL_STRBIN_TAPNUM_DEFAULT	0x3
> +#define DLL_TXCLK_TAPNUM_DEFAULT	0xA
> +
> +#define DLL_STRBIN_TAPNUM_DEFAULT	0x8
> +#define DLL_STRBIN_TAPNUM_FROM_SW	BIT(24)
> +#define DLL_STRBIN_DELAY_NUM_SEL	BIT(26)
> +#define DLL_STRBIN_DELAY_NUM_OFFSET	16
> +#define DLL_STRBIN_DELAY_NUM_DEFAULT	0x16
> +
>   #define DLL_TXCLK_TAPNUM_FROM_SW	BIT(24)
>   #define DWCMSHC_EMMC_DLL_LOCKED		BIT(8)
>   #define DWCMSHC_EMMC_DLL_TIMEOUT	BIT(9)
> @@ -327,7 +343,8 @@ static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clo
>   		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_TXCLK);
>   
>   		extra = DWCMSHC_EMMC_DLL_DLYENA |
> -			DLL_STRBIN_TAPNUM_DEFAULT;
> +			DLL_STRBIN_TAPNUM_DEFAULT |
> +			DLL_STRBIN_TAPNUM_FROM_SW;
>   		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
>   	} else {
>   		/* reset the clock phase when the frequency is lower than 100MHz */
> @@ -335,7 +352,15 @@ static int rk3568_sdhci_emmc_set_clock(struct sdhci_host *host, unsigned int clo
>   		extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
>   		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
>   		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
> -		sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> +		/*
> +		 * Before switching to hs400es mode, the driver will enable
> +		 * enhanced strobe first. PHY needs to configure the parameters
> +		 * of enhanced strobe first.
> +		 */
> +		extra = DWCMSHC_EMMC_DLL_DLYENA |
> +			DLL_STRBIN_DELAY_NUM_SEL |
> +			DLL_STRBIN_DELAY_NUM_DEFAULT << DLL_STRBIN_DELAY_NUM_OFFSET;
> +		sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
>   	}
>   
>   	return 0;
> @@ -346,11 +371,30 @@ static int rk3568_emmc_get_phy(struct udevice *dev)
>   	return 0;
>   }
>   
> +static int rk3568_sdhci_set_enhanced_strobe(struct sdhci_host *host)
> +{
> +	struct mmc *mmc = host->mmc;
> +	u32 vendor;
> +	int reg;
> +
> +	reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
> +	      + DWCMSHC_EMMC_CONTROL;
> +
> +	vendor = sdhci_readl(host, reg);
> +	if (mmc->selected_mode == MMC_HS_400_ES)
> +		vendor |= DWCMSHC_ENHANCED_STROBE;
> +	else
> +		vendor &= ~DWCMSHC_ENHANCED_STROBE;
> +	sdhci_writel(host, vendor, reg);
> +
> +	return 0;
> +}
> +
>   static int rk3568_sdhci_set_ios_post(struct sdhci_host *host)
>   {
>   	struct mmc *mmc = host->mmc;
>   	uint clock = mmc->tran_speed;
> -	u32 reg;
> +	u32 reg, vendor_reg;
>   
>   	if (!clock)
>   		clock = mmc->clock;
> @@ -360,8 +404,15 @@ static int rk3568_sdhci_set_ios_post(struct sdhci_host *host)
>   	if (mmc->selected_mode == MMC_HS_400 || mmc->selected_mode == MMC_HS_400_ES) {
>   		reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>   		reg &= ~SDHCI_CTRL_UHS_MASK;
> -		reg |= SDHCI_CTRL_HS400;
> +		reg |= DWCMSHC_CTRL_HS400;
>   		sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> +
> +		vendor_reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
> +			     + DWCMSHC_EMMC_CONTROL;
> +		/* set CARD_IS_EMMC bit to enable Data Strobe for HS400 */
> +		reg = sdhci_readw(host, vendor_reg);
> +		reg |= DWCMSHC_CARD_IS_EMMC;
> +		sdhci_writew(host, reg, vendor_reg);
>   	} else {
>   		sdhci_set_uhs_timing(host);
>   	}
> @@ -554,6 +605,7 @@ static const struct sdhci_data rk3568_data = {
>   	.get_phy = rk3568_emmc_get_phy,
>   	.emmc_phy_init = rk3568_emmc_phy_init,
>   	.set_ios_post = rk3568_sdhci_set_ios_post,
> +	.set_enhanced_strobe = rk3568_sdhci_set_enhanced_strobe,
>   };
>   
>   static const struct udevice_id sdhci_ids[] = {