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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The YAML conversion introduces structural differences and omits previously required properties, causing schema validation to fail for existing in-tree device trees.
- [Medium] Incorrect use of 'additionalProperties: false' in conjunction with inherited schemas via 'allOf: - $ref', blocking standard SPI properties.
- [Medium] The `spi-cs-high` property was required in the original TXT binding but is missing from the YAML `required` list.
- [Medium] The YAML binding strictly requires exactly two compatible strings, contradicting the original TXT binding which allowed 'One or both'.
--

commit a23d8229dfec6cdc65afe36d12b859535df8cfc9
Author: Svyatoslav Ryhel <clamor95@gmail.com>

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

> 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 <clamor95@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: motorola,cpcap
> +      - const: st,6556002

[Severity: Medium]
Does this strict array configuration restrict compatible strings too much?
The original txt binding indicated that "One or both" strings were
acceptable, but this yaml structure enforces exactly both strings in this
specific order. Could this cause a regression for hardware implementations
that only use a single compatible string, which the driver supports?

> +
> +  reg:
> +    maxItems: 1

[Severity: High]
Are #address-cells and #size-cells missing from this top-level node?
The original txt binding explicitly required these, and existing in-tree
device trees (like arch/arm/boot/dts/ti/omap/motorola-cpcap-mapphone.dtsi)
use them. With additionalProperties: false set, will this cause a regression
where existing valid dts files fail validation?

[ ... ]

> +  audio-codec:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
[ ... ]
> +      ports:
> +        $ref: /schemas/graph.yaml#/properties/ports
> +
> +        properties:
> +          port@0:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description: port connected to the Stereo HiFi DAC
> +
> +          port@1:
> +            $ref: /schemas/graph.yaml#/properties/port
> +            description: port connected to the Voice DAC
> +
> +        required:
> +          - port@0
> +          - port@1

[Severity: High]
Does this structurally conflict with existing in-tree device trees?
The new schema requires a ports container for the audio codec, but existing
in-tree device trees define port@0 and port@1 directly under the audio-codec
node without a wrapper. Since no patch in this series updates the
corresponding dtsi files, won't this cause a regression where existing device
trees are incorrectly rejected?

[ ... ]

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

[Severity: High]
Will this correctly match the existing device tree nodes?
The schema expects the subnode to be named key-power, but existing in-tree
device trees use the name button (e.g., power_button: button { ... }).
This mismatch will likely cause a regression for the subnode validation.

[ ... ]

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

[Severity: Medium]
Should spi-cs-high be included in this required list?
The original txt binding explicitly listed spi-cs-high as a required property,
and the pmic physically requires an active-high spi chip select to function.
If it is omitted from this list, could a device tree missing this hardware
constraint incorrectly pass schema validation?

> +
> +additionalProperties: false

[Severity: Medium]
Should this use unevaluatedProperties: false instead of
additionalProperties: false?

When inheriting from another schema like spi-peripheral-props.yaml via an
allOf reference, additionalProperties: false will reject the inherited
properties unless they are manually redefined in this file. Using
unevaluatedProperties would properly handle the inherited spi properties
and remove the need to manually declare spi-cs-high, spi-cpol, and
spi-cpha above.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520182124.117863-1-clamor95@gmail.com?part=3