On 1/13/20 10:54 PM, Vadim Pasternak wrote:
-----Original Message-----
From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
Sent: Monday, January 13, 2020 10:56 PM
To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
Cc: robh+dt@xxxxxxxxxx; vijaykhemka@xxxxxx; linux-hwmon@xxxxxxxxxxxxxxx;
devicetree@xxxxxxxxxxxxxxx; Michael Shych <michaelsh@xxxxxxxxxxxx>; Ofer
Levy <oferl@xxxxxxxxxxxx>
Subject: Re: [RFC PATCH hwmon-next v1 5/5] hwmon: (pmbus/tps53679) Extend
device list supported by driver
Hi Vadim,
On Mon, Jan 13, 2020 at 12:25:44PM +0000, Vadim Pasternak wrote:
-----Original Message-----
From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
Sent: Wednesday, January 08, 2020 6:48 PM
To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
Cc: robh+dt@xxxxxxxxxx; vijaykhemka@xxxxxx;
linux-hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; Michael
Shych <michaelsh@xxxxxxxxxxxx>
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.
Hi Guenter,
I didn't think about such possibility.
It sounds promising.
So, we can add our platform driver to "drivers/platform/mellanox",
which can:
- fetch "vrm" mapping per each relevant device.
- for each allocate device node, create properties table with vrm mode
mapping like "vrm-mode-vr11", "vrm-mode-vr12", "vrm-mode-vr13",
"vrm-mode-imvp9", "vrm-mode-amd625mv", coded accordingly to
"enum vrm_version".
- attach this node to "i2c_board_info" and instantiate it with
i2c_new_device() for "pmbus" type.
And i"pmbus" will get these properties like
device_property_read_8(dev, "vrm-mode-vr11")) etcetera.
Did I get your suggestion correctly?
Correct. We'll need a bindings document, though, to make it official.
The binding would look something like "vrm-mode = <number>", where
<number> is well defined (possibly in an include file). There are many
bindings include files which you can use as examples.
We'll need to get DT maintainer approval for the exact binding name;
maybe it has to be "pmbus,vrm-mode" or something like that.
Thanks,
Guenter
