devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] hwmon: pmbus: max20730: adjust the vout base on
@ 2020-10-04  3:14 Chu Lin
  2020-10-04  3:14 ` [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730 Chu Lin
  2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
  0 siblings, 2 replies; 10+ messages in thread
From: Chu Lin @ 2020-10-04  3:14 UTC (permalink / raw)
  To: linchuyuan, jdelvare, linux, robh+dt, linux-hwmon, devicetree,
	linux-kernel

The patchset includes:
Patch #1 - Implmentation of adjusting vout base on voltage divider
Patch #2 - device tree binding documentation

ChangeLog v1 -> v2
  hwmon: pmbus: max20730:
  - Don't do anything to the ret if an error is returned from pmbus_read_word
  - avoid overflow when doing multiplication

ChangeLog v2 -> v3
  dt-bindings: hwmon: max20730:
  - Provide the binding documentation in yaml format
  hwmon: pmbus: max20730:
  - No change

ChangeLog v3 -> v4
  dt-bindings: hwmon: max20730:
  - Fix highefficiency to high efficiency in description
  - Fix presents to present in vout-voltage-divider
  - Add additionalProperties: false
  hwmon: pmbus: max20730:
  - No change

Chu Lin (2):
  dt-bindings: hwmon: max20730: adding device tree doc for max20730
  hwmon: pmbus: max20730: adjust the vout reading given voltage divider

 .../bindings/hwmon/maxim,max20730.yaml        | 65 +++++++++++++++++++
 drivers/hwmon/pmbus/max20730.c                | 18 +++++
 2 files changed, 83 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml

-- 
2.28.0.806.g8561365e88-goog


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

* [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730
  2020-10-04  3:14 [PATCH v4 0/2] hwmon: pmbus: max20730: adjust the vout base on Chu Lin
@ 2020-10-04  3:14 ` Chu Lin
  2020-10-06 21:17   ` Rob Herring
  2020-10-06 21:55   ` Guenter Roeck
  2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
  1 sibling, 2 replies; 10+ messages in thread
From: Chu Lin @ 2020-10-04  3:14 UTC (permalink / raw)
  To: linchuyuan, jdelvare, linux, robh+dt, linux-hwmon, devicetree,
	linux-kernel

max20730 Integrated, Step-Down Switching Regulator with PMBus

Signed-off-by: Chu Lin <linchuyuan@google.com>
---
ChangeLog v1 -> v2
  hwmon: pmbus: max20730:
  - Don't do anything to the ret if an error is returned from pmbus_read_word
  - avoid overflow when doing multiplication

ChangeLog v2 -> v3
  dt-bindings: hwmon: max20730:
  - Provide the binding documentation in yaml format
  hwmon: pmbus: max20730:
  - No change

ChangeLog v3 -> v4
  dt-bindings: hwmon: max20730:
  - Fix highefficiency to high efficiency in description
  - Fix presents to present in vout-voltage-divider
  - Add additionalProperties: false
  hwmon: pmbus: max20730:
  - No change

 .../bindings/hwmon/maxim,max20730.yaml        | 65 +++++++++++++++++++
 1 file changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml

diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
new file mode 100644
index 000000000000..93e86e3b4602
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/maxim,max20730.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim max20730
+
+maintainers:
+  - Jean Delvare <jdelvare@suse.com>
+  - Guenter Roeck <linux@roeck-us.net>
+
+description: |
+  The MAX20730 is a fully integrated, highly efficient switching regulator
+  with PMBus for applications operating from 4.5V to 16V and requiring
+  up to 25A (max) load. This single-chip regulator provides extremely
+  compact, high efficiency power-delivery solutions with high-precision
+  output voltages and excellent transient response.
+
+  Datasheets:
+    https://datasheets.maximintegrated.com/en/ds/MAX20730.pdf
+    https://datasheets.maximintegrated.com/en/ds/MAX20734.pdf
+    https://datasheets.maximintegrated.com/en/ds/MAX20743.pdf
+
+properties:
+  compatible:
+    enum:
+      - maxim,max20730
+      - maxim,max20734
+      - maxim,max20743
+
+  reg:
+    maxItems: 1
+
+  vout-voltage-divider:
+    description: |
+      If voltage divider present at vout, the voltage at voltage sensor pin
+      will be scaled. The properties will convert the raw reading to a more
+      meaningful number if voltage divider present. It has two numbers,
+      the first number is the output resistor, the second number is the total
+      resistance. Therefore, the adjusted vout is equal to
+      Vout = Vout * output_resistance / total resistance.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    maxItems: 2
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max20730@10 {
+        compatible = "maxim,max20730";
+        reg = <0x10>;
+        vout-voltage-divider = <1000 2000>; // vout would be scaled to 0.5
+      };
+    };
-- 
2.28.0.806.g8561365e88-goog


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

* [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-04  3:14 [PATCH v4 0/2] hwmon: pmbus: max20730: adjust the vout base on Chu Lin
  2020-10-04  3:14 ` [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730 Chu Lin
@ 2020-10-04  3:14 ` Chu Lin
  2020-10-04 15:43   ` Guenter Roeck
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Chu Lin @ 2020-10-04  3:14 UTC (permalink / raw)
  To: linchuyuan, jdelvare, linux, robh+dt, linux-hwmon, devicetree,
	linux-kernel

Problem:
We use voltage dividers so that the voltage presented at the voltage
sense pins is confusing. We might need to convert these readings to more
meaningful readings given the voltage divider.

Solution:
Read the voltage divider resistance from dts and convert the voltage
reading to a more meaningful reading.

Testing:
max20730 with voltage divider

Signed-off-by: Chu Lin <linchuyuan@google.com>
---
ChangeLog v1 -> v2
  hwmon: pmbus: max20730:
  - Don't do anything to the ret if an error is returned from pmbus_read_word
  - avoid overflow when doing multiplication

ChangeLog v2 -> v3
  dt-bindings: hwmon: max20730:
  - Provide the binding documentation in yaml format
  hwmon: pmbus: max20730:
  - No change

ChangeLog v3 -> v4
  dt-bindings: hwmon: max20730:
  - Fix highefficiency to high efficiency in description
  - Fix presents to present in vout-voltage-divider
  - Add additionalProperties: false
  hwmon: pmbus: max20730:
  - No change

 drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
index a151a2b588a5..fbf2f1e6c969 100644
--- a/drivers/hwmon/pmbus/max20730.c
+++ b/drivers/hwmon/pmbus/max20730.c
@@ -31,6 +31,7 @@ struct max20730_data {
 	struct pmbus_driver_info info;
 	struct mutex lock;	/* Used to protect against parallel writes */
 	u16 mfr_devset1;
+	u32 vout_voltage_divider[2];
 };
 
 #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
@@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
 		max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
 		ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
 		break;
+	case PMBUS_READ_VOUT:
+		ret = pmbus_read_word_data(client, page, phase, reg);
+		if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
+			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
+							 data->vout_voltage_divider[0]);
+			ret = clamp_val(temp, 0, 0xffff);
+		}
+		break;
 	default:
 		ret = -ENODATA;
 		break;
@@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
 	data->id = chip_id;
 	mutex_init(&data->lock);
 	memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
+	if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
+				       data->vout_voltage_divider,
+				       ARRAY_SIZE(data->vout_voltage_divider)) != 0)
+		memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
+	if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
+		dev_err(dev,
+			"The total resistance of voltage divider is less than output resistance\n");
+		return -ENODEV;
+	}
 
 	ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
 	if (ret < 0)
-- 
2.28.0.806.g8561365e88-goog


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

* Re: [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
@ 2020-10-04 15:43   ` Guenter Roeck
  2020-10-05  3:07     ` Chu Lin
  2020-10-05 15:03   ` Guenter Roeck
  2020-10-06 21:55   ` Guenter Roeck
  2 siblings, 1 reply; 10+ messages in thread
From: Guenter Roeck @ 2020-10-04 15:43 UTC (permalink / raw)
  To: Chu Lin; +Cc: jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On Sun, Oct 04, 2020 at 03:14:45AM +0000, Chu Lin wrote:
> Problem:
> We use voltage dividers so that the voltage presented at the voltage
> sense pins is confusing. We might need to convert these readings to more
> meaningful readings given the voltage divider.
> 
> Solution:
> Read the voltage divider resistance from dts and convert the voltage
> reading to a more meaningful reading.
> 
> Testing:
> max20730 with voltage divider
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> ---
> ChangeLog v1 -> v2
>   hwmon: pmbus: max20730:
>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>   - avoid overflow when doing multiplication
> 
> ChangeLog v2 -> v3
>   dt-bindings: hwmon: max20730:
>   - Provide the binding documentation in yaml format
>   hwmon: pmbus: max20730:
>   - No change
> 
> ChangeLog v3 -> v4
>   dt-bindings: hwmon: max20730:
>   - Fix highefficiency to high efficiency in description
>   - Fix presents to present in vout-voltage-divider
>   - Add additionalProperties: false
>   hwmon: pmbus: max20730:
>   - No change

You claim that there have been no changes since v2 of this patch,
yet you dropped my Reviewed-by: tag. Any reason ?

Guenter

> 
>  drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
> index a151a2b588a5..fbf2f1e6c969 100644
> --- a/drivers/hwmon/pmbus/max20730.c
> +++ b/drivers/hwmon/pmbus/max20730.c
> @@ -31,6 +31,7 @@ struct max20730_data {
>  	struct pmbus_driver_info info;
>  	struct mutex lock;	/* Used to protect against parallel writes */
>  	u16 mfr_devset1;
> +	u32 vout_voltage_divider[2];
>  };
>  
>  #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
> @@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
>  		max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
>  		ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
>  		break;
> +	case PMBUS_READ_VOUT:
> +		ret = pmbus_read_word_data(client, page, phase, reg);
> +		if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
> +			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
> +							 data->vout_voltage_divider[0]);
> +			ret = clamp_val(temp, 0, 0xffff);
> +		}
> +		break;
>  	default:
>  		ret = -ENODATA;
>  		break;
> @@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
>  	data->id = chip_id;
>  	mutex_init(&data->lock);
>  	memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
> +	if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
> +				       data->vout_voltage_divider,
> +				       ARRAY_SIZE(data->vout_voltage_divider)) != 0)
> +		memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
> +	if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
> +		dev_err(dev,
> +			"The total resistance of voltage divider is less than output resistance\n");
> +		return -ENODEV;
> +	}
>  
>  	ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
>  	if (ret < 0)
> -- 
> 2.28.0.806.g8561365e88-goog
> 

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

* Re: [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-04 15:43   ` Guenter Roeck
@ 2020-10-05  3:07     ` Chu Lin
  2020-10-05  5:24       ` Guenter Roeck
  0 siblings, 1 reply; 10+ messages in thread
From: Chu Lin @ 2020-10-05  3:07 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On Sun, Oct 4, 2020 at 8:43 AM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Sun, Oct 04, 2020 at 03:14:45AM +0000, Chu Lin wrote:
> > Problem:
> > We use voltage dividers so that the voltage presented at the voltage
> > sense pins is confusing. We might need to convert these readings to more
> > meaningful readings given the voltage divider.
> >
> > Solution:
> > Read the voltage divider resistance from dts and convert the voltage
> > reading to a more meaningful reading.
> >
> > Testing:
> > max20730 with voltage divider
> >
> > Signed-off-by: Chu Lin <linchuyuan@google.com>
> > ---
> > ChangeLog v1 -> v2
> >   hwmon: pmbus: max20730:
> >   - Don't do anything to the ret if an error is returned from pmbus_read_word
> >   - avoid overflow when doing multiplication
> >
> > ChangeLog v2 -> v3
> >   dt-bindings: hwmon: max20730:
> >   - Provide the binding documentation in yaml format
> >   hwmon: pmbus: max20730:
> >   - No change
> >
> > ChangeLog v3 -> v4
> >   dt-bindings: hwmon: max20730:
> >   - Fix highefficiency to high efficiency in description
> >   - Fix presents to present in vout-voltage-divider
> >   - Add additionalProperties: false
> >   hwmon: pmbus: max20730:
> >   - No change
>
> You claim that there have been no changes since v2 of this patch,
> yet you dropped my Reviewed-by: tag. Any reason ?
>
> Guenter
Sorry for the confusion. I thought I can't tag the patch with the Review-by tag.
Just to make sure I do correctly next time, once you tagged a certain
patch in the batch
If there is no change from version to version for this patch, I should
carry the tags when
submitting new revisions. Also, please let me know what is the best
way to fix this revision?
Should I submit a new V5 with the tag attached?

Sincerely,
Chu

>
> >
> >  drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
> > index a151a2b588a5..fbf2f1e6c969 100644
> > --- a/drivers/hwmon/pmbus/max20730.c
> > +++ b/drivers/hwmon/pmbus/max20730.c
> > @@ -31,6 +31,7 @@ struct max20730_data {
> >       struct pmbus_driver_info info;
> >       struct mutex lock;      /* Used to protect against parallel writes */
> >       u16 mfr_devset1;
> > +     u32 vout_voltage_divider[2];
> >  };
> >
> >  #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
> > @@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
> >               max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
> >               ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
> >               break;
> > +     case PMBUS_READ_VOUT:
> > +             ret = pmbus_read_word_data(client, page, phase, reg);
> > +             if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
> > +                     u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
> > +                                                      data->vout_voltage_divider[0]);
> > +                     ret = clamp_val(temp, 0, 0xffff);
> > +             }
> > +             break;
> >       default:
> >               ret = -ENODATA;
> >               break;
> > @@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
> >       data->id = chip_id;
> >       mutex_init(&data->lock);
> >       memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
> > +     if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
> > +                                    data->vout_voltage_divider,
> > +                                    ARRAY_SIZE(data->vout_voltage_divider)) != 0)
> > +             memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
> > +     if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
> > +             dev_err(dev,
> > +                     "The total resistance of voltage divider is less than output resistance\n");
> > +             return -ENODEV;
> > +     }
> >
> >       ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
> >       if (ret < 0)
> > --
> > 2.28.0.806.g8561365e88-goog
> >

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

* Re: [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-05  3:07     ` Chu Lin
@ 2020-10-05  5:24       ` Guenter Roeck
  0 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-10-05  5:24 UTC (permalink / raw)
  To: Chu Lin; +Cc: jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On 10/4/20 8:07 PM, Chu Lin wrote:
> On Sun, Oct 4, 2020 at 8:43 AM Guenter Roeck <linux@roeck-us.net> wrote:
>>
>> On Sun, Oct 04, 2020 at 03:14:45AM +0000, Chu Lin wrote:
>>> Problem:
>>> We use voltage dividers so that the voltage presented at the voltage
>>> sense pins is confusing. We might need to convert these readings to more
>>> meaningful readings given the voltage divider.
>>>
>>> Solution:
>>> Read the voltage divider resistance from dts and convert the voltage
>>> reading to a more meaningful reading.
>>>
>>> Testing:
>>> max20730 with voltage divider
>>>
>>> Signed-off-by: Chu Lin <linchuyuan@google.com>
>>> ---
>>> ChangeLog v1 -> v2
>>>   hwmon: pmbus: max20730:
>>>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>>>   - avoid overflow when doing multiplication
>>>
>>> ChangeLog v2 -> v3
>>>   dt-bindings: hwmon: max20730:
>>>   - Provide the binding documentation in yaml format
>>>   hwmon: pmbus: max20730:
>>>   - No change
>>>
>>> ChangeLog v3 -> v4
>>>   dt-bindings: hwmon: max20730:
>>>   - Fix highefficiency to high efficiency in description
>>>   - Fix presents to present in vout-voltage-divider
>>>   - Add additionalProperties: false
>>>   hwmon: pmbus: max20730:
>>>   - No change
>>
>> You claim that there have been no changes since v2 of this patch,
>> yet you dropped my Reviewed-by: tag. Any reason ?
>>
>> Guenter
> Sorry for the confusion. I thought I can't tag the patch with the Review-by tag.
> Just to make sure I do correctly next time, once you tagged a certain
> patch in the batch
> If there is no change from version to version for this patch, I should
> carry the tags when
> submitting new revisions. Also, please let me know what is the best
> way to fix this revision?
> Should I submit a new V5 with the tag attached?
> 

Actually, if you did not make changes to a patch, you are _expected_
to carry tags forward. In many cases, submitters will even send a new
version of a patch with all new tags included (and no other changes).
This helps reviewers to keep track on what has been reviewed, acked,
or tested in a series. Dropping a tag means that you changed
a patch so substantially that the tag no longer applies (for example,
fixing a spelling error or some formatting does not normally warrant
dropping a tag). If you do drop a tag, you should explain the reason
in the change log.

That is, of course, a honor system that must not be abused
(unfortunately I have seen that abuse happen a couple of times
recently, but that is another story).

No need to re-send in this case - I'll just send another tag
to remind myself that I reviewed this patch.

Guenter

> Sincerely,
> Chu
> 
>>
>>>
>>>  drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
>>> index a151a2b588a5..fbf2f1e6c969 100644
>>> --- a/drivers/hwmon/pmbus/max20730.c
>>> +++ b/drivers/hwmon/pmbus/max20730.c
>>> @@ -31,6 +31,7 @@ struct max20730_data {
>>>       struct pmbus_driver_info info;
>>>       struct mutex lock;      /* Used to protect against parallel writes */
>>>       u16 mfr_devset1;
>>> +     u32 vout_voltage_divider[2];
>>>  };
>>>
>>>  #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
>>> @@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
>>>               max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
>>>               ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
>>>               break;
>>> +     case PMBUS_READ_VOUT:
>>> +             ret = pmbus_read_word_data(client, page, phase, reg);
>>> +             if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
>>> +                     u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
>>> +                                                      data->vout_voltage_divider[0]);
>>> +                     ret = clamp_val(temp, 0, 0xffff);
>>> +             }
>>> +             break;
>>>       default:
>>>               ret = -ENODATA;
>>>               break;
>>> @@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
>>>       data->id = chip_id;
>>>       mutex_init(&data->lock);
>>>       memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
>>> +     if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
>>> +                                    data->vout_voltage_divider,
>>> +                                    ARRAY_SIZE(data->vout_voltage_divider)) != 0)
>>> +             memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
>>> +     if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
>>> +             dev_err(dev,
>>> +                     "The total resistance of voltage divider is less than output resistance\n");
>>> +             return -ENODEV;
>>> +     }
>>>
>>>       ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
>>>       if (ret < 0)
>>> --
>>> 2.28.0.806.g8561365e88-goog
>>>


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

* Re: [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
  2020-10-04 15:43   ` Guenter Roeck
@ 2020-10-05 15:03   ` Guenter Roeck
  2020-10-06 21:55   ` Guenter Roeck
  2 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-10-05 15:03 UTC (permalink / raw)
  To: Chu Lin, jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On 10/3/20 8:14 PM, Chu Lin wrote:
> Problem:
> We use voltage dividers so that the voltage presented at the voltage
> sense pins is confusing. We might need to convert these readings to more
> meaningful readings given the voltage divider.
> 
> Solution:
> Read the voltage divider resistance from dts and convert the voltage
> reading to a more meaningful reading.
> 
> Testing:
> max20730 with voltage divider
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>

For my reference (carried from v2):

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Still waiting for DT approval.

> ---
> ChangeLog v1 -> v2
>   hwmon: pmbus: max20730:
>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>   - avoid overflow when doing multiplication
> 
> ChangeLog v2 -> v3
>   dt-bindings: hwmon: max20730:
>   - Provide the binding documentation in yaml format
>   hwmon: pmbus: max20730:
>   - No change
> 
> ChangeLog v3 -> v4
>   dt-bindings: hwmon: max20730:
>   - Fix highefficiency to high efficiency in description
>   - Fix presents to present in vout-voltage-divider
>   - Add additionalProperties: false
>   hwmon: pmbus: max20730:
>   - No change
> 
>  drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
> index a151a2b588a5..fbf2f1e6c969 100644
> --- a/drivers/hwmon/pmbus/max20730.c
> +++ b/drivers/hwmon/pmbus/max20730.c
> @@ -31,6 +31,7 @@ struct max20730_data {
>  	struct pmbus_driver_info info;
>  	struct mutex lock;	/* Used to protect against parallel writes */
>  	u16 mfr_devset1;
> +	u32 vout_voltage_divider[2];
>  };
>  
>  #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
> @@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
>  		max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
>  		ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
>  		break;
> +	case PMBUS_READ_VOUT:
> +		ret = pmbus_read_word_data(client, page, phase, reg);
> +		if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
> +			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
> +							 data->vout_voltage_divider[0]);
> +			ret = clamp_val(temp, 0, 0xffff);
> +		}
> +		break;
>  	default:
>  		ret = -ENODATA;
>  		break;
> @@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
>  	data->id = chip_id;
>  	mutex_init(&data->lock);
>  	memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
> +	if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
> +				       data->vout_voltage_divider,
> +				       ARRAY_SIZE(data->vout_voltage_divider)) != 0)
> +		memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
> +	if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
> +		dev_err(dev,
> +			"The total resistance of voltage divider is less than output resistance\n");
> +		return -ENODEV;
> +	}
>  
>  	ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
>  	if (ret < 0)
> 


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

* Re: [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730
  2020-10-04  3:14 ` [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730 Chu Lin
@ 2020-10-06 21:17   ` Rob Herring
  2020-10-06 21:55   ` Guenter Roeck
  1 sibling, 0 replies; 10+ messages in thread
From: Rob Herring @ 2020-10-06 21:17 UTC (permalink / raw)
  To: Chu Lin; +Cc: linux-hwmon, linux, robh+dt, linux-kernel, jdelvare, devicetree

On Sun, 04 Oct 2020 03:14:44 +0000, Chu Lin wrote:
> max20730 Integrated, Step-Down Switching Regulator with PMBus
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> ---
> ChangeLog v1 -> v2
>   hwmon: pmbus: max20730:
>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>   - avoid overflow when doing multiplication
> 
> ChangeLog v2 -> v3
>   dt-bindings: hwmon: max20730:
>   - Provide the binding documentation in yaml format
>   hwmon: pmbus: max20730:
>   - No change
> 
> ChangeLog v3 -> v4
>   dt-bindings: hwmon: max20730:
>   - Fix highefficiency to high efficiency in description
>   - Fix presents to present in vout-voltage-divider
>   - Add additionalProperties: false
>   hwmon: pmbus: max20730:
>   - No change
> 
>  .../bindings/hwmon/maxim,max20730.yaml        | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>

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

* Re: [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730
  2020-10-04  3:14 ` [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730 Chu Lin
  2020-10-06 21:17   ` Rob Herring
@ 2020-10-06 21:55   ` Guenter Roeck
  1 sibling, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-10-06 21:55 UTC (permalink / raw)
  To: Chu Lin; +Cc: jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On Sun, Oct 04, 2020 at 03:14:44AM +0000, Chu Lin wrote:
> max20730 Integrated, Step-Down Switching Regulator with PMBus
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> Reviewed-by: Rob Herring <robh@kernel.org>

Applied to hwmon-next.

Thanks,
Guenter

> ---
> ChangeLog v1 -> v2
>   hwmon: pmbus: max20730:
>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>   - avoid overflow when doing multiplication
> 
> ChangeLog v2 -> v3
>   dt-bindings: hwmon: max20730:
>   - Provide the binding documentation in yaml format
>   hwmon: pmbus: max20730:
>   - No change
> 
> ChangeLog v3 -> v4
>   dt-bindings: hwmon: max20730:
>   - Fix highefficiency to high efficiency in description
>   - Fix presents to present in vout-voltage-divider
>   - Add additionalProperties: false
>   hwmon: pmbus: max20730:
>   - No change
> 
>  .../bindings/hwmon/maxim,max20730.yaml        | 65 +++++++++++++++++++
>  1 file changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml b/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
> new file mode 100644
> index 000000000000..93e86e3b4602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/maxim,max20730.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/maxim,max20730.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim max20730
> +
> +maintainers:
> +  - Jean Delvare <jdelvare@suse.com>
> +  - Guenter Roeck <linux@roeck-us.net>
> +
> +description: |
> +  The MAX20730 is a fully integrated, highly efficient switching regulator
> +  with PMBus for applications operating from 4.5V to 16V and requiring
> +  up to 25A (max) load. This single-chip regulator provides extremely
> +  compact, high efficiency power-delivery solutions with high-precision
> +  output voltages and excellent transient response.
> +
> +  Datasheets:
> +    https://datasheets.maximintegrated.com/en/ds/MAX20730.pdf
> +    https://datasheets.maximintegrated.com/en/ds/MAX20734.pdf
> +    https://datasheets.maximintegrated.com/en/ds/MAX20743.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - maxim,max20730
> +      - maxim,max20734
> +      - maxim,max20743
> +
> +  reg:
> +    maxItems: 1
> +
> +  vout-voltage-divider:
> +    description: |
> +      If voltage divider present at vout, the voltage at voltage sensor pin
> +      will be scaled. The properties will convert the raw reading to a more
> +      meaningful number if voltage divider present. It has two numbers,
> +      the first number is the output resistor, the second number is the total
> +      resistance. Therefore, the adjusted vout is equal to
> +      Vout = Vout * output_resistance / total resistance.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 2
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      max20730@10 {
> +        compatible = "maxim,max20730";
> +        reg = <0x10>;
> +        vout-voltage-divider = <1000 2000>; // vout would be scaled to 0.5
> +      };
> +    };

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

* Re: [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider
  2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
  2020-10-04 15:43   ` Guenter Roeck
  2020-10-05 15:03   ` Guenter Roeck
@ 2020-10-06 21:55   ` Guenter Roeck
  2 siblings, 0 replies; 10+ messages in thread
From: Guenter Roeck @ 2020-10-06 21:55 UTC (permalink / raw)
  To: Chu Lin; +Cc: jdelvare, robh+dt, linux-hwmon, devicetree, linux-kernel

On Sun, Oct 04, 2020 at 03:14:45AM +0000, Chu Lin wrote:
> Problem:
> We use voltage dividers so that the voltage presented at the voltage
> sense pins is confusing. We might need to convert these readings to more
> meaningful readings given the voltage divider.
> 
> Solution:
> Read the voltage divider resistance from dts and convert the voltage
> reading to a more meaningful reading.
> 
> Testing:
> max20730 with voltage divider
> 
> Signed-off-by: Chu Lin <linchuyuan@google.com>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Applied to hwmon-next.

> ---
> ChangeLog v1 -> v2
>   hwmon: pmbus: max20730:
>   - Don't do anything to the ret if an error is returned from pmbus_read_word
>   - avoid overflow when doing multiplication
> 
> ChangeLog v2 -> v3
>   dt-bindings: hwmon: max20730:
>   - Provide the binding documentation in yaml format
>   hwmon: pmbus: max20730:
>   - No change
> 
> ChangeLog v3 -> v4
>   dt-bindings: hwmon: max20730:
>   - Fix highefficiency to high efficiency in description
>   - Fix presents to present in vout-voltage-divider
>   - Add additionalProperties: false
>   hwmon: pmbus: max20730:
>   - No change
> 
>  drivers/hwmon/pmbus/max20730.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/hwmon/pmbus/max20730.c b/drivers/hwmon/pmbus/max20730.c
> index a151a2b588a5..fbf2f1e6c969 100644
> --- a/drivers/hwmon/pmbus/max20730.c
> +++ b/drivers/hwmon/pmbus/max20730.c
> @@ -31,6 +31,7 @@ struct max20730_data {
>  	struct pmbus_driver_info info;
>  	struct mutex lock;	/* Used to protect against parallel writes */
>  	u16 mfr_devset1;
> +	u32 vout_voltage_divider[2];
>  };
>  
>  #define to_max20730_data(x)  container_of(x, struct max20730_data, info)
> @@ -114,6 +115,14 @@ static int max20730_read_word_data(struct i2c_client *client, int page,
>  		max_c = max_current[data->id][(data->mfr_devset1 >> 5) & 0x3];
>  		ret = val_to_direct(max_c, PSC_CURRENT_OUT, info);
>  		break;
> +	case PMBUS_READ_VOUT:
> +		ret = pmbus_read_word_data(client, page, phase, reg);
> +		if (ret > 0 && data->vout_voltage_divider[0] && data->vout_voltage_divider[1]) {
> +			u64 temp = DIV_ROUND_CLOSEST_ULL((u64)ret * data->vout_voltage_divider[1],
> +							 data->vout_voltage_divider[0]);
> +			ret = clamp_val(temp, 0, 0xffff);
> +		}
> +		break;
>  	default:
>  		ret = -ENODATA;
>  		break;
> @@ -364,6 +373,15 @@ static int max20730_probe(struct i2c_client *client,
>  	data->id = chip_id;
>  	mutex_init(&data->lock);
>  	memcpy(&data->info, &max20730_info[chip_id], sizeof(data->info));
> +	if (of_property_read_u32_array(client->dev.of_node, "vout-voltage-divider",
> +				       data->vout_voltage_divider,
> +				       ARRAY_SIZE(data->vout_voltage_divider)) != 0)
> +		memset(data->vout_voltage_divider, 0, sizeof(data->vout_voltage_divider));
> +	if (data->vout_voltage_divider[1] < data->vout_voltage_divider[0]) {
> +		dev_err(dev,
> +			"The total resistance of voltage divider is less than output resistance\n");
> +		return -ENODEV;
> +	}
>  
>  	ret = i2c_smbus_read_word_data(client, MAX20730_MFR_DEVSET1);
>  	if (ret < 0)

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

end of thread, other threads:[~2020-10-06 21:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-04  3:14 [PATCH v4 0/2] hwmon: pmbus: max20730: adjust the vout base on Chu Lin
2020-10-04  3:14 ` [PATCH v4 1/2] dt-bindings: hwmon: max20730: adding device tree doc for max20730 Chu Lin
2020-10-06 21:17   ` Rob Herring
2020-10-06 21:55   ` Guenter Roeck
2020-10-04  3:14 ` [PATCH v4 2/2] hwmon: pmbus: max20730: adjust the vout reading given voltage divider Chu Lin
2020-10-04 15:43   ` Guenter Roeck
2020-10-05  3:07     ` Chu Lin
2020-10-05  5:24       ` Guenter Roeck
2020-10-05 15:03   ` Guenter Roeck
2020-10-06 21:55   ` Guenter Roeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).