Re: [PATCH v3 2/3] dt-bindings: mfd: sprd,sc2731: Include SC2730 regulator bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The newly referenced `sprd,sc2730-regulator.yaml` schema contains a critical bug that causes it to falsely reject valid device tree nodes.
- [Medium] Unconditional relaxation of the `regulators` node schema permits incorrect hardware configurations to silently pass validation.
--

commit 0ef24a84813b14435877f202403f8d49b85173ac
Author: Otto Pflüger <otto.pflueger@abscue.de>

dt-bindings: mfd: sprd,sc2731: Include SC2730 regulator bindings

This commit updates the SC2731 MFD binding to allow using the SC2730
regulator bindings for the regulators node. The SC2730 PMIC provides a
different set of regulators than the SC2731.

> diff --git a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
> index b023e1ef8d3cc..e74ec49709941 100644
> --- a/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
> +++ b/Documentation/devicetree/bindings/mfd/sprd,sc2731.yaml
> @@ -54,7 +54,9 @@ properties:
>  
>    regulators:
>      type: object
> -    $ref: /schemas/regulator/sprd,sc2731-regulator.yaml#
> +    oneOf:
> +      - $ref: /schemas/regulator/sprd,sc2730-regulator.yaml#

Does the newly referenced sprd,sc2730-regulator.yaml schema contain an
issue that causes it to falsely reject valid device tree nodes?

This issue appears to be introduced by the preceding patch in the series
(commit "regulator: dt-bindings: Add Unisoc SC2730 PMIC") but manifests
here because this commit adds the reference to the broken schema.

The sprd,sc2730-regulator.yaml file specifies "required: [compatible]"
and "additionalProperties: false", but entirely omits the properties block
necessary to define the compatible property.

Because dt-schema rules forbid any property not explicitly defined when
"additionalProperties: false" is set, it seems any device tree node
defining an SC2730 regulator node will instantly fail dtbs_check for
containing an additional property named compatible.

Does this unconditional oneOf block permit incorrect hardware configurations
to silently pass validation?

This change replaces a single $ref for the regulators node with an
unconditional oneOf block. This allows any PMIC compatible string
supported by this MFD binding to legally declare either the SC2730 or
SC2731 regulator bindings.

Because the commit message correctly notes that the SC2730 provides a
different set of regulators, should the schema strictly associate the
regulator binding with the corresponding top-level PMIC compatible string?

This could be implemented using an allOf list of if-then conditional blocks
that evaluate the parent node's compatible string to selectively apply the
correct $ref to the regulators node, preventing cross-generational hardware
mismatches from silently passing validation.

> +      - $ref: /schemas/regulator/sprd,sc2731-regulator.yaml#

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-sc2730-regulators-v3-0-5bf0e02507e3@abscue.de?part=2