Re: [PATCH v7 3/6] dt-bindings: mfd: motorola-cpcap: convert to DT schema

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] YAML schema omits `#address-cells` and `#size-cells` properties for the root node, causing validation failures for existing device trees.
- [Medium] Schema strictly expects the power button node to be named `key-power`, breaking validation for the legacy `button` node.
- [Medium] The `audio-codec` schema expects a `ports` wrapper, breaking validation for legacy direct `port@0` definitions.
- [Low] The `spi-cs-high` property was dropped from the schema's `required` list.
- [Low] Commit message description is not capitalized, violating MFD subsystem guidelines.
--

--- Patch [3]: [PATCH v7 3/6] dt-bindings: mfd: motorola-cpcap: convert to DT schema ---
commit 4de37a9bd5e5179740a9f015e1900ab235da93ae
Author: Svyatoslav Ryhel <[email protected]>

dt-bindings: mfd: motorola-cpcap: convert to DT schema
    
Convert devicetree bindings for the Motorola CPCAP MFD from TXT to YAML.

> dt-bindings: mfd: motorola-cpcap: convert to DT schema

[Severity: Low]
This isn't a bug, but the MFD subsystem guidelines require the description
after the subsystem prefix to be capitalized. Should "convert" be "Convert"?

> diff --git a/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
> new file mode 100644
> index 0000000000000..76705ea56805a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
> @@ -0,0 +1,403 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/motorola,cpcap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Motorola CPCAP PMIC MFD
> +
> +maintainers:
> +  - Svyatoslav Ryhel <[email protected]>
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: motorola,cpcap
> +      - const: st,6556002

[Severity: Medium]
Does omitting #address-cells and #size-cells from the properties block
break validation for existing device trees?

Because this schema sets additionalProperties: false, running make dtbs_check
against existing in-tree device trees (like
arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi, which uses
#address-cells = <1> and #size-cells = <0> on the pmic@0 node) will falsely
report validation errors.

[ ... ]

> +  audio-codec:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      interrupts:
> +        items:
> +          - description: headset detect interrupt
> +          - description: microphone bias 2 detect interrupt
> +
> +      interrupt-names:
> +        items:
> +          - const: hs
> +          - const: mb2
> +
> +      "#sound-dai-cells":
> +        const: 1
> +
> +      VAUDIO-supply:
> +        description:
> +          Codec power supply, usually VAUDIO regulator of CPCAP.
> +
> +      ports:

[Severity: Medium]
Will this strict ports wrapper requirement cause validation failures for
legacy device trees?

Existing device trees place port@0 directly inside the audio-codec node:

arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi:
    cpcap_audio: audio-codec {
        ...
        port@0 {
            cpcap_audio_codec0: endpoint { ... };
        };
    };

With additionalProperties: false set on the audio-codec node, dtbs_check
will flag these direct ports as illegal properties.

[ ... ]

> +  charger:
> +    $ref: /schemas/power/supply/cpcap-charger.yaml#
> +
> +  key-power:
> +    $ref: /schemas/input/motorola,cpcap-pwrbutton.yaml#

[Severity: Medium]
Does hardcoding the node name as key-power break validation for the legacy
button node?

Existing device trees use the button node name:

arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi:
    power_button: button {
        compatible = "motorola,cpcap-pwrbutton";
        ...
    };

Since this schema enforces the exact name key-power, make dtbs_check
will flag the existing button node as an invalid property.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - spi-max-frequency

[Severity: Low]
Is it intentional to drop spi-cs-high from this list?

The legacy text binding explicitly documented spi-cs-high as a required
property, which reflects the hardware's active-high chip select requirement.
Omitting it here relaxes the constraint and might allow inaccurate device trees
to pass validation.

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