Re: [PATCH v3 2/2] dt-bindings: iio: Add ltc2983 documentation

[Rob Herring <robh@kernel.org>]

On Tue, Oct 08, 2019 at 10:22:04AM +0000, Sa, Nuno wrote:
> Hi Rob,
> 
> Somethings that I would like to clarify before sending a v4 since I'm
> having errors and I'm not really sure about the fix I'm using.
> 
> On Tue, 2019-10-08 at 07:45 +0000, Sa, Nuno wrote:
> > 
> > On Mon, 2019-10-07 at 12:46 -0500, Rob Herring wrote:
> > > On Mon, Oct 7, 2019 at 11:17 AM Sa, Nuno <Nuno.Sa@analog.com>
> > > wrote:
> > > > On Mon, 2019-10-07 at 09:45 -0500, Rob Herring wrote:
> > > > > On Fri, Oct 4, 2019 at 8:55 AM Nuno Sá <nuno.sa@analog.com>
> > > > > wrote:
> > > > > > Document the LTC2983 temperature sensor devicetree bindings.
> > > > > > 
> > > > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com>


> > > > > > +          Represents a thermocouple sensor which is
> > > > > > connected
> > > > > > to
> > > > > > one of the device
> > > > > > +          channels.
> > > > > > +
> > > > > > +        properties:
> > > > > > +          adi,sensor-type:
> > > > > > +            description: |
> > > > > > +              Identifies the type of thermocouple connected
> > > > > > to
> > > > > > the
> > > > > > device.
> > > > > > +              1 - Type J Thermocouple
> > > > > > +              2 - Type K Thermocouple
> > > > > > +              3 - Type E Thermocouple
> > > > > > +              4 - Type N Thermocouple
> > > > > > +              5 - Type R Thermocouple
> > > > > > +              6 - Type S Thermocouple
> > > > > > +              7 - Type T Thermocouple
> > > > > > +              8 - Type B Thermocouple
> > > > > > +              9 - Custom Thermocouple
> > > > > > +            allOf:
> > > > > > +              - $ref:
> > > > > > /schemas/types.yaml#/definitions/uint32
> > > > > 
> > > > > You can move the type definition under ".*@([1-9]|1[0-9]|20)$"
> > > > > and
> > > > > then just have the min/max here.
> > > > 
> > > > And how could I add meaning to the values. Could I add all in the
> > > > "parent" node?
> > > 
> > > No, they have to be at the correct level. You can keep the
> > > descriptions here. Just do:
> > > 
> > > adi,sensor-type:
> > >  description: ...
> > >  $ref: /schemas/types.yaml#/definitions/uint32
> > > 
> > > under ".*@([1-9]|1[0-9]|20)$" and drop the 'allOf: [ $ref: ...]'
> > > part.
> > > 
> 
> So, Im doing like:
> 
> ".*@([1-9]|1[0-9]|20)$":
>   type: object
> 
>   properties:
>     reg:
>       description: ...
>       minimum: 1
>       maximum: 20
>    
>     adi,sensor-type:
>       description: ...
>       $ref: /schemas/types.yaml#/definitions/uint32
>   
>   required:
>     - reg
>     - adi,sensor-type
> 
> Then in:
> 
> "^thermocouple@":
>   type: object
>   description: ...
>   
>   properties:
>     adi,sensor-type:
>       description: ...
>       minimum: 1
>       maximum: 9
>    
> This gives me that adi,sensor-type is not a valid schema. Adding allOf
> before min/max fixes it. The same error comes in:

Forget what I said, you're going to need to keep '$ref: 
/schemas/types.yaml#/definitions/uint32' here.

The problem is the meta-schema enforces that a vendor specific property 
(one with a comma) have a type defined and there's not really any way to 
say it already does elsewhere. I don't want to loosen that up globally 
either.

> 
> "^diode@":
> type: object
>   description: ...
>   
>   properties:
>     adi,sensor-type:
>       description: ...
>       const: 28 #adding allOf here also fixes it...

I need to fix the meta-schema to check what's under 'allOf'. :)

> 
> One last issue that I was not seeing before is with the reg property.
> If you recall, you also suggested to have it under ".*@([1-9]|1[0-
> 9]|20)$": and then add the 'minimum: 2' where needed. Doing that now
> also gives me invalid schema:
> 
> "^rtd@":
> type: object
>   description: ...
> 
>   properties:
>     reg:
>       minimum: 2
> 
> Changing to
>     reg:
>       items:
>         minimum: 2
> 
> fixes it but it feels that the first one should work?

The problem is we require both minimum and maximum to be defined. So 
really the second one should fail too. Or we should loosen this 
requirement.

Rob