Re: [PATCH] hwmon (pmbus): Add client driver for IR35221

[Guenter Roeck <linux@roeck-us.net>]

On Fri, Apr 21, 2017 at 03:55:23PM -0500, Patrick Williams wrote:
> Sam,
> 
> On Fri, Apr 21, 2017 at 02:25:05PM +1000, Samuel Mendoza-Jonas wrote:
> > IR35221 is a Digital DC-DC Multiphase Converter
> > 
> > Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
> > ---
> > - Tested on a ppc64 system which includes several of these devices.
> > - This patch re-implements the linear reg2data/data2reg functions from
> >   pmbus-core like some other drivers in order to scale some results. Is
> >   this something that would be better off being made generic for pmbus
> >   drivers to call?
> > - The resolution of iout0 is apparently configurable between two values,
> >   however the documentation I have access to does not specify how this is
> >   actually configured - currently it is left at the default resolution in
> >   the driver.
> > 
> >  Documentation/hwmon/ir35221   |  87 ++++++++++++
> >  drivers/hwmon/pmbus/Kconfig   |  11 ++
> >  drivers/hwmon/pmbus/Makefile  |   1 +
> >  drivers/hwmon/pmbus/ir35221.c | 322 ++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 421 insertions(+)
> >  create mode 100644 Documentation/hwmon/ir35221
> >  create mode 100644 drivers/hwmon/pmbus/ir35221.c
> 
> Thanks for working on this.
> 
> Would it be possible to enhance it in a follow up to support an
> 'inN_target' also?  I see this is not documented by the sysfs-interface
> for hwmon but we likely need support for this.  There are voltage
> slewing operations we do in system manufacturing.  Without an hwmon
> interface for programming the target voltage we're going to have to
> unbind the device and manually do the i2c operations.
> 
> 
> Guenter,
> 
> I mention 'inN_target' to match 'fanN_target'.  Do you have
> any opposition to either formally adding that as an option for voltage
> readings or adding it to this specific driver?
> 

Please keep in mind that this is a hardware monitoring driver.
Are you looking for regulator functionality ? If so, it might make sense
to improve regulator support (ie support more than enable/disable
when registering with the regulator subsystem) in the pmbus infrastructure.

[ You don't have to unbind the driver to access the i2c interface directly.
  If this is for manufacturing only, you can also access the i2c interface
  directly using I2C_SLAVE_FORCE. ]

Thanks,
Guenter