linux-kernel - Re: [PATCH] hwmon: (pmbus) Add support for 2nd Gen Renesas digital multiphase

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: <20200229154839.GA8048@grant-ubuntu>
Date:   Sat, 29 Feb 2020 09:48:39 -0600
From:   Grant Peltier <grantpeltier93@...il.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
        openbmc@...ts.ozlabs.org, adam.vaughn.xh@...esas.com,
        zaitsev@...gle.com
Subject: Re: [PATCH] hwmon: (pmbus) Add support for 2nd Gen Renesas digital
 multiphase

On Fri, Feb 28, 2020 at 05:55:44PM -0800, Guenter Roeck wrote:
> On 2/28/20 3:52 PM, Grant Peltier wrote:
> > Hi Guenter,
> > 
> > Thank you for your expedient review. I will need to consult with my
> > coworkers to determine a more appropriate driver name. In the meantime I
> > will make the desired changes and I will also create a document for the
> > driver, which I will submit as a linked but separate patch.
> > 
> > With regard to the part numbers, this family of parts is currently in
> > the process of being released and we have not yet published all of the
> > corresponding datasheets. However, I have been assured that all of the
> > parts listed are slated to have a datasheet published publicly in the near
> > future.
> > 
> That would be great.
> 
> As for the driver name, I had a look into drivers/hwmon/pmbus/isl68137.c,
> and I don't immediately see why the new chips would warrant a new driver.
> The only differences seem to be that VMON is a new command, and of course
> only the ISL68137 supports AVL. But then there is, for example, ISL68127,
> which is again quite similar. The only other difference as far as I can
> see is input voltage scaling, but that doesn't warrant a separate driver
> (and, of course, I have no means to validate if input voltage scaling
> is indeed different for all the new chips).
> 
> Overall I would suggest to extend the isl68137 driver. I would also
> suggest to not add separate tables for each of the rail configurations
> but use the three-phase entry as starting point, copy it, and adjust its
> values as needed.
> 
> For the multi-phase chips, I question if reporting the input voltage
> for each phase make sense. Is it really a different voltage ? For IIN
> and PIN, the question is if the registers are indeed paged, since they
> are not paged in the older chips.
> 
> Guenter

The ISL68137 is part of the first generation of our digital multiphase
parts which are all exclusively 2-rail (2-page) devices. There are a
couple of reasons that we are opting for a new driver for the new
generation of devices:

1) Gen 2 has multiple rail configurations (1, 2, or 3) with different scaling
parameters than Gen 1
2) We are planning to support some of the non-generic PMBus functions of
the Gen 2 devices using the debugfs interface.

I am currently working on point 2 and those features are not
quite ready to be included in a patch set but we wanted to move forward
with the hwmon functionality for now as that is useful on it's own.

Fair point on the global vs paged commands. I will modify the page
functions so that global commands are only read from page 0.

Thank you,
Grant