Re: [PATCH] mmc: sdhci-of-dwcmshc: Restore missing regs for RK3576 platform

[Adrian Hunter <adrian.hunter@intel.com>]

On 22/11/24 11:24, Shawn Lin wrote:
> DECMSHC_EMMC_MISC_CON is introduced from RK3576 which need to be
> saved and restore after .reset() with SDHCI_RESET_ALL is called,
> because we assert the reset line. Otherwise the entire controller
> is broken anyway.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 8999b97..aa46655 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -81,6 +81,8 @@
>  #define DWCMSHC_EMMC_DLL_TXCLK		0x808
>  #define DWCMSHC_EMMC_DLL_STRBIN		0x80c
>  #define DECMSHC_EMMC_DLL_CMDOUT		0x810
> +#define MISC_INTCLK_EN			BIT(1)
> +#define DECMSHC_EMMC_MISC_CON		0x81c
>  #define DWCMSHC_EMMC_DLL_STATUS0	0x840
>  #define DWCMSHC_EMMC_DLL_START		BIT(0)
>  #define DWCMSHC_EMMC_DLL_LOCKED		BIT(8)
> @@ -213,6 +215,7 @@ enum dwcmshc_rk_type {
>  struct rk35xx_priv {
>  	struct reset_control *reset;
>  	enum dwcmshc_rk_type devtype;
> +	bool has_misc_con;
>  	u8 txclk_tapnum;
>  };
>  
> @@ -720,6 +723,7 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>  	struct dwcmshc_priv *dwc_priv = sdhci_pltfm_priv(pltfm_host);
>  	struct rk35xx_priv *priv = dwc_priv->priv;
> +	int misc_con;
>  
>  	if (mask & SDHCI_RESET_ALL && priv->reset) {
>  		reset_control_assert(priv->reset);
> @@ -728,6 +732,12 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
>  	}
>  
>  	sdhci_reset(host, mask);
> +
> +	/* Restore DECMSHC_EMMC_MISC_CON */
> +	if (priv->has_misc_con) {
> +		misc_con = sdhci_readl(host, DECMSHC_EMMC_MISC_CON);
> +		sdhci_writel(host, misc_con | MISC_INTCLK_EN, DECMSHC_EMMC_MISC_CON);
> +	}
>  }
>  
>  static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
> @@ -735,7 +745,7 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
>  {
>  	static const char * const clk_ids[] = {"axi", "block", "timer"};
>  	struct rk35xx_priv *priv;
> -	int err;
> +	int err, misc_con;
>  
>  	priv = devm_kzalloc(dev, sizeof(struct rk35xx_priv), GFP_KERNEL);
>  	if (!priv)
> @@ -746,6 +756,9 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
>  	else
>  		priv->devtype = DWCMSHC_RK3568;
>  
> +	if (of_device_is_compatible(dev->of_node, "rockchip,rk3576-dwcmshc"))
> +		priv->has_misc_con = true;

Not needed for this patch, but instead of keeping on adding
'if (of_device_is_compatible())', it would be nicer to get parameters
from a table.  Using sdhci_dwcmshc_dt_ids[] might be overkill, but
dwcmshc_rk35xx could have its own little table for has_misc_con and
whatever devtype is for, read using of_match_node() for example.

> +
>  	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
>  	if (IS_ERR(priv->reset)) {
>  		err = PTR_ERR(priv->reset);
> @@ -767,6 +780,10 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
>  	/* Reset previous settings */
>  	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_TXCLK);
>  	sdhci_writel(host, 0, DWCMSHC_EMMC_DLL_STRBIN);
> +	if (priv->has_misc_con) {
> +		misc_con = sdhci_readl(host, DECMSHC_EMMC_MISC_CON);
> +		sdhci_writel(host, misc_con | MISC_INTCLK_EN, DECMSHC_EMMC_MISC_CON);
> +	}

Probably a bit neater to put that in a separate function since it
is the same code as in rk35xx_sdhci_reset()

>  
>  	dwc_priv->priv = priv;
>  


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip