[PATCH 0/3] Add support for attaching a regulator to w1: ds2482

[PATCH 0/3] Add support for attaching a regulator to w1: ds2482

[Kryštof Černý via B4 Relay]

This series adds a support for attaching a regulator to the ds2482 I2C to 1-Wire converter.
The device I work on has a separate regulator for this chip and using always on regulator
is not working well, so I tried to make a proper solution. This is my first kernel code
modification, so any feedback will be highly appreciated. Patch was tested on the real hw.

Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
---
Kryštof Černý (3):
      w1: ds2482: Add regulator support
      w1: ds2482: Fix datasheet URL
      dt-bindings: w1: ds2482: Add vcc-supply property

 .../devicetree/bindings/w1/maxim,ds2482.yaml       |  3 +++
 drivers/w1/masters/ds2482.c                        | 23 +++++++++++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)
---
base-commit: 6d59cab07b8d74d0f0422b750038123334f6ecc2
change-id: 20241111-ds2482-add-reg-fe13200ad7d6

Best regards,
-- 
Kryštof Černý <cleverline1mc@gmail.com>

[PATCH 1/3] w1: ds2482: Add regulator support

[Kryštof Černý via B4 Relay]

From: Kryštof Černý <cleverline1mc@gmail.com>

Adds a support for attaching a supply regulator.

Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
---
 drivers/w1/masters/ds2482.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/w1/masters/ds2482.c b/drivers/w1/masters/ds2482.c
index a2ecbb863c57f38bffc8e3cd463db1940e603179..3fb35e92fc1587dc4e609c0061fa5057e0027a80 100644
--- a/drivers/w1/masters/ds2482.c
+++ b/drivers/w1/masters/ds2482.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/i2c.h>
 #include <linux/delay.h>
+#include <linux/regulator/consumer.h>
 
 #include <linux/w1.h>
 
@@ -117,6 +118,9 @@ struct ds2482_data {
 	u8			channel;
 	u8			read_prt;	/* see DS2482_PTR_CODE_xxx */
 	u8			reg_config;
+
+	/* reference to the optional regulator */
+	struct regulator *vcc_reg;
 };
 
 
@@ -445,6 +449,7 @@ static int ds2482_probe(struct i2c_client *client)
 	int err = -ENODEV;
 	int temp1;
 	int idx;
+	int ret;
 
 	if (!i2c_check_functionality(client->adapter,
 				     I2C_FUNC_SMBUS_WRITE_BYTE_DATA |
@@ -457,6 +462,18 @@ static int ds2482_probe(struct i2c_client *client)
 		goto exit;
 	}
 
+	/* Get the vcc regulator */
+	data->vcc_reg = devm_regulator_get(&client->dev, "vcc");
+	if (IS_ERR(data->vcc_reg))
+		return PTR_ERR(data->vcc_reg);
+
+	/* Enable the vcc regulator */
+	ret = regulator_enable(data->vcc_reg);
+	if (ret) {
+		dev_err(&client->dev, "Fail to enable regulator\n");
+		return ret;
+	}
+
 	data->client = client;
 	i2c_set_clientdata(client, data);
 
@@ -517,6 +534,7 @@ static int ds2482_probe(struct i2c_client *client)
 			w1_remove_master_device(&data->w1_ch[idx].w1_bm);
 	}
 exit_free:
+	regulator_disable(data->vcc_reg);
 	kfree(data);
 exit:
 	return err;
@@ -533,6 +551,9 @@ static void ds2482_remove(struct i2c_client *client)
 			w1_remove_master_device(&data->w1_ch[idx].w1_bm);
 	}
 
+	/* Disable the vcc regulator */
+	regulator_disable(data->vcc_reg);
+
 	/* Free the memory */
 	kfree(data);
 }

-- 
2.39.5

[PATCH 2/3] w1: ds2482: Fix datasheet URL

[Kryštof Černý via B4 Relay]

From: Kryštof Černý <cleverline1mc@gmail.com>

Current link does redirect to wrong place.

Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
---
 drivers/w1/masters/ds2482.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/w1/masters/ds2482.c b/drivers/w1/masters/ds2482.c
index 3fb35e92fc1587dc4e609c0061fa5057e0027a80..ba86b5953b8d3ed1fe40d40a1a2b018c57fb91b8 100644
--- a/drivers/w1/masters/ds2482.c
+++ b/drivers/w1/masters/ds2482.c
@@ -7,7 +7,7 @@
  * It is a I2C to 1-wire bridge.
  * There are two variations: -100 and -800, which have 1 or 8 1-wire ports.
  * The complete datasheet can be obtained from MAXIM's website at:
- *   http://www.maxim-ic.com/quick_view2.cfm/qv_pk/4382
+ *   https://www.analog.com/en/products/ds2482-100.html
  */
 
 #include <linux/module.h>

-- 
2.39.5

[PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Kryštof Černý via B4 Relay]

From: Kryštof Černý <cleverline1mc@gmail.com>

Adds the newly added vcc-supply property to bindings.

Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
---
 Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
--- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
+++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
@@ -25,6 +25,9 @@ properties:
   reg:
     maxItems: 1
 
+  vcc-supply:
+    description: phandle of the regulator that provides the supply voltage.
+
 required:
   - compatible
   - reg

-- 
2.39.5

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Conor Dooley]

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

On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
> From: Kryštof Černý <cleverline1mc@gmail.com>
> 
> Adds the newly added vcc-supply property to bindings.

This commit message is a circular argument. You're adding it to the
binding, which of course means it is newly added.

> 
> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
> ---
>  Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
> @@ -25,6 +25,9 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  vcc-supply:
> +    description: phandle of the regulator that provides the supply voltage.

"vcc-supply: true" should suffice.

> +
>  required:
>    - compatible
>    - reg
> 
> -- 
> 2.39.5
> 
> 

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

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Kryštof Černý]

Hello,

> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>
>> Adds the newly added vcc-supply property to bindings.
> 
> This commit message is a circular argument. You're adding it to the
> binding, which of course means it is newly added.

You are right, I will replace with "Adds the vcc-supply property to 
bindings." in the next version.

>>
>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>> ---
>>   Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>> @@ -25,6 +25,9 @@ properties:
>>     reg:
>>       maxItems: 1
>>   
>> +  vcc-supply:
>> +    description: phandle of the regulator that provides the supply voltage.
> 
> "vcc-supply: true" should suffice.
> 

Right, I suppose you mean to remove the description and just have 
"vcc-supply: true".
If so, could you explain why no description? Is it some standard property
or because the name is self-explanatory? If you mean to keep both, 
please reply.

>> +
>>   required:
>>     - compatible
>>     - reg
>>
>> -- 
>> 2.39.5
>>
>>

Thank you for review,
Kryštof Černý

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Krzysztof Kozlowski]

On 20/11/2024 09:34, Kryštof Černý wrote:
> Hello,
> 
>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>
>>> Adds the newly added vcc-supply property to bindings.
>>
>> This commit message is a circular argument. You're adding it to the
>> binding, which of course means it is newly added.
> 
> You are right, I will replace with "Adds the vcc-supply property to 
> bindings." in the next version.

No, please say why, e.g. because it was missing and device has it
according to datasheet.

> 
>>>
>>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>>> ---
>>>   Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>> @@ -25,6 +25,9 @@ properties:
>>>     reg:
>>>       maxItems: 1
>>>   
>>> +  vcc-supply:
>>> +    description: phandle of the regulator that provides the supply voltage.
>>
>> "vcc-supply: true" should suffice.
>>
> 
> Right, I suppose you mean to remove the description and just have 
> "vcc-supply: true".
> If so, could you explain why no description? Is it some standard property
> or because the name is self-explanatory? If you mean to keep both, 
> please reply.

It's almost self-explanatory and your description does not give any more
information. git grep for existing code - you will find also examples
which give actual information, e.g. detailed PIN name and accepted voltages.



Best regards,
Krzysztof

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Kryštof Černý]

Hello,

> On 20/11/2024 09:34, Kryštof Černý wrote:
>> Hello,
>>
>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>
>>>> Adds the newly added vcc-supply property to bindings.
>>>
>>> This commit message is a circular argument. You're adding it to the
>>> binding, which of course means it is newly added.
>>
>> You are right, I will replace with "Adds the vcc-supply property to
>> bindings." in the next version.
> 
> No, please say why, e.g. because it was missing and device has it
> according to datasheet.

Right, what about:

Adds the optional vcc-supply property to bindings, informs if the device 
needs a regulator to be turned on for its operation.


>>
>>>>
>>>> Signed-off-by: Kryštof Černý <cleverline1mc@gmail.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/w1/maxim,ds2482.yaml | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> index 422becc6e1fa8d58665c5586ebdc611cd0b2c760..a6b9e0658ec858cb24b21cf64443a061bb43e4ef 100644
>>>> --- a/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> +++ b/Documentation/devicetree/bindings/w1/maxim,ds2482.yaml
>>>> @@ -25,6 +25,9 @@ properties:
>>>>      reg:
>>>>        maxItems: 1
>>>>    
>>>> +  vcc-supply:
>>>> +    description: phandle of the regulator that provides the supply voltage.
>>>
>>> "vcc-supply: true" should suffice.
>>>
>>
>> Right, I suppose you mean to remove the description and just have
>> "vcc-supply: true".
>> If so, could you explain why no description? Is it some standard property
>> or because the name is self-explanatory? If you mean to keep both,
>> please reply.
> 
> It's almost self-explanatory and your description does not give any more
> information. git grep for existing code - you will find also examples
> which give actual information, e.g. detailed PIN name and accepted voltages.
> 

Thanks for the tip, best description I have come up with:

   vcc-supply:
     description:
       Regulator that drives the VCC pin (2.9-5.5 V). Depending on the 
hardware
       design, it may set the vcc voltage of 3 wire 1-Wire bus variant.

Would be glad for feedback if you think it's a good idea to use it, or
just keep "vcc-supply: true".

> 
> Best regards,
> Krzysztof

Thank you,
Kryštof Černý

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Krzysztof Kozlowski]

On 20/11/2024 23:53, Kryštof Černý wrote:
> Hello,
> 
>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>> Hello,
>>>
>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>
>>>>> Adds the newly added vcc-supply property to bindings.
>>>>
>>>> This commit message is a circular argument. You're adding it to the
>>>> binding, which of course means it is newly added.
>>>
>>> You are right, I will replace with "Adds the vcc-supply property to
>>> bindings." in the next version.
>>
>> No, please say why, e.g. because it was missing and device has it
>> according to datasheet.
> 
> Right, what about:
> 
> Adds the optional vcc-supply property to bindings, informs if the device 
> needs a regulator to be turned on for its operation

It does not inform at all. All devices needs power, don't they? And what
operation?



Best regards,
Krzysztof

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Kryštof Černý]

Dne 21.11.2024 v 8:43 Krzysztof Kozlowski napsal(a):
> On 20/11/2024 23:53, Kryštof Černý wrote:
>> Hello,
>>
>>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>>> Hello,
>>>>
>>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>>
>>>>>> Adds the newly added vcc-supply property to bindings.
>>>>>
>>>>> This commit message is a circular argument. You're adding it to the
>>>>> binding, which of course means it is newly added.
>>>>
>>>> You are right, I will replace with "Adds the vcc-supply property to
>>>> bindings." in the next version.
>>>
>>> No, please say why, e.g. because it was missing and device has it
>>> according to datasheet.
>>
>> Right, what about:
>>
>> Adds the optional vcc-supply property to bindings, informs if the device
>> needs a regulator to be turned on for its operation
> 
> It does not inform at all. All devices needs power, don't they? And what
> operation?

This is probably the best I can do:

	ds2482 has a VCC pin, accepting 2.9-5.5 V. Allow optionally specifying 
a voltage regulator to power the chip.

If you don't find this satisfactory, please provide a suggestion.

> Best regards,
> Krzysztof

Thank you for your patience,
Kryštof Černý

Re: [PATCH 3/3] dt-bindings: w1: ds2482: Add vcc-supply property

[Krzysztof Kozlowski]

On 21/11/2024 16:56, Kryštof Černý wrote:
> 
> 
> Dne 21.11.2024 v 8:43 Krzysztof Kozlowski napsal(a):
>> On 20/11/2024 23:53, Kryštof Černý wrote:
>>> Hello,
>>>
>>>> On 20/11/2024 09:34, Kryštof Černý wrote:
>>>>> Hello,
>>>>>
>>>>>> On Fri, Nov 15, 2024 at 03:58:06PM +0100, Kryštof Černý via B4 Relay wrote:
>>>>>>> From: Kryštof Černý <cleverline1mc@gmail.com>
>>>>>>>
>>>>>>> Adds the newly added vcc-supply property to bindings.
>>>>>>
>>>>>> This commit message is a circular argument. You're adding it to the
>>>>>> binding, which of course means it is newly added.
>>>>>
>>>>> You are right, I will replace with "Adds the vcc-supply property to
>>>>> bindings." in the next version.
>>>>
>>>> No, please say why, e.g. because it was missing and device has it
>>>> according to datasheet.
>>>
>>> Right, what about:
>>>
>>> Adds the optional vcc-supply property to bindings, informs if the device
>>> needs a regulator to be turned on for its operation
>>
>> It does not inform at all. All devices needs power, don't they? And what
>> operation?
> 
> This is probably the best I can do:
> 
> 	ds2482 has a VCC pin, accepting 2.9-5.5 V. Allow optionally specifying 
> a voltage regulator to power the chip.

Keep only first sentence. Second is obvious, no need to repeat schema in
free form text (and schema defines it is optional).

Best regards,
Krzysztof