All of lore.kernel.org
 help / color / mirror / Atom feed
From: Janani Sunil <jan.sun97@gmail.com>
To: Jonathan Cameron <jic23@kernel.org>
Cc: "Rodrigo Alencar" <455.rodrigo.alencar@gmail.com>,
	"Janani Sunil" <janani.sunil@analog.com>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Michael Hennerich" <Michael.Hennerich@analog.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <skhan@linuxfoundation.org>,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	"Mark Brown" <broonie@kernel.org>
Subject: Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
Date: Fri, 19 Jun 2026 12:33:11 +0200	[thread overview]
Message-ID: <076d7d2d-81a0-49c2-af94-bd65ead66c09@gmail.com> (raw)
In-Reply-To: <20260614204455.408c4d40@jic23-huawei>


On 6/14/26 21:44, Jonathan Cameron wrote:
> On Tue, 9 Jun 2026 16:47:23 +0200
> Janani Sunil <jan.sun97@gmail.com> wrote:
>
>> On 5/26/26 15:11, Rodrigo Alencar wrote:
>>> On 26/05/19 05:42PM, Janani Sunil wrote:
>>>> Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
>>>> buffered voltage output digital-to-analog converter (DAC) with an
>>>> integrated precision reference.
>>> ...
>>> Probably others may comment on that, but...
>>>
>>> This parent node may support device addressing for multi-device support through
>>> those ID pins. I suppose that each device may have its own power supplies or
>>> other resources like the toggle pins or reset and enable.
>>>
>>> That way I suppose that an example would look like...
>>>   
>>>> +
>>>> +patternProperties:
>>>> +  "^channel@([0-9]|1[0-5])$":
>>>> +    type: object
>>>> +    description: Child nodes for individual channel configuration
>>>> +
>>>> +    properties:
>>>> +      reg:
>>>> +        description: Channel number.
>>>> +        minimum: 0
>>>> +        maximum: 15
>>>> +
>>>> +      adi,output-range-microvolt:
>>>> +        description: |
>>>> +          Output voltage range for this channel as [min, max] in microvolts.
>>>> +          If not specified, defaults to 0V to 5V range.
>>>> +        oneOf:
>>>> +          - items:
>>>> +              - const: 0
>>>> +              - enum: [5000000, 10000000, 20000000, 40000000]
>>>> +          - items:
>>>> +              - const: -5000000
>>>> +              - const: 5000000
>>>> +          - items:
>>>> +              - const: -10000000
>>>> +              - const: 10000000
>>>> +          - items:
>>>> +              - const: -15000000
>>>> +              - const: 15000000
>>>> +          - items:
>>>> +              - const: -20000000
>>>> +              - const: 20000000
>>>> +
>>>> +    required:
>>>> +      - reg
>>>> +
>>>> +    additionalProperties: false
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +  - vdd-supply
>>>> +  - avdd-supply
>>>> +  - hvdd-supply
>>>> +
>>>> +dependencies:
>>>> +  spi-cpha: [ spi-cpol ]
>>>> +  spi-cpol: [ spi-cpha ]
>>>> +
>>>> +allOf:
>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>> +
>>>> +unevaluatedProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    #include <dt-bindings/gpio/gpio.h>
>>>> +
>>>> +    spi {
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <0>;
>>>> +
>>>> +        dac@0 {
>>>> +            compatible = "adi,ad5529r-16";
>>>> +            reg = <0>;
>>>> +            spi-max-frequency = <25000000>;
>>>> +
>>>> +            vdd-supply = <&vdd_regulator>;
>>>> +            avdd-supply = <&avdd_regulator>;
>>>> +            hvdd-supply = <&hvdd_regulator>;
>>>> +            hvss-supply = <&hvss_regulator>;
>>>> +
>>>> +            reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>> +
>>>> +            #address-cells = <1>;
>>>> +            #size-cells = <0>;
>>>> +
>>>> +            channel@0 {
>>>> +                reg = <0>;
>>>> +                adi,output-range-microvolt = <0 5000000>;
>>>> +            };
>>>> +
>>>> +            channel@1 {
>>>> +                reg = <1>;
>>>> +                adi,output-range-microvolt = <(-10000000) 10000000>;
>>>> +            };
>>>> +
>>>> +            channel@2 {
>>>> +                reg = <2>;
>>>> +                adi,output-range-microvolt = <0 40000000>;
>>>> +            };
>>>> +        };
>>>> +    };
>>> ...
>>>
>>> 	spi {
>>> 		#address-cells = <1>;
>>> 		#size-cells = <0>;
>>>
>>> 		multi-dac@0 {
>>> 			compatible = "adi,ad5529r-16";
>>> 			reg = <0>;
>>> 			spi-max-frequency = <25000000>;
>>>
>>> 			#address-cells = <1>;
>>> 			#size-cells = <0>;
>>>
>>> 			dac@0 {
>>> 				reg = <0>;
>>> 				vdd-supply = <&vdd_regulator>;
>>> 				avdd-supply = <&avdd_regulator>;
>>> 				hvdd-supply = <&hvdd_regulator>;
>>> 				hvss-supply = <&hvss_regulator>;
>>>
>>> 				reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
>>>
>>> 				#address-cells = <1>;
>>> 				#size-cells = <0>;
>>>
>>> 				channel@0 {
>>> 					reg = <0>;
>>> 					adi,output-range-microvolt = <0 5000000>;
>>> 				};
>>>
>>> 				channel@1 {
>>> 					reg = <1>;
>>> 					adi,output-range-microvolt = <(-10000000) 10000000>;
>>> 				};
>>>
>>> 				channel@2 {
>>> 					reg = <2>;
>>> 					adi,output-range-microvolt = <0 40000000>;
>>> 				};
>>> 			}
>>>
>>> 			dac@1 {
>>> 				reg = <1>;
>>> 				vdd-supply = <&vdd_regulator>;
>>> 				avdd-supply = <&avdd_regulator>;
>>> 				hvdd-supply = <&hvdd_regulator>;
>>> 				hvss-supply = <&hvss_regulator>;
>>>
>>> 				reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
>>>
>>> 				#address-cells = <1>;
>>> 				#size-cells = <0>;
>>>
>>> 				channel@0 {
>>> 					reg = <0>;
>>> 					adi,output-range-microvolt = <0 5000000>;
>>> 				};
>>>
>>> 				channel@1 {
>>> 					reg = <1>;
>>> 					adi,output-range-microvolt = <(-10000000) 10000000>;
>>> 				};
>>> 			}
>>> 		};
>>> 	};
>>>
>>> then you might need something like:
>>>
>>> 	patternProperties:
>>> 		"^dac@[0-3]$":
>>>
>>> and put most of the things under this node pattern.
>>>
>>> So the main driver that you're putting together might need to handle up to four instances.
>>> Even if your current driver cannot handle this, the dt-bindings might need cover that.
>>>
>>> Need to double check if each dac node needs a separate compatible, so you would maybe populate
>>> a platform data to be shared with the child nodes, which would be a separate driver.
>>> (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
>> Hi Rodrigo,
>>
>> Thank you for looking at this.
>>
>> For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
>> hardware/use case we have only needs one device node and the driver is written around that model as well.
>> While the device addressing pins could allow multi-device topology, we do not have an actual platform using
>> that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
>> speculatively without a validating use case.
> Interesting feature - kind of similar to address control on a typical i2c bus device, or
> looking at it another way a kind of distributed SPI mux.
>
> Challenge of a binding is we need to anticipate the future.  So I think we do need something
> like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> That would leave the path open to supporting the addressing at a later date.
> An alternative might be to look at it like a chained device setup. In those we pretend there
> is just one device with a lot of channels etc.  The snag is that here things are more loosely
> coupled whereas for those devices it tends to be you have to read / write the same register
> in all devices in the chain as one big SPI message.
>
> +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> - Each of these device has 2 ID pins.  The SPI transfers have to contain the 2 bit
> value that matches that or they are ignored.  Thus a single bus + 1 chip select can
> be used to talk to 4 devices.  Question is what that looks like in device tree + I guess
> longer term how to support it cleanly in SPI.
>
> Jonathan

Hi Jonathan, Rob, Krzysztof, Conor,

One possible model that would also allow mixing the 12-bit and 16-bit variants would be to treat the parent node
as the shared SPI transport only, and let each dac@N child carry its own compatible.

Rob, Krzysztof, Conor — wanted to get your input on whether this is an acceptable binding pattern.

properties:
   compatible:
     const: adi,ad5529r-bus

patternProperties:
   "^dac@[0-3]$":
     type: object
     properties:
       compatible:
         enum:
           - adi,ad5529r-16
           - adi,ad5529r-12
       reg:
         minimum: 0
         maximum: 3

With a DT example such as:

ad5529r@0 {
         compatible = "adi,ad5529r-bus";
         reg = <0>;

         dac@0 {
                 compatible = "adi,ad5529r-16";
                 reg = <0>;
         };

         dac@1 {
                 compatible = "adi,ad5529r-12";
                 reg = <1>;
         };
};

The downside is that it introduces adi,ad5529r-bus as a compatible that does not correspond to an actual
standalone device variant - it would require a parent driver to manage the shared SPI transport and enumerate the
child devices. The actual DAC functionality is handled by the matching per-child compatibles(12 or 16 bit).
Is this an acceptable pattern, or is there a preferred way to model this type of addressing scheme?

Regards,
Janani Sunil


  reply	other threads:[~2026-06-19 10:33 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-19 15:42 [PATCH v3 0/2] iio: dac: Add support for AD5529R DAC Janani Sunil
2026-05-19 15:42 ` [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R Janani Sunil
2026-05-19 15:55   ` sashiko-bot
2026-05-22 17:02   ` Jonathan Cameron
2026-05-25 16:30     ` Conor Dooley
2026-05-26 12:35       ` Jonathan Cameron
2026-05-26 13:11   ` Rodrigo Alencar
2026-06-09 14:47     ` Janani Sunil
2026-06-14 19:44       ` Jonathan Cameron
2026-06-19 10:33         ` Janani Sunil [this message]
2026-06-19 11:31           ` Nuno Sá
2026-06-19 11:36           ` Conor Dooley
2026-06-19 11:40             ` Conor Dooley
2026-06-19 13:01               ` Nuno Sá
2026-06-19 14:12                 ` Conor Dooley
2026-06-19 15:54                   ` Nuno Sá
2026-06-21 14:33                     ` Jonathan Cameron
2026-06-21 18:35                       ` Conor Dooley
2026-05-19 15:42 ` [PATCH v3 2/2] iio: dac: Add AD5529R DAC driver support Janani Sunil
2026-05-19 16:22   ` sashiko-bot
2026-05-22 17:24   ` Jonathan Cameron

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=076d7d2d-81a0-49c2-af94-bd65ead66c09@gmail.com \
    --to=jan.sun97@gmail.com \
    --cc=455.rodrigo.alencar@gmail.com \
    --cc=Michael.Hennerich@analog.com \
    --cc=andy@kernel.org \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=janani.sunil@analog.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=lars@metafoo.de \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.