Re: [PATCH 3/3] dt-bindings: iio: Add ltc2947 documentation

["Sa, Nuno" <Nuno.Sa@analog.com>]

On Wed, 2019-10-02 at 09:19 -0500, Rob Herring wrote:
> 
> On Tue, Sep 24, 2019 at 02:49:45PM +0200, Nuno Sá wrote:
> > Document the LTC2947 device devicetree bindings.
> > 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  .../bindings/hwmon/adi,ltc2947.yaml           | 101
> > ++++++++++++++++++
> >  MAINTAINERS                                   |   1 +
> >  2 files changed, 102 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
> > b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
> > new file mode 100644
> > index 000000000000..2ea0187421d4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
> > @@ -0,0 +1,101 @@
> 
> Missing license. Please make new bindings (GPL-2.0-only OR BSD-2-
> Clause)

ack.

> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/bindings/hwmon/adi,ltc2947.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices LTC2947 high precision power and energy
> > monitor
> > +
> > +maintainers:
> > +  - Nuno Sá <nuno.sa@analog.com>
> > +
> > +description: |
> > +  Analog Devices LTC2947 high precision power and energy monitor
> > over SPI or I2C.
> > +
> > +  
> > https://www.analog.com/media/en/technical-documentation/data-sheets/LTC2947.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,ltc2947
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    description:
> > +      The LTC2947 uses either a trimmed internal oscillator or an
> > external clock
> > +      as the time base for determining the integration period to
> > represent time,
> > +      charge and energy. When an external clock is used, this
> > property must be
> > +      set accordingly.
> > +    maxItems: 1
> > +
> > +  adi,accumulator-ctl-pol:
> > +    description:
> > +      This property controls the polarity of current that is
> > accumulated to
> > +      calculate charge and energy so that, they can be only
> > accumulated for
> > +      positive current for example. Since there are two sets of
> > registers for
> > +      the accumulated values, this entry can also have two items
> > which sets
> > +      energy1/charge1 and energy2/charger2 respectively. Check
> > table 12 of the
> > +      datasheet for more information on the supported options.
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      - enum: [0, 1, 2, 3]
> > +      - minItems: 2
> > +      - maxItems: 2
> > +    default: [0, 0]
> 
> This should be:
> 
> allOf:
>   - $ref: ...
> items:
>   enum: [0, 1, 2, 3]
>   default: 0
> minItems: 2
> maxItems: 2

I was expecting this already. When upstreaming another device, I
realized some errors with `make dt_binding_check` when evaluating the
example. Just for my understanding, is this also incorrect?

allOf:
  - $ref: /schemas/types.yaml#/definitions/uint32-array
  - minItems: 2
    maxItems: 2
    items:
      enum: [0, 1, 2, 3]
    default: 0

I used a similar pattern in another device that is also waiting for
review :).
> > +
> > +  adi,accumulation-deadband-microamp:
> > +    description:
> > +      This property controls the Accumulation Dead band which
> > allows to set the
> > +      level of current below which no accumulation takes place.
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > +      - maximum: 255
> 
> maximum should be at same indent as allOf. Or default should be at
> the 
> same level as maximum (under a single '-' list entry).

So it should be?

allOf:
  - $ref: /schemas/types.yaml#/definitions/uint32
  - maximum: 255
    default: 0

> > +    default: 0
> > +
> > +  adi,gpio-out-pol:
> > +    description:
> > +      This property controls the GPIO polarity. Setting it to one
> > makes the GPIO
> > +      active high, setting it to zero makets it active low. When
> > this property
> > +      is present, the GPIO is automatically configured as output
> > and set to
> > +      control a fan as a function of measured temperature.
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32
> > +      - enum: [0, 1]
> > +    default: 0
> 
> Same here.
> 
> > +
> > +  adi,gpio-in-accum:
> > +    description:
> > +      When set, this property sets the GPIO as input. It is then
> > used to control
> > +      the accumulation of charge, energy and time. This function
> > can be
> > +      enabled/configured separately for each of the two sets of
> > accumulation
> > +      registers. Check table 13 of the datasheet for more
> > information on the
> > +      supported options. This property cannot be used together
> > with
> > +      adi,gpio-out-pol.
> > +    allOf:
> > +      - $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      - enum: [0, 1, 2]
> > +      - minItems: 2
> > +      - maxItems: 2
> > +    default: [0, 0]
> 
> Similar here.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +
> > +examples:
> > +  - |
> > +    spi0 {
> 
> Just 'spi'

ack.

> > +           #address-cells = <1>;
> > +           #size-cells = <0>;
> > +
> > +           ltc2947_spi: ltc2947@0 {
> > +                   compatible = "adi,ltc2947";
> > +                   reg = <0>;
> > +                   /* accumulation takes place always for
> > energ1/charge1. */
> > +                   /* accumulation only on positive current for
> > energy2/charge2. */
> > +                   adi,accumulator-ctl-pol = <0 1>;
> > +           };
> > +    };
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 889f38c1c930..820bdde2044b 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9505,6 +9505,7 @@ F:	drivers/hwmon/ltc2947-core.c
> >  F:	drivers/hwmon/ltc2947-spi.c
> >  F:	drivers/hwmon/ltc2947-i2c.c
> >  F:	drivers/hwmon/ltc2947.h
> > +F:	Documentation/devicetree/bindings/hwmon/adi,ltc2947.yaml
> >  
> >  LTC4306 I2C MULTIPLEXER DRIVER
> >  M:	Michael Hennerich <michael.hennerich@analog.com>
> > -- 
> > 2.23.0
> >