All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ariana Lazar" <ariana.lazar@microchip.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: iio: dac: add support for MCP48FEB02 SPI
Date: Thu, 23 Jul 2026 13:50:59 +0000	[thread overview]
Message-ID: <20260723135115.2AF451F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260723-mcp47feb02_refactor-v1-2-ee59e63672bc@microchip.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema retains `additionalProperties: false` at the root level while introducing a conditional `$ref` to `spi-peripheral-props.yaml` in the `allOf` block. This breaks schema validation for standard SPI properties.
- [Low] The newly added SPI device tree example uses an inappropriate SPI chip select value (`0x60`), copied from the I2C device address example.
--

--- Patch [2]: [PATCH 2/3] dt-bindings: iio: dac: add support for MCP48FEB02 SPI ---
commit a357ffde05c7807cd64bca71103d780d8dde416b
Author: Ariana Lazar <ariana.lazar@microchip.com>

dt-bindings: iio: dac: add support for MCP48FEB02 SPI

This commit adds SPI support for the MCP48FxBy1/2/4/8 DAC families to the
existing MCP47FEB02 device tree binding. It introduces the new compatible
strings, the spi-max-frequency property, and a devicetree example for SPI.

> diff --git a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> index d131f136bd151..d25c763241a4f 100644
> --- a/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/microchip,mcp47feb02.yaml
> @@ -178,6 +239,13 @@ required:
>    - vdd-supply
>  
>  allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            pattern: "^microchip,mcp48f[ev]b[0-2][1248]$"
> +    then:
> +      $ref: /schemas/spi/spi-peripheral-props.yaml#

[ ... ]

> +    then:
> +      properties:
> +        spi-max-frequency: false
> +
>  additionalProperties: false

[Severity: Medium]
Since this schema now includes a conditional $ref to spi-peripheral-props.yaml
in the allOf block, should this be changed to unevaluatedProperties: false?

JSON Schema limits additionalProperties: false to only evaluating properties
explicitly declared in the root properties or patternProperties. Any standard
SPI properties pulled in via the conditional $ref will be incorrectly rejected
by dt_binding_check.

>  examples:
> @@ -299,4 +424,29 @@ examples:
>              };
>          };
>      };
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        dac@60 {
> +          compatible = "microchip,mcp48feb02";
> +          reg = <0x60>;

[Severity: Low]
Is 0x60 intended to be used as an SPI chip select here, or was this copied
from the I2C example?

Typically, an SPI chip select would be 0 or 1, and the node would be named
dac@0 with reg = <0>.

> +          vdd-supply = <&vdac_vdd>;
> +          vref-supply = <&vref_reg>;
> +          spi-max-frequency = <10000000>;
> +
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          channel@0 {
> +            reg = <0>;
> +            label = "Adjustable_voltage_ch0";
> +          };
> +
> +          channel@1 {
> +            reg = <0x1>;
> +            label = "Adjustable_voltage_ch1";
> +          };
> +        };
> +    };
>  ...

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723-mcp47feb02_refactor-v1-0-ee59e63672bc@microchip.com?part=2

  reply	other threads:[~2026-07-23 13:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-23 13:43 [PATCH 0/3] Refactor Microchip MCP47FEB02 I2C driver in separate modules to add support for MCP48FEB02 SPI driver Ariana Lazar
2026-07-23 13:43 ` [PATCH 1/3] iio: dac: mcp47feb02: refactor MCP47FEB02 I2C driver into two modules Ariana Lazar
2026-07-23 14:01   ` sashiko-bot
2026-07-23 21:18   ` Joshua Crofts
2026-07-23 13:43 ` [PATCH 2/3] dt-bindings: iio: dac: add support for MCP48FEB02 SPI Ariana Lazar
2026-07-23 13:50   ` sashiko-bot [this message]
2026-07-23 16:36     ` Conor Dooley
2026-07-23 16:38   ` Conor Dooley
2026-07-23 13:43 ` [PATCH 3/3] iio: dac: add support for Microchip MCP48FEB02 Ariana Lazar
2026-07-23 14:03   ` sashiko-bot
2026-07-23 21:02   ` Joshua Crofts

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=20260723135115.2AF451F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=ariana.lazar@microchip.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --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.