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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
> Sent: Monday, January 06, 2020 4:53 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/6/20 4:16 AM, Vadim Pasternak wrote:
> >
> >
> >> -----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).
> >
> 
> Or maybe SN1906016 means something else. Unless we have explicit
> confirmation that the chip exists (or will exist) we should not add it to the list.
> 
> >>
> >>> 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.
> >
> 
> And same as TI, including VOUT_MODE ? Also, did they confirm that the
> unpublished chips do or will actually exist ?

Hi Gunther,

According to the input I got from Infineon guys, these device are already
available for the customers, like XPDE12284, which is equipped on new
Mellanox 400Gx32 Ethernet system, on which we are working now.

But I'll re-check if all these devices are available today to be on the safe
Side.

Regarding VOUT modes:
TPS53679 uses modes - 0x01, 0x02, 0x04, 0x05, 0x07
TPS53688 uses modes - 0x04, 0x07
XDPE122xxx uses modes - 0x01, 0x02, 0x03 and additionally 0x10, which is
for 6.25mV VID table (AMD application).

I didn't add support for mode 0x10 in the patch.

The VID table for the AMD application is different from the
Intel VID tables.

A value of 0x0 corresponds to the highest output voltage of 1.55V.
The voltage is reduced in 6.25mV steps down to the value 0xd8,
which corresponds to 0.2V.

The formula for the calculation of the output voltage would be:

	case DON’T_NOW_HOW_TO_CALL_IT:
		if (val >= 0x00 && val <= 0xd8)
              			rv = 1550 – (val *6.25);

I doubled check it.

Do you think it should added as well?

Thanks,
Vadim.

> 
> Sorry, to be persistent, but give my thanks to Infineon.
> 
> >>
> >>> 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 :)
> >
> 
> Surprise.
> 
> Thanks,
> Guenter




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux