Re: [RFC PATCH hwmon-next v1 5/5] hwmon: (pmbus/tps53679) Extend device list supported by driver

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

Hi Vadim,

On Mon, Jan 13, 2020 at 12:25:44PM +0000, Vadim Pasternak wrote:
> 
> 
> > -----Original Message-----
> > From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck
> > Sent: Wednesday, January 08, 2020 6:48 PM
> > To: Vadim Pasternak <vadimp@mellanox.com>
> > Cc: robh+dt@kernel.org; vijaykhemka@fb.com; linux-hwmon@vger.kernel.org;
> > devicetree@vger.kernel.org; Michael Shych <michaelsh@mellanox.com>
> > Subject: Re: [RFC PATCH hwmon-next v1 5/5] hwmon: (pmbus/tps53679) Extend
> > device list supported by driver
> > 
> > On Wed, Jan 08, 2020 at 02:10:50PM +0000, Vadim Pasternak wrote:
> > >
> > > Hi Guenter,
> > >
> > > We are looking for possibility to provide some kind of flexible driver
> > > to handle different devices from different vendors, but which have
> > > common nature, like support of two pages for telemetry with same set
> > > of functions and same formats. (Actually driver could be flexible regarding the
> > number of pages).
> > >
> > > While the difference only in VID codes mapping.
> > >
> > > The motivation for that is to give free hand to HW design to choose
> > > which particular device to use on the same system type.
> > > There are two main reasons for such requirement:
> > > - Quality of device (we already had a serios problems with ucd9224 and
> > >   tps53679, caused system meltdown). In such case the intention is to move
> > >   to fallback devices in the next batches.
> > > - Device availability in stock. Sometimes vendors can't supply in time the
> > >    necessary quantity.
> > >
> > > Currently there are the devices from three vendor: TI tps536xx,
> > > Infineon
> > > xdpe122 and MPS mp2975.
> > > All have different mapping of VID codes.
> > >
> > > It could be also few additional devices, which are supposed to be used
> > > as fallback devices.
> > >
> > > We have several very big customers ordering now huge quantity of our
> > > systems, which are very conservative regarding image upgrade.
> > > Means we can provide now support for tps536xx, xdpe122xx and mp2975
> > > but in case new devices are coming soon, we will be able to support it
> > > in kernel for their image after year or even more.
> > >
> > > We can provide ACPI pmbus driver, which will read VID mapping from
> > > ACPI DSDT table. This mapping table will contain the names of
> > > available modes and specific vendor code for this mode. Like:
> > > PMBVR11=1
> > > PMBVR12=2
> > > PMBVR13=5
> > > PMBIMVP9=10
> > > And driver will set info->vrm_version[i] according to this mapping.
> > >
> > 
> > The DSDT would have to provide all properties, not just the VID modes.
> > At the very least that would have to be the number of pages, data formats, vrm
> > version, and the supported attributes per page. It should be possible to also add
> > m/b/R information for each of the sensor classes, but I guess the actual
> > implementation could be postponed until it is needed.
> > 
> > This could all be done through the existing generic driver (pmbus.c); it would not
> > really require a new driver. ACPI/DSDT could provide firmware properties, and
> > pmbus.c could read those using device_property API functions (eg
> > device_property_read_u32(dev, "vrm-mode")). Would that work for you ?
> 
> Hi Guenter,
> 
> We thought that it's possible to provide just modified DSDT with the specific
> configuration as an external table, which is loaded during system boot.
> 
> It should not be integrated into BIOS image.
> We want to avoid releasing of new BIOS or new each time system configuration
> is changed.
> New BIOS is released only when new CPU type should be supported.
> Also BIOS overwriting is not an option, sine we have to support secured BIOS.
> 
> It should not be located in initrd.
> The new system batch is released with BIOS, SMBIOS and with customer's OS or
> with install environment like ONIE.
> Means no kernel changes.
> Also not all our customers use initrd.
>  
> So, it seems there is no place, when we can locate modified DSDT and load
> it dynamically.
> But we should think more about possible methods for doing it.
> 
> Today all the static devices are instantiated  from the user space.
> It's supposed to work for us while we have to support some pre-defined set of
> devices, for which we can detect the specific configuration through DMI tables
> (SKU in particular).
> But it'll not work for some new coming devices.
> 
> We have a possibility to provide VID mapping info through CPLD device.
> But in this case we'll have to implement Mellanox specific PMBus driver aware
> of CPLD register map.
> Not sure if such approach is accepted?
> 

How about a platform driver which loads the parameters into device
properties from whatever location and instantiates the pmbus driver ?
The PMBus driver would then read the device attributes and instantiate
itself accordingly.

If the other PMBus attributes can be auto-detected, it might actually be
sufficient to have a per-page vrm-mode property and otherwise use the
auto-detect mechanism of pmbus.c.

Thanks,
Guenter