linux-kernel - Re: [PATCH 08/10] hwmon: (amc6821) Drop unnecessary enum chips

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <cdcedb9a-3619-4448-9047-c250e7df1a81@roeck-us.net>
Date: Mon, 1 Jul 2024 07:47:05 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Quentin Schulz <quentin.schulz@...rry.de>, linux-hwmon@...r.kernel.org
Cc: linux-kernel@...r.kernel.org, Farouk Bouabid <farouk.bouabid@...rry.de>
Subject: Re: [PATCH 08/10] hwmon: (amc6821) Drop unnecessary enum chips

On 7/1/24 04:36, Quentin Schulz wrote:
> Hi Guenter,
> 
> On 6/28/24 5:13 PM, Guenter Roeck wrote:
>> The driver only supports a single chip, so an enum
>> to determine the chip type is unnecessary. Drop it.
>>
>> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
>> ---
>>   drivers/hwmon/amc6821.c | 5 +----
>>   1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
>> index 042e2044de7b..ebffc9393c3d 100644
>> --- a/drivers/hwmon/amc6821.c
>> +++ b/drivers/hwmon/amc6821.c
>> @@ -36,8 +36,6 @@ module_param(pwminv, int, 0444);
>>   static int init = 1; /*Power-on initialization.*/
>>   module_param(init, int, 0444);
>> -enum chips { amc6821 };
>> -
>>   #define AMC6821_REG_DEV_ID        0x3D
>>   #define AMC6821_REG_COMP_ID        0x3E
>>   #define AMC6821_REG_CONF1        0x00
>> @@ -943,7 +941,7 @@ static int amc6821_probe(struct i2c_client *client)
>>   }
>>   static const struct i2c_device_id amc6821_id[] = {
>> -    { "amc6821", amc6821 },
>> +    { "amc6821", 0 },
> 
> amc6821_id being a global variable, its content should be initialized by the compiler if omitted, and the default value of kernel_ulong_t (aka unsigned long) is 0. So I think we could just remove the second parameter there.
> 
> Doesn't hurt to keep it though and it seems there's a mix of implicit and explicit initialization among the i2c kernel drivers, so with that said:
> 

I may use that inconsistently myself. I am never sure if I want to say
"keep default" or "explicitly state that the value is 0". I'll keep
it as-is.

> Reviewed-by: Quentin Schulz <quentin.schulz@...rry.de>
> 

Thanks,
Guenter