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

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

[Artem Shimko]

Hi,

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

Patch 1 replaces udelay and usleep_range with fsleep,
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

ChangeLog:
  v1: 
    * https://lore.kernel.org/all/20260518110034.142587-1-a.shimko.dev@gmail.com/T/#t
  v2:
    * Remove redundant "Error:" prefix from dev_err_probe() message
    * Put dev_err_probe() call on a single line
    * Replace usleep_range() with fsleep(midpoint) as primary change
    * Reword commit message to clarify the purpose

Artem Shimko (3):
  mmc: sdhci-of-dwcmshc: improve delay and sleep 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 | 50 ++++++++++++-----------------
 1 file changed, 20 insertions(+), 30 deletions(-)

-- 
2.43.0

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

[Artem Shimko]

Replace usleep_range() and udelay(1) (for consistency) with fsleep()
in several places to follow kernel documentation recommendation [1]
of using fsleep() for delays in non-atomic contexts.

The fsleep() function automatically selects the optimal delay mechanism:
udelay() for delays <= 10 us, usleep_range() for 10-20000 us, and msleep()
for longer delays. This makes the code more consistent and simplifies
future adjustments if delay durations ever change.

[1] https://docs.kernel.org/timers/delay_sleep_functions.html

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

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 0b2158a7e409..4cccd6e42e60 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);
 	}
 
@@ -1136,7 +1136,7 @@ static void cv18xx_sdhci_set_tap(struct sdhci_host *host, int tap)
 
 	clk |= SDHCI_CLOCK_CARD_EN;
 	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
-	usleep_range(1000, 2000);
+	fsleep(1500);
 }
 
 static int cv18xx_retry_tuning(struct mmc_host *mmc, u32 opcode, int *cmd_error)
@@ -1490,7 +1490,7 @@ static void sdhci_eic7700_config_phy(struct sdhci_host *host)
 		val |= PHY_PAD_RXSEL_1V8;
 		sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
 	}
-	usleep_range(2000, 3000);
+	fsleep(2500);
 	sdhci_writel(host, drv | PHY_CNFG_RSTN_DEASSERT, PHY_CNFG_R);
 	sdhci_eic7700_config_phy_delay(host, dwc_priv->delay_line);
 }
@@ -1520,7 +1520,7 @@ static int sdhci_eic7700_reset_init(struct device *dev, struct eic7700_priv *pri
 		dev_err(dev, "Failed to assert reset signals: %d\n", ret);
 		return ret;
 	}
-	usleep_range(2000, 2100);
+	fsleep(2050);
 	ret = reset_control_deassert(priv->reset);
 	if (ret) {
 		dev_err(dev, "Failed to deassert reset signals: %d\n", ret);
@@ -1565,7 +1565,7 @@ static int sdhci_eic7700_delay_tuning(struct sdhci_host *host, u32 opcode)
 		ret = mmc_send_tuning(host->mmc, opcode, &cmd_error);
 		if (ret) {
 			host->ops->reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
-			usleep_range(200, 210);
+			fsleep(205);
 			if (delay_min != -1 && delay_max != -1)
 				break;
 		} else {
@@ -1736,7 +1736,7 @@ static void sdhci_eic7700_set_uhs_signaling(struct sdhci_host *host, unsigned in
 		 */
 		sdhci_writew(host, 0xffff, PHY_DLLBT_CNFG_R);
 		sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
-		usleep_range(100, 110);
+		fsleep(105);
 
 		ret = read_poll_timeout(sdhci_readb, status, status & DLL_LOCK_STS, 100, 1000000,
 					false, host, PHY_DLL_STATUS_R);
-- 
2.43.0

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

[Adrian Hunter]

On 21/05/2026 11:35, Artem Shimko wrote:
> Replace usleep_range() and udelay(1) (for consistency) with fsleep()
> in several places to follow kernel documentation recommendation [1]
> of using fsleep() for delays in non-atomic contexts.

The usleep_range()'s have presumably been chosen and validated.
They cannot be arbitrarily changed.  And then there is no point
changing udelay() to fsleep().  Please let's just leave this.

> 
> The fsleep() function automatically selects the optimal delay mechanism:
> udelay() for delays <= 10 us, usleep_range() for 10-20000 us, and msleep()
> for longer delays. This makes the code more consistent and simplifies
> future adjustments if delay durations ever change.
> 
> [1] https://docs.kernel.org/timers/delay_sleep_functions.html
> 
> Signed-off-by: Artem Shimko <a.shimko.dev@gmail.com>
> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 0b2158a7e409..4cccd6e42e60 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);
>  	}
>  
> @@ -1136,7 +1136,7 @@ static void cv18xx_sdhci_set_tap(struct sdhci_host *host, int tap)
>  
>  	clk |= SDHCI_CLOCK_CARD_EN;
>  	sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> -	usleep_range(1000, 2000);
> +	fsleep(1500);
>  }
>  
>  static int cv18xx_retry_tuning(struct mmc_host *mmc, u32 opcode, int *cmd_error)
> @@ -1490,7 +1490,7 @@ static void sdhci_eic7700_config_phy(struct sdhci_host *host)
>  		val |= PHY_PAD_RXSEL_1V8;
>  		sdhci_writew(host, val, PHY_STBPAD_CNFG_R);
>  	}
> -	usleep_range(2000, 3000);
> +	fsleep(2500);
>  	sdhci_writel(host, drv | PHY_CNFG_RSTN_DEASSERT, PHY_CNFG_R);
>  	sdhci_eic7700_config_phy_delay(host, dwc_priv->delay_line);
>  }
> @@ -1520,7 +1520,7 @@ static int sdhci_eic7700_reset_init(struct device *dev, struct eic7700_priv *pri
>  		dev_err(dev, "Failed to assert reset signals: %d\n", ret);
>  		return ret;
>  	}
> -	usleep_range(2000, 2100);
> +	fsleep(2050);
>  	ret = reset_control_deassert(priv->reset);
>  	if (ret) {
>  		dev_err(dev, "Failed to deassert reset signals: %d\n", ret);
> @@ -1565,7 +1565,7 @@ static int sdhci_eic7700_delay_tuning(struct sdhci_host *host, u32 opcode)
>  		ret = mmc_send_tuning(host->mmc, opcode, &cmd_error);
>  		if (ret) {
>  			host->ops->reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
> -			usleep_range(200, 210);
> +			fsleep(205);
>  			if (delay_min != -1 && delay_max != -1)
>  				break;
>  		} else {
> @@ -1736,7 +1736,7 @@ static void sdhci_eic7700_set_uhs_signaling(struct sdhci_host *host, unsigned in
>  		 */
>  		sdhci_writew(host, 0xffff, PHY_DLLBT_CNFG_R);
>  		sdhci_writeb(host, PHY_DLL_CTRL_ENABLE, PHY_DLL_CTRL_R);
> -		usleep_range(100, 110);
> +		fsleep(105);
>  
>  		ret = read_poll_timeout(sdhci_readb, status, status & DLL_LOCK_STS, 100, 1000000,
>  					false, host, PHY_DLL_STATUS_R);

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

[Artem Shimko]

Hi Adrian,

On Fri, May 22, 2026 at 9:01 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> On 21/05/2026 11:35, Artem Shimko wrote:
> > Replace usleep_range() and udelay(1) (for consistency) with fsleep()
> > in several places to follow kernel documentation recommendation [1]
> > of using fsleep() for delays in non-atomic contexts.
>
> The usleep_range()'s have presumably been chosen and validated.
> They cannot be arbitrarily changed.  And then there is no point
> changing udelay() to fsleep().  Please let's just leave this.

Sure.

Should I send v3 without the commit?

Thank you!
--
Best regards,
Artem

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

[Adrian Hunter]

On 22/05/2026 09:58, Artem Shimko wrote:
> Hi Adrian,
> 
> On Fri, May 22, 2026 at 9:01 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>>
>> On 21/05/2026 11:35, Artem Shimko wrote:
>>> Replace usleep_range() and udelay(1) (for consistency) with fsleep()
>>> in several places to follow kernel documentation recommendation [1]
>>> of using fsleep() for delays in non-atomic contexts.
>>
>> The usleep_range()'s have presumably been chosen and validated.
>> They cannot be arbitrarily changed.  And then there is no point
>> changing udelay() to fsleep().  Please let's just leave this.
> 
> Sure.
> 
> Should I send v3 without the commit?

Sure

[PATCH v2 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.

Acked-by: Adrian Hunter <adrian.hunter@intel.com>
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 4cccd6e42e60..925fff1c0f3a 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 v2 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 | 30 +++++++++++------------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
index 925fff1c0f3a..b16027f5dd8c 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,8 @@ 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, "No device match data found\n");
 
 	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
 				sizeof(struct dwcmshc_priv));
-- 
2.43.0

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

[Adrian Hunter]

On 21/05/2026 11:35, 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>

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

> ---
>  drivers/mmc/host/sdhci-of-dwcmshc.c | 30 +++++++++++------------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-dwcmshc.c b/drivers/mmc/host/sdhci-of-dwcmshc.c
> index 925fff1c0f3a..b16027f5dd8c 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,8 @@ 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, "No device match data found\n");
>  
>  	host = sdhci_pltfm_init(pdev, &pltfm_data->pdata,
>  				sizeof(struct dwcmshc_priv));