From: ChiYuan Huang <[email protected]>
Add the binding document for Richtek RT5739.
Signed-off-by: ChiYuan Huang <[email protected]>
---
.../bindings/regulator/richtek,rt5739.yaml | 80 ++++++++++++++++++++++
1 file changed, 80 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
new file mode 100644
index 00000000..7dc4f78
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
+
+maintainers:
+ - ChiYuan Huang <[email protected]>
+
+description: |
+ The RT5739 is a step-down switching voltage regulator that delivers a
+ digitally programmable output from an input voltage supply of 2.5V to 5.5V.
+ The output voltage is programmed through an I2C interface capable of
+ operating up to 3.4MHz.
+
+ Using a proprietary architecture with synchronous rectification, the RT5739
+ is capable of delivering 3.5A continuously at over 80% efficiency,
+ maintaining that efficiency at load current as low as 10mA. The regulator
+ operates at a normal fixed frequency of 2.4MHz, which reduces the value of
+ the external components. Additional output capacitance can be added to
+ improve regulation during load transients without affecting stability.
+
+allOf:
+ - $ref: regulator.yaml#
+
+properties:
+ compatible:
+ enum:
+ - richtek,rt5739
+
+ reg:
+ maxItems: 1
+
+ enable-gpios:
+ maxItems: 1
+
+ richtek,vsel-active-high:
+ description: |
+ If property is present, use the 'VSEL1' register group for buck control.
+ Else, use the 'VSEL0' register group. This depends on external hardware
+ 'VSEL' pin connecton.
+ type: boolean
+
+ regulator-allowed-modes:
+ description: |
+ buck allowed operating mode
+ 0: Auto PFM/PWM mode
+ 1: Forced PWM mode
+ maxItems: 2
+ items:
+ enum: [0, 1]
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ regulator@50 {
+ compatible = "richtek,rt5739";
+ reg = <0x50>;
+ enable-gpios = <&gpio26 1 GPIO_ACTIVE_HIGH>;
+ richtek,vsel-active-high;
+ regulator-name = "richtek,rt5739-buck";
+ regulator-min-microvolt = <3000000>;
+ regulator-max-microvolt = <13000000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-boot-on;
+ };
+ };
--
2.7.4
On Wed, Feb 15, 2023 at 10:00:56AM +0800, [email protected] wrote:
> + richtek,vsel-active-high:
> + description: |
> + If property is present, use the 'VSEL1' register group for buck control.
> + Else, use the 'VSEL0' register group. This depends on external hardware
> + 'VSEL' pin connecton.
> + type: boolean
I would expect this to be GPIO controlled rather than fixed in
the design, at least for some systems? The name suggests it's
supposed to control the polarity of a GPIO too. Sadly we don't
really have good infrastructure for fixed GPIOs AFAIK... ideally
this would be a GPIO and then if we need to hold it high then
there would be some binding we could connect to the GPIO that
says that. We don't have that though so for now a fixed property
might be OK.
Hi,
Mark Brown <[email protected]> 於 2023年2月15日 週三 下午9:03寫道：
>
> On Wed, Feb 15, 2023 at 10:00:56AM +0800, [email protected] wrote:
>
> > + richtek,vsel-active-high:
> > + description: |
> > + If property is present, use the 'VSEL1' register group for buck control.
> > + Else, use the 'VSEL0' register group. This depends on external hardware
> > + 'VSEL' pin connecton.
> > + type: boolean
>
> I would expect this to be GPIO controlled rather than fixed in
> the design, at least for some systems? The name suggests it's
> supposed to control the polarity of a GPIO too. Sadly we don't
> really have good infrastructure for fixed GPIOs AFAIK... ideally
> this would be a GPIO and then if we need to hold it high then
> there would be some binding we could connect to the GPIO that
> says that. We don't have that though so for now a fixed property
> might be OK.
It's most like two phase buck controllers internally. 'Vsel' to
control which buck output.
Each phase has its registers (voltage/enable/mode).
The common usage of 'vsel' pin is to connect the pin by SoC suspend/resume pin.
This can be used to implement system low power design (like as system
awake 0.6V, sleep 0.3V or sleep buck off).
I haven't seen the usage to connect it directly by GPIO.
On 15/02/2023 03:00, [email protected] wrote:
> From: ChiYuan Huang <[email protected]>
>
> Add the binding document for Richtek RT5739.
Subject: drop second/last, redundant "bindings for". The "dt-bindings"
prefix is already stating that these are bindings.
>
> Signed-off-by: ChiYuan Huang <[email protected]>
> ---
> .../bindings/regulator/richtek,rt5739.yaml | 80 ++++++++++++++++++++++
> 1 file changed, 80 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> new file mode 100644
> index 00000000..7dc4f78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
> +
> +maintainers:
> + - ChiYuan Huang <[email protected]>
> +
> +description: |
> + The RT5739 is a step-down switching voltage regulator that delivers a
> + digitally programmable output from an input voltage supply of 2.5V to 5.5V.
> + The output voltage is programmed through an I2C interface capable of
> + operating up to 3.4MHz.
> +
> + Using a proprietary architecture with synchronous rectification, the RT5739
> + is capable of delivering 3.5A continuously at over 80% efficiency,
> + maintaining that efficiency at load current as low as 10mA. The regulator
> + operates at a normal fixed frequency of 2.4MHz, which reduces the value of
> + the external components.
Can we drop the marketing from kernel? Last part of sentence is not
related to this submission at all. The internal frequency also looks
unrelated to the topic...
> Additional output capacitance can be added to
> + improve regulation during load transients without affecting stability.
> +
> +allOf:
> + - $ref: regulator.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - richtek,rt5739
> +
> + reg:
> + maxItems: 1
> +
> + enable-gpios:
> + maxItems: 1
> +
> + richtek,vsel-active-high:
> + description: |
> + If property is present, use the 'VSEL1' register group for buck control.
> + Else, use the 'VSEL0' register group. This depends on external hardware
> + 'VSEL' pin connecton.
> + type: boolean
> +
> + regulator-allowed-modes:
> + description: |
> + buck allowed operating mode
> + 0: Auto PFM/PWM mode
> + 1: Forced PWM mode
> + maxItems: 2
> + items:
> + enum: [0, 1]
So you always require two items? Thus I wonder what's the point of
having it in DT? To skip the property entirely if none of the modes are
allowed?
Best regards,
Krzysztof
On Thu, Feb 16, 2023 at 10:12:15AM +0100, Krzysztof Kozlowski wrote:
> On 15/02/2023 03:00, [email protected] wrote:
> > From: ChiYuan Huang <[email protected]>
> >
> > Add the binding document for Richtek RT5739.
>
> Subject: drop second/last, redundant "bindings for". The "dt-bindings"
> prefix is already stating that these are bindings.
>
Then, refine it to "dt-bindings: regulator: Add Richtek RT5739 document"
> >
> > Signed-off-by: ChiYuan Huang <[email protected]>
> > ---
> > .../bindings/regulator/richtek,rt5739.yaml | 80 ++++++++++++++++++++++
> > 1 file changed, 80 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> > new file mode 100644
> > index 00000000..7dc4f78
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
> > +
> > +maintainers:
> > + - ChiYuan Huang <[email protected]>
> > +
> > +description: |
> > + The RT5739 is a step-down switching voltage regulator that delivers a
> > + digitally programmable output from an input voltage supply of 2.5V to 5.5V.
> > + The output voltage is programmed through an I2C interface capable of
> > + operating up to 3.4MHz.
> > +
> > + Using a proprietary architecture with synchronous rectification, the RT5739
> > + is capable of delivering 3.5A continuously at over 80% efficiency,
> > + maintaining that efficiency at load current as low as 10mA. The regulator
> > + operates at a normal fixed frequency of 2.4MHz, which reduces the value of
> > + the external components.
>
> Can we drop the marketing from kernel? Last part of sentence is not
> related to this submission at all. The internal frequency also looks
> unrelated to the topic...
>
Okay, too much marketing text. I'll shorten it and simply describe the function or
voltage range only.
> > Additional output capacitance can be added to
> > + improve regulation during load transients without affecting stability.
> > +
> > +allOf:
> > + - $ref: regulator.yaml#
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - richtek,rt5739
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + enable-gpios:
> > + maxItems: 1
> > +
> > + richtek,vsel-active-high:
> > + description: |
> > + If property is present, use the 'VSEL1' register group for buck control.
> > + Else, use the 'VSEL0' register group. This depends on external hardware
> > + 'VSEL' pin connecton.
> > + type: boolean
> > +
> > + regulator-allowed-modes:
> > + description: |
> > + buck allowed operating mode
> > + 0: Auto PFM/PWM mode
> > + 1: Forced PWM mode
> > + maxItems: 2
> > + items:
> > + enum: [0, 1]
>
> So you always require two items? Thus I wonder what's the point of
> having it in DT? To skip the property entirely if none of the modes are
> allowed?
>
Not always need two. So does it mean no need to describe the 'maxItems' and 'Items'.
Keep the description of 'regulator-allowed-modes' to tell the user what the valid value is.
>
> Best regards,
> Krzysztof
>
On 16/02/2023 10:57, ChiYuan Huang wrote:
> On Thu, Feb 16, 2023 at 10:12:15AM +0100, Krzysztof Kozlowski wrote:
>> On 15/02/2023 03:00, [email protected] wrote:
>>> From: ChiYuan Huang <[email protected]>
>>>
>>> Add the binding document for Richtek RT5739.
>>
>> Subject: drop second/last, redundant "bindings for". The "dt-bindings"
>> prefix is already stating that these are bindings.
>>
> Then, refine it to "dt-bindings: regulator: Add Richtek RT5739 document"
I propose also to drop "document" - it is also redundant. Can bindings
be something else than document?
>>>
>>> Signed-off-by: ChiYuan Huang <[email protected]>
>>> ---
>>> .../bindings/regulator/richtek,rt5739.yaml | 80 ++++++++++++++++++++++
>>> 1 file changed, 80 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
>>> new file mode 100644
>>> index 00000000..7dc4f78
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
>>> @@ -0,0 +1,80 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
>>> +
>>> +maintainers:
>>> + - ChiYuan Huang <[email protected]>
>>> +
>>> +description: |
>>> + The RT5739 is a step-down switching voltage regulator that delivers a
>>> + digitally programmable output from an input voltage supply of 2.5V to 5.5V.
>>> + The output voltage is programmed through an I2C interface capable of
>>> + operating up to 3.4MHz.
>>> +
>>> + Using a proprietary architecture with synchronous rectification, the RT5739
>>> + is capable of delivering 3.5A continuously at over 80% efficiency,
>>> + maintaining that efficiency at load current as low as 10mA. The regulator
>>> + operates at a normal fixed frequency of 2.4MHz, which reduces the value of
>>> + the external components.
>>
>> Can we drop the marketing from kernel? Last part of sentence is not
>> related to this submission at all. The internal frequency also looks
>> unrelated to the topic...
>>
> Okay, too much marketing text. I'll shorten it and simply describe the function or
> voltage range only.
>>> Additional output capacitance can be added to
>>> + improve regulation during load transients without affecting stability.
>>> +
>>> +allOf:
>>> + - $ref: regulator.yaml#
>>> +
>>> +properties:
>>> + compatible:
>>> + enum:
>>> + - richtek,rt5739
>>> +
>>> + reg:
>>> + maxItems: 1
>>> +
>>> + enable-gpios:
>>> + maxItems: 1
>>> +
>>> + richtek,vsel-active-high:
>>> + description: |
>>> + If property is present, use the 'VSEL1' register group for buck control.
>>> + Else, use the 'VSEL0' register group. This depends on external hardware
>>> + 'VSEL' pin connecton.
>>> + type: boolean
>>> +
>>> + regulator-allowed-modes:
>>> + description: |
>>> + buck allowed operating mode
>>> + 0: Auto PFM/PWM mode
>>> + 1: Forced PWM mode
>>> + maxItems: 2
>>> + items:
>>> + enum: [0, 1]
>>
>> So you always require two items? Thus I wonder what's the point of
>> having it in DT? To skip the property entirely if none of the modes are
>> allowed?
>>
> Not always need two. So does it mean no need to describe the 'maxItems' and 'Items'.
Your minItems is 2, so you always need two. If you accept one, the add
minItems: 1.
Best regards,
Krzysztof
On Thu, Feb 16, 2023 at 11:03:39AM +0100, Krzysztof Kozlowski wrote:
> On 16/02/2023 10:57, ChiYuan Huang wrote:
> > On Thu, Feb 16, 2023 at 10:12:15AM +0100, Krzysztof Kozlowski wrote:
> >> On 15/02/2023 03:00, [email protected] wrote:
> >>> From: ChiYuan Huang <[email protected]>
> >>>
> >>> Add the binding document for Richtek RT5739.
> >>
> >> Subject: drop second/last, redundant "bindings for". The "dt-bindings"
> >> prefix is already stating that these are bindings.
> >>
> > Then, refine it to "dt-bindings: regulator: Add Richtek RT5739 document"
>
> I propose also to drop "document" - it is also redundant. Can bindings
> be something else than document?
>
Yes, you'r right.
>
> >>>
> >>> Signed-off-by: ChiYuan Huang <[email protected]>
> >>> ---
> >>> .../bindings/regulator/richtek,rt5739.yaml | 80 ++++++++++++++++++++++
> >>> 1 file changed, 80 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> >>> new file mode 100644
> >>> index 00000000..7dc4f78
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/regulator/richtek,rt5739.yaml
> >>> @@ -0,0 +1,80 @@
> >>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/regulator/richtek,rt5739.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Richtek RT5739 2.4MHz 3.5A Step-Down Converter
> >>> +
> >>> +maintainers:
> >>> + - ChiYuan Huang <[email protected]>
> >>> +
> >>> +description: |
> >>> + The RT5739 is a step-down switching voltage regulator that delivers a
> >>> + digitally programmable output from an input voltage supply of 2.5V to 5.5V.
> >>> + The output voltage is programmed through an I2C interface capable of
> >>> + operating up to 3.4MHz.
> >>> +
> >>> + Using a proprietary architecture with synchronous rectification, the RT5739
> >>> + is capable of delivering 3.5A continuously at over 80% efficiency,
> >>> + maintaining that efficiency at load current as low as 10mA. The regulator
> >>> + operates at a normal fixed frequency of 2.4MHz, which reduces the value of
> >>> + the external components.
> >>
> >> Can we drop the marketing from kernel? Last part of sentence is not
> >> related to this submission at all. The internal frequency also looks
> >> unrelated to the topic...
> >>
> > Okay, too much marketing text. I'll shorten it and simply describe the function or
> > voltage range only.
> >>> Additional output capacitance can be added to
> >>> + improve regulation during load transients without affecting stability.
> >>> +
> >>> +allOf:
> >>> + - $ref: regulator.yaml#
> >>> +
> >>> +properties:
> >>> + compatible:
> >>> + enum:
> >>> + - richtek,rt5739
> >>> +
> >>> + reg:
> >>> + maxItems: 1
> >>> +
> >>> + enable-gpios:
> >>> + maxItems: 1
> >>> +
> >>> + richtek,vsel-active-high:
> >>> + description: |
> >>> + If property is present, use the 'VSEL1' register group for buck control.
> >>> + Else, use the 'VSEL0' register group. This depends on external hardware
> >>> + 'VSEL' pin connecton.
> >>> + type: boolean
> >>> +
> >>> + regulator-allowed-modes:
> >>> + description: |
> >>> + buck allowed operating mode
> >>> + 0: Auto PFM/PWM mode
> >>> + 1: Forced PWM mode
> >>> + maxItems: 2
> >>> + items:
> >>> + enum: [0, 1]
> >>
> >> So you always require two items? Thus I wonder what's the point of
> >> having it in DT? To skip the property entirely if none of the modes are
> >> allowed?
> >>
> > Not always need two. So does it mean no need to describe the 'maxItems' and 'Items'.
>
> Your minItems is 2, so you always need two. If you accept one, the add
> minItems: 1.
>
https://elixir.bootlin.com/linux/v6.2-rc8/source/drivers/regulator/of_regulator.c#L198
It seems no need to limit the maxItems. Regulator core will call 'of_map_mode' to check the value.
Even the value is repeat.
And for minItems, 'regulator.yaml' already said it's uint32-array. Must be lager than zero, right?
So how about just keep 'items' and remove the 'maxItems'?
>
>
> Best regards,
> Krzysztof
>
On 16/02/2023 11:35, ChiYuan Huang wrote:
> https://elixir.bootlin.com/linux/v6.2-rc8/source/drivers/regulator/of_regulator.c#L198
> It seems no need to limit the maxItems. Regulator core will call 'of_map_mode' to check the value.
> Even the value is repeat.
>
> And for minItems, 'regulator.yaml' already said it's uint32-array. Must be lager than zero, right?
>
> So how about just keep 'items' and remove the 'maxItems'?
Yeah, go ahead, with that enum, it should be correct..
Best regards,
Krzysztof