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

From: Stephen Kitt
Date: Fri Aug 07 2020 - 02:39:47 EST


On Thu, 6 Aug 2020 14:48:58 -0700, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
> 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:
[...]
> >> 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.

I agree, and I wasn’t intending on encouraging re-inventing the wheel in each
driver. Let’s focus on probe_new for now...

What did you mean by “to make it easier to add more devices to the various
drivers if that happens in the future”? There are already many drivers with
multiple devices but no driver_data, dropping the explicit driver_data
initialisation doesn’t necessarily make it harder to add devices, does it?

Regards,

Stephen

Attachment: pgpsyt9H1DdjQ.pgp
Description: OpenPGP digital signature