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

[Guenter Roeck]

On 8/6/20 1:12 PM, Stephen Kitt wrote:
> On Thu, 6 Aug 2020 12:15:55 -0700, Guenter Roeck <linux@xxxxxxxxxxxx> 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

Attachment: signature.asc
Description: OpenPGP digital signature