All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: regulator: Add dt property
@ 2023-04-18 14:50 Naresh Solanki
  2023-04-18 14:50 ` [PATCH 2/2] regulator: userspace-consumer: Multiple regulators Naresh Solanki
  2023-04-20  0:01 ` [PATCH 1/2] dt-bindings: regulator: Add dt property Zev Weiss
  0 siblings, 2 replies; 12+ messages in thread
From: Naresh Solanki @ 2023-04-18 14:50 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski, Zev Weiss
  Cc: Naresh Solanki, linux-kernel, devicetree

Add DT property regulator-supplies.
This enables us to couple one or more regulator output to gether. This
is use in case of Single connector having 2 or more supplies.

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/regulator/regulator-output.yaml b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
index 078b37a1a71a..17f683d3c1f3 100644
--- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
+++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
@@ -21,13 +21,19 @@ properties:
   compatible:
     const: regulator-output
 
-  vout-supply:
+  regulator-supplies:
     description:
-      Phandle of the regulator supplying the output.
+      Specifies the name of the output supply provided by the regulator.
+      Defaults to "vout".
+    default: "vout"
+
+patternProperties:
+  ".*-supply":
+    description:
+      Specified the phandle for various supplies
 
 required:
   - compatible
-  - vout-supply
 
 additionalProperties: false
 

base-commit: c55470f8b0616b0adb758077dbae9b19c5aac005
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] regulator: userspace-consumer: Multiple regulators
  2023-04-18 14:50 [PATCH 1/2] dt-bindings: regulator: Add dt property Naresh Solanki
@ 2023-04-18 14:50 ` Naresh Solanki
  2023-04-20  0:02   ` Zev Weiss
  2023-04-20  0:01 ` [PATCH 1/2] dt-bindings: regulator: Add dt property Zev Weiss
  1 sibling, 1 reply; 12+ messages in thread
From: Naresh Solanki @ 2023-04-18 14:50 UTC (permalink / raw)
  To: Liam Girdwood, Mark Brown; +Cc: Naresh Solanki, linux-kernel

Use property regulator-supplies to determine all regulator
supplies.
This is useful in case of a connector having 2 or more supplies.
Example: PCIe connector on mainboard can be powered by 12V & 3.3V
suplies.

Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
---
 drivers/regulator/userspace-consumer.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
index 97f075ed68c9..0bb49547b926 100644
--- a/drivers/regulator/userspace-consumer.c
+++ b/drivers/regulator/userspace-consumer.c
@@ -120,7 +120,10 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 	struct regulator_userspace_consumer_data tmpdata;
 	struct regulator_userspace_consumer_data *pdata;
 	struct userspace_consumer_data *drvdata;
-	int ret;
+	struct device_node *np = pdev->dev.of_node;
+	struct property *prop;
+	const char *supply;
+	int ret, count = 0;
 
 	pdata = dev_get_platdata(&pdev->dev);
 	if (!pdata) {
@@ -131,11 +134,19 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
 		memset(pdata, 0, sizeof(*pdata));
 
 		pdata->no_autoswitch = true;
-		pdata->num_supplies = 1;
-		pdata->supplies = devm_kzalloc(&pdev->dev, sizeof(*pdata->supplies), GFP_KERNEL);
+		pdata->num_supplies = of_property_count_strings(np, "regulator-supplies");
+		if (pdata->num_supplies < 0) {
+			dev_err(&pdev->dev, "could not parse property regulator-supplies");
+			return -EINVAL;
+		}
+		pdata->supplies = devm_kzalloc(&pdev->dev,
+					       sizeof(*pdata->supplies) * pdata->num_supplies,
+					       GFP_KERNEL);
 		if (!pdata->supplies)
 			return -ENOMEM;
-		pdata->supplies[0].supply = "vout";
+
+		of_property_for_each_string(np, "regulator-supplies", prop, supply)
+			pdata->supplies[count++].supply = supply;
 	}
 
 	if (pdata->num_supplies < 1) {
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-18 14:50 [PATCH 1/2] dt-bindings: regulator: Add dt property Naresh Solanki
  2023-04-18 14:50 ` [PATCH 2/2] regulator: userspace-consumer: Multiple regulators Naresh Solanki
@ 2023-04-20  0:01 ` Zev Weiss
  2023-04-20  8:22   ` Naresh Solanki
  1 sibling, 1 reply; 12+ messages in thread
From: Zev Weiss @ 2023-04-20  0:01 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-kernel, devicetree

On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
>Add DT property regulator-supplies.
>This enables us to couple one or more regulator output to gether. This
>is use in case of Single connector having 2 or more supplies.
>
>Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>---
> .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/Documentation/devicetree/bindings/regulator/regulator-output.yaml b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>index 078b37a1a71a..17f683d3c1f3 100644
>--- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>+++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>@@ -21,13 +21,19 @@ properties:
>   compatible:
>     const: regulator-output
>
>-  vout-supply:
>+  regulator-supplies:
>     description:
>-      Phandle of the regulator supplying the output.
>+      Specifies the name of the output supply provided by the regulator.
>+      Defaults to "vout".
>+    default: "vout"
>+

Was this meant to be specified as a string-array to allow providing 
multiple names?

>+patternProperties:
>+  ".*-supply":
>+    description:
>+      Specified the phandle for various supplies
>
> required:
>   - compatible
>-  - vout-supply
>
> additionalProperties: false
>
>

I think it would be nice to also update the examples to show what a 
multi-supply instance would look like.

A slightly more descriptive subject line would also be good -- "Add dt 
property" is a bit vague.

>base-commit: c55470f8b0616b0adb758077dbae9b19c5aac005
>-- 
>2.39.1
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] regulator: userspace-consumer: Multiple regulators
  2023-04-18 14:50 ` [PATCH 2/2] regulator: userspace-consumer: Multiple regulators Naresh Solanki
@ 2023-04-20  0:02   ` Zev Weiss
  2023-04-20  8:46     ` Naresh Solanki
  0 siblings, 1 reply; 12+ messages in thread
From: Zev Weiss @ 2023-04-20  0:02 UTC (permalink / raw)
  To: Naresh Solanki; +Cc: Liam Girdwood, Mark Brown, linux-kernel

On Tue, Apr 18, 2023 at 07:50:51AM PDT, Naresh Solanki wrote:
>Use property regulator-supplies to determine all regulator
>supplies.
>This is useful in case of a connector having 2 or more supplies.
>Example: PCIe connector on mainboard can be powered by 12V & 3.3V
>suplies.
>
>Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>---
> drivers/regulator/userspace-consumer.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/regulator/userspace-consumer.c b/drivers/regulator/userspace-consumer.c
>index 97f075ed68c9..0bb49547b926 100644
>--- a/drivers/regulator/userspace-consumer.c
>+++ b/drivers/regulator/userspace-consumer.c
>@@ -120,7 +120,10 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
> 	struct regulator_userspace_consumer_data tmpdata;
> 	struct regulator_userspace_consumer_data *pdata;
> 	struct userspace_consumer_data *drvdata;
>-	int ret;
>+	struct device_node *np = pdev->dev.of_node;
>+	struct property *prop;
>+	const char *supply;
>+	int ret, count = 0;
>
> 	pdata = dev_get_platdata(&pdev->dev);
> 	if (!pdata) {
>@@ -131,11 +134,19 @@ static int regulator_userspace_consumer_probe(struct platform_device *pdev)
> 		memset(pdata, 0, sizeof(*pdata));
>
> 		pdata->no_autoswitch = true;
>-		pdata->num_supplies = 1;
>-		pdata->supplies = devm_kzalloc(&pdev->dev, sizeof(*pdata->supplies), GFP_KERNEL);
>+		pdata->num_supplies = of_property_count_strings(np, "regulator-supplies");
>+		if (pdata->num_supplies < 0) {
>+			dev_err(&pdev->dev, "could not parse property regulator-supplies");
>+			return -EINVAL;
>+		}
>+		pdata->supplies = devm_kzalloc(&pdev->dev,
>+					       sizeof(*pdata->supplies) * pdata->num_supplies,
>+					       GFP_KERNEL);

AFAICT this doesn't appear to implement the "vout" default specified in 
the dt-binding patch?

Also, since the core of the userspace-consumer driver itself already 
supports multiple regulators, it might be nice for the subject line to 
mention DT supplies or something a bit more specifically.

> 		if (!pdata->supplies)
> 			return -ENOMEM;
>-		pdata->supplies[0].supply = "vout";
>+
>+		of_property_for_each_string(np, "regulator-supplies", prop, supply)
>+			pdata->supplies[count++].supply = supply;
> 	}
>
> 	if (pdata->num_supplies < 1) {
>-- 
>2.39.1
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-20  0:01 ` [PATCH 1/2] dt-bindings: regulator: Add dt property Zev Weiss
@ 2023-04-20  8:22   ` Naresh Solanki
  2023-04-20 10:41     ` Zev Weiss
  2023-04-21 18:10     ` Rob Herring
  0 siblings, 2 replies; 12+ messages in thread
From: Naresh Solanki @ 2023-04-20  8:22 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-kernel, devicetree

Hi Zev,

On 20-04-2023 05:31 am, Zev Weiss wrote:
> On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
>> Add DT property regulator-supplies.
>> This enables us to couple one or more regulator output to gether. This
>> is use in case of Single connector having 2 or more supplies.
>>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>> ---
>> .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git 
>> a/Documentation/devicetree/bindings/regulator/regulator-output.yaml 
>> b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>> index 078b37a1a71a..17f683d3c1f3 100644
>> --- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>> +++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>> @@ -21,13 +21,19 @@ properties:
>>   compatible:
>>     const: regulator-output
>>
>> -  vout-supply:
>> +  regulator-supplies:
>>     description:
>> -      Phandle of the regulator supplying the output.
>> +      Specifies the name of the output supply provided by the regulator.
>> +      Defaults to "vout".
>> +    default: "vout"
>> +
> 
> Was this meant to be specified as a string-array to allow providing 
> multiple names?
Yes. This is string-array.
> 
>> +patternProperties:
>> +  ".*-supply":
>> +    description:
>> +      Specified the phandle for various supplies
>>
>> required:
>>   - compatible
>> -  - vout-supply
>>
>> additionalProperties: false
>>
>>
> 
> I think it would be nice to also update the examples to show what a 
> multi-supply instance would look like.
Ack. Will do that.
> 
> A slightly more descriptive subject line would also be good -- "Add dt 
> property" is a bit vague.
Suggestion ?
How about like 'Allow multiple supplies' or 'Add support for multiple 
supplies'
> 
>> base-commit: c55470f8b0616b0adb758077dbae9b19c5aac005
>> -- 
>> 2.39.1
>>
Regards,
Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] regulator: userspace-consumer: Multiple regulators
  2023-04-20  0:02   ` Zev Weiss
@ 2023-04-20  8:46     ` Naresh Solanki
  2023-04-20 10:57       ` Zev Weiss
  0 siblings, 1 reply; 12+ messages in thread
From: Naresh Solanki @ 2023-04-20  8:46 UTC (permalink / raw)
  To: Zev Weiss; +Cc: Liam Girdwood, Mark Brown, linux-kernel

Hi Zev,

On 20-04-2023 05:32 am, Zev Weiss wrote:
> On Tue, Apr 18, 2023 at 07:50:51AM PDT, Naresh Solanki wrote:
>> Use property regulator-supplies to determine all regulator
>> supplies.
>> This is useful in case of a connector having 2 or more supplies.
>> Example: PCIe connector on mainboard can be powered by 12V & 3.3V
>> suplies.
>>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>> ---
>> drivers/regulator/userspace-consumer.c | 19 +++++++++++++++----
>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/regulator/userspace-consumer.c 
>> b/drivers/regulator/userspace-consumer.c
>> index 97f075ed68c9..0bb49547b926 100644
>> --- a/drivers/regulator/userspace-consumer.c
>> +++ b/drivers/regulator/userspace-consumer.c
>> @@ -120,7 +120,10 @@ static int 
>> regulator_userspace_consumer_probe(struct platform_device *pdev)
>>     struct regulator_userspace_consumer_data tmpdata;
>>     struct regulator_userspace_consumer_data *pdata;
>>     struct userspace_consumer_data *drvdata;
>> -    int ret;
>> +    struct device_node *np = pdev->dev.of_node;
>> +    struct property *prop;
>> +    const char *supply;
>> +    int ret, count = 0;
>>
>>     pdata = dev_get_platdata(&pdev->dev);
>>     if (!pdata) {
>> @@ -131,11 +134,19 @@ static int 
>> regulator_userspace_consumer_probe(struct platform_device *pdev)
>>         memset(pdata, 0, sizeof(*pdata));
>>
>>         pdata->no_autoswitch = true;
>> -        pdata->num_supplies = 1;
>> -        pdata->supplies = devm_kzalloc(&pdev->dev, 
>> sizeof(*pdata->supplies), GFP_KERNEL);
>> +        pdata->num_supplies = of_property_count_strings(np, 
>> "regulator-supplies");
>> +        if (pdata->num_supplies < 0) {
>> +            dev_err(&pdev->dev, "could not parse property 
>> regulator-supplies");
>> +            return -EINVAL;
>> +        }
>> +        pdata->supplies = devm_kzalloc(&pdev->dev,
>> +                           sizeof(*pdata->supplies) * 
>> pdata->num_supplies,
>> +                           GFP_KERNEL);
> 
> AFAICT this doesn't appear to implement the "vout" default specified in 
> the dt-binding patch?
The "regulator-supplies" property will hold the default value of "vout" 
unless specified otherwise. As a result, the string enumeration 
retrieves the value of "vout" by default, and the "vout-supply" property 
is utilized for the regulator.

> 
> Also, since the core of the userspace-consumer driver itself already 
> supports multiple regulators, it might be nice for the subject line to 
> mention DT supplies or something a bit more specifically.
Sure. How about 'Support multiple supplies' ?
> 
>>         if (!pdata->supplies)
>>             return -ENOMEM;
>> -        pdata->supplies[0].supply = "vout";
>> +
>> +        of_property_for_each_string(np, "regulator-supplies", prop, 
>> supply)
>> +            pdata->supplies[count++].supply = supply;
>>     }
>>
>>     if (pdata->num_supplies < 1) {
>> -- 
>> 2.39.1
>>
>>
Regards,
Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-20  8:22   ` Naresh Solanki
@ 2023-04-20 10:41     ` Zev Weiss
  2023-04-20 11:32       ` Naresh Solanki
  2023-04-21 18:10     ` Rob Herring
  1 sibling, 1 reply; 12+ messages in thread
From: Zev Weiss @ 2023-04-20 10:41 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-kernel, devicetree

On Thu, Apr 20, 2023 at 01:22:30AM PDT, Naresh Solanki wrote:
>Hi Zev,
>
>On 20-04-2023 05:31 am, Zev Weiss wrote:
>>On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
>>>Add DT property regulator-supplies.
>>>This enables us to couple one or more regulator output to gether. This
>>>is use in case of Single connector having 2 or more supplies.
>>>
>>>Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>---
>>>.../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
>>>1 file changed, 9 insertions(+), 3 deletions(-)
>>>
>>>diff --git 
>>>a/Documentation/devicetree/bindings/regulator/regulator-output.yaml 
>>>b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>index 078b37a1a71a..17f683d3c1f3 100644
>>>--- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>+++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>@@ -21,13 +21,19 @@ properties:
>>>  compatible:
>>>    const: regulator-output
>>>
>>>-  vout-supply:
>>>+  regulator-supplies:
>>>    description:
>>>-      Phandle of the regulator supplying the output.
>>>+      Specifies the name of the output supply provided by the regulator.
>>>+      Defaults to "vout".
>>>+    default: "vout"
>>>+
>>
>>Was this meant to be specified as a string-array to allow providing 
>>multiple names?
>Yes. This is string-array.

Okay -- in that case I think it should include

   $ref: /schemas/types.yaml#/definitions/string-array

>>
>>>+patternProperties:
>>>+  ".*-supply":
>>>+    description:
>>>+      Specified the phandle for various supplies
>>>
>>>required:
>>>  - compatible
>>>-  - vout-supply
>>>
>>>additionalProperties: false
>>>
>>>
>>
>>I think it would be nice to also update the examples to show what a 
>>multi-supply instance would look like.
>Ack. Will do that.
>>
>>A slightly more descriptive subject line would also be good -- "Add 
>>dt property" is a bit vague.
>Suggestion ?
>How about like 'Allow multiple supplies' or 'Add support for multiple 
>supplies'

Sure, both of those sound fine to me.

>>
>>>base-commit: c55470f8b0616b0adb758077dbae9b19c5aac005
>>>-- 
>>>2.39.1
>>>
>Regards,
>Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] regulator: userspace-consumer: Multiple regulators
  2023-04-20  8:46     ` Naresh Solanki
@ 2023-04-20 10:57       ` Zev Weiss
  2023-04-20 12:27         ` Naresh Solanki
  0 siblings, 1 reply; 12+ messages in thread
From: Zev Weiss @ 2023-04-20 10:57 UTC (permalink / raw)
  To: Naresh Solanki; +Cc: Liam Girdwood, Mark Brown, linux-kernel

On Thu, Apr 20, 2023 at 01:46:14AM PDT, Naresh Solanki wrote:
>Hi Zev,
>
>On 20-04-2023 05:32 am, Zev Weiss wrote:
>>On Tue, Apr 18, 2023 at 07:50:51AM PDT, Naresh Solanki wrote:
>>>Use property regulator-supplies to determine all regulator
>>>supplies.
>>>This is useful in case of a connector having 2 or more supplies.
>>>Example: PCIe connector on mainboard can be powered by 12V & 3.3V
>>>suplies.
>>>
>>>Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>---
>>>drivers/regulator/userspace-consumer.c | 19 +++++++++++++++----
>>>1 file changed, 15 insertions(+), 4 deletions(-)
>>>
>>>diff --git a/drivers/regulator/userspace-consumer.c 
>>>b/drivers/regulator/userspace-consumer.c
>>>index 97f075ed68c9..0bb49547b926 100644
>>>--- a/drivers/regulator/userspace-consumer.c
>>>+++ b/drivers/regulator/userspace-consumer.c
>>>@@ -120,7 +120,10 @@ static int 
>>>regulator_userspace_consumer_probe(struct platform_device *pdev)
>>>    struct regulator_userspace_consumer_data tmpdata;
>>>    struct regulator_userspace_consumer_data *pdata;
>>>    struct userspace_consumer_data *drvdata;
>>>-    int ret;
>>>+    struct device_node *np = pdev->dev.of_node;
>>>+    struct property *prop;
>>>+    const char *supply;
>>>+    int ret, count = 0;
>>>
>>>    pdata = dev_get_platdata(&pdev->dev);
>>>    if (!pdata) {
>>>@@ -131,11 +134,19 @@ static int 
>>>regulator_userspace_consumer_probe(struct platform_device *pdev)
>>>        memset(pdata, 0, sizeof(*pdata));
>>>
>>>        pdata->no_autoswitch = true;
>>>-        pdata->num_supplies = 1;
>>>-        pdata->supplies = devm_kzalloc(&pdev->dev, 
>>>sizeof(*pdata->supplies), GFP_KERNEL);
>>>+        pdata->num_supplies = of_property_count_strings(np, 
>>>"regulator-supplies");
>>>+        if (pdata->num_supplies < 0) {
>>>+            dev_err(&pdev->dev, "could not parse property 
>>>regulator-supplies");
>>>+            return -EINVAL;
>>>+        }
>>>+        pdata->supplies = devm_kzalloc(&pdev->dev,
>>>+                           sizeof(*pdata->supplies) * 
>>>pdata->num_supplies,
>>>+                           GFP_KERNEL);
>>
>>AFAICT this doesn't appear to implement the "vout" default specified 
>>in the dt-binding patch?
>The "regulator-supplies" property will hold the default value of 
>"vout" unless specified otherwise. As a result, the string enumeration 
>retrieves the value of "vout" by default, and the "vout-supply" 
>property is utilized for the regulator.
>

With the disclaimer that I'm not a DT expert, that's not my 
understanding of how DT works.  I don't think the 'default' value 
specified in the binding forces the fdt to always include that value if 
it's not present in the dts (since I'm pretty sure dtc doesn't even look 
at the binding to know that a default exists when compiling the dts); 
rather, it's information meant to be used by the software implementing 
support for that device (e.g. a driver for it) about what value to 
assume if the property isn't present in the fdt.

>>
>>Also, since the core of the userspace-consumer driver itself already 
>>supports multiple regulators, it might be nice for the subject line 
>>to mention DT supplies or something a bit more specifically.
>Sure. How about 'Support multiple supplies' ?

I meant that it should explicitly mention "DT" (or perhaps "OF").  The 
driver's structure has supported multiple supplies since it was first 
introduced in 2009, so "Support multiple supplies" sounds like this 
commit is adding functionality that was already there.  What this patch 
is doing is connecting that existing support to the OF support logic so 
that it can be used in a device-tree context.

>>
>>>        if (!pdata->supplies)
>>>            return -ENOMEM;
>>>-        pdata->supplies[0].supply = "vout";
>>>+
>>>+        of_property_for_each_string(np, "regulator-supplies", 
>>>prop, supply)
>>>+            pdata->supplies[count++].supply = supply;
>>>    }
>>>
>>>    if (pdata->num_supplies < 1) {
>>>-- 
>>>2.39.1
>>>
>>>
>Regards,
>Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-20 10:41     ` Zev Weiss
@ 2023-04-20 11:32       ` Naresh Solanki
  0 siblings, 0 replies; 12+ messages in thread
From: Naresh Solanki @ 2023-04-20 11:32 UTC (permalink / raw)
  To: Zev Weiss
  Cc: Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	linux-kernel, devicetree

Hi Zev,

On 20-04-2023 04:11 pm, Zev Weiss wrote:
> On Thu, Apr 20, 2023 at 01:22:30AM PDT, Naresh Solanki wrote:
>> Hi Zev,
>>
>> On 20-04-2023 05:31 am, Zev Weiss wrote:
>>> On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
>>>> Add DT property regulator-supplies.
>>>> This enables us to couple one or more regulator output to gether. This
>>>> is use in case of Single connector having 2 or more supplies.
>>>>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>> ---
>>>> .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/regulator/regulator-output.yaml 
>>>> b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> index 078b37a1a71a..17f683d3c1f3 100644
>>>> --- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> +++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> @@ -21,13 +21,19 @@ properties:
>>>>   compatible:
>>>>     const: regulator-output
>>>>
>>>> -  vout-supply:
>>>> +  regulator-supplies:
>>>>     description:
>>>> -      Phandle of the regulator supplying the output.
>>>> +      Specifies the name of the output supply provided by the 
>>>> regulator.
>>>> +      Defaults to "vout".
>>>> +    default: "vout"
>>>> +
>>>
>>> Was this meant to be specified as a string-array to allow providing 
>>> multiple names?
>> Yes. This is string-array.
> 
> Okay -- in that case I think it should include
> 
>    $ref: /schemas/types.yaml#/definitions/string-array
Ack
> 
>>>
>>>> +patternProperties:
>>>> +  ".*-supply":
>>>> +    description:
>>>> +      Specified the phandle for various supplies
>>>>
>>>> required:
>>>>   - compatible
>>>> -  - vout-supply
>>>>
>>>> additionalProperties: false
>>>>
>>>>
>>>
>>> I think it would be nice to also update the examples to show what a 
>>> multi-supply instance would look like.
>> Ack. Will do that.
>>>
>>> A slightly more descriptive subject line would also be good -- "Add 
>>> dt property" is a bit vague.
>> Suggestion ?
>> How about like 'Allow multiple supplies' or 'Add support for multiple 
>> supplies'
> 
> Sure, both of those sound fine to me.
Thanks :)
> 
>>>
>>>> base-commit: c55470f8b0616b0adb758077dbae9b19c5aac005
>>>> -- 
>>>> 2.39.1
>>>>
>> Regards,
>> Naresh
Regards,
Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] regulator: userspace-consumer: Multiple regulators
  2023-04-20 10:57       ` Zev Weiss
@ 2023-04-20 12:27         ` Naresh Solanki
  0 siblings, 0 replies; 12+ messages in thread
From: Naresh Solanki @ 2023-04-20 12:27 UTC (permalink / raw)
  To: Zev Weiss; +Cc: Liam Girdwood, Mark Brown, linux-kernel

Hi Zev,

On 20-04-2023 04:27 pm, Zev Weiss wrote:
> On Thu, Apr 20, 2023 at 01:46:14AM PDT, Naresh Solanki wrote:
>> Hi Zev,
>>
>> On 20-04-2023 05:32 am, Zev Weiss wrote:
>>> On Tue, Apr 18, 2023 at 07:50:51AM PDT, Naresh Solanki wrote:
>>>> Use property regulator-supplies to determine all regulator
>>>> supplies.
>>>> This is useful in case of a connector having 2 or more supplies.
>>>> Example: PCIe connector on mainboard can be powered by 12V & 3.3V
>>>> suplies.
>>>>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>> ---
>>>> drivers/regulator/userspace-consumer.c | 19 +++++++++++++++----
>>>> 1 file changed, 15 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/regulator/userspace-consumer.c 
>>>> b/drivers/regulator/userspace-consumer.c
>>>> index 97f075ed68c9..0bb49547b926 100644
>>>> --- a/drivers/regulator/userspace-consumer.c
>>>> +++ b/drivers/regulator/userspace-consumer.c
>>>> @@ -120,7 +120,10 @@ static int 
>>>> regulator_userspace_consumer_probe(struct platform_device *pdev)
>>>>     struct regulator_userspace_consumer_data tmpdata;
>>>>     struct regulator_userspace_consumer_data *pdata;
>>>>     struct userspace_consumer_data *drvdata;
>>>> -    int ret;
>>>> +    struct device_node *np = pdev->dev.of_node;
>>>> +    struct property *prop;
>>>> +    const char *supply;
>>>> +    int ret, count = 0;
>>>>
>>>>     pdata = dev_get_platdata(&pdev->dev);
>>>>     if (!pdata) {
>>>> @@ -131,11 +134,19 @@ static int 
>>>> regulator_userspace_consumer_probe(struct platform_device *pdev)
>>>>         memset(pdata, 0, sizeof(*pdata));
>>>>
>>>>         pdata->no_autoswitch = true;
>>>> -        pdata->num_supplies = 1;
>>>> -        pdata->supplies = devm_kzalloc(&pdev->dev, 
>>>> sizeof(*pdata->supplies), GFP_KERNEL);
>>>> +        pdata->num_supplies = of_property_count_strings(np, 
>>>> "regulator-supplies");
>>>> +        if (pdata->num_supplies < 0) {
>>>> +            dev_err(&pdev->dev, "could not parse property 
>>>> regulator-supplies");
>>>> +            return -EINVAL;
>>>> +        }
>>>> +        pdata->supplies = devm_kzalloc(&pdev->dev,
>>>> +                           sizeof(*pdata->supplies) * 
>>>> pdata->num_supplies,
>>>> +                           GFP_KERNEL);
>>>
>>> AFAICT this doesn't appear to implement the "vout" default specified 
>>> in the dt-binding patch?
>> The "regulator-supplies" property will hold the default value of 
>> "vout" unless specified otherwise. As a result, the string enumeration 
>> retrieves the value of "vout" by default, and the "vout-supply" 
>> property is utilized for the regulator.
>>
> 
> With the disclaimer that I'm not a DT expert, that's not my 
> understanding of how DT works.  I don't think the 'default' value 
> specified in the binding forces the fdt to always include that value if 
> it's not present in the dts (since I'm pretty sure dtc doesn't even look 
> at the binding to know that a default exists when compiling the dts); 
> rather, it's information meant to be used by the software implementing 
> support for that device (e.g. a driver for it) about what value to 
> assume if the property isn't present in the fdt.
I apologize for the oversight on my part. Upon further testing, I have 
discovered that the default did not reflect in the debug prints. I will 
address this issue and include the proper fix in the next patchset. 
Thank you for bringing this to my attention
> 
>>>
>>> Also, since the core of the userspace-consumer driver itself already 
>>> supports multiple regulators, it might be nice for the subject line 
>>> to mention DT supplies or something a bit more specifically.
>> Sure. How about 'Support multiple supplies' ?
> 
> I meant that it should explicitly mention "DT" (or perhaps "OF").  The 
> driver's structure has supported multiple supplies since it was first 
> introduced in 2009, so "Support multiple supplies" sounds like this 
> commit is adding functionality that was already there.  What this patch 
> is doing is connecting that existing support to the OF support logic so 
> that it can be used in a device-tree context.
Sure. Will make it something like "Support multiple supplies in DT".
> 
>>>
>>>>         if (!pdata->supplies)
>>>>             return -ENOMEM;
>>>> -        pdata->supplies[0].supply = "vout";
>>>> +
>>>> +        of_property_for_each_string(np, "regulator-supplies", prop, 
>>>> supply)
>>>> +            pdata->supplies[count++].supply = supply;
>>>>     }
>>>>
>>>>     if (pdata->num_supplies < 1) {
>>>> -- 
>>>> 2.39.1
>>>>
>>>>
>> Regards,
>> Naresh
Regards,
Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-20  8:22   ` Naresh Solanki
  2023-04-20 10:41     ` Zev Weiss
@ 2023-04-21 18:10     ` Rob Herring
  2023-04-24  7:49       ` Naresh Solanki
  1 sibling, 1 reply; 12+ messages in thread
From: Rob Herring @ 2023-04-21 18:10 UTC (permalink / raw)
  To: Naresh Solanki
  Cc: Zev Weiss, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	linux-kernel, devicetree

On Thu, Apr 20, 2023 at 01:52:30PM +0530, Naresh Solanki wrote:
> Hi Zev,
> 
> On 20-04-2023 05:31 am, Zev Weiss wrote:
> > On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
> > > Add DT property regulator-supplies.
> > > This enables us to couple one or more regulator output to gether. This
> > > is use in case of Single connector having 2 or more supplies.
> > > 
> > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> > > ---
> > > .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
> > > 1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
> > > b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
> > > index 078b37a1a71a..17f683d3c1f3 100644
> > > --- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
> > > +++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
> > > @@ -21,13 +21,19 @@ properties:
> > >   compatible:
> > >     const: regulator-output
> > > 
> > > -  vout-supply:
> > > +  regulator-supplies:
> > >     description:
> > > -      Phandle of the regulator supplying the output.
> > > +      Specifies the name of the output supply provided by the regulator.
> > > +      Defaults to "vout".
> > > +    default: "vout"
> > > +
> > 
> > Was this meant to be specified as a string-array to allow providing
> > multiple names?
> Yes. This is string-array.
> > 
> > > +patternProperties:
> > > +  ".*-supply":
> > > +    description:
> > > +      Specified the phandle for various supplies
> > > 
> > > required:
> > >   - compatible
> > > -  - vout-supply
> > > 
> > > additionalProperties: false
> > > 
> > > 
> > 
> > I think it would be nice to also update the examples to show what a
> > multi-supply instance would look like.
> Ack. Will do that.
> > 
> > A slightly more descriptive subject line would also be good -- "Add dt
> > property" is a bit vague.
> Suggestion ?
> How about like 'Allow multiple supplies' or 'Add support for multiple
> supplies'

And indicate this is for regulator-output, As-is looks like it's 
something for all regulators.

Rob

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: regulator: Add dt property
  2023-04-21 18:10     ` Rob Herring
@ 2023-04-24  7:49       ` Naresh Solanki
  0 siblings, 0 replies; 12+ messages in thread
From: Naresh Solanki @ 2023-04-24  7:49 UTC (permalink / raw)
  To: Rob Herring
  Cc: Zev Weiss, Liam Girdwood, Mark Brown, Krzysztof Kozlowski,
	linux-kernel, devicetree

Hi Rob,

On 21-04-2023 11:40 pm, Rob Herring wrote:
> On Thu, Apr 20, 2023 at 01:52:30PM +0530, Naresh Solanki wrote:
>> Hi Zev,
>>
>> On 20-04-2023 05:31 am, Zev Weiss wrote:
>>> On Tue, Apr 18, 2023 at 07:50:50AM PDT, Naresh Solanki wrote:
>>>> Add DT property regulator-supplies.
>>>> This enables us to couple one or more regulator output to gether. This
>>>> is use in case of Single connector having 2 or more supplies.
>>>>
>>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
>>>> ---
>>>> .../bindings/regulator/regulator-output.yaml         | 12 +++++++++---
>>>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git
>>>> a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> index 078b37a1a71a..17f683d3c1f3 100644
>>>> --- a/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> +++ b/Documentation/devicetree/bindings/regulator/regulator-output.yaml
>>>> @@ -21,13 +21,19 @@ properties:
>>>>    compatible:
>>>>      const: regulator-output
>>>>
>>>> -  vout-supply:
>>>> +  regulator-supplies:
>>>>      description:
>>>> -      Phandle of the regulator supplying the output.
>>>> +      Specifies the name of the output supply provided by the regulator.
>>>> +      Defaults to "vout".
>>>> +    default: "vout"
>>>> +
>>>
>>> Was this meant to be specified as a string-array to allow providing
>>> multiple names?
>> Yes. This is string-array.
>>>
>>>> +patternProperties:
>>>> +  ".*-supply":
>>>> +    description:
>>>> +      Specified the phandle for various supplies
>>>>
>>>> required:
>>>>    - compatible
>>>> -  - vout-supply
>>>>
>>>> additionalProperties: false
>>>>
>>>>
>>>
>>> I think it would be nice to also update the examples to show what a
>>> multi-supply instance would look like.
>> Ack. Will do that.
>>>
>>> A slightly more descriptive subject line would also be good -- "Add dt
>>> property" is a bit vague.
>> Suggestion ?
>> How about like 'Allow multiple supplies' or 'Add support for multiple
>> supplies'
> 
> And indicate this is for regulator-output, As-is looks like it's
> something for all regulators.
Thanks for bringing that to my notice. Will make:
dt-bindings: regulator-output: Add dt property
> 
> Rob
Regards,
Naresh

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-04-24  7:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-18 14:50 [PATCH 1/2] dt-bindings: regulator: Add dt property Naresh Solanki
2023-04-18 14:50 ` [PATCH 2/2] regulator: userspace-consumer: Multiple regulators Naresh Solanki
2023-04-20  0:02   ` Zev Weiss
2023-04-20  8:46     ` Naresh Solanki
2023-04-20 10:57       ` Zev Weiss
2023-04-20 12:27         ` Naresh Solanki
2023-04-20  0:01 ` [PATCH 1/2] dt-bindings: regulator: Add dt property Zev Weiss
2023-04-20  8:22   ` Naresh Solanki
2023-04-20 10:41     ` Zev Weiss
2023-04-20 11:32       ` Naresh Solanki
2023-04-21 18:10     ` Rob Herring
2023-04-24  7:49       ` Naresh Solanki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.