linux-kernel - Re: [PATCH] drivers/hwmon/pmbus: Add support for raa229141 in isl68137

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: <CAMdV+4f93oZ3jhxo+oZDCiK4tHO=gYqfiLAMoxuzMn9Wn24B3A@mail.gmail.com>
Date: Thu, 6 Nov 2025 17:33:01 +0800
From: Jeff Lin <jefflin994697@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: jdelvare@...e.com, cedricjustine.encarnacion@...log.com, 
	ninad@...ux.ibm.com, andriy.shevchenko@...ux.intel.com, 
	johnerasmusmari.geronimo@...log.com, Mariel.Tinaco@...log.com, 
	jbrunet@...libre.com, kimseer.paller@...log.com, leo.yang.sy0@...il.com, 
	nuno.sa@...log.com, chiang.brian@...entec.com, gregkh@...uxfoundation.org, 
	grantpeltier93@...il.com, peterz@...radead.org, william@...nnington.com, 
	krzysztof.kozlowski@...aro.org, tzungbi@...nel.org, thorsten.blum@...ux.dev, 
	linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drivers/hwmon/pmbus: Add support for raa229141 in isl68137

Sorry for the late reply.
On Fri, Sep 26, 2025 at 4:27 PM Guenter Roeck <linux@...ck-us.net> wrote:
>
> On 9/25/25 18:45, Jeff Lin wrote:
> > In chip RAA229141 there exist ISYS pin which can report the current data
> > for the device connected to this chip through this pin by routed by Direct
> > Memory Access(DMA) command. To read the data in ISYS pin, we have to set
> > the DMA address to 0xC5 and then read the DMA data from 0xC7. And then use
> > the Direct read format with 10mA per LSB to transfer the data in 0xC7.
> > And for ISYS input pin, the DMA address is 0xE0D3 and for ISYS output pin,
> > the DMA address is 0xEE42.
> >
> > Signed-off-by: Jeff Lin <jefflin994697@...il.com>
>
> As submitted this is a no-go, for several reasons.
>
> The description should describe what is done and why (i.e., here, describe
> the added chip), not implementation details. Those are useful as comments
> in the code, not as patch description.
>
> Introducing a new sensor class is unacceptable. This is a current (I think),
> treat it as such.
>
> Changes in the core together with other changes are unacceptable.
>
> A new virtual command (or commands) would have to be discussed and be generic.
>
> A new Kconfig symbol when adding support for a new chip to an existing driver
> is unacceptable. Besides, the new Kconfig symbol would have no effect if
> the driver supporting the chip is not enabled, so this is not only unacceptable
> but wrong.
>
> Regarding the code itself: Stick with existing coding style. Do not use C++ comments,
> declare variables at the beginning of code blocks.
Thank you for your advice. I will keep it in mind.
> I would suggest to add support for RAA229141 in one patch, then we can discuss
> what ISYS_{IN,OUT} actually measures, how it differs from IIN/IOUT, if it indeed
> requires new virtual commands and how those command might look like, or if it
> can be handled by mapping a existing commands.
>
> The datasheet for RAA229141 is not public, so be prepared to provide a detailed
> description.
For RAA229141, the controller can provide the fast input power monitor
SVID device
specified by the VR14 PSYS requirements. When using the VR14 PSYS
capability, connect
the ISYS signal to pin 44 or 45 and the VSYS signal to pin 45 or 46
(depending on
configuration).

Base on the standard version of the specification for this chip from
Renesas, we use
the config of taking pin44 as ISYS for input current sensing and pin45
as VSYS for
input voltage sensing.
For pin 44, ISYS is multifunction pin which sense the system input current.
For pin 45, VSYS is multifunction pin which sense either the voltage
near PSU or the
system input current.

However, in my machine the pin45 is customized as output current
sensing for near PSU
device.

To read these multifuction pin data, it need to use Dicrect Memory Access(DMA)
command codes.
DMA Address(Command code 0xc7): Used to set the register address to
use with other
DMA commands.
DMA Data(Command code 0xc5): Used to read from or write to the
register selected by
the DMA Address command.

And to read pin45, it needs to be routed by DMA and should be the 0xEE42, it's
direct read with 10mA/LSB; pin44 is located at 0xE0D3 and it's direct read with
10mA/LSB.

That is, write 0xEE42 to 0xc7 and then read the data from 0xc5 with
direct read with
10mA/LSB for pin45(VSYS) and it is the near PSU device's output current.

Since the address 0xc5 is different from IIN(0x89), IOUT(0x8C), I
think it might need
new command to do it.

Thank you for your review. I’ve submitted a v2 of the patch incorporating your
suggestions. I appreciate your time and feedback.
> Guenter
>
Thank you
Jeff