[PATCH v2 1/2] regulator: Add bindings for TPS6287x

Mårten Lindahl posted 2 patches 2 years, 7 months ago
There is a newer version of this series
[PATCH v2 1/2] regulator: Add bindings for TPS6287x
Posted by Mårten Lindahl 2 years, 7 months ago
Add bindings for the TPS62870/TPS62871/TPS62872/TPS62873 voltage
regulators.

Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
---
 .../devicetree/bindings/regulator/ti,tps62870.yaml | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
new file mode 100644
index 000000000000..32f259f16314
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/ti,tps62870.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI TPS62870/TPS62871/TPS62872/TPS62873 voltage regulator
+
+maintainers:
+  - Mårten Lindahl <marten.lindahl@axis.com>
+
+properties:
+  compatible:
+    enum:
+      - ti,tps62870
+      - ti,tps62871
+      - ti,tps62872
+      - ti,tps62873
+
+  reg:
+    maxItems: 1
+
+  regulators:
+    type: object
+
+    properties:
+      "vout":
+        type: object
+        $ref: regulator.yaml#
+        unevaluatedProperties: false
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - regulators
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      regulator@41 {
+        compatible = "ti,tps62873";
+        reg = <0x41>;
+
+        regulators {
+          vout {
+            regulator-name = "+0.75V";
+            regulator-min-microvolt = <400000>;
+            regulator-max-microvolt = <1675000>;
+            regulator-initial-mode = <2>;
+          };
+        };
+      };
+    };
+
+...

-- 
2.30.2

Re: [PATCH v2 1/2] regulator: Add bindings for TPS6287x
Posted by Krzysztof Kozlowski 2 years, 7 months ago
On 04/05/2023 10:30, Mårten Lindahl wrote:
> Add bindings for the TPS62870/TPS62871/TPS62872/TPS62873 voltage
> regulators.
> 

Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

Just a hint - I in general ignore all the emails without dt-bindings prefix.

> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
> ---
>  .../devicetree/bindings/regulator/ti,tps62870.yaml | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
> new file mode 100644
> index 000000000000..32f259f16314
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/ti,tps62870.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI TPS62870/TPS62871/TPS62872/TPS62873 voltage regulator
> +
> +maintainers:
> +  - Mårten Lindahl <marten.lindahl@axis.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,tps62870
> +      - ti,tps62871
> +      - ti,tps62872
> +      - ti,tps62873
> +
> +  reg:
> +    maxItems: 1
> +
> +  regulators:
> +    type: object
> +
> +    properties:
> +      "vout":

Drop quotes.

Why do you need entire "regulators" node for one regulator? Why do you
need child at first place. Drop it entirely.


> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false

You missed that piece of explanation:

"The set of possible operating modes depends on the capabilities of
every hardware so each device binding documentation explains which
values the regulator supports."



Best regards,
Krzysztof

Re: [PATCH v2 1/2] regulator: Add bindings for TPS6287x
Posted by Mårten Lindahl 2 years, 7 months ago
Hi Krzysztof!

On 5/4/23 11:34, Krzysztof Kozlowski wrote:
> On 04/05/2023 10:30, Mårten Lindahl wrote:
>> Add bindings for the TPS62870/TPS62871/TPS62872/TPS62873 voltage
>> regulators.
>>
> Use subject prefixes matching the subsystem (which you can get for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching).
>
> Just a hint - I in general ignore all the emails without dt-bindings prefix.
Ok, I'll prefix it "dt-bindings: regulator:"
>
>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
>> ---
>>   .../devicetree/bindings/regulator/ti,tps62870.yaml | 62 ++++++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>> new file mode 100644
>> index 000000000000..32f259f16314
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/regulator/ti,tps62870.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: TI TPS62870/TPS62871/TPS62872/TPS62873 voltage regulator
>> +
>> +maintainers:
>> +  - Mårten Lindahl <marten.lindahl@axis.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,tps62870
>> +      - ti,tps62871
>> +      - ti,tps62872
>> +      - ti,tps62873
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  regulators:
>> +    type: object
>> +
>> +    properties:
>> +      "vout":
> Drop quotes.
>
> Why do you need entire "regulators" node for one regulator? Why do you
> need child at first place. Drop it entirely.
I will remove the regulators node. I think the vout node is needed to 
get the of_get_regulator_init_data.
>
>
>> +        type: object
>> +        $ref: regulator.yaml#
>> +        unevaluatedProperties: false
> You missed that piece of explanation:
>
> "The set of possible operating modes depends on the capabilities of
> every hardware so each device binding documentation explains which
> values the regulator supports."

Ok, I will add a description for the valid regulator-initial-mode values.

Thanks!

Kind regards

Mårten

>
>
>
> Best regards,
> Krzysztof
>
Re: [PATCH v2 1/2] regulator: Add bindings for TPS6287x
Posted by Krzysztof Kozlowski 2 years, 7 months ago
On 04/05/2023 17:08, Mårten Lindahl wrote:
> Hi Krzysztof!
> 
> On 5/4/23 11:34, Krzysztof Kozlowski wrote:
>> On 04/05/2023 10:30, Mårten Lindahl wrote:
>>> Add bindings for the TPS62870/TPS62871/TPS62872/TPS62873 voltage
>>> regulators.
>>>
>> Use subject prefixes matching the subsystem (which you can get for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching).
>>
>> Just a hint - I in general ignore all the emails without dt-bindings prefix.
> Ok, I'll prefix it "dt-bindings: regulator:"

You got command to run, so run it.  This semi-automated response is made
longer for the purpose to help you, not to be quickly scrolled and
ignored. When you run it you will see the order is opposite, regulator
followed by dt-bindings.

You can apply such habit for other subsystems where maintainers also
expect certain prefixes.


>>
>>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
>>> ---
>>>   .../devicetree/bindings/regulator/ti,tps62870.yaml | 62 ++++++++++++++++++++++
>>>   1 file changed, 62 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>>> new file mode 100644
>>> index 000000000000..32f259f16314
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>>> @@ -0,0 +1,62 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/regulator/ti,tps62870.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI TPS62870/TPS62871/TPS62872/TPS62873 voltage regulator
>>> +
>>> +maintainers:
>>> +  - Mårten Lindahl <marten.lindahl@axis.com>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,tps62870
>>> +      - ti,tps62871
>>> +      - ti,tps62872
>>> +      - ti,tps62873
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  regulators:
>>> +    type: object
>>> +
>>> +    properties:
>>> +      "vout":
>> Drop quotes.
>>
>> Why do you need entire "regulators" node for one regulator? Why do you
>> need child at first place. Drop it entirely.
> I will remove the regulators node. I think the vout node is needed to 
> get the of_get_regulator_init_data.

Hmmm, how other simple regulators deal with it? Like all the fixed ones
and few other one-regulator-devices?


Best regards,
Krzysztof

Re: [PATCH v2 1/2] regulator: Add bindings for TPS6287x
Posted by Mårten Lindahl 2 years, 7 months ago
On 5/4/23 17:11, Krzysztof Kozlowski wrote:
> On 04/05/2023 17:08, Mårten Lindahl wrote:
>> Hi Krzysztof!
>>
>> On 5/4/23 11:34, Krzysztof Kozlowski wrote:
>>> On 04/05/2023 10:30, Mårten Lindahl wrote:
>>>> Add bindings for the TPS62870/TPS62871/TPS62872/TPS62873 voltage
>>>> regulators.
>>>>
>>> Use subject prefixes matching the subsystem (which you can get for
>>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>>> your patch is touching).
>>>
>>> Just a hint - I in general ignore all the emails without dt-bindings prefix.
>> Ok, I'll prefix it "dt-bindings: regulator:"
> You got command to run, so run it.  This semi-automated response is made
> longer for the purpose to help you, not to be quickly scrolled and
> ignored. When you run it you will see the order is opposite, regulator
> followed by dt-bindings.
>
> You can apply such habit for other subsystems where maintainers also
> expect certain prefixes.

Hi! Sorry, I did run the command and followed the latest commit I could 
see (1ba7dfb905b3) as the result is mixed with prefixes.

But I will of course do as you request: "regulator: dt-bindings: ".

>
>
>>>> Signed-off-by: Mårten Lindahl <marten.lindahl@axis.com>
>>>> ---
>>>>    .../devicetree/bindings/regulator/ti,tps62870.yaml | 62 ++++++++++++++++++++++
>>>>    1 file changed, 62 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>>>> new file mode 100644
>>>> index 000000000000..32f259f16314
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/regulator/ti,tps62870.yaml
>>>> @@ -0,0 +1,62 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/regulator/ti,tps62870.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: TI TPS62870/TPS62871/TPS62872/TPS62873 voltage regulator
>>>> +
>>>> +maintainers:
>>>> +  - Mårten Lindahl <marten.lindahl@axis.com>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - ti,tps62870
>>>> +      - ti,tps62871
>>>> +      - ti,tps62872
>>>> +      - ti,tps62873
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  regulators:
>>>> +    type: object
>>>> +
>>>> +    properties:
>>>> +      "vout":
>>> Drop quotes.
>>>
>>> Why do you need entire "regulators" node for one regulator? Why do you
>>> need child at first place. Drop it entirely.
>> I will remove the regulators node. I think the vout node is needed to
>> get the of_get_regulator_init_data.
> Hmmm, how other simple regulators deal with it? Like all the fixed ones
> and few other one-regulator-devices?

Ok, I added of_get_regulator_init_data to the driver probe and then it 
works fine. I'll drop the child node.

Thanks!

Kind regards

Mårten

>
>
> Best regards,
> Krzysztof
>