From: alinayu <[email protected]>
Add bindings for Richtek RTQ2208 IC controlled SubPMIC
Signed-off-by: alinayu <[email protected]>
---
.../regulator/richtek,rtq2208-regulator.yaml | 228 +++++++++++++++++++++
1 file changed, 228 insertions(+)
create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
new file mode 100644
index 0000000..2a060ce
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
@@ -0,0 +1,228 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/regulator/richtek,rtq2208-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Richtek RTQ2208 SubPMIC Regulator
+
+maintainers:
+ - Alina Yu <[email protected]>
+
+description: |
+ The RTQ2208 is a highly integrated multi-configurable power converter that
+ offers functional safety embedded dual multi-configurable synchronous buck
+ converters and two LDOs.
+
+ Bucks support "regulator-allowed-modes" and "regulator-mode". The former defines the permitted
+ swiching operation in normal mode; the latter defines the operation in suspend to RAM mode.
+
+ No matter the RTQ2208 is configured in normal or suspend to RAM mode, there are two switching
+ operation modes for all buck rails, automic power saving mode (Auto mode) and forced continious
+ conduction mode (FCCM).
+
+ The meaning of modes is defined in the datasheet which is avaliabe in below link
+ and their meaning is::
+ 0 - Auto mode for power saving, which reducing the switching frequency at light load condition
+ to maintain high frequency.
+ 1 - FCCM to meet the strict voltage regulation accuracy, which keeping constant switching frequency.
+
+ Datasheet will be available soon at
+ https://www.richtek.com/assets/Products
+
+ The standard "regulator-mode" property can only be used for bucks that
+ changing their mode to suspend to RAM mode. Also, it only takes effect if the
+ regulator has been enabled for the given suspend state using "regulator-on-in-suspend".
+
+properties:
+ compatible:
+ enum:
+ - richtek,rtq2208
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ richtek,mtp-sel:
+ description: |
+ Buck and ldo vout selection is based on this value.
+ There are two independently programmable voltage settings named as mtp-sel0 and
+ mtp-sel1 for RTQ2208 bucks vout voltage. 0 which means this property isn't present
+ and 1 which means this property is present corresponds to different adjustable registers.
+
+ 0 - DVS0 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL0 register to en/disable vout.
+ 1 - DVS1 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL1 register to en/disable vout.
+
+ type: boolean
+
+ regulators:
+ type: object
+
+ patternProperties:
+ "^buck_[A-H]$":
+ type: object
+ $ref: regulator.yaml#
+ description: |
+ regulator description for buck[A-H].
+
+ properties:
+ regulator-compatible:
+ pattern: "^BUCK_[A-H]$"
+
+ regulator-allowed-modes:
+ description: |
+ describe buck permitted modes.
+
+ "^ldo[1-2]$":
+ type: object
+ $ref: regulator.yaml#
+ description: |
+ regulator description for ldo[1-2].
+
+ properties:
+ regulator-compatible:
+ pattern: "^LDO[1-2]$"
+
+ richtek,fixed_uV:
+ $ref: "/schemas/types.yaml#/definitions/uint32"
+ enum: [ 900000, 1200000, 1800000, 3300000 ]
+ description: |
+ the fixed voltage in microvolt which is descided at the factory.
+
+ regulator-state-(mem):
+ type: object
+ additionalProperties: true
+ properties:
+ regulator-on-in-suspend: false
+ regulator-mode: false
+
+required:
+ - compatible
+ - reg
+ - regulators
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtq2208@10 {
+ compatible = "richtek,rtq2208";
+ reg = <0x10>;
+ interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
+ richtek,mtp-sel;
+
+ regulators {
+ BUCK_A:buck_A {
+ regulator-compatible = "BUCK_A";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_B:buck_B {
+ regulator-compatible = "BUCK_B";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_C:buck_C {
+ regulator-compatible = "BUCK_C";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_D:buck_D {
+ regulator-compatible = "BUCK_D";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_E:buck_E {
+ regulator-compatible = "BUCK_E";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_F:buck_F {
+ regulator-compatible = "BUCK_F";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_G:buck_G {
+ regulator-compatible = "BUCK_G";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ BUCK_H:buck_H {
+ regulator-compatible = "BUCK_H";
+ regulator-min-microvolt = <400000>;
+ regulator-max-microvolt = <2050000>;
+ regulator-allowed-modes = <0 1>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ regulator-mode = <1>;
+ };
+ };
+ LDO1:ldo1 {
+ regulator-compatible = "LDO1";
+ richtek,fixed_uV = <1200000>;
+ regulator-always-on;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+ LDO2:ldo2 {
+ regulator-compatible = "LDO2";
+ regulator-always-on;
+ richtek,fixed_uV = <3300000>;
+ regulator-state-mem {
+ regulator-on-in-suspend;
+ };
+ };
+ };
+ };
+ };
--
2.7.4
On 05/07/2023 17:27, [email protected] wrote:
> From: alinayu <[email protected]>
>
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
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.
You missed at least DT list (maybe more), so this won't be tested by our
tools. Performing review on untested code might be a waste of time, thus
I will skip this patch entirely till you follow the process allowing the
patch to be tested.
Please kindly resend and include all necessary To/Cc entries.
Limited review follows.
>
> Signed-off-by: alinayu <[email protected]>
> ---
> .../regulator/richtek,rtq2208-regulator.yaml | 228 +++++++++++++++++++++
> 1 file changed, 228 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
>
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
> new file mode 100644
> index 0000000..2a060ce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208-regulator.yaml
Filename like compatible.
> @@ -0,0 +1,228 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rtq2208-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RTQ2208 SubPMIC Regulator
> +
> +maintainers:
> + - Alina Yu <[email protected]>
> +
> +description: |
> + The RTQ2208 is a highly integrated multi-configurable power converter that
> + offers functional safety embedded dual multi-configurable synchronous buck
> + converters and two LDOs.
> +
> + Bucks support "regulator-allowed-modes" and "regulator-mode". The former defines the permitted
> + swiching operation in normal mode; the latter defines the operation in suspend to RAM mode.
> +
> + No matter the RTQ2208 is configured in normal or suspend to RAM mode, there are two switching
> + operation modes for all buck rails, automic power saving mode (Auto mode) and forced continious
> + conduction mode (FCCM).
> +
> + The meaning of modes is defined in the datasheet which is avaliabe in below link
You have several typos here and before. Please run spellcheck.
> + and their meaning is::
> + 0 - Auto mode for power saving, which reducing the switching frequency at light load condition
> + to maintain high frequency.
> + 1 - FCCM to meet the strict voltage regulation accuracy, which keeping constant switching frequency.
> +
> + Datasheet will be available soon at
> + https://www.richtek.com/assets/Products
> +
> + The standard "regulator-mode" property can only be used for bucks that
> + changing their mode to suspend to RAM mode. Also, it only takes effect if the
> + regulator has been enabled for the given suspend state using "regulator-on-in-suspend".
> +
> +properties:
> + compatible:
> + enum:
> + - richtek,rtq2208
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + richtek,mtp-sel:
> + description: |
> + Buck and ldo vout selection is based on this value.
> + There are two independently programmable voltage settings named as mtp-sel0 and
> + mtp-sel1 for RTQ2208 bucks vout voltage. 0 which means this property isn't present
> + and 1 which means this property is present corresponds to different adjustable registers.
> +
> + 0 - DVS0 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL0 register to en/disable vout.
> + 1 - DVS1 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL1 register to en/disable vout.
I read it three times and still don't understand. This is bool, not 0/1,
so are these "0" refer to DVS0 or to presence of the property? Maybe
Mark will understand it, I don't get it.
> +
No blank line. Just put type before description.
> + type: boolean
> +
> + regulators:
> + type: object
> +
> + patternProperties:
> + "^buck_[A-H]$":
No underscores in node names. Lowercase names.
> + type: object
> + $ref: regulator.yaml#
Missing unevaluatedProperties: false
> + description: |
> + regulator description for buck[A-H].
> +
> + properties:
> + regulator-compatible:
> + pattern: "^BUCK_[A-H]$"
Drop the property.
> +
> + regulator-allowed-modes:
> + description: |
Do not need '|' unless you need to preserve formatting.
> + describe buck permitted modes.
Exactly: describe them
> +
> + "^ldo[1-2]$":
> + type: object
> + $ref: regulator.yaml#
> + description: |
> + regulator description for ldo[1-2].
> +
> + properties:
> + regulator-compatible:
> + pattern: "^LDO[1-2]$"
> +
> + richtek,fixed_uV:
> + $ref: "/schemas/types.yaml#/definitions/uint32"
> + enum: [ 900000, 1200000, 1800000, 3300000 ]
> + description: |
> + the fixed voltage in microvolt which is descided at the factory.
> +
> + regulator-state-(mem):
That's not a pattern.
> + type: object
> + additionalProperties: true
Why?
> + properties:
> + regulator-on-in-suspend: false
> + regulator-mode: false
> +
> +required:
> + - compatible
> + - reg
> + - regulators
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + rtq2208@10 {
Node names should be generic. See also explanation and list of examples
in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "richtek,rtq2208";
> + reg = <0x10>;
> + interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
> + richtek,mtp-sel;
> +
> + regulators {
> + BUCK_A:buck_A {
Drop labels.
Best regards,
Krzysztof
On Wed, Jul 05, 2023 at 07:58:53PM +0200, Krzysztof Kozlowski wrote:
> On 05/07/2023 17:27, [email protected] wrote:
> > From: alinayu <[email protected]>
> >
> > Add bindings for Richtek RTQ2208 IC controlled SubPMIC
>
> 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.
>
> You missed at least DT list (maybe more), so this won't be tested by our
> tools. Performing review on untested code might be a waste of time, thus
> I will skip this patch entirely till you follow the process allowing the
> patch to be tested.
>
> Please kindly resend and include all necessary To/Cc entries.
>
> Limited review follows.
>
Sorry, I will add all necessary people to CC list to v3.
...
> > + Buck and ldo vout selection is based on this value.
> > + There are two independently programmable voltage settings named as mtp-sel0 and
> > + mtp-sel1 for RTQ2208 bucks vout voltage. 0 which means this property isn't present
> > + and 1 which means this property is present corresponds to different adjustable registers.
> > +
> > + 0 - DVS0 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL0 register to en/disable vout.
> > + 1 - DVS1 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL1 register to en/disable vout.
>
> I read it three times and still don't understand. This is bool, not 0/1,
> so are these "0" refer to DVS0 or to presence of the property? Maybe
> Mark will understand it, I don't get it.
>
Yes, "0" refers to DVS0 registers, and "1" refers to DVS1.
and there is only DVS0 and DVS1, so I use boolean to check which one is used.
Is it more understandable if I modify that to enum ? And description will be like this
richtek,mtp-sel:
enum: [0, 1]
description: |
vout register selection besed on this value.
0 - Using DVS0 register setting to adjust vout
1 - Using DVS1 register setting to adjust vout
...
> > +
> > + regulator-state-(mem):
>
> That's not a pattern.
>
Should I revise that like this ?
patternProperties:
"^regulator-state-mem$":
> > + type: object
> > + additionalProperties: true
>
> Why?
Does "additionalProperties: true" mean I need to define my own property ?
If yes, I misunderstood additionalProperties as properties like "regulator-on-in-suspend" or "regulator-mode".
>
> > + properties:
> > + regulator-on-in-suspend: false
> > + regulator-mode: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - regulators
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + rtq2208@10 {
>
> Node names should be generic. See also explanation and list of examples
> in DT specification:
> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>
I'll modify the node name to
regulator@10
in v3
> > + compatible = "richtek,rtq2208";
> > + reg = <0x10>;
> > + interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
> > + richtek,mtp-sel;
> > +
> > + regulators {
> > + BUCK_A:buck_A {
>
> Drop labels.
>
>
BR,
Alina
On 06/07/2023 12:30, Alina Yu wrote:
>>> +
>>> + regulator-state-(mem):
>>
>> That's not a pattern.
>>
>
> Should I revise that like this ?
>
> patternProperties:
> "^regulator-state-mem$":
It's still not a pattern, but a regular property, so keep it in properties.
I don't even understand what you want to say with this. You put it
outside of any regulator. I don't think this was tested at all. :(
>
>
>>> + type: object
>>> + additionalProperties: true
>>
>> Why?
>
> Does "additionalProperties: true" mean I need to define my own property ?
No.
> If yes, I misunderstood additionalProperties as properties like "regulator-on-in-suspend" or "regulator-mode".
Please open recent regulator bindings and use them as example.
>
>>
>>> + properties:
>>> + regulator-on-in-suspend: false
>>> + regulator-mode: false
>>> +
>>> +required:
>>> + - compatible
>>> + - reg
>>> + - regulators
>>> +
>>> +unevaluatedProperties: false
>>> +
>>> +examples:
>>> + - |
>>> + #include <dt-bindings/interrupt-controller/irq.h>
>>> + i2c {
>>> + #address-cells = <1>;
>>> + #size-cells = <0>;
>>> +
>>> + rtq2208@10 {
>>
>> Node names should be generic. See also explanation and list of examples
>> in DT specification:
>> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
>>
>
> I'll modify the node name to
>
> regulator@10
If this is PMIC, then "pmic" as name.
Best regards,
Krzysztof
Hi, Krzystof
On Thu, Jul 06, 2023 at 06:30:40PM +0800, Alina Yu wrote:
> On Wed, Jul 05, 2023 at 07:58:53PM +0200, Krzysztof Kozlowski wrote:
> > On 05/07/2023 17:27, [email protected] wrote:
> > > From: alinayu <[email protected]>
> > >
> > > Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> >
>
> ...
>
> > > + Buck and ldo vout selection is based on this value.
> > > + There are two independently programmable voltage settings named as mtp-sel0 and
> > > + mtp-sel1 for RTQ2208 bucks vout voltage. 0 which means this property isn't present
> > > + and 1 which means this property is present corresponds to different adjustable registers.
> > > +
> > > + 0 - DVS0 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL0 register to en/disable vout.
> > > + 1 - DVS1 registers to adjust buck vout and BUCK_[A-H]_EN_NR_MTP_SEL1 register to en/disable vout.
> >
> > I read it three times and still don't understand. This is bool, not 0/1,
> > so are these "0" refer to DVS0 or to presence of the property? Maybe
> > Mark will understand it, I don't get it.
> >
>
> Yes, "0" refers to DVS0 registers, and "1" refers to DVS1.
> and there is only DVS0 and DVS1, so I use boolean to check which one is used.
>
> Is it more understandable if I modify that to enum ? And description will be like this
>
> richtek,mtp-sel:
> enum: [0, 1]
> description: |
> vout register selection besed on this value.
> 0 - Using DVS0 register setting to adjust vout
> 1 - Using DVS1 register setting to adjust vout
>
May I ask one more question ?
If I modify the name into "richtek,mtp-sel-high", is that more understandable ?
It will be like this,
richtek,mtp-sel-high:
type: boolean
description:
vout register selection besed on this value.
0 - Using DVS0 register setting to adjust vout
1 - Using DVS1 register setting to adjust vout
>
> >
> >
>
> BR,
> Alina
On 10/07/2023 05:08, Alina Yu wrote:
>> Yes, "0" refers to DVS0 registers, and "1" refers to DVS1.
>> and there is only DVS0 and DVS1, so I use boolean to check which one is used.
>>
>> Is it more understandable if I modify that to enum ? And description will be like this
>>
>> richtek,mtp-sel:
>> enum: [0, 1]
>> description: |
>> vout register selection besed on this value.
>> 0 - Using DVS0 register setting to adjust vout
>> 1 - Using DVS1 register setting to adjust vout
>>
>
> May I ask one more question ?
> If I modify the name into "richtek,mtp-sel-high", is that more understandable ?
> It will be like this,
>
> richtek,mtp-sel-high:
> type: boolean
> description:
> vout register selection besed on this value.
> 0 - Using DVS0 register setting to adjust vout
> 1 - Using DVS1 register setting to adjust vout
You don't have 0 or 1 values in such case. The property can be bool, but
description is not good.
Best regards,
Krzysztof
On Mon, Jul 10, 2023 at 08:02:39AM +0200, Krzysztof Kozlowski wrote:
> On 10/07/2023 05:08, Alina Yu wrote:
> >> Yes, "0" refers to DVS0 registers, and "1" refers to DVS1.
> >> and there is only DVS0 and DVS1, so I use boolean to check which one is used.
> >>
> >> Is it more understandable if I modify that to enum ? And description will be like this
> >>
> >> richtek,mtp-sel:
> >> enum: [0, 1]
> >> description: |
> >> vout register selection besed on this value.
> >> 0 - Using DVS0 register setting to adjust vout
> >> 1 - Using DVS1 register setting to adjust vout
> >>
> >
> > May I ask one more question ?
> > If I modify the name into "richtek,mtp-sel-high", is that more understandable ?
> > It will be like this,
> >
> > richtek,mtp-sel-high:
> > type: boolean
> > description:
> > vout register selection besed on this value.
> > 0 - Using DVS0 register setting to adjust vout
> > 1 - Using DVS1 register setting to adjust vout
>
> You don't have 0 or 1 values in such case. The property can be bool, but
> description is not good.
>
May I modify the description like this ?
richtek,mtp-sel-high:
type: boolean
description:
vout register selection besed on this boolean value.
false - Using DVS0 register setting to adjust vout
true - Using DVS1 register setting to adjust vout
BR,
Alina.