[PATCH 0/2] Add specified fixed LDO VOUT propery

[PATCH 0/2] Add specified fixed LDO VOUT propery

[Alina Yu]

Hi,

There are two types of LDO VOUT: fixed voltage mode and adjustable voltage mode.

As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
the constraints for this scenario are not suitable to represent both modes.
Therefore, A property is added to specify the fixed LDO VOUT.

Alina Yu (2):
  regulator: rtq2208: Add fixed LDO VOUT property and check that matches
    the constraints
  regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

 .../devicetree/bindings/regulator/richtek,rtq2208.yaml        | 11 ++++++++++-
 drivers/regulator/rtq2208-regulator.c                         | 11 +++++++++--
 2 files changed, 19 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH 1/2] regulator: rtq2208: Add fixed LDO VOUT property and check that matches the constraints

[Alina Yu]

A fixed LDO VOUT property has been added to specify the fixed_uV of the regulator_desc.
Additionally, a check has been included in this version
to ensure that the fixed_uV matches the constraints.

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
 drivers/regulator/rtq2208-regulator.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/rtq2208-regulator.c b/drivers/regulator/rtq2208-regulator.c
index 39147d6..c2c1689 100644
--- a/drivers/regulator/rtq2208-regulator.c
+++ b/drivers/regulator/rtq2208-regulator.c
@@ -377,6 +377,7 @@ static int rtq2208_of_get_ldo_dvs_ability(struct device *dev)
 	struct of_regulator_match *match;
 	struct regulator_desc *desc;
 	struct regulator_init_data *init_data;
+	u32 fixed_uV;
 	int ret, i;
 
 	if (!dev->of_node)
@@ -401,9 +402,15 @@ static int rtq2208_of_get_ldo_dvs_ability(struct device *dev)
 		if (!init_data || !desc)
 			continue;
 
-		if (init_data->constraints.min_uV == init_data->constraints.max_uV) {
+		/* specify working fixed voltage if the propery exists */
+		ret = of_property_read_u32(match->of_node, "richtek,fixed-microvolt", &fixed_uV);
+
+		if (!ret) {
+			if (fixed_uV != init_data->constraints.min_uV ||
+				fixed_uV != init_data->constraints.max_uV)
+				return -EINVAL;
 			desc->n_voltages = 1;
-			desc->fixed_uV = init_data->constraints.min_uV;
+			desc->fixed_uV = fixed_uV;
 			desc->ops = &rtq2208_regulator_ldo_fix_ops;
 		} else {
 			desc->n_voltages = ARRAY_SIZE(rtq2208_ldo_volt_table);
-- 
2.7.4

[PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Alina Yu]

As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
the constraints for this scenario are not suitable to represent both modes.
Therefore, A property is added to specify the fixed LDO VOUT.

Examples of fixed LDO VOUT and adjustable LDO VOUT is also added to this version.

Signed-off-by: Alina Yu <alina_yu@richtek.com>
---
 .../devicetree/bindings/regulator/richtek,rtq2208.yaml        | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
index 609c066..87accc6 100644
--- a/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
+++ b/Documentation/devicetree/bindings/regulator/richtek,rtq2208.yaml
@@ -75,6 +75,12 @@ properties:
         description:
           regulator description for ldo[1-2].
 
+        properties:
+          richtek,fixed-microvolt:
+            description: |
+              This property can be used to set a fixed operating voltage that lies outside
+              the range of the regulator's adjustable mode.
+
 required:
   - compatible
   - reg
@@ -177,6 +183,8 @@ examples:
             };
           };
           ldo1 {
+            /* Fixed LDO VOUT */
+            richtek,fixed-microvolt = <1200000>;
             regulator-min-microvolt = <1200000>;
             regulator-max-microvolt = <1200000>;
             regulator-always-on;
@@ -185,7 +193,8 @@ examples:
             };
           };
           ldo2 {
-            regulator-min-microvolt = <3300000>;
+            /* Adjustable LDO VOUT */
+            regulator-min-microvolt = <1800000>;
             regulator-max-microvolt = <3300000>;
             regulator-always-on;
             regulator-state-mem {
-- 
2.7.4

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Krzysztof Kozlowski]

On 16/05/2024 11:20, Alina Yu wrote:
> As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
> the constraints for this scenario are not suitable to represent both modes.
> Therefore, A property is added to specify the fixed LDO VOUT.
> 
> Examples of fixed LDO VOUT and adjustable LDO VOUT is also added to this version.
> 
> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> ---

This is a v1 but I am pretty sure I saw it somewhere and there was
already some sort of discussion. Confused... :(

Best regards,
Krzysztof

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Alina Yu]

On Thu, May 16, 2024 at 02:34:02PM +0200, Krzysztof Kozlowski wrote:
> On 16/05/2024 11:20, Alina Yu wrote:
> > As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
> > the constraints for this scenario are not suitable to represent both modes.
> > Therefore, A property is added to specify the fixed LDO VOUT.
> > 
> > Examples of fixed LDO VOUT and adjustable LDO VOUT is also added to this version.
> > 
> > Signed-off-by: Alina Yu <alina_yu@richtek.com>
> > ---
> 
> This is a v1 but I am pretty sure I saw it somewhere and there was
> already some sort of discussion. Confused... :(
> 
> Best regards,
> Krzysztof
> 

The discussion regarding this matter took place during v2 and v3.
Due to the fixed LDO VOUT being outside the range of the adjustable one,
a special-use property has been added to avoid overusing the constraints.


Thanks,
Alina

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Krzysztof Kozlowski]

On 22/05/2024 11:03, Alina Yu wrote:
> On Thu, May 16, 2024 at 02:34:02PM +0200, Krzysztof Kozlowski wrote:
>> On 16/05/2024 11:20, Alina Yu wrote:
>>> As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
>>> the constraints for this scenario are not suitable to represent both modes.
>>> Therefore, A property is added to specify the fixed LDO VOUT.
>>>
>>> Examples of fixed LDO VOUT and adjustable LDO VOUT is also added to this version.
>>>
>>> Signed-off-by: Alina Yu <alina_yu@richtek.com>
>>> ---
>>
>> This is a v1 but I am pretty sure I saw it somewhere and there was
>> already some sort of discussion. Confused... :(
>>
>> Best regards,
>> Krzysztof
>>
> 
> The discussion regarding this matter took place during v2 and v3.

So in the future?

> Due to the fixed LDO VOUT being outside the range of the adjustable one,
> a special-use property has been added to avoid overusing the constraints.

Hm, why exactly this is not a bool property? What are the benefits?

Best regards,
Krzysztof

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Alina Yu]

On Wed, May 22, 2024 at 11:27:06AM +0200, Krzysztof Kozlowski wrote:
> On 22/05/2024 11:03, Alina Yu wrote:
> > On Thu, May 16, 2024 at 02:34:02PM +0200, Krzysztof Kozlowski wrote:
> >> On 16/05/2024 11:20, Alina Yu wrote:
> >>> As the fixed voltage for the LDO is outside the range of the adjustable voltage mode,
> >>> the constraints for this scenario are not suitable to represent both modes.
> >>> Therefore, A property is added to specify the fixed LDO VOUT.
> >>>
> >>> Examples of fixed LDO VOUT and adjustable LDO VOUT is also added to this version.
> >>>
> >>> Signed-off-by: Alina Yu <alina_yu@richtek.com>
> >>> ---
> >>
> >> This is a v1 but I am pretty sure I saw it somewhere and there was
> >> already some sort of discussion. Confused... :(
> >>
> >> Best regards,
> >> Krzysztof
> >>
> > 
> > The discussion regarding this matter took place during v2 and v3.
> 
> So in the future?

Based on the previous discussion, it is scheduled to be included in the linux-next tree.
Will we start a further discussions on this topic?

> 
> > Due to the fixed LDO VOUT being outside the range of the adjustable one,
> > a special-use property has been added to avoid overusing the constraints.
> 
> Hm, why exactly this is not a bool property? What are the benefits?
> 
> Best regards,
> Krzysztof
>

As only the user will know the exact fixed voltage,
the property is defined as a u32 to allow user customization and decision-making.


Thanks,
Alina

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 652 bytes --]

On Wed, May 22, 2024 at 11:27:06AM +0200, Krzysztof Kozlowski wrote:
> On 22/05/2024 11:03, Alina Yu wrote:

> > Due to the fixed LDO VOUT being outside the range of the adjustable one,
> > a special-use property has been added to avoid overusing the constraints.

> Hm, why exactly this is not a bool property? What are the benefits?

It avoids confusion between invalid constraints specified on the
variable voltage regulator and allows us to validate any constraints
that happen to be specified (though it'd be pointless to specify
constraints).  The fact that the regulator could also be variable
voltage is asking for confusion if we use boolean.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] regulator: dt-bindings: rtq2208: Add specified fixed LDO VOUT property

[Krzysztof Kozlowski]

On 22/05/2024 13:35, Mark Brown wrote:
> On Wed, May 22, 2024 at 11:27:06AM +0200, Krzysztof Kozlowski wrote:
>> On 22/05/2024 11:03, Alina Yu wrote:
> 
>>> Due to the fixed LDO VOUT being outside the range of the adjustable one,
>>> a special-use property has been added to avoid overusing the constraints.
> 
>> Hm, why exactly this is not a bool property? What are the benefits?
> 
> It avoids confusion between invalid constraints specified on the
> variable voltage regulator and allows us to validate any constraints
> that happen to be specified (though it'd be pointless to specify
> constraints).  The fact that the regulator could also be variable
> voltage is asking for confusion if we use boolean.


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 1/2] regulator: rtq2208: Add fixed LDO VOUT property and check that matches the constraints

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

On Thu, May 16, 2024 at 05:20:33PM +0800, Alina Yu wrote:
> A fixed LDO VOUT property has been added to specify the fixed_uV of the regulator_desc.
> Additionally, a check has been included in this version
> to ensure that the fixed_uV matches the constraints.

This doesn't apply against current code, please check and resend.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/2] regulator: rtq2208: Add fixed LDO VOUT property and check that matches the constraints

[Alina Yu]

On Mon, May 27, 2024 at 02:00:47PM +0100, Mark Brown wrote:
> On Thu, May 16, 2024 at 05:20:33PM +0800, Alina Yu wrote:
> > A fixed LDO VOUT property has been added to specify the fixed_uV of the regulator_desc.
> > Additionally, a check has been included in this version
> > to ensure that the fixed_uV matches the constraints.
> 
> This doesn't apply against current code, please check and resend.

Sorry, I didn't notice these links missed in linux-next tree:

https://lore.kernel.org/all/a1e19b4026d7fea27526ba94c398500a3826b282.1714467553.git.alina_yu@richtek.com/

https://lore.kernel.org/all/ffeecd61c194df1f7f049bd50cb2bbbad3cf1025.1714467553.git.alina_yu@richtek.com/

I have now integrated these missing parts into the resent patches.

Thanks,
Alina

Re: [PATCH 1/2] regulator: rtq2208: Add fixed LDO VOUT property and check that matches the constraints

[Alina Yu]

On Mon, May 27, 2024 at 02:00:47PM +0100, Mark Brown wrote:
> On Thu, May 16, 2024 at 05:20:33PM +0800, Alina Yu wrote:
> > A fixed LDO VOUT property has been added to specify the fixed_uV of the regulator_desc.
> > Additionally, a check has been included in this version
> > to ensure that the fixed_uV matches the constraints.
> 
> This doesn't apply against current code, please check and resend.


Regarding the previous discussion in

https://lore.kernel.org/all/20240528060731.GA25526@linuxcarl2.richtek.com/

I found some patches missing in the linux-next tree.

So I merged the missing part in my latest resent sereies.

The issue has been addressed in

In '[RESEND 2/4] regulator: rtq2208: Fix LDO to be compatible with both fixed and adjustable vout'
https://lore.kernel.org/all/5ad4c7728c7fa7f6286db36b99d31c9d0f5d16c7.1716870419.git.alina_yu@richtek.com/


@@ -219,7 +219,7 @@ static const struct regulator_ops rtq2208_regulator_buck_ops = {
 	.set_suspend_mode = rtq2208_set_suspend_mode,
};
	  
-static const struct regulator_ops rtq2208_regulator_ldo_ops = {
+static const struct regulator_ops rtq2208_regulator_ldo_fix_ops = {
	.enable = regulator_enable_regmap,
	.disable = regulator_disable_regmap,
	.is_enabled = regulator_is_enabled_regmap,

...

Thanks,
Alina