Re: [PATCH v3 1/2] dt-bindings: iio: light: add Broadcom APDS9999

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unnecessary schema split and contradiction regarding the vled-supply property.
--

commit 16ae176ed1deb8d1509cb369a4821fa40c4b1e66
Author: Jose A. Perez de Azpillaga <azpijr@gmail.com>

dt-bindings: iio: light: add Broadcom APDS9999

This commit adds a new device tree binding for the Broadcom APDS9999 ambient
light and proximity sensor. The commit message states that a separate binding
file is used rather than merging with the existing APDS9300 binding due to
the addition of a vled-supply property.

> A separate binding file is used rather
> than merging with avago,apds9300.yaml because the APDS9999
> has an additional vled-supply for the VCSEL.

Does the implementation match this justification?

> diff --git a/Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml b/Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/brcm,apds9999.yaml
[ ... ]
> +properties:
[ ... ]
> +  vled-supply:
> +    description: VCSEL power supply
> +
> +  interrupts:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +examples:

If the vled-supply property is the primary hardware difference that warrants a
separate schema file, should it be included in the required properties list?

Alternatively, if the supply is genuinely optional, could this have been
handled with a simple conditional in the existing avago,apds9300.yaml schema
instead of splitting into a new file? The device tree binding guidelines
recommend splitting bindings into separate files only when the conditionals
become too unwieldy.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1779181370.git.azpijr@gmail.com?part=1