Re: [PATCH v2 1/2] regulator: dt-bindings: qcom,rpmh: document supplies per variant

From: Rob Herring
Date: Thu Apr 14 2022 - 12:08:49 EST


On Tue, Apr 12, 2022 at 09:31:22AM +0200, Krzysztof Kozlowski wrote:
> The RPMH regulator binding covers several devices with different
> regulator supplies, so it uses patterns matching broad range of these
> supplies. This works fine but is not specific and might miss actual
> mistakes when a wrong supply property is used for given variant.
>
> Describe the supplies depending on the compatible, using a defs-allOf
> method.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> ---
> .../regulator/qcom,rpmh-regulator.yaml | 290 +++++++++++++++++-
> 1 file changed, 276 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> index 842ccef691b8..773536fe37c7 100644
> --- a/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/qcom,rpmh-regulator.yaml
> @@ -95,35 +95,297 @@ properties:
> vdd-rgb-supply:
> description: Input supply phandle of rgb.
>
> - vin-lvs-1-2-supply:
> - description: Input supply phandle of one or more regulators.
> -
> - vdd-bob-supply:
> - description: BOB regulator parent supply phandle.
> -
> bob:
> type: object
> $ref: "regulator.yaml#"
> description: BOB regulator node.
>
> patternProperties:
> - "^vdd-s([0-9]+)-supply$":
> - description: Input supply phandle(s) of one or more regulators.
> -
> - "^vdd-(l[0-9]+[-]){1,5}supply$":
> - description: Input supply phandle(s) of one or more regulators.
> -
> "^(smps|ldo|lvs)[0-9]+$":
> type: object
> $ref: "regulator.yaml#"
> description: smps/ldo regulator nodes(s).
>
> -additionalProperties: false
> -
> required:
> - compatible
> - qcom,pmic-id
>
> +allOf:
> + - $ref: "#/$defs/pm6150"
> + - $ref: "#/$defs/pm6150l"
> + - $ref: "#/$defs/pm7325"
> + - $ref: "#/$defs/pm8005"
> + - $ref: "#/$defs/pm8009"
> + - $ref: "#/$defs/pm8150"
> + - $ref: "#/$defs/pm8150l"
> + - $ref: "#/$defs/pm8350"
> + - $ref: "#/$defs/pm8350c"
> + - $ref: "#/$defs/pm8450"
> + - $ref: "#/$defs/pm8998"
> + - $ref: "#/$defs/pmg1110"
> + - $ref: "#/$defs/pmi8998"
> + - $ref: "#/$defs/pmr735a"
> + - $ref: "#/$defs/pmx55"
> + - $ref: "#/$defs/pmx65"
> +
> +unevaluatedProperties: false
> +
> +$defs:

I'm not following on why you need $defs here rather than putting the
if/then schemas under the 'allOf'. $defs should primarily only be used
where it saves duplicating a schema 2 or more times. That could be the
case here if there's a case that's a subset of another case.


> + pm6150:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm6150-rpmh-regulators
> + then:
> + properties:
> + vdd-l2-l3-supply: true
> + vdd-l4-l7-l8-supply: true
> + vdd-l5-l16-l17-l18-l19-supply: true
> + vdd-l10-l14-l15-supply: true
> + vdd-l11-l12-l13-supply: true
> + patternProperties:
> + "^vdd-l[169]-supply$": true
> + "^vdd-s[1-5]-supply$": true
> +
> + pm6150l:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm6150l-rpmh-regulators
> + then:
> + properties:
> + vdd-bob-supply:
> + description: BOB regulator parent supply phandle.
> + vdd-l1-l8-supply: true
> + vdd-l2-l3-supply: true
> + vdd-l4-l5-l6-supply: true
> + vdd-l7-l11-supply: true
> + vdd-l9-l10-supply: true
> + patternProperties:
> + "^vdd-s[1-8]-supply$": true
> +
> + pm7325:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm7325-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l4-l12-l15-supply: true
> + vdd-l2-l7-supply: true
> + vdd-l6-l9-l10-supply: true
> + vdd-l11-l17-l18-l19-supply: true
> + vdd-l13-supply: true
> + vdd-l14-l16-supply: true
> + patternProperties:
> + "^vdd-l[358]-supply$": true
> + "^vdd-s[1-8]-supply$": true
> +
> + pm8005:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8005-rpmh-regulators
> + then:
> + patternProperties:
> + "^vdd-s[1-4]-supply$": true
> +
> + pm8009:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8009-rpmh-regulators
> + - qcom,pm8009-1-rpmh-regulators
> + then:
> + properties:
> + vdd-l5-l6-supply: true
> + patternProperties:
> + "^vdd-l[1-47]-supply$": true
> + "^vdd-s[1-2]-supply$": true
> +
> + pm8150:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8150-rpmh-regulators
> + - qcom,pmm8155au-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l8-l11-supply: true
> + vdd-l2-l10-supply: true
> + vdd-l3-l4-l5-l18-supply: true
> + vdd-l6-l9-supply: true
> + vdd-l7-l12-l14-l15-supply: true
> + vdd-l13-l16-l17-supply: true
> + patternProperties:
> + "^vdd-s([1-9]|10)-supply$": true
> +
> + pm8150l:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8150l-rpmh-regulators
> + then:
> + properties:
> + vdd-bob-supply:
> + description: BOB regulator parent supply phandle.
> + vdd-l1-l8-supply: true
> + vdd-l2-l3-supply: true
> + vdd-l4-l5-l6-supply: true
> + vdd-l7-l11-supply: true
> + vdd-l9-l10-supply: true
> + patternProperties:
> + "^vdd-s[1-8]-supply$": true
> +
> + pm8350:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8350-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l4-supply: true
> + vdd-l2-l7-supply: true
> + vdd-l3-l5-supply: true
> + vdd-l6-l9-l10-supply: true
> + vdd-l8-supply: true
> + patternProperties:
> + "^vdd-s([1-9]|1[0-2])-supply$": true
> +
> + pm8350c:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8350c-rpmh-regulators
> + then:
> + properties:
> + vdd-bob-supply:
> + description: BOB regulator parent supply phandle.
> + vdd-l1-l12-supply: true
> + vdd-l2-l8-supply: true
> + vdd-l3-l4-l5-l7-l13-supply: true
> + vdd-l6-l9-l11-supply: true
> + vdd-l10-supply: true
> + patternProperties:
> + "^vdd-s([1-9]|10)-supply$": true
> +
> + pm8450:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8450-rpmh-regulators
> + then:
> + patternProperties:
> + "^vdd-l[1-4]-supply$": true
> + "^vdd-s[1-6]-supply$": true
> +
> + pm8998:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pm8998-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l27-supply: true
> + vdd-l2-l8-l17-supply: true
> + vdd-l3-l11-supply: true
> + vdd-l4-l5-supply: true
> + vdd-l6-supply: true
> + vdd-l7-l12-l14-l15-supply: true
> + vdd-l9-supply: true
> + vdd-l10-l23-l25-supply: true
> + vdd-l13-l19-l21-supply: true
> + vdd-l16-l28-supply: true
> + vdd-l18-l22-supply: true
> + vdd-l20-l24-supply: true
> + vdd-l26-supply: true
> + vin-lvs-1-2-supply: true
> + patternProperties:
> + "^vdd-s([1-9]|1[0-3])-supply$": true
> +
> + pmg1110:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pmg1110-rpmh-regulators
> + then:
> + properties:
> + vdd-s1-supply: true
> +
> + pmi8998:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pmi8998-rpmh-regulators
> + then:
> + properties:
> + vdd-bob-supply:
> + description: BOB regulator parent supply phandle.
> +
> + pmr735a:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pmr735a-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l2-supply: true
> + vdd-l3-supply: true
> + vdd-l4-supply: true
> + vdd-l5-l6-supply: true
> + patternProperties:
> + "^vdd-s[1-3]-supply$": true
> +
> + pmx55:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pmx55-rpmh-regulators
> + then:
> + properties:
> + vdd-l1-l2-supply: true
> + vdd-l3-l9-supply: true
> + vdd-l4-l12-supply: true
> + vdd-l5-l6-supply: true
> + vdd-l7-l8-supply: true
> + vdd-l10-l11-l13-supply: true
> + patternProperties:
> + "^vdd-l1[4-6]-supply$": true
> + "^vdd-s[1-7]-supply$": true
> +
> + pmx65:
> + if:
> + properties:
> + compatible:
> + enum:
> + - qcom,pmx65-rpmh-regulators
> + then:
> + properties:
> + vdd-l2-l18-supply: true
> + vdd-l5-l6-l16-supply: true
> + vdd-l8-l9-supply: true
> + vdd-l11-l13-supply: true
> + patternProperties:
> + "^vdd-l[1347]-supply$": true
> + "^vdd-l1[0245789]-supply$": true
> + "^vdd-l2[01]-supply$": true
> + "^vdd-s[1-8]-supply$": true
> +
> examples:
> - |
> #include <dt-bindings/regulator/qcom,rpmh-regulator.h>
> --
> 2.32.0
>
>