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

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:   Mon, 30 Oct 2023 07:59:12 +0000
From:   "Miclaus, Antoniu" <Antoniu.Miclaus@...log.com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     Jean Delvare <jdelvare@...e.com>, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>
Subject: RE: [PATCH v5 2/2] hwmon: ltc2991: add driver support


> 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@...log.com>
> 
> 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