[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <yhah2mrild37ntk77u75oysjzf3mpegvqeg5es2vzq6tencwco@lytu235eymoa>
Date: Wed, 27 Nov 2024 09:00:22 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Marcelo Schmitt <marcelo.schmitt@...log.com>
Cc: linux-iio@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, lars@...afoo.de, Michael.Hennerich@...log.com, jic23@...nel.org,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
marcelo.schmitt1@...il.com
Subject: Re: [PATCH v4 1/4] dt-bindings: iio: adc: adi,ad4000: Add PulSAR
On Tue, Nov 26, 2024 at 08:15:05PM -0300, Marcelo Schmitt wrote:
> Extend the AD4000 series device tree documentation to also describe
> PulSAR devices.
>
> The single-channel series of PulSAR devices is similar to the AD4000 series
> except PulSAR devices sample at slower rates and don't have a
> configuration register. Because PulSAR devices don't have a configuration
> register, they don't support all features of AD4000 devices and thus fewer
> interfaces are provided to user space. Also, while AD4000 may have their
> SDI pin connected to SPI host MOSI line, PulSAR SDI pin is never connected
> to MOSI.
>
> Some devices within the PulSAR series are just faster versions of others.
> >From fastest to slowest, AD7980, AD7988-5, AD7686, AD7685, and AD7988-1 are
> all 16-bit pseudo-differential pin-for-pin compatible ADCs. Devices that
> only vary on the sample rate are documented with a common fallback
> compatible.
>
> Signed-off-by: Marcelo Schmitt <marcelo.schmitt@...log.com>
> ---
> Change log v3 -> v4
> - Sorted compatible strings in alphabetical order.
> - Left only fallback compatibles in allOf check list for adi,sdi-pin property.
> - Improved patch description with explanation about how the AD4000 and PulSAR
> devices are different.
>
> Well, I didn't manage to get a dtbs_check message for all the cases I was
> expecting to cover, yet. I trust the test done by maintainers while I don't
> figure out what's wrong with my setup.
>
> FWIW, here's what I tried:
>
> Cloned dt-binding tree from git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
> Fetched and checked out dt/next branch.
> Applied AD4000/PulSAR patches.
> - <62dd96ac9cd> ("iio: adc: ad4000: fix reading unsigned data")
> - <8ebfd0925521> ("iio: adc: ad4000: Check for error code from devm_mutex_init() call")
> - the patches from this patch series
>
> Cloned dtc from git://git.kernel.org/pub/scm/utils/dtc/dtc.git into a directory
> at the same level of linux kernel source dir.
>
> Cloned dt-schema from https://github.com/devicetree-org/dt-schema.git into a
> directory at the same level of linux kernel source dir.
> Within dt-schema,
> mkdir venv
> python3 -m venv venv/
> source venv/bin/activate
> python3 -m ensurepip --default-pip
> python3 -m pip install --upgrade pip setuptools wheel
> pip install yamllint
> pip install dtschema --upgrade
> pip install -e .
>
> export ARCH=arm; export CROSS_COMPILE=arm-linux-gnueabi-
> Ran `./scripts/dtc/update-dtc-source.sh` from the top level of Linux source tree.
> make defconfig
> Added zynq-coraz7s-ad7685.dts to arch/arm/boot/dts/xilinx/.
> Added zynq-coraz7s-ad7685.dtb to arch/arm/boot/dts/xilinx/Makefile.
> make -j4 dtbs_check # but it didn't print anything about adi,sdi-pin value.
> Changed the compatible from "adi,ad7685" to "adi,ad7687" and dtbs_check prints
> arch/arm/boot/dts/xilinx/zynq-coraz7s-ad7685.dtb: adc@0: adi,sdi-pin:0: 'sdi' is not one of ['high', 'low', 'cs']
>
Your process is weird. None of these are needed, especially dtc, except
pip install yamllint and dtschema. Installing dtc suggests you are
working on some old kernel and that's a mistake on its own. Plaese work
on latest mainline / maintainer / next tree.
> -zynq-coraz7s-ad7685.dts file {
> // SPDX-License-Identifier: GPL-2.0
>
> /dts-v1/;
> #include "zynq-7000.dtsi"
>
> / {
> adc_vref: regulator-vref {
> compatible = "regulator-fixed";
> regulator-name = "EVAL 5V Vref";
> regulator-min-microvolt = <5000000>;
> regulator-max-microvolt = <5000000>;
> regulator-always-on;
> };
>
> adc_vdd: regulator-vdd {
> compatible = "regulator-fixed";
> regulator-name = "Eval VDD supply";
> regulator-min-microvolt = <5000000>;
> regulator-max-microvolt = <5000000>;
> regulator-always-on;
> };
>
> adc_vio: regulator-vio {
> compatible = "regulator-fixed";
> regulator-name = "Eval VIO supply";
> regulator-min-microvolt = <3300000>;
> regulator-max-microvolt = <3300000>;
> regulator-always-on;
> };
> };
>
> &spi0 {
> adc@0 {
> compatible = "adi,ad7685";
> reg = <0>;
> spi-max-frequency = <40000000>;
> vdd-supply = <&adc_vdd>;
> vio-supply = <&adc_vio>;
> ref-supply = <&adc_vref>;
> adi,sdi-pin = "sdi";
> };
> };
> -} zynq-coraz7s-ad7685.dts file
>
> .../bindings/iio/adc/adi,ad4000.yaml | 56 +++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> index e413a9d8d2a2..3c1171c7f0e1 100644
> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad4000.yaml
> @@ -19,6 +19,20 @@ description: |
> https://www.analog.com/media/en/technical-documentation/data-sheets/ad4020-4021-4022.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4001.pdf
> https://www.analog.com/media/en/technical-documentation/data-sheets/adaq4003.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7685.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7686.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7687.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7688.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7690.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7691.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7693.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7942.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7946.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7980.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7982.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7983.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7984.pdf
> + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7988-1_7988-5.pdf
>
> $ref: /schemas/spi/spi-peripheral-props.yaml#
>
> @@ -63,6 +77,32 @@ properties:
>
> - const: adi,adaq4003
>
> + - items:
> + - enum:
> + - adi,ad7685
> + - adi,ad7686
> + - adi,ad7980
> + - adi,ad7988-1
> + - adi,ad7988-5
> + - const: adi,ad7983
> +
> + - items:
> + - enum:
> + - adi,ad7688
> + - adi,ad7693
> + - const: adi,ad7687
I don't see where you allow adi,ad7687 alone. Same problem with other
cases.
> +
> + - items:
> + - enum:
> + - adi,ad7690
> + - adi,ad7982
> + - adi,ad7984
> + - const: adi,ad7691
Best regards,
Krzysztof
Powered by blists - more mailing lists