linux-kernel - Re: [PATCH] hwmon/pmbus: use simple i2c probe function

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]

Date:   Thu, 6 Aug 2020 14:48:58 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Stephen Kitt <steve@....org>
Cc:     Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hwmon/pmbus: use simple i2c probe function

On 8/6/20 1:12 PM, Stephen Kitt wrote:
> On Thu, 6 Aug 2020 12:15:55 -0700, Guenter Roeck <linux@...ck-us.net> wrote:
>> On 8/6/20 9:16 AM, Stephen Kitt wrote:
>>> pmbus_do_probe doesn't use the id information provided in its second
>>> argument, so this can be removed, which then allows using the
>>> single-parameter i2c probe function ("probe_new") for probes which
>>> don't use the id information either.
>>>
>>> This avoids scanning the identifier tables during probes.
>>>
>>> Additionally, in cases where the id information (driver_data) isn't
>>> used, the corresponding declarations are removed from the id_table,
>>> and .name is specified explicitly.
>>>   
>>
>> The ultimate idea seems to be to remove the "old" i2c probe function
>> entirely. This means we'll have to touch the various drivers again
>> to make that happen if they are not converted to probe_new.
>>
>> With that in mind, since we are at it, why not use probe_new() in
>> every driver and call i2c_match_id() in cases where it is actually
>> needed/used ?
> 
> Yes, I was planning on doing that in a second phase, but I can do it right
> now (perhaps as a patch series) if that would be better.
> 
>> Also, I am not convinced that replacements such as
>>
>> -	{ "ipsps1", 0 },
>> +	{ .name = "ipsps1" },
>>
>> are an improvement. I would suggest to leave that alone for
>> consistency (and to make it easier to add more devices to the
>> various drivers if that happens in the future).
> 
> From reading through all the drivers using id_table, it seems to me that we
> could do away with driver_data altogether and move all that to driver-local
> structures, in many cases covering more than just an id. By only initialising
> the elements of the structure that are really needed, I was hoping to (a)
> make it more obvious that driver_data isn’t used, and (b) allow removing it
> without touching all the code again.
> 

I don't see it as an improvement to replace a common data structure with
per-driver data structures. That sounds too much like "let's re-invent
the wheel over and over again". If that is where things are going, I'd
rather have it implemented everywhere else first. I am ok with the other
changes, but not with this.

Guenter



Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)