Re: [PATCH 2/2] regulator: pf530x: dt-bindings: nxp,pf530x-regulator

[Krzysztof Kozlowski <krzk@kernel.org>]

On 02/09/2025 16:21, Woodrow Douglass wrote:
> Bindings for the pf530x series of voltage regulators
> 
> Signed-off-by: Woodrow Douglass <wdouglass@carnegierobotics.com>
> ---
>  .../regulator/nxp,pf530x-regulator.yaml       | 74 +++++++++++++++++++

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

You missed at least devicetree list (maybe more), so this won't be
tested by automated tooling. Performing review on untested code might be
a waste of time.

Please kindly resend and include all necessary To/Cc entries.
</form letter>


>  1 file changed, 74 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/nxp,pf530x-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/nxp,pf530x-regulator.yaml b/Documentation/devicetree/bindings/regulator/nxp,pf530x-regulator.yaml
> new file mode 100644
> index 000000000000..f1065b167491
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/nxp,pf530x-regulator.yaml


Filename should match compatible, so nxp,pf5300.yaml.

> @@ -0,0 +1,74 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/nxp,pf530x-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP PF5300/PF5301/PF5302 PMIC regulators
> +
> +maintainers:
> +  - Woodrow Douglass <wdouglass@carnegierobotics.com>
> +
> +description: |
> +  The PF5300, PF5301, and PF5302 integrate high-performance buck converters, 12 A, 8 A,
> +  and 15 A, respectively, to power high-end automotive and industrial processors. With adaptive
> +  voltage positioning and a high-bandwidth loop, they offer transient regulation to minimize capacitor
> +  requirements.

Wrap according to Linux coding style.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,pf5300
> +      - nxp,pf5301
> +      - nxp,pf5302

Your driver clearly suggests these are compatible, so express it (see
example schema).

> +
> +  reg:
> +    maxItems: 1
> +
> +  regulators:

No need for this node.

> +    type: object
> +    description: |
> +      list of regulators provided by this controller
> +
> +    properties:
> +      SW1:

No need, drop the node.

> +        type: object
> +        $ref: regulator.yaml#
> +        description:
> +          Properties for the regulator.
> +
> +        properties:
> +          regulator-name:
> +            pattern: "^SW1$"

No, drop entirely regulator-name. Just embed the properties in parent node.

> +            description:
> +              Name of the single regulator
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c1 {

i2c

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        vddi_0_75@28 {

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
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

See also DTS coding style.



Best regards,
Krzysztof