Re: [PATCH] regulator: dt-bindigs: ti,pbias-omap: Convert to DT schema

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Top-level `$ref: regulator.yaml#` incorrectly applies standard regulator constraints to the parent container node.
- [Medium] The schema incorrectly uses `oneOf` to restrict the parent node to exactly one regulator subnode, which prevents valid configurations like OMAP3 from defining both MMC and SIM regulators.
--

--- Patch [1]: [PATCH] regulator: dt-bindigs: ti,pbias-omap: Convert to DT schema ---
commit e24c9bc6e58ceda1b221ec8e8ce86f428408d847
Author: Bhargav Joshi <j.bhargav.u@gmail.com>

regulator: dt-bindigs: ti,pbias-omap: Convert to DT schema

Convert Texas Instruments PBIAS internal regulator from text to DT
schema. No Functional changes are introduced.

> diff --git a/Documentation/devicetree/bindings/regulator/ti,pbias-omap.yaml b/Documentation/devicetree/bindings/regulator/ti,pbias-omap.yaml
> new file mode 100644
> index 0000000000000..61a4dc87c48c9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/ti,pbias-omap.yaml

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - syscon
> +
> +oneOf:
> +  - required:
> +      - pbias_mmc_omap2430
> +  - required:
> +      - pbias_sim_omap3
> +  - required:
> +      - pbias_mmc_omap4
> +  - required:
> +      - pbias_mmc_omap5

[Severity: Medium]
Will this reject valid configurations that define more than one child
regulator?

The JSON schema oneOf keyword requires exactly one condition to match. Since
OMAP3 SoCs can have both MMC and SIM PBIAS regulators, and the driver
supports them concurrently, having both defined would cause oneOf to fail.
Should this use anyOf instead?

> +
> +allOf:
> +  - $ref: regulator.yaml#

[Severity: Medium]
Does applying the standard regulator schema to the top-level parent node
neutralize validation?

This allows a device tree author to mistakenly place properties like
regulator-min-microvolt directly on the parent node instead of the
subnodes. Such properties would be ignored by the kernel but silently pass
the schema check.

> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - ti,pbias-omap2
> +    then:
> +      required:
> +        - pbias_mmc_omap2430
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: ti,pbias-omap3
> +    then:
> +      oneOf:
> +        - required:
> +            - pbias_mmc_omap2430
> +        - required:
> +            - pbias_sim_omap3

[Severity: Medium]
Similarly, does this restrict OMAP3 to exactly one regulator subnode? 

This would prevent valid hardware configurations from using both the MMC and
SIM regulators simultaneously. Should this also use anyOf?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-ti-pbias-omap-v1-1-527848131801@gmail.com?part=1