Re: [PATCH v2 1/3] dt-bindings: Add InvenSense ICM-42370-p accelerometer

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Incomplete device tree binding schema rejects valid hardware configurations for SPI, I3C, and reset lines.
- [Low] Missing IIO subsystem mailing list in MAINTAINERS entry.
- [Low] Contradictory schema constraints for the `interrupts` property.
--

--- Patch [1]: [PATCH v2 1/3] dt-bindings: Add InvenSense ICM-42370-p accelerometer ---
commit 580d5dd4de723a5224325d048b11cddaf0d9a2b1
Author: Kanak Shilledar <[email protected]>

dt-bindings: Add InvenSense ICM-42370-p accelerometer

This commit adds device tree binding documentation for the InvenSense
ICM-42370-P 3-axis accelerometer. The device supports I2C, SPI, and I3C,
but this initial binding is geared towards I2C. The commit updates the
MAINTAINERS file with the new binding.

> diff --git a/Documentation/devicetree/bindings/iio/accel/invensense,icm42370p.yaml b/Documentation/devicetree/bindings/iio/accel/invensense,icm42370p.yaml
> new file mode 100644
> index 0000000000000..df23406921a51
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/invensense,icm42370p.yaml
[ ... ]
> +dependencies:
> +  interrupt-names: [ interrupts ]
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

[Severity: Low]
Does this schema unnecessarily enforce interrupts as a strict requirement?

Since interrupts is listed in the required array, the dependency check 
for interrupt-names: [ interrupts ] above is dead logic. Also, since IIO
sensors typically support polling, making interrupts strictly mandatory 
might prevent valid polling-based hardware designs from passing validation.

> +  - vdd-supply
> +  - vddio-supply
> +
> +unevaluatedProperties: false

[Severity: Medium]
Will this strict constraint actively reject valid hardware descriptions for
SPI, I3C, or reset lines?

The commit message mentions support for SPI, I3C, and resets, but since
standard SPI/I3C peripheral property $ref inclusions and the resets property
are missing, unevaluatedProperties: false will cause validation to fail when
standard properties like spi-max-frequency or resets are provided in the
device tree.

[ ... ]
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 04fa5322d9f74..e4764489ea971 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
[ ... ]
> +INVENSENSE ICM-42370-P ACCELEROMETER
> +M:	Kanak Shilledar <[email protected]>
> +M:	Henrik Grimler <[email protected]>
> +L:	[email protected]

[Severity: Low]
Should this MAINTAINERS entry include the standard IIO subsystem mailing list?

Currently, it only lists a corporate mailing list, omitting 
[email protected] which is typically mandatory for IIO entries. This
means patches modifying this binding will not be automatically CC'd to the 
subsystem list by get_maintainer.pl.

> +S:	Maintained
> +W:	https://invensense.tdk.com/
> +F:	Documentation/devicetree/bindings/iio/accel/invensense,icm42370p.yaml

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1