linux-kernel - Re: [PATCH v3 3/7] hwmon: (adt7x10) Use devm_hwmon_device_register_with

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: <03bf1d99-5411-daca-bc96-319e30b5e836@gmail.com>
Date:   Wed, 22 Dec 2021 08:35:32 +0200
From:   Cosmin Tanislav <demonsingur@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     cosmin.tanislav@...log.com, Jean Delvare <jdelvare@...e.com>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/7] hwmon: (adt7x10) Use
 devm_hwmon_device_register_with_info

On 12/22/21 05:08, Guenter Roeck wrote:
> On 12/21/21 1:58 PM, Cosmin Tanislav wrote:
>> From: Cosmin Tanislav <cosmin.tanislav@...log.com>
>>
> [ ... ]
> 
>> +
>> +static int adt7x10_read(struct device *dev, enum hwmon_sensor_types 
>> type,
>> +            u32 attr, int channel, long *val)
>> +{
>> +    struct adt7x10_data *data = dev_get_drvdata(dev);
>> +
>> +    switch (attr) {
>> +    case hwmon_temp_input:
>> +        return adt7x10_temp_read(data, 0, val);
>> +    case hwmon_temp_max:
>> +        return adt7x10_temp_read(data, 1, val);
>> +    case hwmon_temp_min:
>> +        return adt7x10_temp_read(data, 2, val);
>> +    case hwmon_temp_crit:
>> +        return adt7x10_temp_read(data, 3, val);
> 
> Ok, so you want to keep using the internal "index" to indicate the
> array position in the register cache. I _did_ specifically ask
> to use defines for index values in this case. You did not explain
> why you ignored this. So now you'll have to explain 1) why you
> ignored my request and 2) why you want to keep the code as is.
> 
> And, _please_, add a To: recipient to your patches. I am getting
> tired having to handle the fallout.
> 
> Guenter

First of all, maybe you should compare the time these patches were
sent to the time that your reply that "I ignored" was sent.
I sent the patches before you "specifically asked me to use defines for
index values in this case".

Second of all, this specific place is a 1:1 conversion from the original 
code. If I change to using defines here, I'll have to change to using
defines everywhere else in the code, which doesn't seem to be the scope
of this exact patch. Of course it looks bad, but it looked equally bad
before. I can introduce more following patches that fix some obvious
non-functional problems with the driver.

Third of all, why are you so tense? You're making both of our lives
harder. I understand being an upstream maintainer is hard work, but
everyone's job is hard work. It's not like I wanted to work on
refactoring ADT7x10, I just had to so I can later introduce a
patch that implements debugfs reg access...