[v5,1/2] regulator: dt-bindings: rtq2208: Add Richtek RTQ2208 SubPMIC

  Add bindings for Richtek RTQ2208 IC controlled SubPMIC

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
v5
- Revise filename from $id
- Remove "regulator-compatible" for ldos
- Add missing "high" for "richtek,mtp-sel-high"
- Remove "regulator-mode" property
- Add and modify "unevaluatedProperties: false" and "additionalProperties: false"
- Remove "richtek,fixed-uV"
- Remove redundant space in "buck-a" and "ldo2"
v4
- Modify filename to "richtek,rtq2208"
- Add more desciptions for "regulator-allowed-modes"
---
 .../bindings/regulator/richtek,rtq2208.yaml        | 196 +++++++++++++++++++++
 1 file changed, 196 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
  

On 26/07/2023 09:13, Alina Yu wrote:
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>

Thank you for your patch. There is something to discuss/improve.

> +    
> +  richtek,mtp-sel-high:
> +    type: boolean
> +    description:
> +      vout register selection based on this boolean value.
> +      false - Using DVS0 register setting to adjust vout
> +      true - Using DVS1 register setting to adjust vout
> +
> +  regulators:
> +    type: object
> +

On this level of indentation:

    additionalProperties: false

I think I missed this in previous versions. I am sorry.

Rest looks good to me, but this needs to be fixed.

Best regards,
Krzysztof
  

On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
Hi,
> ---
> v5
> - Revise filename from $id
> - Remove "regulator-compatible" for ldos
> - Add missing "high" for "richtek,mtp-sel-high"
> - Remove "regulator-mode" property
> - Add and modify "unevaluatedProperties: false" and "additionalProperties: false"
> - Remove "richtek,fixed-uV"
> - Remove redundant space in "buck-a" and "ldo2"
> v4
> - Modify filename to "richtek,rtq2208"
> - Add more desciptions for "regulator-allowed-modes"
> ---
>  .../bindings/regulator/richtek,rtq2208.yaml        | 196 +++++++++++++++++++++
>  1 file changed, 196 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> 
> diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> new file mode 100644
> index 0000000..63453f2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
> @@ -0,0 +1,196 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/richtek,rtq2208.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Richtek RTQ2208 SubPMIC Regulator
> +
> +maintainers:
> +  - Alina Yu <alina_yu@richtek.com>
> +
> +description: |
> +  RTQ2208 is a highly integrated power converter that offers functional safety dual
> +  multi-configurable synchronous buck converters and two LDOs.
> +
> +  Bucks support "regulator-allowed-modes" and "regulator-mode". The former defines the permitted
> +  switching operation in normal mode; the latter defines the operation in suspend to RAM mode.
> +
> +  No matter the RTQ2208 is configured to normal or suspend to RAM mode, there are two switching
> +  operation modes for all buck rails, automatic power saving mode (Auto mode) and forced continuous
> +  conduction mode (FCCM).
> +
> +  The definition of modes is in the datasheet which is available 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
> +
> +properties:
> +  compatible:
> +    enum:
> +      - richtek,rtq2208
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +    
> +  richtek,mtp-sel-high:
> +    type: boolean
> +    description:
> +      vout register selection based on this boolean value.
> +      false - Using DVS0 register setting to adjust vout
> +      true - Using DVS1 register setting to adjust vout
> +
> +  regulators:
> +    type: object
Just curious.
It seems this PMIC only support buck/ldo ouput.
Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
AFAIK, if there's no more function included, to put all on the top level may be better.
> +
> +    patternProperties:
> +      "^buck-[a-h]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          description for buck-[a-h] regulator.
> +
> +        properties:
> +          regulator-allowed-modes:
> +            description:
> +              two buck modes in different switching accuracy.
> +              0 - Auto mode
> +              1 - FCCM
> +            items:
> +              enum: [0, 1]
> +
> +      "^ldo[1-2]$":
> +        type: object
> +        $ref: regulator.yaml#
> +        unevaluatedProperties: false
> +        description:
> +          regulator description for ldo[1-2].
> +
> +required:
> +  - compatible
> +  - reg
> +  - regulators
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      pmic@10 {
> +        compatible = "richtek,rtq2208";
> +        reg = <0x10>;
> +        interrupts-extended = <&gpio26 0 IRQ_TYPE_LEVEL_LOW>;
> +        richtek,mtp-sel-high;
> +
> +        regulators {
> +          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 {
> +            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 {
> +            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 {
> +            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 {
> +            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 {
> +            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 {
> +            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 {
> +            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 {
> +            regulator-min-microvolt = <1200000>;
> +            regulator-max-microvolt = <1200000>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +            };
> +          };
> +          ldo2 {
> +            regulator-min-microvolt = <3300000>;
> +            regulator-max-microvolt = <3300000>;
> +            regulator-always-on;
> +            regulator-state-mem {
> +              regulator-on-in-suspend;
> +            };
> +          };
> +        };
> +      };
> +    };
> -- 
> 2.7.4
>
  

On 03/08/2023 08:36, ChiYuan Huang wrote:
> On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
>> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
>>
>> Signed-off-by: Alina Yu <alina_yu@richtek.com>


>> +    
>> +  richtek,mtp-sel-high:
>> +    type: boolean
>> +    description:
>> +      vout register selection based on this boolean value.
>> +      false - Using DVS0 register setting to adjust vout
>> +      true - Using DVS1 register setting to adjust vout
>> +
>> +  regulators:
>> +    type: object
> Just curious.
> It seems this PMIC only support buck/ldo ouput.
> Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
> AFAIK, if there's no more function included, to put all on the top level may be better.
>> +

Indeed we tend to avoid empty nodes just as a corresponding part to
Linux device.

Here, I don't know Rob's and Conor's opinion, but for cases of having
more than 2 regulators, having "regulators" node makes sense to me -
nicely organizes that piece of DT which can grow quite big. I can also
live without the "regulators" node.

Best regards,
Krzysztof
  

Hi
Krzysztof,

On Sat, Aug 05, 2023 at 11:27:00PM +0200, Krzysztof Kozlowski wrote:
> On 03/08/2023 08:36, ChiYuan Huang wrote:
> > On Wed, Jul 26, 2023 at 03:13:11PM +0800, Alina Yu wrote:
> >> Add bindings for Richtek RTQ2208 IC controlled SubPMIC
> >>
> >> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> 
> 
> >> +    
> >> +  richtek,mtp-sel-high:
> >> +    type: boolean
> >> +    description:
> >> +      vout register selection based on this boolean value.
> >> +      false - Using DVS0 register setting to adjust vout
> >> +      true - Using DVS1 register setting to adjust vout
> >> +
> >> +  regulators:
> >> +    type: object
> > Just curious.
> > It seems this PMIC only support buck/ldo ouput.
> > Since Krzysztof already reviewed it, not sure whether to group it into one node is common or not.
> > AFAIK, if there's no more function included, to put all on the top level may be better.
> >> +
> 
> Indeed we tend to avoid empty nodes just as a corresponding part to
> Linux device.
> 
> Here, I don't know Rob's and Conor's opinion, but for cases of having
> more than 2 regulators, having "regulators" node makes sense to me -
> nicely organizes that piece of DT which can grow quite big. I can also
> live without the "regulators" node.
>

Thank you for your kind advice.
I will keep regulator node in v7

Best regards,
Alina