public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
Cc: Jonathan Cameron <jic23@kernel.org>,
	 David Lechner <dlechner@baylibre.com>,
	Nuno Sa <nuno.sa@analog.com>, Andy Shevchenko <andy@kernel.org>,
	 Michal Simek <michal.simek@amd.com>,
	Rob Herring <robh@kernel.org>,
	 Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	linux-iio@vger.kernel.org,  devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	 linux-kernel@vger.kernel.org, saikrishna12468@gmail.com,
	git@amd.com
Subject: Re: [PATCH 5/5] dt-bindings: iio: adc: xilinx-xadc: convert to YAML format
Date: Sat, 21 Feb 2026 11:38:26 +0100	[thread overview]
Message-ID: <20260221-dancing-papaya-wolverine-db8afd@quoll> (raw)
In-Reply-To: <20260220053941.611415-6-sai.krishna.potthuri@amd.com>

On Fri, Feb 20, 2026 at 11:09:41AM +0530, Sai Krishna Potthuri wrote:
> Convert the xilinx-xadc.txt Devicetree binding to a YAML schema format
> and remove the old text binding.
> 
> Signed-off-by: Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
> ---
>  .../bindings/iio/adc/xilinx-xadc.txt          | 141 -------------
>  .../bindings/iio/adc/xilinx-xadc.yaml         | 194 ++++++++++++++++++
>  2 files changed, 194 insertions(+), 141 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/xilinx-xadc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
> deleted file mode 100644
> index f42e18078376..000000000000
> --- a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.txt
> +++ /dev/null
> @@ -1,141 +0,0 @@
> -Xilinx XADC device driver
> -
> -This binding document describes the bindings for the Xilinx 7 Series XADC as well
> -as the UltraScale/UltraScale+ System Monitor.
> -
> -The Xilinx XADC is an ADC that can be found in the Series 7 FPGAs from Xilinx.
> -The XADC has a DRP interface for communication. Currently two different
> -frontends for the DRP interface exist. One that is only available on the ZYNQ
> -family as a hardmacro in the SoC portion of the ZYNQ. The other one is available
> -on all series 7 platforms and is a softmacro with a AXI interface. This binding
> -document describes the bindings for both of them since the bindings are very
> -similar.
> -
> -The Xilinx System Monitor is an ADC that is found in the UltraScale and
> -UltraScale+ FPGAs from Xilinx. The System Monitor provides a DRP interface for
> -communication. Xilinx provides a standard IP core that can be used to access the
> -System Monitor through an AXI interface in the FPGA fabric. This IP core is
> -called the Xilinx System Management Wizard. This document describes the bindings
> -for this IP.
> -
> -Required properties:
> -	- compatible: Should be one of
> -		* "xlnx,zynq-xadc-1.00.a": When using the ZYNQ device
> -		  configuration interface to interface to the XADC hardmacro.
> -		* "xlnx,axi-xadc-1.00.a": When using the axi-xadc pcore to
> -		  interface to the XADC hardmacro.
> -		* "xlnx,system-management-wiz-1.3": When using the
> -		  Xilinx System Management Wizard fabric IP core to access the
> -		  UltraScale and UltraScale+ System Monitor.
> -	- reg: Address and length of the register set for the device
> -	- interrupts: Interrupt for the XADC control interface.
> -	- clocks: When using the ZYNQ this must be the ZYNQ PCAP clock,
> -	  when using the axi-xadc or the axi-system-management-wizard this must be
> -	  the clock that provides the clock to the AXI bus interface of the core.
> -
> -Optional properties:
> -	- xlnx,external-mux:
> -		* "none": No external multiplexer is used, this is the default
> -		  if the property is omitted.
> -		* "single": External multiplexer mode is used with one
> -		   multiplexer.
> -		* "dual": External multiplexer mode is used with two
> -		  multiplexers for simultaneous sampling.
> -	- xlnx,external-mux-channel: Configures which pair of pins is used to
> -	  sample data in external mux mode.
> -	  Valid values for single external multiplexer mode are:
> -		0: VP/VN
> -		1: VAUXP[0]/VAUXN[0]
> -		2: VAUXP[1]/VAUXN[1]
> -		...
> -		16: VAUXP[15]/VAUXN[15]
> -	  Valid values for dual external multiplexer mode are:
> -		1: VAUXP[0]/VAUXN[0] - VAUXP[8]/VAUXN[8]
> -		2: VAUXP[1]/VAUXN[1] - VAUXP[9]/VAUXN[9]
> -		...
> -		8: VAUXP[7]/VAUXN[7] - VAUXP[15]/VAUXN[15]
> -
> -	  This property needs to be present if the device is configured for
> -	  external multiplexer mode (either single or dual). If the device is
> -	  not using external multiplexer mode the property is ignored.
> -	- xnlx,channels: List of external channels that are connected to the ADC
> -	  Required properties:
> -		* #address-cells: Should be 1.
> -		* #size-cells: Should be 0.
> -
> -	  The child nodes of this node represent the external channels which are
> -	  connected to the ADC. If the property is no present no external
> -	  channels will be assumed to be connected.
> -
> -	  Each child node represents one channel and has the following
> -	  properties:
> -		Required properties:
> -			* reg: Pair of pins the channel is connected to.
> -				0: VP/VN
> -				1: VAUXP[0]/VAUXN[0]
> -				2: VAUXP[1]/VAUXN[1]
> -				...
> -				16: VAUXP[15]/VAUXN[15]
> -			  Note each channel number should only be used at most
> -			  once.
> -		Optional properties:
> -			* xlnx,bipolar: If set the channel is used in bipolar
> -			  mode.
> -
> -
> -Examples:
> -	xadc@f8007100 {
> -		compatible = "xlnx,zynq-xadc-1.00.a";
> -		reg = <0xf8007100 0x20>;
> -		interrupts = <0 7 4>;
> -		interrupt-parent = <&gic>;
> -		clocks = <&pcap_clk>;
> -
> -		xlnx,channels {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -			channel@0 {
> -				reg = <0>;
> -			};
> -			channel@1 {
> -				reg = <1>;
> -			};
> -			channel@8 {
> -				reg = <8>;
> -			};
> -		};
> -	};
> -
> -	xadc@43200000 {
> -		compatible = "xlnx,axi-xadc-1.00.a";
> -		reg = <0x43200000 0x1000>;
> -		interrupts = <0 53 4>;
> -		interrupt-parent = <&gic>;
> -		clocks = <&fpga1_clk>;
> -
> -		xlnx,channels {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -			channel@0 {
> -				reg = <0>;
> -				xlnx,bipolar;
> -			};
> -		};
> -	};
> -
> -	adc@80000000 {
> -		compatible = "xlnx,system-management-wiz-1.3";
> -		reg = <0x80000000 0x1000>;
> -		interrupts = <0 81 4>;
> -		interrupt-parent = <&gic>;
> -		clocks = <&fpga1_clk>;
> -
> -		xlnx,channels {
> -			#address-cells = <1>;
> -			#size-cells = <0>;
> -			channel@0 {
> -				reg = <0>;
> -				xlnx,bipolar;
> -			};
> -		};
> -	};
> diff --git a/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.yaml b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.yaml
> new file mode 100644
> index 000000000000..17508fef1f43
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/xilinx-xadc.yaml

Filename matching compatible, e.g. xlnx,axi-xadc.yaml


> @@ -0,0 +1,194 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/xilinx-xadc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx XADC and System Monitor ADC
> +
> +description:
> +  The Xilinx XADC is an ADC that can be found in the Series 7 FPGAs from Xilinx.
> +  The XADC has a DRP interface for communication. Currently two different
> +  frontends for the DRP interface exist. One that is only available on the ZYNQ
> +  family as a hardmacro in the SoC portion of the ZYNQ. The other one is available
> +  on all series 7 platforms and is a softmacro with an AXI interface.
> +
> +  The Xilinx System Monitor is an ADC that is found in the UltraScale and
> +  UltraScale+ FPGAs from Xilinx. The System Monitor provides a DRP interface for
> +  communication. Xilinx provides a standard IP core that can be used to access the
> +  System Monitor through an AXI interface in the FPGA fabric. This IP core is
> +  called the Xilinx System Management Wizard.
> +
> +  The System Management Wizard can also be accessed via I2C interface for remote
> +  monitoring scenarios where the system Management Wizard is located on a different chip.

Not wrapped correctly. Please read coding style.

> +
> +maintainers:
> +  - Lars-Peter Clausen <lars@metafoo.de>
> +  - Sai Krishna Potthuri <sai.krishna.potthuri@amd.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - xlnx,zynq-xadc-1.00.a
> +      - xlnx,axi-xadc-1.00.a
> +      - xlnx,system-management-wiz-1.3
> +      - xlnx,system-management-wiz-1.3-remote

There was no such compatible and nothing explains why it appeared. You
cannot just add new ABI and claim it is just a conversion.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +    description:
> +      When using the ZYNQ this must be the ZYNQ PCAP clock,
> +      when using the axi-xadc or the axi-system-management-wizard this must be
> +      the clock that provides the clock to the AXI bus interface of the core.
> +
> +  xlnx,external-mux:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - none
> +      - single
> +      - dual
> +    default: none
> +    description: |
> +      External multiplexer configuration:
> +      - "none": No external multiplexer is used (default)

Drop "default", don't repeat constraints in free form text.

> +      - "single": External multiplexer mode is used with one multiplexer
> +      - "dual": External multiplexer mode is used with two multiplexers
> +        for simultaneous sampling
> +
> +  xlnx,external-mux-channel:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 16
> +    description: |
> +      Configures which pair of pins is used to sample data in external mux mode.
> +      Valid values for single external multiplexer mode are 0-16:
> +      0: VP/VN
> +      1-16: VAUXP[0-15]/VAUXN[0-15]
> +      Valid values for dual external multiplexer mode are 1-8:
> +      1-8: VAUXP[0-7]/VAUXN[0-7] - VAUXP[8-15]/VAUXN[8-15]
> +      This property needs to be present if the device is configured for
> +      external multiplexer mode (either single or dual).
> +
> +  xlnx,channels:
> +    $ref: '#/$defs/channels'
> +
> +allOf:

Missing ref since you use unevaluatedProperties...

> +  - if:
> +      required:
> +        - xlnx,external-mux
> +      properties:
> +        xlnx,external-mux:
> +          enum:
> +            - single
> +            - dual
> +    then:
> +      required:
> +        - xlnx,external-mux-channel
> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false

or you meant additionalProperties?

> +
> +$defs:

Why this is a def, not used directly? I see only one usage of this def.

> +  channels:
> +    type: object
> +    description: List of external channels that are connected to the ADC
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      "^channel@([0-9]|1[0-6])$":
> +        type: object
> +        properties:
> +          reg:
> +            minimum: 0
> +            maximum: 16
> +            description: |
> +              Pair of pins the channel is connected to:
> +                0: VP/VN
> +                1-16: VAUXP[0-15]/VAUXN[0-15]
> +              Note each channel number should only be used at most once.
> +
> +          xlnx,bipolar:
> +            type: boolean
> +            description: If set, the channel is used in bipolar mode
> +
> +        required:
> +          - reg
> +
> +        unevaluatedProperties: false

Again, where is any $ref?

> +
> +    required:
> +      - '#address-cells'
> +      - '#size-cells'
> +
> +    unevaluatedProperties: false

And here, please read writing schema and writing bindings docs.

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    xadc@f8007100 {

adc

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

> +        compatible = "xlnx,zynq-xadc-1.00.a";
> +        reg = <0xf8007100 0x20>;
> +        interrupts = <0 7 4>;

Use proper defines.

> +        clocks = <&pcap_clk>;
> +
> +        xlnx,channels {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            channel@0 {
> +                reg = <0>;
> +            };
> +            channel@1 {
> +                reg = <1>;
> +            };
> +            channel@8 {
> +                reg = <8>;
> +            };
> +        };
> +    };
> +
> +  - |
> +    xadc@43200000 {

One example is enough, I don't see differences here.

> +        compatible = "xlnx,axi-xadc-1.00.a";
> +        reg = <0x43200000 0x1000>;
> +        interrupts = <0 53 4>;
> +        clocks = <&fpga1_clk>;
> +
> +        xlnx,channels {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            channel@0 {
> +                reg = <0>;
> +                xlnx,bipolar;
> +            };
> +        };
> +    };
> +
> +  - |
> +    adc@80000000 {

Again, one example is enough, unless you have multiple differences in
properties.

Best regards,
Krzysztof


  reply	other threads:[~2026-02-21 10:38 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20  5:39 [PATCH 0/5] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Sai Krishna Potthuri
2026-02-20  5:39 ` [PATCH 1/5] iio: adc: xilinx-xadc: Add helper functions for the device setup Sai Krishna Potthuri
2026-02-20  7:50   ` Andy Shevchenko
2026-02-20  5:39 ` [PATCH 2/5] iio: adc: xilinx-xadc: Add setup_channels function pointer to ops structure Sai Krishna Potthuri
2026-02-20  7:52   ` Andy Shevchenko
2026-02-20  5:39 ` [PATCH 3/5] iio: adc: xilinx-xadc: Replace module macro with custom init/exit functions Sai Krishna Potthuri
2026-02-20  7:52   ` Andy Shevchenko
2026-02-20  7:54     ` Michal Simek
2026-02-20  8:09       ` Andy Shevchenko
2026-03-18  9:13         ` Sai Krishna Potthuri
2026-03-18  9:46           ` Andy Shevchenko
2026-02-20  5:39 ` [PATCH 4/5] iio: adc: xilinx-xadc: Add I2C interface support Sai Krishna Potthuri
2026-02-20  7:58   ` Andy Shevchenko
2026-02-20  5:39 ` [PATCH 5/5] dt-bindings: iio: adc: xilinx-xadc: convert to YAML format Sai Krishna Potthuri
2026-02-21 10:38   ` Krzysztof Kozlowski [this message]
2026-03-19 13:52     ` Sai Krishna Potthuri
2026-03-19 14:23       ` David Lechner
2026-03-19 14:49         ` Sai Krishna Potthuri
2026-03-19 14:58           ` David Lechner
2026-03-19 15:10             ` Sai Krishna Potthuri
2026-03-19 15:35               ` David Lechner
2026-03-19 15:49                 ` Sai Krishna Potthuri
2026-03-19 16:49                   ` Krzysztof Kozlowski
2026-03-19 19:07                     ` David Lechner
2026-03-22  9:55                       ` Krzysztof Kozlowski
2026-02-21 10:39   ` Krzysztof Kozlowski
2026-02-20  8:00 ` [PATCH 0/5] iio: adc: xilinx-xadc: Add I2C interface support for System Management Wizard Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260221-dancing-papaya-wolverine-db8afd@quoll \
    --to=krzk@kernel.org \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=git@amd.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=sai.krishna.potthuri@amd.com \
    --cc=saikrishna12468@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox