Re: [PATCH] dt-bindings: pmic: mt6366: add binding document

[zhiyong.tao <zhiyong.tao@mediatek.com>]

On Tue, 2022-05-10 at 12:14 +0200, Krzysztof Kozlowski wrote:
> On 10/05/2022 08:46, Zhiyong Tao wrote:
> > From: "Zhiyong.Tao" <zhiyong.tao@mediatek.com>
> > 
> > The commit adds mt6366 binding document.
> 
> 
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
==>
Hi Krzysztof,

we will fix it in v2.

> > 
> > Signed-off-by: Zhiyong.Tao <zhiyong.tao@mediatek.com>
> > ---
> >  .../bindings/regulator/mt6366-regulator.yaml  | 405
> > ++++++++++++++++++
> >  1 file changed, 405 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/regulator/mt6366-regulator.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/regulator/mt6366-
> > regulator.yaml
> > b/Documentation/devicetree/bindings/regulator/mt6366-regulator.yaml
> > new file mode 100644
> > index 000000000000..1f125f5ba860
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/mt6366-
> > regulator.yaml
> 
> Filename with vendor, so "mediatek,mt6366-regulator.yaml"
> 
> > @@ -0,0 +1,405 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> > http://devicetree.org/schemas/regulator/mt6366-regulator.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MT6366 Regulator from MediaTek Integrated
> > +
> > +maintainers:
> > +  - Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>
> > +
> > +description: |
> > +  List of regulators provided by this controller. It is named
> > +  according to its regulator type, buck_<name> and ldo_<name>.
> > +  MT6366 regulators node should be sub node of the MT6397 MFD
> > node.
> > +
> 
> Missing properties node, missing compatible. How is it supposed to
> match? If you plan to use it as part of other binding, please send
> entire patchset (including the binding which uses this one).
> 
> In current form this looks untestable, unusable.
==>We will  add properties node and compatiblein v2.
> 
> > +patternProperties:
> > +  "^buck_v(dram1|core|pa|proc11|proc12|gpu|s2|modem|s1)$":
> > +    type: object
> > +    $ref: "regulator.yaml#"
> > +
> > +    properties:
> > +      regulator-name:
> > +        pattern:
> > "^v(dram1|core|pa|proc11|proc12|gpu|s2|modem|s1)$"
> > +
> > +    unevaluatedProperties: false
> 
> Put this one after $ref (other places as well). Makes it more
> readable.
==>we will fix it in v2.
> 
> > +
> > +  "^ldo_v(ibr|rf12|usb|camio|efuse|xo22)$":
> > +    type: object
> > +    $ref: "regulator.yaml#"
> > +
> > +    properties:
> > +      regulator-name:
> > +        pattern: "^v(ibr|rf12|usb|camio|efuse|xo22)$"
> > +
> > +    unevaluatedProperties: false
> > +
> > +  "^ldo_v(rfck|emc|a12|a09|ufs|bbck)$":
> > +    type: object
> > +    $ref: "regulator.yaml#"
> > +
> > +    properties:
> > +      regulator-name:
> > +        pattern: "^v(rfck|emc|a12|a09|ufs|bbck)$"
> > +
> > +    unevaluatedProperties: false
> > +
> > +  "^ldo_vcn(18|13|33_1_bt|13_1_wifi|33_2_bt|33_2_wifi)$":
> 
> No underscores in node names.
==>we will fix it in v2.
> 
> > +    type: object
> > +    $ref: "regulator.yaml#"
> > +
> > +    properties:
> > +      regulator-name:
> > +        pattern:
> > "^vcn(18|13|33_1_bt|13_1_wifi|33_2_bt|33_2_wifi)$"
> > +
> > +    unevaluatedProperties: false
> > +
> > +  "^ldo_vsram_(proc2|others|md|proc1|others_sshub)$":
> 
> No underscores in node names, in all places.
==>we will fix it in v2.
Thanks.
> 
> > +    type: object
> > +    $ref: "regulator.yaml#"
> > +
> > +    properties:
> > +      regulator-name:
> > +        pattern: "^vsram_(proc2|others|md|proc1|others_sshub)$"
> > +
> > +    unevaluatedProperties: false
> > +
> 
> Best regards,
> Krzysztof