Re: [PATCH] hwmon: Fix WARN_ON() always called from devm_hwmon_device_unregister()

[PJ Waskiewicz]

On Thu, 2024-09-12 at 07:13 -0700, Guenter Roeck wrote:
> On 9/12/24 02:14, Matthew Sanders wrote:
> > devm_hwmon_device_unregister() only takes parent of a devres-
> > managed
> > hwmon device as an argument. This always fails, as devres can't
> > find
> > the hwmon device it needs to unregister with the parent device
> > alone.
> > Without this patch, the WARN_ON() in devm_hwmon_device_unregister()
> > will
> > always be displayed unconditionally:
> > 
> > [    7.969746] WARNING: CPU: 1 PID: 224 at
> > drivers/hwmon/hwmon.c:1205 devm_hwmon_device_unregister+0x28/0x30
> > 
> > This patch adds an extra argument to
> > devm_hwmon_device_unregister(), a
> > pointer to a hwmon device which was previously registered to the
> > parent using devres.
> > 
> > There aren't any drivers which currently make use of this function,
> > so
> > any existing users of devm_hwmon_* shouldn't require any changes as
> > a
> > result of this patch.
> 
> If there are no users, there is no need to keep the function. We
> should drop
> it instead.

There aren't any direct in-tree users of the function.  But some out-
of-tree drivers can find it useful to use, hence Matt hitting this
issue.

If there's a desire to just remove it, that's fine.  But it would
remove a handy hook for out-of-tree stuff.

-PJ

> 
> Also, your patch is not signed and therefore can not be applied.
> 
> Guenter
> 
> > ---
> >   drivers/hwmon/hwmon.c | 6 ++++--
> >   include/linux/hwmon.h | 2 +-
> >   2 files changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> > index a362080d41fa..84945a276320 100644
> > --- a/drivers/hwmon/hwmon.c
> > +++ b/drivers/hwmon/hwmon.c
> > @@ -1199,10 +1199,12 @@ static int devm_hwmon_match(struct device
> > *dev, void *res, void *data)
> >    * devm_hwmon_device_unregister - removes a previously registered
> > hwmon device
> >    *
> >    * @dev: the parent device of the device to unregister
> > + * @hwdev: the hwmon device to unregister
> >    */
> > -void devm_hwmon_device_unregister(struct device *dev)
> > +void devm_hwmon_device_unregister(struct device *dev, struct
> > device *hwdev)
> >   {
> > -	WARN_ON(devres_release(dev, devm_hwmon_release,
> > devm_hwmon_match, dev));
> > +	WARN_ON(devres_release(dev, devm_hwmon_release,
> > devm_hwmon_match,
> > +			       hwdev));
> >   }
> >   EXPORT_SYMBOL_GPL(devm_hwmon_device_unregister);
> >   
> > diff --git a/include/linux/hwmon.h b/include/linux/hwmon.h
> > index e94314760aab..2434c6fc17a7 100644
> > --- a/include/linux/hwmon.h
> > +++ b/include/linux/hwmon.h
> > @@ -481,7 +481,7 @@ devm_hwmon_device_register_with_info(struct
> > device *dev,
> >   				const struct attribute_group
> > **extra_groups);
> >   
> >   void hwmon_device_unregister(struct device *dev);
> > -void devm_hwmon_device_unregister(struct device *dev);
> > +void devm_hwmon_device_unregister(struct device *dev, struct
> > device *hwdev);
> >   
> >   int hwmon_notify_event(struct device *dev, enum
> > hwmon_sensor_types type,
> >   		       u32 attr, int channel);
>