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

Uwe Kleine-König
2025-07-01T17:44:20Z

[-- Attachment #1: Type: text/plain, Size: 2980 bytes --]

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]