Re: [PATCH] Remove error prints for devm_add_action_or_reset()

[Uwe Kleine-König <ukleinek@xxxxxxxxxx>]

Hello,

On Tue, Jul 01, 2025 at 05:03:33PM +0200, Waqar Hameed wrote:
>  drivers/pwm/pwm-meson.c                          | 3 +--

Looking at this driver I tried the following:

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 3809baed42f3..58a2ab74f14c 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -5062,7 +5062,7 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
  *
  * Returns @err.
  */
-int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
+int _dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
 {
 	va_list vargs;
 
@@ -5075,7 +5075,7 @@ int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(dev_err_probe);
+EXPORT_SYMBOL_GPL(_dev_err_probe);
 
 /**
  * dev_warn_probe - probe error check and log helper
diff --git a/include/linux/dev_printk.h b/include/linux/dev_printk.h
index eb2094e43050..23ef250727f1 100644
--- a/include/linux/dev_printk.h
+++ b/include/linux/dev_printk.h
@@ -275,7 +275,13 @@ do {									\
 	WARN_ONCE(condition, "%s %s: " format, \
 			dev_driver_string(dev), dev_name(dev), ## arg)
 
-__printf(3, 4) int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
+__printf(3, 4) int _dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
+
+#define dev_err_probe(dev, err, ...) (								\
+	(__builtin_constant_p(err) && err == -ENOMEM) ? err : _dev_err_probe(dev, err, __VA_ARGS__)	\
+)
+
+
 __printf(3, 4) int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...);
 
 /* Simple helper for dev_err_probe() when ERR_PTR() is to be returned. */
diff --git a/include/linux/device/devres.h b/include/linux/device/devres.h
index ae696d10faff..abfa5152b5a7 100644
--- a/include/linux/device/devres.h
+++ b/include/linux/device/devres.h
@@ -157,8 +157,11 @@ static inline int __devm_add_action_or_reset(struct device *dev, void (*action)(
 	int ret;
 
 	ret = __devm_add_action(dev, action, data, name);
-	if (ret)
+	if (ret) {
+		if (ret != -ENOMEM)
+			__builtin_unreachable();
 		action(data);
+	}
 
 	return ret;
 }

With that

        ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
                                       meson->channels[i].clk);
        if (ret)
                return dev_err_probe(dev, ret,
                                     "Failed to add clk_put action\n");

from drivers/pwm/pwm-meson.c is optimized to

        ret = devm_add_action_or_reset(dev, meson_pwm_s4_put_clk,
                                       meson->channels[i].clk);
        if (ret)
                return ret;

.

I would prefer this approach, because a) there is no need to drop all
dev_err_probe()s after devm_add_action_or_reset() and b) the
dev_err_probe()s could stay for consistency in the error paths of a
driver.

Best regards
Uwe

Attachment: signature.asc
Description: PGP signature