On Thu, Jan 09, 2025 at 06:03:17PM +0100, Heiner Kallweit wrote: > A number of network PHY drivers use the following code: > > name = devm_hwmon_sanitize_name(dev, dev_name(dev)); > if (IS_ERR(name)) > return PTR_ERR(name); > devm_hwmon_device_register_with_info(dev, name, ..); > > Make this a generic fallback option and use the device name if no name > is provided to devm_hwmon_device_register_with_info(). This would allow > to simplify the affected drivers. > > Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx> > --- > drivers/hwmon/hwmon.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > index 685d4ce8d..1fd3d94e1 100644 > --- a/drivers/hwmon/hwmon.c > +++ b/drivers/hwmon/hwmon.c > @@ -1170,6 +1170,11 @@ devm_hwmon_device_register_with_info(struct device *dev, const char *name, > if (!dev) > return ERR_PTR(-EINVAL); > > + if (!name) > + name = devm_hwmon_sanitize_name(dev, dev_name(dev)); > + if (IS_ERR(name)) > + return ERR_CAST(name); > + That introduces an undiscussed change: It handles an ERR_PTR() passed as name parameter, not just NULL. Why would that be warranted ? It may not look like much, but it introduces an inconsistency: Other pointers (dev, chip) are not checked against ERR_PTR(). It is also not immediately obvious to me why the driver would want to check if the passed name is an ER_PTR(). Guenter > ptr = devres_alloc(devm_hwmon_release, sizeof(*ptr), GFP_KERNEL); > if (!ptr) > return ERR_PTR(-ENOMEM); > -- > 2.47.1 >
