[PATCH 0/2] staging: most: dim2: fix error handling in fsl_mx6_enable

Artem Lytkin posted 2 patches 2 weeks, 2 days ago
drivers/staging/most/dim2/dim2.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
[PATCH 0/2] staging: most: dim2: fix error handling in fsl_mx6_enable
Posted by Artem Lytkin 2 weeks, 2 days ago
Fix error handling issues in fsl_mx6_enable() in the DIM2 driver.

Patch 1 fixes a real bug: the return value of clk_prepare_enable()
for the PLL clock is not checked, while the same call for the MLB
clock is properly checked earlier in the function.

Patch 2 addresses review feedback from Dan Carpenter on an earlier
attempt to fix this function [1]: replace IS_ERR_OR_NULL() with
IS_ERR(), use dev_err_probe() to propagate actual error codes and
handle probe deferral correctly.

[1] https://lore.kernel.org/linux-staging/ZLUnW2CrhFMBCCa1@moroto/

Artem Lytkin (2):
  staging: most: dim2: check return value of clk_prepare_enable for PLL
  staging: most: dim2: fix error handling in fsl_mx6_enable

 drivers/staging/most/dim2/dim2.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

-- 
2.43.0
[PATCH v2 0/3] staging: most: dim2: clean up error handling in fsl_mx6_enable
Posted by Artem Lytkin 1 week, 2 days ago
This series cleans up error handling in fsl_mx6_enable(), split into
separate logical changes per Dan Carpenter's review of v1.

Changes since v1:
  - Split combined patch into 3 separate patches (one change per patch)
  - Extended string indirection fix to all three enable functions

Patch 1 replaces IS_ERR_OR_NULL() with IS_ERR() since devm_clk_get()
never returns NULL.

Patch 2 replaces dev_err() + hardcoded -EFAULT with dev_err_probe()
and PTR_ERR() to propagate correct error codes and handle probe
deferral properly.

Patch 3 removes unnecessary string indirection in dev_err() calls
("%s\n", "message" -> "message\n").

Artem Lytkin (3):
  staging: most: dim2: replace IS_ERR_OR_NULL with IS_ERR for
    devm_clk_get
  staging: most: dim2: use dev_err_probe and proper error codes for
    clock
  staging: most: dim2: remove unnecessary string indirection in dev_err

 drivers/staging/most/dim2/dim2.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

-- 
2.43.0
Re: [PATCH v2 0/3] staging: most: dim2: clean up error handling in fsl_mx6_enable
Posted by Greg Kroah-Hartman 1 week, 1 day ago
On Tue, Feb 24, 2026 at 04:37:20AM +0000, Artem Lytkin wrote:
> This series cleans up error handling in fsl_mx6_enable(), split into
> separate logical changes per Dan Carpenter's review of v1.
> 
> Changes since v1:
>   - Split combined patch into 3 separate patches (one change per patch)
>   - Extended string indirection fix to all three enable functions
> 
> Patch 1 replaces IS_ERR_OR_NULL() with IS_ERR() since devm_clk_get()
> never returns NULL.
> 
> Patch 2 replaces dev_err() + hardcoded -EFAULT with dev_err_probe()
> and PTR_ERR() to propagate correct error codes and handle probe
> deferral properly.
> 
> Patch 3 removes unnecessary string indirection in dev_err() calls
> ("%s\n", "message" -> "message\n").
> 
> Artem Lytkin (3):
>   staging: most: dim2: replace IS_ERR_OR_NULL with IS_ERR for
>     devm_clk_get
>   staging: most: dim2: use dev_err_probe and proper error codes for
>     clock
>   staging: most: dim2: remove unnecessary string indirection in dev_err
> 
>  drivers/staging/most/dim2/dim2.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> -- 
> 2.43.0
> 

I see 2 different copies of this series, as can be seen on
lore.kernel.org:
	https://lore.kernel.org/all/20260224043723.1828-1-iprintercanon@gmail.com/

which is "correct"?

Can you send a v3 so I know what to do?

thanks,

greg k-h
[PATCH v2 1/3] staging: most: dim2: replace IS_ERR_OR_NULL with IS_ERR for devm_clk_get
Posted by Artem Lytkin 1 week, 2 days ago
devm_clk_get() never returns NULL, so IS_ERR_OR_NULL() checks are
unnecessary. Replace them with IS_ERR() for both the "mlb" and
"pll8_mlb" clock lookups in fsl_mx6_enable().

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 92c7a7d8fe7e..cea1ba99caf7 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -921,7 +921,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	int ret;
 
 	dev->clk = devm_clk_get(&pdev->dev, "mlb");
-	if (IS_ERR_OR_NULL(dev->clk)) {
+	if (IS_ERR(dev->clk)) {
 		dev_err(&pdev->dev, "unable to get mlb clock\n");
 		return -EFAULT;
 	}
@@ -935,7 +935,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	if (dev->clk_speed >= CLK_2048FS) {
 		/* enable pll */
 		dev->clk_pll = devm_clk_get(&pdev->dev, "pll8_mlb");
-		if (IS_ERR_OR_NULL(dev->clk_pll)) {
+		if (IS_ERR(dev->clk_pll)) {
 			dev_err(&pdev->dev, "unable to get mlb pll clock\n");
 			clk_disable_unprepare(dev->clk);
 			return -EFAULT;
-- 
2.43.0
[PATCH v2 2/3] staging: most: dim2: use dev_err_probe and proper error codes for clock
Posted by Artem Lytkin 1 week, 2 days ago
Replace hardcoded -EFAULT returns with dev_err_probe() and PTR_ERR()
when devm_clk_get() fails in fsl_mx6_enable(). This ensures the
correct error code is propagated (e.g. -EPROBE_DEFER for deferred
probing) and avoids log noise during probe deferral.

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index cea1ba99caf7..e4c8b4995c61 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -921,10 +921,9 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	int ret;
 
 	dev->clk = devm_clk_get(&pdev->dev, "mlb");
-	if (IS_ERR(dev->clk)) {
-		dev_err(&pdev->dev, "unable to get mlb clock\n");
-		return -EFAULT;
-	}
+	if (IS_ERR(dev->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk),
+				     "unable to get mlb clock\n");
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
@@ -936,9 +935,9 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 		/* enable pll */
 		dev->clk_pll = devm_clk_get(&pdev->dev, "pll8_mlb");
 		if (IS_ERR(dev->clk_pll)) {
-			dev_err(&pdev->dev, "unable to get mlb pll clock\n");
 			clk_disable_unprepare(dev->clk);
-			return -EFAULT;
+			return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk_pll),
+					     "unable to get mlb pll clock\n");
 		}
 
 		writel(0x888, dev->io_base + 0x38);
-- 
2.43.0
[PATCH v2 3/3] staging: most: dim2: remove unnecessary string indirection in dev_err
Posted by Artem Lytkin 1 week, 2 days ago
Replace dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed") with
the direct format string dev_err(&pdev->dev, "clk_prepare_enable
failed\n"). The extra level of indirection through %s is unnecessary.

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index e4c8b4995c61..66617e89e028 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -927,7 +927,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
@@ -975,7 +975,7 @@ static int rcar_gen2_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
@@ -1020,7 +1020,7 @@ static int rcar_gen3_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
-- 
2.43.0
[PATCH v2 0/3] staging: most: dim2: clean up error handling in fsl_mx6_enable
Posted by Artem Lytkin 1 week, 2 days ago
This series cleans up error handling in fsl_mx6_enable(), split into
separate logical changes per Dan Carpenter's review of v1.

Changes since v1:
  - Split combined patch into 3 separate patches (one change per patch)
  - Extended string indirection fix to all three enable functions

Patch 1 replaces IS_ERR_OR_NULL() with IS_ERR() since devm_clk_get()
never returns NULL.

Patch 2 replaces dev_err() + hardcoded -EFAULT with dev_err_probe()
and PTR_ERR() to propagate correct error codes and handle probe
deferral properly.

Patch 3 removes unnecessary string indirection in dev_err() calls
("%s\n", "message" -> "message\n").

Artem Lytkin (3):
  staging: most: dim2: replace IS_ERR_OR_NULL with IS_ERR for
    devm_clk_get
  staging: most: dim2: use dev_err_probe and proper error codes for
    clock
  staging: most: dim2: remove unnecessary string indirection in dev_err

 drivers/staging/most/dim2/dim2.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

-- 
2.43.0
[PATCH v2 1/3] staging: most: dim2: replace IS_ERR_OR_NULL with IS_ERR for devm_clk_get
Posted by Artem Lytkin 1 week, 2 days ago
devm_clk_get() never returns NULL, so IS_ERR_OR_NULL() checks are
unnecessary. Replace them with IS_ERR() for both the "mlb" and
"pll8_mlb" clock lookups in fsl_mx6_enable().

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index 92c7a7d8fe7e..cea1ba99caf7 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -921,7 +921,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	int ret;
 
 	dev->clk = devm_clk_get(&pdev->dev, "mlb");
-	if (IS_ERR_OR_NULL(dev->clk)) {
+	if (IS_ERR(dev->clk)) {
 		dev_err(&pdev->dev, "unable to get mlb clock\n");
 		return -EFAULT;
 	}
@@ -935,7 +935,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	if (dev->clk_speed >= CLK_2048FS) {
 		/* enable pll */
 		dev->clk_pll = devm_clk_get(&pdev->dev, "pll8_mlb");
-		if (IS_ERR_OR_NULL(dev->clk_pll)) {
+		if (IS_ERR(dev->clk_pll)) {
 			dev_err(&pdev->dev, "unable to get mlb pll clock\n");
 			clk_disable_unprepare(dev->clk);
 			return -EFAULT;
-- 
2.43.0
[PATCH v2 2/3] staging: most: dim2: use dev_err_probe and proper error codes for clock
Posted by Artem Lytkin 1 week, 2 days ago
Replace hardcoded -EFAULT returns with dev_err_probe() and PTR_ERR()
when devm_clk_get() fails in fsl_mx6_enable(). This ensures the
correct error code is propagated (e.g. -EPROBE_DEFER for deferred
probing) and avoids log noise during probe deferral.

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index cea1ba99caf7..e4c8b4995c61 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -921,10 +921,9 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 	int ret;
 
 	dev->clk = devm_clk_get(&pdev->dev, "mlb");
-	if (IS_ERR(dev->clk)) {
-		dev_err(&pdev->dev, "unable to get mlb clock\n");
-		return -EFAULT;
-	}
+	if (IS_ERR(dev->clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk),
+				     "unable to get mlb clock\n");
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
@@ -936,9 +935,9 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 		/* enable pll */
 		dev->clk_pll = devm_clk_get(&pdev->dev, "pll8_mlb");
 		if (IS_ERR(dev->clk_pll)) {
-			dev_err(&pdev->dev, "unable to get mlb pll clock\n");
 			clk_disable_unprepare(dev->clk);
-			return -EFAULT;
+			return dev_err_probe(&pdev->dev, PTR_ERR(dev->clk_pll),
+					     "unable to get mlb pll clock\n");
 		}
 
 		writel(0x888, dev->io_base + 0x38);
-- 
2.43.0
[PATCH v2 3/3] staging: most: dim2: remove unnecessary string indirection in dev_err
Posted by Artem Lytkin 1 week, 2 days ago
Replace dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed") with
the direct format string dev_err(&pdev->dev, "clk_prepare_enable
failed\n"). The extra level of indirection through %s is unnecessary.

Signed-off-by: Artem Lytkin <iprintercanon@gmail.com>
---
 drivers/staging/most/dim2/dim2.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/dim2/dim2.c b/drivers/staging/most/dim2/dim2.c
index e4c8b4995c61..66617e89e028 100644
--- a/drivers/staging/most/dim2/dim2.c
+++ b/drivers/staging/most/dim2/dim2.c
@@ -927,7 +927,7 @@ static int fsl_mx6_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
@@ -975,7 +975,7 @@ static int rcar_gen2_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
@@ -1020,7 +1020,7 @@ static int rcar_gen3_enable(struct platform_device *pdev)
 
 	ret = clk_prepare_enable(dev->clk);
 	if (ret) {
-		dev_err(&pdev->dev, "%s\n", "clk_prepare_enable failed");
+		dev_err(&pdev->dev, "clk_prepare_enable failed\n");
 		return ret;
 	}
 
-- 
2.43.0