RE: [PATCH v5 2/2] hwmon: ltc2991: add driver support

From: Miclaus, Antoniu
Date: Mon Oct 30 2023 - 03:59:44 EST



> On Thu, Oct 26, 2023 at 01:33:13PM +0300, Antoniu Miclaus wrote:
> > Add support for LTC2991 Octal I2C Voltage, Current, and Temperature
> > Monitor.
> >
> > The LTC2991 is used to monitor system temperatures, voltages and
> > currents. Through the I2C serial interface, the eight monitors can
> > individually measure supply voltages and can be paired for
> > differential measurements of current sense resistors or temperature
> > sensing transistors. Additional measurements include internal
> > temperature and internal VCC.
> >
> > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx>
>
> Applied. I do have one comment (see below) about code which
> I would normally reject, but I am getting tired.
>
> [ ... ]
>
> > +
> > +struct ltc2991_state {
> > + struct device *dev;
>
> It is completely pointless to have a reference to dev in struct
> ltc2991_state because it is only used in the init function and
> dereferenced six times there. It would have been much easier to
> pass it as argument to that function. That would also have avoided
> the wrong assumption or expectation that it is needed/used elsewhere.
>
> Guenter
Sorry for the misunderstanding. I took as reference some old driver
implementations from the mainline kernel.

Is it fine if I do a follow-up patch on this subject in the near future?

Regards,
Antoniu