From: sashiko-bot@kernel.org
To: "Kim Seer Paller" <kimseer.paller@analog.com>
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml
Date: Tue, 21 Jul 2026 09:00:50 +0000 [thread overview]
Message-ID: <20260721090051.1B2831F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260721-iio-ad5710r-upstream-v2-2-324949dc72da@analog.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `adi,range-double` property is defined as a global device property, artificially limiting the hardware's per-channel capabilities and conflicting with current output modes.
- [Medium] The `reg` property inside `patternProperties` incorrectly uses `items` to apply numeric constraints, rendering channel bounds checking completely ineffective.
--
--- Patch [2]: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml ---
commit fe69da2e77f6f1de3acc9e529bec6856901e4e98
Author: Kim Seer Paller <kimseer.paller@analog.com>
dt-bindings: iio: dac: add adi,ad5710r.yaml
Add device tree bindings for the Analog Devices AD5710R/AD5711R
8-channel 12-/16-bit Configurable IDAC/VDAC.
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
> new file mode 100644
> index 0000000000000..17c1bf0092421
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5710r.yaml
[ ... ]
> + adi,range-double:
> + description:
> + Configure the output range for all channels. If the property is present,
> + the output will range from 0V to 2Vref. If the property is not present,
> + the output will range from 0V to Vref.
> + type: boolean
[Severity: Medium]
Since the description notes that each channel can be independently configured
as either a voltage or current output, should this property be placed inside
the channel patternProperties block instead?
Defining it globally seems to restrict configuring the output range on a
per-channel basis, and might also semantically conflict with channels
configured for current output.
> +
> +patternProperties:
> + "^channel@[0-7]$":
> + $ref: /schemas/iio/dac/dac.yaml#
> + type: object
> + description:
> + Represents the external channels which are connected to the DAC.
> +
> + properties:
> + reg:
> + description: Channel number
> + items:
> + minimum: 0
> + maximum: 7
[Severity: Medium]
Does placing minimum and maximum inside an items dictionary correctly apply
the constraints?
In dt-schema, the reg property is mapped to a matrix. Wrapping the constraints
in items applies them to the tuple array instead of the scalar integers,
causing schema validation to ignore them and silently accept out-of-bounds
channel numbers.
Could these bounds be placed directly under the reg property?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-iio-ad5710r-upstream-v2-0-324949dc72da@analog.com?part=2
next prev parent reply other threads:[~2026-07-21 9:00 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-21 8:47 [PATCH v2 0/4] Add support for AD5710R/AD5711R DAC Kim Seer Paller
2026-07-21 8:47 ` [PATCH v2 1/4] iio: ABI: Add DAC current powerdown attributes and 15kohm_to_gnd mode Kim Seer Paller
2026-07-21 8:47 ` [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml Kim Seer Paller
2026-07-21 9:00 ` sashiko-bot [this message]
2026-07-21 15:52 ` Conor Dooley
2026-07-24 21:57 ` Jonathan Cameron
2026-07-28 15:41 ` Conor Dooley
2026-07-24 22:10 ` Jonathan Cameron
2026-07-21 8:47 ` [PATCH v2 3/4] iio: dac: ad3530r: parameterize DAC resolution Kim Seer Paller
2026-07-28 16:40 ` Nuno Sá
2026-07-21 8:47 ` [PATCH v2 4/4] iio: dac: ad3530r: add support for AD5710R/AD5711R Kim Seer Paller
2026-07-21 8:58 ` sashiko-bot
2026-07-21 10:38 ` Andy Shevchenko
2026-07-23 4:54 ` Kim Seer Paller
2026-07-24 22:26 ` 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=20260721090051.1B2831F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=kimseer.paller@analog.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/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.