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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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

> 
> 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.

> +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.

> +
> +  "^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.

> +    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.

> +    type: object
> +    $ref: "regulator.yaml#"
> +
> +    properties:
> +      regulator-name:
> +        pattern: "^vsram_(proc2|others|md|proc1|others_sshub)$"
> +
> +    unevaluatedProperties: false
> +

Best regards,
Krzysztof