Re: [PATCH v2 1/2] dt-bindings: hwmon: ltc2991: add bindings

[Rob Herring <robh@kernel.org>]

On Mon, Oct 02, 2023 at 01:46:53PM +0000, Miclaus, Antoniu wrote:
> 
> 
> --
> Antoniu Miclăuş
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Monday, October 2, 2023 4:01 PM
> > To: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>
> > Cc: Jean Delvare <jdelvare@suse.com>; Guenter Roeck <linux@roeck-
> > us.net>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski
> > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; linux-
> > hwmon@vger.kernel.org; devicetree@vger.kernel.org; linux-
> > kernel@vger.kernel.org; linux-doc@vger.kernel.org
> > Subject: Re: [PATCH v2 1/2] dt-bindings: hwmon: ltc2991: add bindings
> > 
> > [External]
> > 
> > Hey,
> > 
> > On Mon, Oct 02, 2023 at 01:18:14PM +0300, Antoniu Miclaus wrote:
> > > Add dt-bindings for ltc2991 octal i2c voltage, current and temperature
> > > monitor.
> > >
> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > ---
> > > changes in v2:
> > >  - make compatible const type
> > >  - remove `|` where not necessary
> > >  - switch to micro-ohms for the shunt resistor property
> > >  - add vendor prefix for temperature-enable
> > 
> > Thanks for the updates...
> > 
> > >  .../bindings/hwmon/adi,ltc2991.yaml           | 114 ++++++++++++++++++
> > >  1 file changed, 114 insertions(+)
> > >  create mode 100644
> > Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> > b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> > > new file mode 100644
> > > index 000000000000..3811ea07a04f
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2991.yaml
> > > @@ -0,0 +1,114 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +
> > > +$id: http://devicetree.org/schemas/hwmon/adi,ltc2991.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Analog Devices LTC2991 Octal I2C Voltage, Current and Temperature
> > Monitor
> > > +
> > > +maintainers:
> > > +  - Antoniu Miclaus <antoniu.miclaus@analog.com>
> > > +
> > > +description: |
> > > +  The LTC2991 is used to monitor system temperatures, voltages and
> > currents.
> > > +  Through the I2C serial interface, the eight monitors can individually
> > measure
> > > +  supply voltages and can be paired for differential measurements of
> > current
> > > +  sense resistors or temperature sensing transistors.
> > > +
> > > +  Datasheet:
> > > +    https://www.analog.com/en/products/ltc2991.html
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: adi,ltc2991
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  '#address-cells':
> > > +    const: 1
> > > +
> > > +  '#size-cells':
> > > +    const: 0
> > > +
> > > +  vcc-supply: true
> > > +
> > > +patternProperties:
> > > +  "^channel@[0-3]$":
> > > +    type: object
> > > +    description:
> > > +      Represents the differential/temperature channels.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description:
> > > +          The channel number. LTC2991 can monitor 4 currents/temperatures.
> > > +        items:
> > > +          minimum: 0
> > > +          maximum: 3
> > > +
> > > +      shunt-resistor-micro-ohms:
> > > +        description:
> > > +          The value of curent sense resistor in miliohms. Enables differential
> > > +          input pair.
> > > +
> > > +      adi,temperature-enable:
> > > +        description:
> > > +          Enables temperature readings for a input pair.
> > > +        type: boolean
> > 
> > ...but I did not see an answer to my question on v1:
> > 	TBH, this seems like it is used just to control software behaviour.
> > 	Why would you want to actually disable this in DT?
> > In other words, is there something in that hardware that precludes
> > measuring temperature for channels that do not contain this property?
> > 
> > Thanks,
> > Conor.
> > 
> Sorry for missing that. I took in consideration this approach based on the pin functions
> described in the datasheet (page 8 of 32). For example the V1 pin of the part can support
> 3 different configurations: "V1 (Pin 1): First Monitor Input. This pin can be configured
> as a single-ended input (0V to 4.9V) or the positive inputfor a differential or remote diode
> temperature measurement (in combination with V2)."
> Moreover, looking at the multiple typical applications examples at the end of the datasheet
> there is a specific adjacent hardware circuit connected to this part for the temperature
> measurements configurations.

Makes sense. Please expand the description with some of this detail so 
the purpose is clear. The description should answer when do I set this 
property or not.

Rob