Re: [PATCH v2 01/10] dt-bindings: iio: adc: ad7606: Set the correct polarity

Uwe Kleine-König
2024-09-21T09:11:35Z

[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]