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

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

[Guenter Roeck]

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(+)

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
-- 
2.39.2

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

[Joe Perches]

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

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@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
>