> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Sunday, January 05, 2020 8:35 PM
> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>; robh+dt@xxxxxxxxxx;
> vijaykhemka@xxxxxx
> Cc: linux-hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> Subject: Re: [RFC PATCH hwmon-next v1 5/5] hwmon: (pmbus/tps53679) Extend
> device list supported by driver
>
> On 1/5/20 8:44 AM, Vadim Pasternak wrote:
> >
> >
> >> -----Original Message-----
> >> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> >> Sent: Sunday, January 05, 2020 6:04 PM
> >> To: Vadim Pasternak <vadimp@xxxxxxxxxxxx>; robh+dt@xxxxxxxxxx;
> >> vijaykhemka@xxxxxx
> >> Cc: linux-hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx
> >> Subject: Re: [RFC PATCH hwmon-next v1 5/5] hwmon: (pmbus/tps53679)
> >> Extend device list supported by driver
> >>
> >> On 1/5/20 2:58 AM, Vadim Pasternak wrote:
> >>> Extends driver with support of the additional devices:
> >>> Texas Instruments Dual channel DCAP+ multiphase controllers:
> >>> TPS53688, SN1906016.
> >>> Infineon Multi-phase Digital VR Controller Sierra devices
> >>> XDPE12286C, XDPE12284C, XDPE12283C, XDPE12254C and XDPE12250C.
> >>>
> >>> Extend Kconfig with added devices.
> >>>
> >>> Signed-off-by: Vadim Pasternak <vadimp@xxxxxxxxxxxx>
> >>> ---
> >>> drivers/hwmon/pmbus/Kconfig | 5 +++--
> >>> drivers/hwmon/pmbus/tps53679.c | 14 ++++++++++++++
> >>> 2 files changed, 17 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/hwmon/pmbus/Kconfig
> >>> b/drivers/hwmon/pmbus/Kconfig index 59859979571d..9e3d197d5322
> >>> 100644
> >>> --- a/drivers/hwmon/pmbus/Kconfig
> >>> +++ b/drivers/hwmon/pmbus/Kconfig
> >>> @@ -200,10 +200,11 @@ config SENSORS_TPS40422
> >>> be called tps40422.
> >>>
> >>> config SENSORS_TPS53679
> >>> - tristate "TI TPS53679"
> >>> + tristate "TI TPS53679, TPS53688, SN1906016, Infineon XDPE122xxx
> >> family"
> >>> help
> >>> If you say yes here you get hardware monitoring support for TI
> >>> - TPS53679.
> >>> + TPS53679, PS53688, SN1906016 and Infineon XDPE12286C,
> >> XDPE12284C,
> >>
> >> TPS53688. For the others, for some I can't even determine if they
> >> exist in the first place (eg SN1906016, XPDE12250C) or how they would
> >> differ from other variants (eg XPDE12284C vs. XPDE12284A).
> >> And why would they all use the same bit map in the VOUT_MODE
> >> register, the same number of PMBus pages (phases), and the same attributes
> in each page ?
> >
> > Hi Guenter,
> >
> > Thank you for reply.
> >
> > On our new system we have device XPDE12284C equipped.
> > I tested this device.
> >
> Sounds good, but did you also make sure that all chips have the same number of
> pages (phases), the same set of commands as the TI chip, and support the same
> bit settings in VOUT_MODE ? It seems a bit unlikely that TI's register definitions
> would make it into an Infineon chip.
>
> Also, what about the SN1906016 ? I don't find that anywhere, except in one
> place where it is listed as MCU from TI.
I'll drop SN1906016.
Datasheet has a title Dual channel DCAP+ multiphase controllers:
TPS53688, SN1906016.
But maybe it's some custom device (anyway I'll try to check it with TI).
>
> > Infineon datasheet refers all these device as XDPE122xxC family and it
> > doesn't specify any differences in register map between these devices.
>
> That is a bit vague, especially when it includes devices which return zero results
> with Google searches.
>
> "A" vs. "C" may distinguish automotive vs. commercial; the "A" device is listed
> under automotive. If the command set is the same, I don't really want the "c" in
> the id.
Got feedback from Infineon guys.
No need 'C' at the end, as you wrote.
All XDPE12250, XDPE12254, XDPE12283, XDPE12284, XDPE12286 are
treated in the same way:
same pages, same VOUT_MODE, VOUT_READ, etcetera.
>
> > Tomorrow we'll have guys from Infineon in our lab and I'll verify if
> > there is any difference.
>
> Tell them that it isn't really helpful to keep their datasheets under wrap.
> Unfortunately, TI started doing the same, which isn't helpful either.
Told them about datasheets availability - got :)
>
> Thanks,
> Guenter
>
> > If yes, I'll leave only XPDE12284C.
> >
> >>
> >> Thanks,
> >> Guenter
> >>
> >>> + XDPE12283C, XDPE12254C, XDPE12250C devices.
> >>>
> >>> This driver can also be built as a module. If so, the module will
> >>> be called tps53679.
> >>> diff --git a/drivers/hwmon/pmbus/tps53679.c
> >>> b/drivers/hwmon/pmbus/tps53679.c index 7ce2fca4acde..f38eb116735b
> >>> 100644
> >>> --- a/drivers/hwmon/pmbus/tps53679.c
> >>> +++ b/drivers/hwmon/pmbus/tps53679.c
> >>> @@ -89,6 +89,13 @@ static int tps53679_probe(struct i2c_client
> >>> *client,
> >>>
> >>> static const struct i2c_device_id tps53679_id[] = {
> >>> {"tps53679", 0},
> >>> + {"tps53688", 0},
> >>> + {"sn1906016", 0},
> >>> + {"xdpe12283c", 0},
> >>> + {"xdpe12250c", 0},
> >>> + {"xdpe12254c", 0},
> >>> + {"xdpe12284c", 0},
> >>> + {"xdpe12286c", 0},
> >>
> >> Alphabetic order, please.
> >>
> >>> {}
> >>> };
> >>>
> >>> @@ -96,6 +103,13 @@ MODULE_DEVICE_TABLE(i2c, tps53679_id);
> >>>
> >>> static const struct of_device_id __maybe_unused tps53679_of_match[] =
> {
> >>> {.compatible = "ti,tps53679"},
> >>> + {.compatible = "ti,tps53688"},
> >>> + {.compatible = "ti,sn1906016"},
> >>> + {.compatible = "infineon,xdpe12283c"},
> >>> + {.compatible = "infineon,xdpe12250c"},
> >>> + {.compatible = "infineon,xdpe12254c"},
> >>> + {.compatible = "infineon,xdpe12284c"},
> >>> + {.compatible = "infineon,xdpe12286c"},
> >>> {}
> >>> };
> >>> MODULE_DEVICE_TABLE(of, tps53679_of_match);
> >>>
> >
