diff mbox series

[PATCHv1,08/19] mmc: sdhci-of-dwcmshc: add reset call back for rockchip Socs

Message ID 20220422170920.401914-9-sebastian.reichel@collabora.com
State Superseded
Headers show
Series Basic RK3588 Support | expand

Commit Message

Sebastian Reichel April 22, 2022, 5:09 p.m. UTC
From: Yifeng Zhao <yifeng.zhao@rock-chips.com>

The reset function build in the SDHCI will not reset the logic
circuit related to the tuning function, which may cause data
reading errors. Resetting the complete SDHCI controller through
the reset controller fixes the issue.

Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
[rebase]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

Comments

Dmitry Osipenko April 23, 2022, 10:32 a.m. UTC | #1
22.04.2022 20:09, Sebastian Reichel пишет:
> From: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> 
> The reset function build in the SDHCI will not reset the logic
> circuit related to the tuning function, which may cause data
> reading errors. Resetting the complete SDHCI controller through
> the reset controller fixes the issue.
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> [rebase]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index bac874ab0b33..d95ae6ca1256 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -15,6 +15,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/reset.h>
>  #include <linux/sizes.h>
>  
>  #include "sdhci-pltfm.h"
> @@ -63,6 +64,7 @@
>  struct rk3568_priv {
>  	/* Rockchip specified optional clocks */
>  	struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS];
> +	struct reset_control *reset;
>  	u8 txclk_tapnum;
>  };
>  
> @@ -255,6 +257,23 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
>  }
>  
> +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;
> +
> +	if (mask & SDHCI_RESET_ALL) {
> +		if (!IS_ERR_OR_NULL(priv->reset)) {

priv->reset can't be a error ptr since probe fails on error.

> +			reset_control_assert(priv->reset);
> +			udelay(1);
> +			reset_control_deassert(priv->reset);
> +		}
> +	}
> +
> +	sdhci_reset(host, mask);
> +}
> +
>  static const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.set_clock		= sdhci_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
> @@ -269,7 +288,7 @@ static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = {
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
>  	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> -	.reset			= sdhci_reset,
> +	.reset			= rk35xx_sdhci_reset,
>  	.adma_write_desc	= dwcmshc_adma_write_desc,
>  };
>  
> @@ -292,6 +311,13 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
>  	int err;
>  	struct rk3568_priv *priv = dwc_priv->priv;
>  
> +	priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc));

The devm_reset_control_array_get_exclusive() never returns NULL.

The devm_reset_control_array_get_optional_exclusive(() may return NULL
if reset is missing in DT, perhaps that's what you actually want?

> +	if (IS_ERR_OR_NULL(priv->reset)) {
> +		err = PTR_ERR(priv->reset);

NULL isn't a error

> +		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);

dev_err_probe()?
Adrian Hunter April 27, 2022, 7:50 a.m. UTC | #2
On 22/04/22 20:09, Sebastian Reichel wrote:
> From: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> 
> The reset function build in the SDHCI will not reset the logic
> circuit related to the tuning function, which may cause data
> reading errors. Resetting the complete SDHCI controller through
> the reset controller fixes the issue.
> 
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> [rebase]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 28 +++++++++++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index bac874ab0b33..d95ae6ca1256 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -15,6 +15,7 @@
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/reset.h>
>  #include <linux/sizes.h>
>  
>  #include "sdhci-pltfm.h"
> @@ -63,6 +64,7 @@
>  struct rk3568_priv {
>  	/* Rockchip specified optional clocks */
>  	struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS];
> +	struct reset_control *reset;
>  	u8 txclk_tapnum;
>  };
>  
> @@ -255,6 +257,23 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
>  	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
>  }
>  
> +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;
> +
> +	if (mask & SDHCI_RESET_ALL) {
> +		if (!IS_ERR_OR_NULL(priv->reset)) {
> +			reset_control_assert(priv->reset);
> +			udelay(1);
> +			reset_control_deassert(priv->reset);
> +		}
> +	}
> +
> +	sdhci_reset(host, mask);
> +}
> +
>  static const struct sdhci_ops sdhci_dwcmshc_ops = {
>  	.set_clock		= sdhci_set_clock,
>  	.set_bus_width		= sdhci_set_bus_width,
> @@ -269,7 +288,7 @@ static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = {
>  	.set_bus_width		= sdhci_set_bus_width,
>  	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
>  	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
> -	.reset			= sdhci_reset,
> +	.reset			= rk35xx_sdhci_reset,
>  	.adma_write_desc	= dwcmshc_adma_write_desc,
>  };
>  
> @@ -292,6 +311,13 @@ static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
>  	int err;
>  	struct rk3568_priv *priv = dwc_priv->priv;
>  
> +	priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc));
> +	if (IS_ERR_OR_NULL(priv->reset)) {
> +		err = PTR_ERR(priv->reset);
> +		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
> +		return err;
> +	}
> +
>  	priv->rockchip_clks[0].id = "axi";
>  	priv->rockchip_clks[1].id = "block";
>  	priv->rockchip_clks[2].id = "timer";
diff mbox series

Patch

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index bac874ab0b33..d95ae6ca1256 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -15,6 +15,7 @@ 
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/reset.h>
 #include <linux/sizes.h>
 
 #include "sdhci-pltfm.h"
@@ -63,6 +64,7 @@ 
 struct rk3568_priv {
 	/* Rockchip specified optional clocks */
 	struct clk_bulk_data rockchip_clks[RK3568_MAX_CLKS];
+	struct reset_control *reset;
 	u8 txclk_tapnum;
 };
 
@@ -255,6 +257,23 @@  static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
 	sdhci_writel(host, extra, DWCMSHC_EMMC_DLL_STRBIN);
 }
 
+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;
+
+	if (mask & SDHCI_RESET_ALL) {
+		if (!IS_ERR_OR_NULL(priv->reset)) {
+			reset_control_assert(priv->reset);
+			udelay(1);
+			reset_control_deassert(priv->reset);
+		}
+	}
+
+	sdhci_reset(host, mask);
+}
+
 static const struct sdhci_ops sdhci_dwcmshc_ops = {
 	.set_clock		= sdhci_set_clock,
 	.set_bus_width		= sdhci_set_bus_width,
@@ -269,7 +288,7 @@  static const struct sdhci_ops sdhci_dwcmshc_rk3568_ops = {
 	.set_bus_width		= sdhci_set_bus_width,
 	.set_uhs_signaling	= dwcmshc_set_uhs_signaling,
 	.get_max_clock		= sdhci_pltfm_clk_get_max_clock,
-	.reset			= sdhci_reset,
+	.reset			= rk35xx_sdhci_reset,
 	.adma_write_desc	= dwcmshc_adma_write_desc,
 };
 
@@ -292,6 +311,13 @@  static int dwcmshc_rk3568_init(struct sdhci_host *host, struct dwcmshc_priv *dwc
 	int err;
 	struct rk3568_priv *priv = dwc_priv->priv;
 
+	priv->reset = devm_reset_control_array_get_exclusive(mmc_dev(host->mmc));
+	if (IS_ERR_OR_NULL(priv->reset)) {
+		err = PTR_ERR(priv->reset);
+		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
+		return err;
+	}
+
 	priv->rockchip_clks[0].id = "axi";
 	priv->rockchip_clks[1].id = "block";
 	priv->rockchip_clks[2].id = "timer";