Re: [PATCH v6 3/5] dt-bindings: iio: adc: microchip,mcp3911: Add spi-device-addr

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Driver code fails to parse the newly introduced `spi-device-addr` devicetree property.
- [Low] The `spi-device-addr` property definition omits the default fallback value that was explicitly documented for the deprecated property.
--

--- Patch [3]: [PATCH v6 3/5] dt-bindings: iio: adc: microchip,mcp3911: Add spi-device-addr ---
commit 477e23e3dcd5151c9814b58cb6b94b490d963f05
Author: Janani Sunil <janani.sunil@analog.com>

dt-bindings: iio: adc: microchip,mcp3911: Add spi-device-addr

This commit adds the generic spi-device-addr property to the binding
schema and marks the vendor-specific microchip,device-addr property
as deprecated.

> diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
> index 3a69ec60edb91..4a60df06bd357 100644
> --- a/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,mcp3911.yaml
> @@ -57,6 +57,12 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [0, 1, 2, 3]
>      default: 0
> +    deprecated: true
> +
> +  spi-device-addr:
> +    maxItems: 1
> +    items:
> +      enum: [0, 1, 2, 3]

[Severity: Low]
Is it intentional to omit the default fallback value here?

The deprecated property explicitly documents a default value of 0, but this
new definition omits it. This causes the schema to lose information about
the fallback behavior when the address property is omitted. Should it
include default: [0]?

[ ... ]

> @@ -86,7 +92,7 @@ examples:
>          interrupts = <15 2>;
>          reset-gpios = <&gpio1 10 GPIO_ACTIVE_LOW>;
>          spi-max-frequency = <20000000>;
> -        microchip,device-addr = <0>;
> +        spi-device-addr = <0>;

[Severity: Medium]
Will the driver fail to communicate with the hardware if a device tree uses
this newly recommended property?

The devicetree example explicitly advertises the new spi-device-addr
property, but looking at drivers/iio/adc/mcp3911.c:mcp3911_probe(), the
driver only attempts to read the older properties:

	ret = device_property_read_u32(dev, "microchip,device-addr", &adc->dev_addr);
	if (ret)
		device_property_read_u32(dev, "device-addr", &adc->dev_addr);

Similarly, in drivers/iio/adc/mcp3564.c:mcp3564_config(), the driver only
attempts to read the deprecated vendor property:

	device_property_read_u32(dev, "microchip,hw-device-address", &tmp);

Because the driver code does not parse spi-device-addr, it seems it will
silently fall back to the default address if a device tree provides the
new property instead of the deprecated ones.

>          vref-supply = <&vref_reg>;
>          clocks = <&xtal>;
>        };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-ad5529r-driver-v6-0-cfdf8b9f5ee3@analog.com?part=3