Re: [PATCH v4 4/4] rockchip: sdhci: Add HS400 Enhanced Strobe support for RK3568

[Jaehoon Chung <jh80.chung@samsung.com>]

On 1/29/22 07:42, 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.
> 
> 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).
> 
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

As you mentioned, if someone test this on RK3568 board, it will be more better.

Best Regards,
Jaehoon Chung

> ---
> Only build-tested as I don't have a RK3568 board.
> 
> (no changes since v3)
> 
> 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 | 42 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
> 
> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
> index f4d5a59036a2..1d96c4696b6f 100644
> --- a/drivers/mmc/rockchip_sdhci.c
> +++ b/drivers/mmc/rockchip_sdhci.c
> @@ -45,6 +45,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)
> @@ -279,11 +287,25 @@ static int rk3568_emmc_phy_init(struct udevice *dev)
>  {
>  	struct rockchip_sdhc *prv = dev_get_priv(dev);
>  	struct sdhci_host *host = &prv->host;
> +	struct mmc *mmc = host->mmc;
>  	u32 extra;
> +	u32 vendor;
> +	int reg;
>  
>  	extra = DLL_RXCLK_NO_INVERTER << DWCMSHC_EMMC_DLL_RXCLK_SRCSEL;
>  	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_RXCLK);
>  
> +	/* set CARD_IS_EMMC bit to enable Data Strobe for HS400 and HS400ES */
> +	reg = (sdhci_readl(host, DWCMSHC_P_VENDOR_AREA1) & DWCMSHC_AREA1_MASK)
> +	      + DWCMSHC_EMMC_CONTROL;
> +
> +	vendor = sdhci_readw(host, reg);
> +	if (IS_MMC(mmc))
> +		vendor |= DWCMSHC_CARD_IS_EMMC;
> +	else
> +		vendor &= ~DWCMSHC_CARD_IS_EMMC;
> +	sdhci_writew(host, vendor, reg);
> +
>  	return 0;
>  }
>  
> @@ -346,6 +368,25 @@ 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;
> @@ -554,6 +595,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[] = {