Add a device tree bindings for ltc4286 driver.
Signed-off-by: Delphine CC Chiu <[email protected]>
Changelog:
v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
- Add type for adi,vrange-select-25p6
- Revise rsense-micro-ohms to shunt-resistor-micro-ohms
---
.../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++
MAINTAINERS | 10 ++++
2 files changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
new file mode 100644
index 000000000000..17022de657bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: LTC4286 power monitors
+
+maintainers:
+ - Delphine CC Chiu <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - lltc,ltc4286
+ - lltc,ltc4287
+
+ reg:
+ maxItems: 1
+
+ adi,vrange-select-25p6:
+ description:
+ This property is a bool parameter to represent the
+ voltage range is 25.6 or not for this chip.
+ type: boolean
+
+ shunt-resistor-micro-ohms:
+ description:
+ Resistor value in micro-Ohm
+
+required:
+ - compatible
+ - reg
+ - shunt-resistor-micro-ohms
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ power-sensor@40 {
+ compatible = "lltc,ltc4286";
+ reg = <0x40>;
+ adi,vrange-select-25p6;
+ shunt-resistor-micro-ohms = <300>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 668d1e24452d..89e5fff4bba9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12466,6 +12466,16 @@ S: Maintained
F: Documentation/hwmon/ltc4261.rst
F: drivers/hwmon/ltc4261.c
+LTC4286 HARDWARE MONITOR DRIVER
+M: Delphine CC Chiu <[email protected]>
+L: [email protected]
+S: Maintained
+F: Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
+F: Documentation/devicetree/bindings/hwmon/ltc4286.rst
+F: drivers/hwmon/pmbus/Kconfig
+F: drivers/hwmon/pmbus/Makefile
+F: drivers/hwmon/pmbus/ltc4286.c
+
LTC4306 I2C MULTIPLEXER DRIVER
M: Michael Hennerich <[email protected]>
L: [email protected]
--
2.25.1
Hey,
On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> Add a device tree bindings for ltc4286 driver.
Bindings are for devices, not for drivers.
>
> Signed-off-by: Delphine CC Chiu <[email protected]>
>
> Changelog:
> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> - Add type for adi,vrange-select-25p6
> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> ---
> .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++
> MAINTAINERS | 10 ++++
> 2 files changed, 60 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> new file mode 100644
> index 000000000000..17022de657bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LTC4286 power monitors
> +
> +maintainers:
> + - Delphine CC Chiu <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - lltc,ltc4286
> + - lltc,ltc4287
I don't recall seeing an answer to Guenter about this ltc4287 device:
https://lore.kernel.org/all/[email protected]/
> +
> + reg:
> + maxItems: 1
> +
> + adi,vrange-select-25p6:
> + description:
> + This property is a bool parameter to represent the
> + voltage range is 25.6 or not for this chip.
25.6 what? Volts? microvolts?
What about Guenter's suggestion to name this so that it better matches
the other, similar properties?
Cheers,
Conor.
On 10/26/23 07:25, Conor Dooley wrote:
> Hey,
>
> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
>> Add a device tree bindings for ltc4286 driver.
>
> Bindings are for devices, not for drivers.
>
>>
>> Signed-off-by: Delphine CC Chiu <[email protected]>
>>
>> Changelog:
>> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
>> - Add type for adi,vrange-select-25p6
>> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
>> ---
>> .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++
>> MAINTAINERS | 10 ++++
>> 2 files changed, 60 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>> new file mode 100644
>> index 000000000000..17022de657bb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>> @@ -0,0 +1,50 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: LTC4286 power monitors
>> +
>> +maintainers:
>> + - Delphine CC Chiu <[email protected]>
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - lltc,ltc4286
>> + - lltc,ltc4287
>
> I don't recall seeing an answer to Guenter about this ltc4287 device:
> https://lore.kernel.org/all/[email protected]/
>
At least the chip does officially exist now, and a datasheet is available.
https://www.analog.com/en/products/ltc4287.html
It shows that coefficients for the telemetry commands are different,
meaning that indeed both chips need to be explicitly referenced
in the properties description (and handled in the driver, which proves
my point of needing a datasheet before accepting such a driver).
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + adi,vrange-select-25p6:
>> + description:
>> + This property is a bool parameter to represent the
>> + voltage range is 25.6 or not for this chip.
>
> 25.6 what? Volts? microvolts?
> What about Guenter's suggestion to name this so that it better matches
> the other, similar properties?
>
I still would prefer one of the more common properties.
I still prefer adi,vrange-high-enable.
Guenter
On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote:
> On 10/26/23 07:25, Conor Dooley wrote:
> > Hey,
> >
> > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> > > Add a device tree bindings for ltc4286 driver.
> >
> > Bindings are for devices, not for drivers.
> >
> > >
> > > Signed-off-by: Delphine CC Chiu <[email protected]>
> > >
> > > Changelog:
> > > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> > > - Add type for adi,vrange-select-25p6
> > > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> > > ---
> > > .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++
> > > MAINTAINERS | 10 ++++
> > > 2 files changed, 60 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > new file mode 100644
> > > index 000000000000..17022de657bb
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > @@ -0,0 +1,50 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: LTC4286 power monitors
> > > +
> > > +maintainers:
> > > + - Delphine CC Chiu <[email protected]>
> > > +
> > > +properties:
> > > + compatible:
> > > + enum:
> > > + - lltc,ltc4286
> > > + - lltc,ltc4287
> >
> > I don't recall seeing an answer to Guenter about this ltc4287 device:
> > https://lore.kernel.org/all/[email protected]/
> >
>
> At least the chip does officially exist now, and a datasheet is available.
>
> https://www.analog.com/en/products/ltc4287.html
>
> It shows that coefficients for the telemetry commands are different,
> meaning that indeed both chips need to be explicitly referenced
> in the properties description (and handled in the driver, which proves
> my point of needing a datasheet before accepting such a driver).
Oh neat, would've been good if that'd been mentioned!
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + adi,vrange-select-25p6:
> > > + description:
> > > + This property is a bool parameter to represent the
> > > + voltage range is 25.6 or not for this chip.
> >
> > 25.6 what? Volts? microvolts?
> > What about Guenter's suggestion to name this so that it better matches
> > the other, similar properties?
> >
>
> I still would prefer one of the more common properties.
> I still prefer adi,vrange-high-enable.
I think, from reading the previous version, that this is actually the
lower voltage range, as there is a 102.x $unit range too that is the
default in the hardware. Obviously, the use of the property could be
inverted to match that naming however.
Cheers,
Conor.
On 10/26/23 08:26, Conor Dooley wrote:
> On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote:
>> On 10/26/23 07:25, Conor Dooley wrote:
>>> Hey,
>>>
>>> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
>>>> Add a device tree bindings for ltc4286 driver.
>>>
>>> Bindings are for devices, not for drivers.
>>>
>>>>
>>>> Signed-off-by: Delphine CC Chiu <[email protected]>
>>>>
>>>> Changelog:
>>>> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
>>>> - Add type for adi,vrange-select-25p6
>>>> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
>>>> ---
>>>> .../bindings/hwmon/lltc,ltc4286.yaml | 50 +++++++++++++++++++
>>>> MAINTAINERS | 10 ++++
>>>> 2 files changed, 60 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>>>> new file mode 100644
>>>> index 000000000000..17022de657bb
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
>>>> @@ -0,0 +1,50 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: LTC4286 power monitors
>>>> +
>>>> +maintainers:
>>>> + - Delphine CC Chiu <[email protected]>
>>>> +
>>>> +properties:
>>>> + compatible:
>>>> + enum:
>>>> + - lltc,ltc4286
>>>> + - lltc,ltc4287
>>>
>>> I don't recall seeing an answer to Guenter about this ltc4287 device:
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>
>> At least the chip does officially exist now, and a datasheet is available.
>>
>> https://www.analog.com/en/products/ltc4287.html
>>
>> It shows that coefficients for the telemetry commands are different,
>> meaning that indeed both chips need to be explicitly referenced
>> in the properties description (and handled in the driver, which proves
>> my point of needing a datasheet before accepting such a driver).
>
> Oh neat, would've been good if that'd been mentioned!
>
Actually, turns out there is some contradiction in the LTC4286 datasheet.
It mentions different coefficients in different places. It is all but
impossible to determine if the datasheet is wrong or if the chip uses
a variety of coefficients unless one has a real chip available. Sigh :-(.
>>>> +
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + adi,vrange-select-25p6:
>>>> + description:
>>>> + This property is a bool parameter to represent the
>>>> + voltage range is 25.6 or not for this chip.
>>>
>>> 25.6 what? Volts? microvolts?
>>> What about Guenter's suggestion to name this so that it better matches
>>> the other, similar properties?
>>>
>>
>> I still would prefer one of the more common properties.
>> I still prefer adi,vrange-high-enable.
>
> I think, from reading the previous version, that this is actually the
> lower voltage range, as there is a 102.x $unit range too that is the
> default in the hardware. Obviously, the use of the property could be
> inverted to match that naming however.
>
It needs to be programmed either way because it is unknown how the chip
has been programmed before. That means some action is needed no matter
if the property is provided or not.
Sure, we could add something like adi,vrange-low-enable. Not sure if
that would be any better.
Actually, after looking at the datasheets, I'd be more interested
in the impact of other configuration buts such as VPWR_SELECT
which selects the voltage used for power calculations, or
all the settings in MFR_ADC_CONFIG. The datasheet doesn't really
explain (or I can't figure out how it does) how the different
configurations affect the reported telemetry values. But that is
a question for the driver, not for the property description.
Guenter
Hi Delphine,
kernel test robot noticed the following build warnings:
[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.6-rc7 next-20231026]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Delphine-CC-Chiu/dt-bindings-hwmon-Add-lltc-ltc4286-driver-bindings/20231026-161739
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/20231026081514.3610343-2-Delphine_CC_Chiu%40Wiwynn.com
patch subject: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver bindings
reproduce: (https://download.01.org/0day-ci/archive/20231027/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/ltc4286.rst
>> MAINTAINERS:27681: WARNING: unknown document: ../devicetree/bindings/hwmon/ltc4286
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Thursday, October 26, 2023 10:26 PM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <[email protected]>
> Cc: [email protected]; Jean Delvare <[email protected]>; Guenter Roeck
> <[email protected]>; Rob Herring <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Jonathan Corbet <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver
> bindings
>
> Hey,
>
> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> > Add a device tree bindings for ltc4286 driver.
>
> Bindings are for devices, not for drivers.
>
> >
> > Signed-off-by: Delphine CC Chiu <[email protected]>
> >
> > Changelog:
> > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> > - Add type for adi,vrange-select-25p6
> > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> > ---
> > .../bindings/hwmon/lltc,ltc4286.yaml | 50
> +++++++++++++++++++
> > MAINTAINERS | 10 ++++
> > 2 files changed, 60 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > new file mode 100644
> > index 000000000000..17022de657bb
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > @@ -0,0 +1,50 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: LTC4286 power monitors
> > +
> > +maintainers:
> > + - Delphine CC Chiu <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - lltc,ltc4286
> > + - lltc,ltc4287
>
> I don't recall seeing an answer to Guenter about this ltc4287 device:
> https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roeck-us.
> net/
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + adi,vrange-select-25p6:
> > + description:
> > + This property is a bool parameter to represent the
> > + voltage range is 25.6 or not for this chip.
>
> 25.6 what? Volts? microvolts?
25.6 volts
> What about Guenter's suggestion to name this so that it better matches the
> other, similar properties?
>
> Cheers,
> Conor.
> -----Original Message-----
> From: Conor Dooley <[email protected]>
> Sent: Thursday, October 26, 2023 11:26 PM
> To: Guenter Roeck <[email protected]>
> Cc: Delphine_CC_Chiu/WYHQ/Wiwynn <[email protected]>;
> [email protected]; Jean Delvare <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Jonathan Corbet <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver
> bindings
>
> On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote:
> > On 10/26/23 07:25, Conor Dooley wrote:
> > > Hey,
> > >
> > > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> > > > Add a device tree bindings for ltc4286 driver.
> > >
> > > Bindings are for devices, not for drivers.
> > >
> > > >
> > > > Signed-off-by: Delphine CC Chiu <[email protected]>
> > > >
> > > > Changelog:
> > > > v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> > > > - Add type for adi,vrange-select-25p6
> > > > - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> > > > ---
> > > > .../bindings/hwmon/lltc,ltc4286.yaml | 50
> +++++++++++++++++++
> > > > MAINTAINERS | 10 ++++
> > > > 2 files changed, 60 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > > b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > > new file mode 100644
> > > > index 000000000000..17022de657bb
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> > > > @@ -0,0 +1,50 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> > > > +1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/hwmon/lltc,ltc4286.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: LTC4286 power monitors
> > > > +
> > > > +maintainers:
> > > > + - Delphine CC Chiu <[email protected]>
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - lltc,ltc4286
> > > > + - lltc,ltc4287
> > >
> > > I don't recall seeing an answer to Guenter about this ltc4287 device:
> > > https://lore.kernel.org/all/22f6364c-611c-ffb6-451c-9ddc20418d0a@roe
> > > ck-us.net/
> > >
> >
> > At least the chip does officially exist now, and a datasheet is available.
> >
> > https://www.analog.com/en/products/ltc4287.html
> >
> > It shows that coefficients for the telemetry commands are different,
> > meaning that indeed both chips need to be explicitly referenced in the
> > properties description (and handled in the driver, which proves my
> > point of needing a datasheet before accepting such a driver).
>
> Oh neat, would've been good if that'd been mentioned!
>
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + adi,vrange-select-25p6:
> > > > + description:
> > > > + This property is a bool parameter to represent the
> > > > + voltage range is 25.6 or not for this chip.
> > >
> > > 25.6 what? Volts? microvolts?
> > > What about Guenter's suggestion to name this so that it better
> > > matches the other, similar properties?
> > >
> >
> > I still would prefer one of the more common properties.
> > I still prefer adi,vrange-high-enable.
>
> I think, from reading the previous version, that this is actually the lower voltage
> range, as there is a 102.x $unit range too that is the default in the hardware.
> Obviously, the use of the property could be inverted to match that naming
> however.
We will use adi,vrange-low-enable instead of adi,vrange-select-25p6
>
> Cheers,
> Conor.
> -----Original Message-----
> From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
> Sent: Friday, October 27, 2023 12:49 AM
> To: Conor Dooley <[email protected]>
> Cc: Delphine_CC_Chiu/WYHQ/Wiwynn <[email protected]>;
> [email protected]; Jean Delvare <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Jonathan Corbet <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver
> bindings
>
> Security Reminder: Please be aware that this email is sent by an external
> sender.
>
> On 10/26/23 08:26, Conor Dooley wrote:
> > On Thu, Oct 26, 2023 at 08:09:52AM -0700, Guenter Roeck wrote:
> >> On 10/26/23 07:25, Conor Dooley wrote:
> >>> Hey,
> >>>
> >>> On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> >>>> Add a device tree bindings for ltc4286 driver.
> >>>
> >>> Bindings are for devices, not for drivers.
> >>>
> >>>>
> >>>> Signed-off-by: Delphine CC Chiu <[email protected]>
> >>>>
> >>>> Changelog:
> >>>> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> >>>> - Add type for adi,vrange-select-25p6
> >>>> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> >>>> ---
> >>>> .../bindings/hwmon/lltc,ltc4286.yaml | 50
> +++++++++++++++++++
> >>>> MAINTAINERS | 10 ++++
> >>>> 2 files changed, 60 insertions(+)
> >>>> create mode 100644
> >>>> Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >>>>
> >>>> diff --git
> >>>> a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >>>> b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >>>> new file mode 100644
> >>>> index 000000000000..17022de657bb
> >>>> --- /dev/null
> >>>> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >>>> @@ -0,0 +1,50 @@
> >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML
> >>>> +1.2
> >>>> +---
> >>>> +$id:
> >>>> +https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd
> >>>>
> +evicetree.org%2Fschemas%2Fhwmon%2Flltc%2Cltc4286.yaml%23&data=05%
> 7
> >>>>
> +C01%7CWayne_SC_Liu%40wiwynn.com%7Cf555db9618454d17e0b508dbd64
> 36721
> >>>>
> +%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6383393572109674
> 95%7
> >>>>
> +CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB
> TiI
> >>>>
> +6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=cPOOqJ5y3K%2B
> D%2Frsp
> >>>> +gVhpKDIC0gC5nKBbRp7Up0OxDpM%3D&reserved=0
> >>>> +$schema:
> >>>> +https://apc01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fd
> >>>>
> +evicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CWayne
> _S
> >>>>
> +C_Liu%40wiwynn.com%7Cf555db9618454d17e0b508dbd6436721%7Cda6e06
> 28fc
> >>>>
> +834caf9dd273061cbab167%7C0%7C0%7C638339357210967495%7CUnknown
> %7CTW
> >>>>
> +FpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> >>>>
> +VCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HjOhDm8bwPaNpWgumCSlak%2
> FiqoagwZc
> >>>> +0eb95J1wnKUE%3D&reserved=0
> >>>> +
> >>>> +title: LTC4286 power monitors
> >>>> +
> >>>> +maintainers:
> >>>> + - Delphine CC Chiu <[email protected]>
> >>>> +
> >>>> +properties:
> >>>> + compatible:
> >>>> + enum:
> >>>> + - lltc,ltc4286
> >>>> + - lltc,ltc4287
> >>>
> >>> I don't recall seeing an answer to Guenter about this ltc4287 device:
> >>> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> >>>
> re.kernel.org%2Fall%2F22f6364c-611c-ffb6-451c-9ddc20418d0a%40roeck-u
> >>>
> s.net%2F&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7Cf555db96184
> 54d17
> >>>
> e0b508dbd6436721%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6
> 38339
> >>>
> 357210967495%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQI
> joiV2l
> >>>
> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oekIX
> %2FlP
> >>> %2B%2F4KhrSlK8Xp1zR0fdX1Emmr0Ox5GPS9Dz0%3D&reserved=0
> >>>
> >>
> >> At least the chip does officially exist now, and a datasheet is available.
> >>
> >> https://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww
> >> .analog.com%2Fen%2Fproducts%2Fltc4287.html&data=05%7C01%7CWayne
> _SC_Li
> >>
> u%40wiwynn.com%7Cf555db9618454d17e0b508dbd6436721%7Cda6e0628fc8
> 34caf9
> >>
> dd273061cbab167%7C0%7C0%7C638339357210967495%7CUnknown%7CTWF
> pbGZsb3d8
> >>
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C
> >>
> 3000%7C%7C%7C&sdata=FqRheGXFi%2FmSH3cnRZ7eSbwD3JfZj8powcGBCJcP
> wWQ%3D&
> >> reserved=0
> >>
> >> It shows that coefficients for the telemetry commands are different,
> >> meaning that indeed both chips need to be explicitly referenced in
> >> the properties description (and handled in the driver, which proves
> >> my point of needing a datasheet before accepting such a driver).
> >
> > Oh neat, would've been good if that'd been mentioned!
> >
>
> Actually, turns out there is some contradiction in the LTC4286 datasheet.
> It mentions different coefficients in different places. It is all but impossible to
> determine if the datasheet is wrong or if the chip uses a variety of coefficients
> unless one has a real chip available. Sigh :-(.
We are not the chip vendor, but we could forward your question to vendor.
Could you point out the exact places (which pages) where are the contradiction in LTC4286 datasheet?
>
> >>>> +
> >>>> + reg:
> >>>> + maxItems: 1
> >>>> +
> >>>> + adi,vrange-select-25p6:
> >>>> + description:
> >>>> + This property is a bool parameter to represent the
> >>>> + voltage range is 25.6 or not for this chip.
> >>>
> >>> 25.6 what? Volts? microvolts?
> >>> What about Guenter's suggestion to name this so that it better
> >>> matches the other, similar properties?
> >>>
> >>
> >> I still would prefer one of the more common properties.
> >> I still prefer adi,vrange-high-enable.
> >
> > I think, from reading the previous version, that this is actually the
> > lower voltage range, as there is a 102.x $unit range too that is the
> > default in the hardware. Obviously, the use of the property could be
> > inverted to match that naming however.
> >
>
> It needs to be programmed either way because it is unknown how the chip has
> been programmed before. That means some action is needed no matter if the
> property is provided or not.
>
> Sure, we could add something like adi,vrange-low-enable. Not sure if that
> would be any better.
>
> Actually, after looking at the datasheets, I'd be more interested in the impact
> of other configuration buts such as VPWR_SELECT which selects the voltage
> used for power calculations, or all the settings in MFR_ADC_CONFIG. The
> datasheet doesn't really explain (or I can't figure out how it does) how the
> different configurations affect the reported telemetry values. But that is a
> question for the driver, not for the property description.
We have sent an e-mail about this question.
>
> Guenter
> -----Original Message-----
> From: Guenter Roeck <[email protected]> On Behalf Of Guenter Roeck
> Sent: Thursday, October 26, 2023 11:10 PM
> To: Conor Dooley <[email protected]>; Delphine_CC_Chiu/WYHQ/Wiwynn
> <[email protected]>
> Cc: [email protected]; Jean Delvare <[email protected]>; Rob Herring
> <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Jonathan Corbet <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: Add lltc ltc4286 driver
> bindings
>
> Security Reminder: Please be aware that this email is sent by an external
> sender.
>
> On 10/26/23 07:25, Conor Dooley wrote:
> > Hey,
> >
> > On Thu, Oct 26, 2023 at 04:15:11PM +0800, Delphine CC Chiu wrote:
> >> Add a device tree bindings for ltc4286 driver.
> >
> > Bindings are for devices, not for drivers.
> >
> >>
> >> Signed-off-by: Delphine CC Chiu <[email protected]>
> >>
> >> Changelog:
> >> v2 - Revise vrange_select_25p6 to adi,vrange-select-25p6
> >> - Add type for adi,vrange-select-25p6
> >> - Revise rsense-micro-ohms to shunt-resistor-micro-ohms
> >> ---
> >> .../bindings/hwmon/lltc,ltc4286.yaml | 50
> +++++++++++++++++++
> >> MAINTAINERS | 10 ++++
> >> 2 files changed, 60 insertions(+)
> >> create mode 100644
> >> Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >>
> >> diff --git
> >> a/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >> b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >> new file mode 100644
> >> index 000000000000..17022de657bb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/hwmon/lltc,ltc4286.yaml
> >> @@ -0,0 +1,50 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >> +---
> >> +$id:
> >> +http://dev/
> >>
> +icetree.org%2Fschemas%2Fhwmon%2Flltc%2Cltc4286.yaml%23&data=05%7C
> 01%
> >>
> +7CWayne_SC_Liu%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51
> %7Cda6
> >>
> +e0628fc834caf9dd273061cbab167%7C0%7C0%7C638339298041650948%7CU
> nknown
> >>
> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiL
> >>
> +CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=gGQEDl33zRIJbfbinu2%2Bi2cC
> Ay6y0o
> >> +DSLzBpLL7hA%2F8%3D&reserved=0
> >> +$schema:
> >> +http://dev/
> >>
> +icetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C01%7CWayne_S
> C_Li
> >>
> +u%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51%7Cda6e0628fc8
> 34caf
> >>
> +9dd273061cbab167%7C0%7C0%7C638339298041650948%7CUnknown%7CT
> WFpbGZsb3
> >>
> +d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> %3D
> >>
> +%7C3000%7C%7C%7C&sdata=nl1IM1HpYptsJOHfiuXtKFmD%2FVlGMCW1IkoK
> HYco0sk
> >> +%3D&reserved=0
> >> +
> >> +title: LTC4286 power monitors
> >> +
> >> +maintainers:
> >> + - Delphine CC Chiu <[email protected]>
> >> +
> >> +properties:
> >> + compatible:
> >> + enum:
> >> + - lltc,ltc4286
> >> + - lltc,ltc4287
> >
> > I don't recall seeing an answer to Guenter about this ltc4287 device:
> > https://lore/
> > .kernel.org%2Fall%2F22f6364c-611c-ffb6-451c-9ddc20418d0a%40roeck-us.n
> e
> >
> t%2F&data=05%7C01%7CWayne_SC_Liu%40wiwynn.com%7Cb250e206c9ef48f
> bc1c108
> >
> dbd6359e51%7Cda6e0628fc834caf9dd273061cbab167%7C0%7C0%7C6383392
> 9804165
> >
> 0948%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz
> IiLCJBT
> >
> iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=OpwfD3rBS0vQBF
> jUhszrMg
> > 4mq581jU7gx54Ln8V3gUA%3D&reserved=0
> >
>
> At least the chip does officially exist now, and a datasheet is available.
>
> https://www.ana/
> log.com%2Fen%2Fproducts%2Fltc4287.html&data=05%7C01%7CWayne_SC_Li
> u%40wiwynn.com%7Cb250e206c9ef48fbc1c108dbd6359e51%7Cda6e0628fc83
> 4caf9dd273061cbab167%7C0%7C0%7C638339298041650948%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
> CJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=CwT16Uipl8RymnFZ1WuBJzUJv
> fLCKdK3VlTcTBN0xxk%3D&reserved=0
>
> It shows that coefficients for the telemetry commands are different, meaning
> that indeed both chips need to be explicitly referenced in the properties
> description (and handled in the driver, which proves my point of needing a
> datasheet before accepting such a driver).
We will check the difference of coefficients for the telemetry commands with vendor.
>
> >> +
> >> + reg:
> >> + maxItems: 1
> >> +
> >> + adi,vrange-select-25p6:
> >> + description:
> >> + This property is a bool parameter to represent the
> >> + voltage range is 25.6 or not for this chip.
> >
> > 25.6 what? Volts? microvolts?
> > What about Guenter's suggestion to name this so that it better matches
> > the other, similar properties?
> >
>
> I still would prefer one of the more common properties.
> I still prefer adi,vrange-high-enable.
>
> Guenter
On 10/30/23 23:25, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
[ ... ]
>>
>> Actually, turns out there is some contradiction in the LTC4286 datasheet.
>> It mentions different coefficients in different places. It is all but impossible to
>> determine if the datasheet is wrong or if the chip uses a variety of coefficients
>> unless one has a real chip available. Sigh :-(.
> We are not the chip vendor, but we could forward your question to vendor.
> Could you point out the exact places (which pages) where are the contradiction in LTC4286 datasheet?
>
See "PMBUS COMMAND SUMMARY", default values:
"IOUT_OC_WARN_LIMIT" says "21.3 mV/RSENSE"
"PIN_OP_WARN_LIMIT" says "2.8/RSENSE"
This seems to contradict "Table 8. PMBus M, B, and R Parameters". But then,
reading it again (and again), I think it is just an odd and confusing way
of trying to describe the 0x7fff default register values. Sorry for the noise.
Guenter