[PATCH 0/2] leds: aw2013: Add vdd regulator

[PATCH 0/2] leds: aw2013: Add vdd regulator

[Lin, Meng-Bo]

Implement support for a "vdd" that is enabled by the aw2013 driver so that
the regulator gets enabled when needed.

[PATCH 1/2] dt-bindings: leds: aw2013: Document vdd-supply

[Lin, Meng-Bo]

Document vdd-supply, a regulator providing power to the "VDD" pin.

Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
---
 Documentation/devicetree/bindings/leds/leds-aw2013.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
index 08f3e1cfc1b1..51a58f4b8ed6 100644
--- a/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
+++ b/Documentation/devicetree/bindings/leds/leds-aw2013.yaml
@@ -23,6 +23,9 @@ properties:
   vcc-supply:
     description: Regulator providing power to the "VCC" pin.
 
+  vdd-supply:
+    description: Regulator providing power to the "VDD" pin.
+
   "#address-cells":
     const: 1
 
-- 
2.30.2

[PATCH 2/2] leds: aw2013: Add vdd regulator

[Lin, Meng-Bo]

Implement support for a "vdd" that is enabled by the aw2013 driver so that
the regulator gets enabled when needed.

Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
---
 drivers/leds/leds-aw2013.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 0b52fc9097c6..91d720edb857 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -62,7 +62,7 @@ struct aw2013_led {
 
 struct aw2013 {
 	struct mutex mutex; /* held when writing to registers */
-	struct regulator *vcc_regulator;
+	struct regulator_bulk_data regulators[2];
 	struct i2c_client *client;
 	struct aw2013_led leds[AW2013_MAX_LEDS];
 	struct regmap *regmap;
@@ -106,7 +106,8 @@ static void aw2013_chip_disable(struct aw2013 *chip)
 
 	regmap_write(chip->regmap, AW2013_GCR, 0);
 
-	ret = regulator_disable(chip->vcc_regulator);
+	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
+				     chip->regulators);
 	if (ret) {
 		dev_err(&chip->client->dev,
 			"Failed to disable regulator: %d\n", ret);
@@ -123,7 +124,8 @@ static int aw2013_chip_enable(struct aw2013 *chip)
 	if (chip->enabled)
 		return 0;
 
-	ret = regulator_enable(chip->vcc_regulator);
+	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
+				    chip->regulators);
 	if (ret) {
 		dev_err(&chip->client->dev,
 			"Failed to enable regulator: %d\n", ret);
@@ -348,16 +350,20 @@ static int aw2013_probe(struct i2c_client *client)
 		goto error;
 	}
 
-	chip->vcc_regulator = devm_regulator_get(&client->dev, "vcc");
-	ret = PTR_ERR_OR_ZERO(chip->vcc_regulator);
-	if (ret) {
+	chip->regulators[0].supply = "vcc";
+	chip->regulators[1].supply = "vdd";
+	ret = devm_regulator_bulk_get(&client->dev,
+				      ARRAY_SIZE(chip->regulators),
+				      chip->regulators);
+	if (ret < 0) {
 		if (ret != -EPROBE_DEFER)
 			dev_err(&client->dev,
 				"Failed to request regulator: %d\n", ret);
 		goto error;
 	}
 
-	ret = regulator_enable(chip->vcc_regulator);
+	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
+				    chip->regulators);
 	if (ret) {
 		dev_err(&client->dev,
 			"Failed to enable regulator: %d\n", ret);
@@ -382,7 +388,8 @@ static int aw2013_probe(struct i2c_client *client)
 	if (ret < 0)
 		goto error_reg;
 
-	ret = regulator_disable(chip->vcc_regulator);
+	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
+				     chip->regulators);
 	if (ret) {
 		dev_err(&client->dev,
 			"Failed to disable regulator: %d\n", ret);
@@ -394,7 +401,8 @@ static int aw2013_probe(struct i2c_client *client)
 	return 0;
 
 error_reg:
-	regulator_disable(chip->vcc_regulator);
+	regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
+			       chip->regulators);
 
 error:
 	mutex_destroy(&chip->mutex);
-- 
2.30.2

Re: [PATCH 1/2] dt-bindings: leds: aw2013: Document vdd-supply

[Krzysztof Kozlowski]

On 20/03/2023 15:22, Lin, Meng-Bo wrote:
> Document vdd-supply, a regulator providing power to the "VDD" pin.
> 

No. This device does not have VDD pin. I checked in datasheet.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: leds: aw2013: Document vdd-supply

[Lee Jones]

On Mon, 20 Mar 2023, Krzysztof Kozlowski wrote:

> On 20/03/2023 15:22, Lin, Meng-Bo wrote:
> > Document vdd-supply, a regulator providing power to the "VDD" pin.
> >
>
> No. This device does not have VDD pin. I checked in datasheet.

I'm confused.  This patch set much do something?

I guess a better commit message would help.  Let me request that.

--
Lee Jones [李琼斯]

Re: [PATCH 2/2] leds: aw2013: Add vdd regulator

[Lee Jones]

On Mon, 20 Mar 2023, Lin, Meng-Bo wrote:

> Implement support for a "vdd" that is enabled by the aw2013 driver so that
> the regulator gets enabled when needed.

There seems to be some dispute over the H/W.

Please improve this commit message to cover the following points.

What is currently broken / not working?

How does applying this patch help with that problem?

> Signed-off-by: Lin, Meng-Bo <linmengbo0689@protonmail.com>
> ---
>  drivers/leds/leds-aw2013.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
> index 0b52fc9097c6..91d720edb857 100644
> --- a/drivers/leds/leds-aw2013.c
> +++ b/drivers/leds/leds-aw2013.c
> @@ -62,7 +62,7 @@ struct aw2013_led {
>
>  struct aw2013 {
>  	struct mutex mutex; /* held when writing to registers */
> -	struct regulator *vcc_regulator;
> +	struct regulator_bulk_data regulators[2];
>  	struct i2c_client *client;
>  	struct aw2013_led leds[AW2013_MAX_LEDS];
>  	struct regmap *regmap;
> @@ -106,7 +106,8 @@ static void aw2013_chip_disable(struct aw2013 *chip)
>
>  	regmap_write(chip->regmap, AW2013_GCR, 0);
>
> -	ret = regulator_disable(chip->vcc_regulator);
> +	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
> +				     chip->regulators);
>  	if (ret) {
>  		dev_err(&chip->client->dev,
>  			"Failed to disable regulator: %d\n", ret);
> @@ -123,7 +124,8 @@ static int aw2013_chip_enable(struct aw2013 *chip)
>  	if (chip->enabled)
>  		return 0;
>
> -	ret = regulator_enable(chip->vcc_regulator);
> +	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
> +				    chip->regulators);
>  	if (ret) {
>  		dev_err(&chip->client->dev,
>  			"Failed to enable regulator: %d\n", ret);
> @@ -348,16 +350,20 @@ static int aw2013_probe(struct i2c_client *client)
>  		goto error;
>  	}
>
> -	chip->vcc_regulator = devm_regulator_get(&client->dev, "vcc");
> -	ret = PTR_ERR_OR_ZERO(chip->vcc_regulator);
> -	if (ret) {
> +	chip->regulators[0].supply = "vcc";
> +	chip->regulators[1].supply = "vdd";
> +	ret = devm_regulator_bulk_get(&client->dev,
> +				      ARRAY_SIZE(chip->regulators),
> +				      chip->regulators);
> +	if (ret < 0) {
>  		if (ret != -EPROBE_DEFER)
>  			dev_err(&client->dev,
>  				"Failed to request regulator: %d\n", ret);
>  		goto error;
>  	}
>
> -	ret = regulator_enable(chip->vcc_regulator);
> +	ret = regulator_bulk_enable(ARRAY_SIZE(chip->regulators),
> +				    chip->regulators);
>  	if (ret) {
>  		dev_err(&client->dev,
>  			"Failed to enable regulator: %d\n", ret);
> @@ -382,7 +388,8 @@ static int aw2013_probe(struct i2c_client *client)
>  	if (ret < 0)
>  		goto error_reg;
>
> -	ret = regulator_disable(chip->vcc_regulator);
> +	ret = regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
> +				     chip->regulators);
>  	if (ret) {
>  		dev_err(&client->dev,
>  			"Failed to disable regulator: %d\n", ret);
> @@ -394,7 +401,8 @@ static int aw2013_probe(struct i2c_client *client)
>  	return 0;
>
>  error_reg:
> -	regulator_disable(chip->vcc_regulator);
> +	regulator_bulk_disable(ARRAY_SIZE(chip->regulators),
> +			       chip->regulators);
>
>  error:
>  	mutex_destroy(&chip->mutex);
> --
> 2.30.2
>
>

--
Lee Jones [李琼斯]

Re: [PATCH 1/2] dt-bindings: leds: aw2013: Document vdd-supply

[Krzysztof Kozlowski]

On 30/03/2023 13:46, Lee Jones wrote:
> On Mon, 20 Mar 2023, Krzysztof Kozlowski wrote:
> 
>> On 20/03/2023 15:22, Lin, Meng-Bo wrote:
>>> Document vdd-supply, a regulator providing power to the "VDD" pin.
>>>
>>
>> No. This device does not have VDD pin. I checked in datasheet.
> 
> I'm confused.  This patch set much do something?
> 
> I guess a better commit message would help.  Let me request that.

There was a v2 - with a discussion - so we wait for v3 which clearly
indicates this is supply on controller side. IMHO, this is still wrong
because it is controller's property, not device's, but Rob gave here
green light, so v3 with proper explanation would be fine.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: leds: aw2013: Document vdd-supply

[Lee Jones]

On Thu, 30 Mar 2023, Krzysztof Kozlowski wrote:

> On 30/03/2023 13:46, Lee Jones wrote:
> > On Mon, 20 Mar 2023, Krzysztof Kozlowski wrote:
> >
> >> On 20/03/2023 15:22, Lin, Meng-Bo wrote:
> >>> Document vdd-supply, a regulator providing power to the "VDD" pin.
> >>>
> >>
> >> No. This device does not have VDD pin. I checked in datasheet.
> >
> > I'm confused.  This patch set much do something?
> >
> > I guess a better commit message would help.  Let me request that.
>
> There was a v2 - with a discussion - so we wait for v3 which clearly
> indicates this is supply on controller side. IMHO, this is still wrong
> because it is controller's property, not device's, but Rob gave here
> green light, so v3 with proper explanation would be fine.

Understood, thanks.

--
Lee Jones [李琼斯]