diff mbox series

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

Message ID 20240815-ad7606_add_iio_backend_support-v1-1-cea3e11b1aa4@baylibre.com (mailing list archive)
State Changes Requested
Headers show
Series Add iio backend compatibility for ad7606 | expand

Commit Message

Guillaume Stols Aug. 15, 2024, 12:11 p.m. UTC
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.

Signed-off-by: Guillaume Stols <gstols@baylibre.com>
---
 .../devicetree/bindings/iio/adc/adi,ad7606.yaml         | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Conor Dooley Aug. 15, 2024, 2:35 p.m. UTC | #1
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
> +then:
> +  allOf:
> +    - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +    - required:
> +        - spi-cpol
>  
> +allOf:
>    - if:
>        properties:
>          compatible:
> 
> -- 
> 2.34.1
>
Jonathan Cameron Aug. 17, 2024, 3:05 p.m. UTC | #2
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
> >
diff mbox series

Patch

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
+then:
+  allOf:
+    - $ref: /schemas/spi/spi-peripheral-props.yaml#
+    - required:
+        - spi-cpol
 
+allOf:
   - if:
       properties:
         compatible: