[PATCH 0/3] mmc: sdhci-of-dwcmshc: Minor cleanups

[PATCH 0/3] mmc: sdhci-of-dwcmshc: Minor cleanups

[Artem Shimko]

Hello,

This series introduces three non-functional cleanups for the Synopsys
DWC MSHC driver.

Patch 1 replaces udelay(1) with fsleep(1) in Rockchip-specific paths,
following the kernel documentation recommendation for delays in
non-atomic contexts.

Patch 2 removes redundant IS_ERR() checks for bus_clk in suspend/resume
paths where the called functions already have internal protection.

Patch 3 converts error handling in probe paths to use dev_err_probe()
macro for more compact code and better deferred probe debugging.
--
Best regards,
Artem

Artem Shimko (3):
  mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()
  mmc: sdhci-of-dwcmshc: remove redundant IS_ERR() check
  mmc: sdhci-of-dwcmshc: use dev_err_probe() to simplify error paths

 drivers/mmc/host/sdhci-of-dwcmshc.c | 41 +++++++++++------------------
 1 file changed, 16 insertions(+), 25 deletions(-)

-- 
2.43.0

[PATCH 1/3] mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()

[Artem Shimko]

The driver currently uses udelay() for short delays, which performs
busy-waiting and may not be optimal even for very short durations.
While udelay() is appropriate for atomic context, these specific delays
occur in non-atomic contexts where sleeping is allowed.

Replace udelay(1) with fsleep(1) in dwcmshc_rk3568_set_clock() and
rk35xx_sdhci_reset(). The fsleep() function automatically selects the
optimal delay mechanism based on the requested duration - for
delays <= 10 microseconds it will use udelay() internally, but provides
better flexibility for future adjustments and makes the code more
consistent with kernel best practices. No functional change is intended.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 0b2158a7e409..8c3ca47c8060 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -816,7 +816,7 @@ static void dwcmshc_rk3568_set_clock(struct sdhci_host *host, unsigned int clock
 
 	/* Reset DLL */
 	sdhci_writel(host, BIT(1), DWCMSHC_EMMC_DLL_CTRL);
-	udelay(1);
+	fsleep(1);
 	sdhci_writel(host, 0x0, DWCMSHC_EMMC_DLL_CTRL);
 
 	/*
@@ -895,7 +895,7 @@ static void rk35xx_sdhci_reset(struct sdhci_host *host, u8 mask)
 
 	if (mask & SDHCI_RESET_ALL && priv->reset) {
 		reset_control_assert(priv->reset);
-		udelay(1);
+		fsleep(1);
 		reset_control_deassert(priv->reset);
 	}
 
-- 
2.43.0

[PATCH 2/3] mmc: sdhci-of-dwcmshc: remove redundant IS_ERR() check

[Artem Shimko]

The clk_disable_unprepare() function has internal protection against
ERR_PTR and NULL pointers (IS_ERR_OR_NULL). Remove the redundant
IS_ERR() check for bus_clk in dwcmshc_suspend() and in the error
path of dwcmshc_resume() to simplify the code.

Note that the clk_prepare_enable() call in dwcmshc_resume() must retain
its IS_ERR() check because clk_prepare() only handles NULL pointers,
not ERR_PTR.

No functional change intended.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 8c3ca47c8060..29bb46783ee5 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -2564,8 +2564,7 @@ static int dwcmshc_suspend(struct device *dev)
 		return ret;
 
 	clk_disable_unprepare(pltfm_host->clk);
-	if (!IS_ERR(priv->bus_clk))
-		clk_disable_unprepare(priv->bus_clk);
+	clk_disable_unprepare(priv->bus_clk);
 
 	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 
@@ -2608,8 +2607,7 @@ static int dwcmshc_resume(struct device *dev)
 disable_other_clks:
 	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
 disable_bus_clk:
-	if (!IS_ERR(priv->bus_clk))
-		clk_disable_unprepare(priv->bus_clk);
+	clk_disable_unprepare(priv->bus_clk);
 disable_clk:
 	clk_disable_unprepare(pltfm_host->clk);
 	return ret;
-- 
2.43.0

[PATCH 3/3] mmc: sdhci-of-dwcmshc: use dev_err_probe() to simplify error paths

[Artem Shimko]

Replace common pattern of dev_err() + return with dev_err_probe() in
probe functions and their callees. This macro provides standardized
error message format with symbolic error names and adds deferred probe
debugging information.

The conversion makes the code more compact and ensures consistent error
logging across all initialization paths.

Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
---
 drivers/mmc/host/sdhci-of-dwcmshc.c | 31 +++++++++++------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 29bb46783ee5..ccd5f4d6fec0 100644
--- a/drivers/mmc/host/sdhci-of-dwcmshc.c
+++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
@@ -917,11 +917,9 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
 		return -ENOMEM;
 
 	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
-	if (IS_ERR(priv->reset)) {
-		err = PTR_ERR(priv->reset);
-		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
-		return err;
-	}
+	if (IS_ERR(priv->reset))
+		return dev_err_probe(mmc_dev(host->mmc), PTR_ERR(priv->reset),
+				     "failed to get reset control\n");
 
 	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
 					    ARRAY_SIZE(clk_ids), clk_ids);
@@ -1781,10 +1779,8 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
 	dwc_priv->priv = priv;
 
 	ret = sdhci_eic7700_reset_init(dev, dwc_priv->priv);
-	if (ret) {
-		dev_err(dev, "failed to reset\n");
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to reset\n");
 
 	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
 					    ARRAY_SIZE(clk_ids), clk_ids);
@@ -1792,16 +1788,14 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
 		return ret;
 
 	ret = of_parse_phandle_with_fixed_args(dev->of_node, "eswin,hsp-sp-csr", 2, 0, &args);
-	if (ret) {
-		dev_err(dev, "Fail to parse 'eswin,hsp-sp-csr' phandle (%d)\n", ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "Fail to parse 'eswin,hsp-sp-csr' phandle\n");
 
 	hsp_regmap = syscon_node_to_regmap(args.np);
 	if (IS_ERR(hsp_regmap)) {
-		dev_err(dev, "Failed to get regmap for 'eswin,hsp-sp-csr'\n");
 		of_node_put(args.np);
-		return PTR_ERR(hsp_regmap);
+		return dev_err_probe(dev, PTR_ERR(hsp_regmap),
+				     "Failed to get regmap for 'eswin,hsp-sp-csr'\n");
 	}
 	hsp_int_status = args.args[0];
 	hsp_pwr_ctrl = args.args[1];
@@ -2408,10 +2402,9 @@ static int dwcmshc_probe(struct platform_device *pdev)
 	u32 extra, caps;
 
 	pltfm_data = device_get_match_data(&pdev->dev);
-	if (!pltfm_data) {
-		dev_err(&pdev->dev, "Error: No device match data found\n");
-		return -ENODEV;
-	}
+	if (!pltfm_data)
+		return dev_err_probe(&pdev->dev, -ENODEV,
+				     "Error: No device match data found\n");
 
 	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
 				sizeof(struct dwcmshc_priv));
-- 
2.43.0

Re: [PATCH 1/3] mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()

[Andy Shevchenko]

On Mon, May 18, 2026 at 02:00:31PM +0300, Artem Shimko wrote:
> The driver currently uses udelay() for short delays, which performs
> busy-waiting and may not be optimal even for very short durations.
> While udelay() is appropriate for atomic context, these specific delays
> occur in non-atomic contexts where sleeping is allowed.
> 
> Replace udelay(1) with fsleep(1) in dwcmshc_rk3568_set_clock() and
> rk35xx_sdhci_reset(). The fsleep() function automatically selects the
> optimal delay mechanism based on the requested duration - for
> delays <= 10 microseconds it will use udelay() internally, but provides
> better flexibility for future adjustments and makes the code more
> consistent with kernel best practices. No functional change is intended.

Read, what you wrote. With the above commit message this is no point in this
patch.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()

[Artem Shimko]

Hi Andy,

On Mon, May 18, 2026 at 5:16 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> Read, what you wrote. With the above commit message this is no point in this
> patch.

I tried to replace *sleep() with fsleep() API. It seems I
misunderstood the hint [1].

Or is that the commit description mistake?

Thank you for reviewing it.

[1] https://lore.kernel.org/all/ad-mCsvOWWgqt-rc@ashevche-desk.local/
--
Best regards,
Artem

Re: [PATCH 1/3] mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()

[Andy Shevchenko]

On Mon, May 18, 2026 at 05:50:55PM +0300, Artem Shimko wrote:
> On Mon, May 18, 2026 at 5:16 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > Read, what you wrote. With the above commit message this is no point in this
> > patch.
> 
> I tried to replace *sleep() with fsleep() API. It seems I
> misunderstood the hint [1].

Yep, the hint was basically about uses of usleep_range(). udelay for 1 or 2
microseconds is fine as is. Of course for the sake of consistency it can be
moved to fsleep() (when the context is non-atomic), but solely udelay(1) -->
fsleep(1) makes no needed churn.

> Or is that the commit description mistake?
> 
> Thank you for reviewing it.
> 
> [1] https://lore.kernel.org/all/ad-mCsvOWWgqt-rc@ashevche-desk.local/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] mmc: sdhci-of-dwcmshc: improve delay handling with fsleep()

[Artem Shimko]

Hi Andy,

On Mon, May 18, 2026 at 9:35 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> Yep, the hint was basically about uses of usleep_range().
Just to clarify before sending V2, for example (midpoint of the range):
s/usleep_range(1000, 2000)/fsleep(1500)/ ?

> microseconds is fine as is.
Since they occur in non-atomic context
(inside set_clock and reset functions), would it be worth replacing
them with fsleep(1) as well for consistency? =)

Thank you!
--
Best regards,
Artem Shimko



>
> > Or is that the commit description mistake?
> >
> > Thank you for reviewing it.
> >
> > [1] https://lore.kernel.org/all/ad-mCsvOWWgqt-rc@ashevche-desk.local/
>
> --
> With Best Regards,
> Andy Shevchenko
>
>

Re: [PATCH 3/3] mmc: sdhci-of-dwcmshc: use dev_err_probe() to simplify error paths

[Adrian Hunter]

On 18/05/2026 14:00, Artem Shimko wrote:
> Replace common pattern of dev_err() + return with dev_err_probe() in
> probe functions and their callees. This macro provides standardized
> error message format with symbolic error names and adds deferred probe
> debugging information.
> 
> The conversion makes the code more compact and ensures consistent error
> logging across all initialization paths.
> 
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 31 +++++++++++------------------
>  1 file changed, 12 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 29bb46783ee5..ccd5f4d6fec0 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -917,11 +917,9 @@ static int dwcmshc_rk35xx_init(struct device *dev, struct sdhci_host *host,
>  		return -ENOMEM;
>  
>  	priv->reset = devm_reset_control_array_get_optional_exclusive(mmc_dev(host->mmc));
> -	if (IS_ERR(priv->reset)) {
> -		err = PTR_ERR(priv->reset);
> -		dev_err(mmc_dev(host->mmc), "failed to get reset control %d\n", err);
> -		return err;
> -	}
> +	if (IS_ERR(priv->reset))
> +		return dev_err_probe(mmc_dev(host->mmc), PTR_ERR(priv->reset),
> +				     "failed to get reset control\n");
>  
>  	err = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
>  					    ARRAY_SIZE(clk_ids), clk_ids);
> @@ -1781,10 +1779,8 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
>  	dwc_priv->priv = priv;
>  
>  	ret = sdhci_eic7700_reset_init(dev, dwc_priv->priv);
> -	if (ret) {
> -		dev_err(dev, "failed to reset\n");
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to reset\n");
>  
>  	ret = dwcmshc_get_enable_other_clks(mmc_dev(host->mmc), dwc_priv,
>  					    ARRAY_SIZE(clk_ids), clk_ids);
> @@ -1792,16 +1788,14 @@ static int eic7700_init(struct device *dev, struct sdhci_host *host, struct dwcm
>  		return ret;
>  
>  	ret = of_parse_phandle_with_fixed_args(dev->of_node, "eswin,hsp-sp-csr", 2, 0, &args);
> -	if (ret) {
> -		dev_err(dev, "Fail to parse 'eswin,hsp-sp-csr' phandle (%d)\n", ret);
> -		return ret;
> -	}
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Fail to parse 'eswin,hsp-sp-csr' phandle\n");
>  
>  	hsp_regmap = syscon_node_to_regmap(args.np);
>  	if (IS_ERR(hsp_regmap)) {
> -		dev_err(dev, "Failed to get regmap for 'eswin,hsp-sp-csr'\n");
>  		of_node_put(args.np);
> -		return PTR_ERR(hsp_regmap);
> +		return dev_err_probe(dev, PTR_ERR(hsp_regmap),
> +				     "Failed to get regmap for 'eswin,hsp-sp-csr'\n");
>  	}
>  	hsp_int_status = args.args[0];
>  	hsp_pwr_ctrl = args.args[1];
> @@ -2408,10 +2402,9 @@ static int dwcmshc_probe(struct platform_device *pdev)
>  	u32 extra, caps;
>  
>  	pltfm_data = device_get_match_data(&pdev->dev);
> -	if (!pltfm_data) {
> -		dev_err(&pdev->dev, "Error: No device match data found\n");
> -		return -ENODEV;
> -	}
> +	if (!pltfm_data)
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +				     "Error: No device match data found\n");

"Error: " is especially redundant with dev_err_probe(), and it can all
be on 1 line e.g.

		return dev_err_probe(&pdev->dev, -ENODEV, "No device match data found\n");

>  
>  	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
>  				sizeof(struct dwcmshc_priv));

Re: [PATCH 2/3] mmc: sdhci-of-dwcmshc: remove redundant IS_ERR() check

[Adrian Hunter]

On 18/05/2026 14:00, Artem Shimko wrote:
> The clk_disable_unprepare() function has internal protection against
> ERR_PTR and NULL pointers (IS_ERR_OR_NULL). Remove the redundant
> IS_ERR() check for bus_clk in dwcmshc_suspend() and in the error
> path of dwcmshc_resume() to simplify the code.
> 
> Note that the clk_prepare_enable() call in dwcmshc_resume() must retain
> its IS_ERR() check because clk_prepare() only handles NULL pointers,
> not ERR_PTR.
> 
> No functional change intended.
> 
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>

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

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 8c3ca47c8060..29bb46783ee5 100644
> --- a/drivers/mmc/host/sdhci-of-dwcmshc.c
> +++ b/drivers/mmc/host/sdhci-of-dwcmshc.c
> @@ -2564,8 +2564,7 @@ static int dwcmshc_suspend(struct device *dev)
>  		return ret;
>  
>  	clk_disable_unprepare(pltfm_host->clk);
> -	if (!IS_ERR(priv->bus_clk))
> -		clk_disable_unprepare(priv->bus_clk);
> +	clk_disable_unprepare(priv->bus_clk);
>  
>  	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  
> @@ -2608,8 +2607,7 @@ static int dwcmshc_resume(struct device *dev)
>  disable_other_clks:
>  	clk_bulk_disable_unprepare(priv->num_other_clks, priv->other_clks);
>  disable_bus_clk:
> -	if (!IS_ERR(priv->bus_clk))
> -		clk_disable_unprepare(priv->bus_clk);
> +	clk_disable_unprepare(priv->bus_clk);
>  disable_clk:
>  	clk_disable_unprepare(pltfm_host->clk);
>  	return ret;

Re: [PATCH 3/3] mmc: sdhci-of-dwcmshc: use dev_err_probe() to simplify error paths

[Artem Shimko]

Hi Adrian,

On Wed, May 20, 2026 at 9:18 AM Adrian Hunter <adrian.hunter@intel.com> wrote:

> > +     if (!pltfm_data)
> > +             return dev_err_probe(&pdev->dev, -ENODEV,
> > +                                  "Error: No device match data found\n");
>
> "Error: " is especially redundant with dev_err_probe(),
Missed that.
> and it can all be on 1 line e.g.
Got it.

Thank you for your review!
--
Best Regards,
Artem