| Message ID | 20240920-ad7606_add_iio_backend_support-v2-1-0e78782ae7d0@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add iio backend compatibility for ad7606 | expand |
Hello Guillaume, On Fri, Sep 20, 2024 at 05:33:21PM +0000, Guillaume Stols wrote: > According to the datasheet, "Data is clocked in from SDI on the falling > edge of SCLK, while data is clocked out on DOUTA on the rising edge of > SCLK". > Also, even if not stated textually in the datasheet, it is made clear on > the diagrams that sclk idles at high. > > So the documentation is erroneously stating that spi-cpha is required, and > the example is erroneously setting both spi-cpol and spi-cpha. I would expect that the communication with the chip is at least unreliable if not outright broken with the wrong polarity. So maybe add something like: On $MyMachine dropping the spi-cpha property reduces IO errors / fixes measurement readout / improves somehow differently. to the commit log? > Fixes: 416f882c3b40 ("dt-bindings: iio: adc: Migrate AD7606 documentation to yaml") > Fixes: 6e33a125df66 ("dt-bindings: iio: adc: Add docs for AD7606 ADC") > > Signed-off-by: Guillaume Stols <gstols@baylibre.com> The empty line between Fixes and S-o-b is unusual. Assuming you resend anyway, please drop it. > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > index 69408cae3db9..75334a033539 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > @@ -29,8 +29,6 @@ properties: > reg: > maxItems: 1 > > - spi-cpha: true > - > spi-cpol: true > > avcc-supply: true > @@ -117,7 +115,7 @@ properties: > required: > - compatible > - reg > - - spi-cpha > + - spi-cpol Adding cpol seems unrelated to this patch. (And you remove it again in patch #2.) > - avcc-supply > - vdrive-supply > - interrupts Best regards Uwe
On Sat, 21 Sep 2024 11:11:31 +0200 Uwe Kleine-König <u.kleine-koenig@baylibre.com> wrote: > Hello Guillaume, > > On Fri, Sep 20, 2024 at 05:33:21PM +0000, Guillaume Stols wrote: > > According to the datasheet, "Data is clocked in from SDI on the falling > > edge of SCLK, while data is clocked out on DOUTA on the rising edge of > > SCLK". > > Also, even if not stated textually in the datasheet, it is made clear on > > the diagrams that sclk idles at high. > > > > So the documentation is erroneously stating that spi-cpha is required, and > > the example is erroneously setting both spi-cpol and spi-cpha. > > I would expect that the communication with the chip is at least > unreliable if not outright broken with the wrong polarity. So maybe add > something like: > > On $MyMachine dropping the spi-cpha property reduces IO errors / fixes > measurement readout / improves somehow differently. > > to the commit log? > > > Fixes: 416f882c3b40 ("dt-bindings: iio: adc: Migrate AD7606 documentation to yaml") > > Fixes: 6e33a125df66 ("dt-bindings: iio: adc: Add docs for AD7606 ADC") > > > > Signed-off-by: Guillaume Stols <gstols@baylibre.com> > > The empty line between Fixes and S-o-b is unusual. Assuming you resend > anyway, please drop it. It's also scanned for in linux-next so to reduce chances I forget to fix this definitely resend. > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > index 69408cae3db9..75334a033539 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > @@ -29,8 +29,6 @@ properties: > > reg: > > maxItems: 1 > > > > - spi-cpha: true > > - > > spi-cpol: true > > > > avcc-supply: true > > @@ -117,7 +115,7 @@ properties: > > required: > > - compatible > > - reg > > - - spi-cpha > > + - spi-cpol > > Adding cpol seems unrelated to this patch. (And you remove it again in > patch #2.) > > > - avcc-supply > > - vdrive-supply > > - interrupts > > Best regards > Uwe
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml index 69408cae3db9..75334a033539 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml @@ -29,8 +29,6 @@ properties: reg: maxItems: 1 - spi-cpha: true - spi-cpol: true avcc-supply: true @@ -117,7 +115,7 @@ properties: required: - compatible - reg - - spi-cpha + - spi-cpol - avcc-supply - vdrive-supply - interrupts @@ -185,7 +183,6 @@ examples: reg = <0>; spi-max-frequency = <1000000>; spi-cpol; - spi-cpha; avcc-supply = <&adc_vref>; vdrive-supply = <&vdd_supply>;
According to the datasheet, "Data is clocked in from SDI on the falling edge of SCLK, while data is clocked out on DOUTA on the rising edge of SCLK". Also, even if not stated textually in the datasheet, it is made clear on the diagrams that sclk idles at high. So the documentation is erroneously stating that spi-cpha is required, and the example is erroneously setting both spi-cpol and spi-cpha. Fixes: 416f882c3b40 ("dt-bindings: iio: adc: Migrate AD7606 documentation to yaml") Fixes: 6e33a125df66 ("dt-bindings: iio: adc: Add docs for AD7606 ADC") Signed-off-by: Guillaume Stols <gstols@baylibre.com> --- Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)