Re: [PATCH] dt-bindings: regulator: mediatek,mt6397-regulator: convert to YAML

[Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>]

On 8/6/24 21:41, Krzysztof Kozlowski wrote:
> 	
>
> External email : Please do not click links or open attachments until you 
> have verified the sender or the content.
>
> On 06/08/2024 14:25, Macpaul Lin wrote:
> > Convert the MediaTek MT6397 regulator bindings to DT schema.
> >
> > Signed-off-by: Sen Chu <sen.chu@xxxxxxxxxxxx>
> > Signed-off-by: Macpaul Lin <macpaul.lin@xxxxxxxxxxxx>
>
>
> > +
> > +patternProperties:
> > +  "^(buck_)?v(core|drm|gpu|io18|pca(7|15)|sramca(7|15))$":
> > +    description: Buck regulators
> > +    type: object
> > +    $ref: regulator.yaml#
> > +    properties:
> > +      regulator-allowed-modes:
> > +        description: |
> > +          BUCK regulators can set regulator-initial-mode and regulator-allowed-modes to
> > +          values specified in dt-bindings/regulator/mediatek,mt6397-regulator.h
> > +        items:
> > +          enum: [0, 1]
> > +      regulator-compatible:
> > +        pattern: "^(buck_)?v(core|drm|gpu|io18|pca(7|15)|sramca(7|15))$"
>
> This should not be needed. Same in every other place.
>
> > +    unevaluatedProperties: false
> > +
> > +  "^(ldo_)?v(tcxo|(a|io)28)$":
> > +    description: LDOs with fixed 2.8V output and 0~100/10mV tuning
> > +    type: object
> > +    $ref: regulator.yaml#
> > +    properties:
> > +      regulator-allowed-modes: false
> > +      regulator-compatible:
> > +        pattern: "^(ldo_)?v(tcxo|(a|io)28)$"
> > +    unevaluatedProperties: false
> > +
> > +  "^(ldo_)?vusb$":
> > +    description: LDOs with fixed 3.0V output and 0~100/10mV tuning
> > +    type: object
> > +    $ref: regulator.yaml#
> > +    properties:
> > +      regulator-allowed-modes: false
> > +      regulator-compatible:
> > +        pattern: "^(ldo_)?vusb$"
> > +    unevaluatedProperties: false
> > +
> > +  "^(ldo_)?v(cama|emc3v3|gp[123456]|ibr|mc|mch)$":
> > +    description: LDOs with variable output and 0~100/10mV tuning
> > +    type: object
> > +    $ref: regulator.yaml#
> > +    properties:
> > +      regulator-allowed-modes: false
> > +      regulator-compatible:
> > +        pattern: "^(ldo_)?v(cama|emc3v3|gp[123456]|ibr|mc|mch)$"
> > +    unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +
> > +    pwrap {
> > +       pmic {
> > +            compatible = "mediatek,mt6397";
>
> Messed indentation.
>
> Use 4 spaces for example indentation.
>
> Anyway, drop top node or better move the example to the parent device
> schema making it complete.
>
> > +
> > +            mt6397regulator: mt6397regulator {
>
> Node names should be generic. See also an explanation and list of
> examples (not exhaustive) in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation  <https://urldefense.com/v3/__https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html*generic-names-recommendation__;Iw!!CTRNKA9wMg0ARbw!ngXiiQwr5k1xuQ409K5BEaN120H9jRtwvbdy_u_VFuelciqoBoAYCD6Pi09Sy5YS3wfeZ5m876QLGXgDi-Pm8V4O08-MZ7s$>
>
>
> > +                compatible = "mediatek,mt6397-regulator";
> > +
> > +                mt6397_vpca15_reg: buck_vpca15 {
> > +                    regulator-compatible = "buck_vpca15";
>
> Drop, same in other places
>
> > +                    regulator-name = "vpca15";
> > +                    regulator-min-microvolt = < 850000>;
> > +                    regulator-max-microvolt = <1350000>;
> > +                    regulator-ramp-delay = <12500>;
> > +                    regulator-enable-ramp-delay = <200>;
> > +                };
> > +
>
>
> ;
>
> Best regards,
> Krzysztof

Thanks for the review.
All above issues should be fixed in patch v2 [1]
[1] https://lkml.org/lkml/2024/8/7/520

Best regards,
Macpaul Lin