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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <dde6077b-83e8-24fa-440a-180ce8123080@roeck-us.net>
Date:   Sun, 2 Jul 2023 16:38:14 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Joe Perches <joe@...ches.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        cocci <cocci@...teme.lip6.fr>
Cc:     Andy Whitcroft <apw@...onical.com>,
        Dwaipayan Ray <dwaipayanray1@...il.com>,
        Lukas Bulwahn <lukas.bulwahn@...il.com>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] checkpatch: Add old hwmon APIs to deprecated list

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