Re: [PATCH 1/8] dt-bindings: iio: adc: ad7606: Make corrections on spi conditions

[Jonathan Cameron <jic23@kernel.org>]

On Thu, 15 Aug 2024 15:35:34 +0100
Conor Dooley <conor@kernel.org> wrote:

> On Thu, Aug 15, 2024 at 12:11:55PM +0000, Guillaume Stols wrote:
> > The SPI conditions are not always required, because there is also a
> > parallel interface. The way used to detect that the SPI interface is
> > used is to check if the reg value is between 0 and 256.
> > There is also a correction on the spi-cpha that is not required when SPI
> > interface is selected, while spi-cpol is.  
> 
> This feels like it should be two patches, with the first having a Fixes:
> tag etc, if the original binding was incorrect.
> 
> > 
> > Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> > ---
> >  .../devicetree/bindings/iio/adc/adi,ad7606.yaml         | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > index 69408cae3db9..c0008d36320f 100644
> > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> > @@ -117,15 +117,26 @@ properties:
> >  required:
> >    - compatible
> >    - reg
> > -  - spi-cpha
> >    - avcc-supply
> >    - vdrive-supply
> >    - interrupts
> >    - adi,conversion-start-gpios
> >  
> > -allOf:
> > -  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +# This checks if reg is a chipselect so the device is on an SPI
> > +# bus, the if-clause will fail if reg is a tuple such as for a
> > +# platform device.
> > +if:
> > +  properties:
> > +    reg:
> > +      minimum: 0
> > +      maximum: 256

That's not particularly nice - in theory the parallel bus memory map could
be at 0 - it's just very unlikely on a real platform.

I'd just do what we do with i2c/spi drivers and just not make it required at all.
Rely on comments to say why.

In ideal case we'd figure out from the parent node if it was an spi bus
but I have no idea how that might be enforced.

> > +then:
> > +  allOf:
> > +    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +    - required:
> > +        - spi-cpol
> >  
> > +allOf:
> >    - if:
> >        properties:
> >          compatible:
> > 
> > -- 
> > 2.34.1
> >