Re: [PATCH] checkpatch: Add old hwmon APIs to deprecated list

[Guenter Roeck]

On 7/2/23 15:42, Joe Perches wrote:
> On Sun, 2023-07-02 at 14:14 -0700, Guenter Roeck wrote:
> > hwmon_device_register() and [devm_]hwmon_device_register_with_groups()
> > have been deprecated. All hardware monitoring drivers should use
> > [devm_]hwmon_device_register_with_info() instead.
> >
> > The problem with the old API functions is that they require sysfs attribute
> > handling in driver code. The new API handles sysfs attributes in the
> > hwmon core. Using the new API typically reduces driver code size by 20-40%.
> >
> > Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
>
> Seems sensible, thanks.
>
> But how big an effort is it to convert all the existing uses
> and remove the code?  There are less than 200 uses.
>
> Perhaps it's not that onerous.
> Is it something that coccinelle could do reasonably well?
>
> $ git grep -w hwmon_device_register | wc -l
> 49
>
> $ git grep -w hwmon_device_register_with_groups | wc -l
> 22
>
> $ git grep -w devm_hwmon_device_register_with_groups | wc -l
> 108

Unfortunately that doesn't work, because the parameters are completely
different. Coccinelle is god, but not that good.

The _with_info API implements sysfs attributes in the hwmon core.
The older APIs implement sysfs attributes in the individual drivers.
I have converted a number of drivers, but it is a lot of work, and it
can only be done safely if one has access to hardware to test the result.

Problem is that I still see submissions using the old API, with arguments
such as "this other driver uses it". On top of that, there have been attempts
to abuse the with_info API by providing only the (old) groups argument.
Commit ddaefa209c4a ("hwmon: Make chip parameter for with_info API mandatory")
made that impossible, and commit aededf875a23 ("Documentation/hwmon:
Remove description of deprecated registration functions") removed the old API
calls from the documentation. Unfortunately that is still insufficient.
The next step would be to print a warning if the with_groups API is used,
but I don't want to go that far yet.

Guenter

> > ---..
> >   scripts/checkpatch.pl | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> > index 7bfa4d39d17f..6d97f1a6028e 100755
> > --- a/scripts/checkpatch.pl
> > +++ b/scripts/checkpatch.pl
> > @@ -842,6 +842,9 @@ our %deprecated_apis = (
> >   	"kunmap"				=> "kunmap_local",
> >   	"kmap_atomic"				=> "kmap_local_page",
> >   	"kunmap_atomic"				=> "kunmap_local",
> > +	"hwmon_device_register"			=> "hwmon_device_register_with_info",
> > +	"hwmon_device_register_with_groups"	=> "hwmon_device_register_with_info",
> > +	"devm_hwmon_device_register_with_groups"=> "devm_hwmon_device_register_with_info",
> >   );
> >   
> >   #Create a search pattern for all these strings to speed up a loop below