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]

Message-ID: <20200806221232.278c3878@heffalump.sk2.org>
Date:   Thu, 6 Aug 2020 22:12:32 +0200
From:   Stephen Kitt <steve@....org>
To:     Guenter Roeck <linux@...ck-us.net>
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 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.

Regards,

Stephen

Content of type "application/pgp-signature" skipped