Message ID | 20230702211450.3789779-1-linux@roeck-us.net (mailing list archive) |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | checkpatch: Add old hwmon APIs to deprecated list | expand |
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@roeck-us.net> 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 > ---.. > 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
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@roeck-us.net> > > 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 >
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
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@roeck-us.net> --- scripts/checkpatch.pl | 3 +++ 1 file changed, 3 insertions(+)