lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ