Linux Hardware Monitor development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration
@ 2026-06-15  3:07 Potin Lai
  2026-06-15  3:07 ` [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
  2026-06-15  3:07 ` [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support Potin Lai
  0 siblings, 2 replies; 8+ messages in thread
From: Potin Lai @ 2026-06-15  3:07 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Zev Weiss
  Cc: linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh,
	Potin Lai, Potin Lai

This series adds support for configuring the current limit behavior via
software override on LM25066-compatible devices (excluding LM25056) using
the DEVICE_SETUP (0xD9) register.

When the 'ti,current-limit' property is specified in the device tree,
the driver configures the DEVICE_SETUP register's Current Limit Configuration
bit (bit 2) to activate SMBus/software override and sets the Current Limit

Setting bit (bit 4) to "low" or "high" threshold accordingly.
Since LM25056 does not support software override (bit 2 of DEVICE_SETUP is
reserved), it is explicitly excluded from this support in both the device
tree binding schema and the driver.

---
Changes in v2:
- Replaced the boolean properties ('ti,cl-smbus-high' and 'ti,cl-smbus-low')
  with a single string property 'ti,current-limit' ('low' or 'high')
- Excluded lm25056 in the driver from parsing/setting the current limit property.
- Link to v1: https://patch.msgid.link/20260611-lm25066-cl-config-v1-0-02e567bf3d91@gmail.com

To: Guenter Roeck <linux@roeck-us.net>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Zev Weiss <zev@bewilderbeest.net>
Cc: linux-hwmon@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Cosmo Chou <cosmo.chou@quantatw.com>
Cc: Mike Hsieh <Mike_Hsieh@quantatw.com>
Cc: Potin Lai <potin.lai@quantatw.com>
Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>

---
Potin Lai (2):
      dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
      hwmon: (pmbus/lm25066) add current limit configuration support

 .../bindings/hwmon/pmbus/ti,lm25066.yaml           | 18 +++++++++++
 drivers/hwmon/pmbus/lm25066.c                      | 37 ++++++++++++++++++++++
 2 files changed, 55 insertions(+)
---
base-commit: 05f7e89ab9731565d8a62e3b5d1ec206485eeb0b
change-id: 20260611-lm25066-cl-config-f81925f7337e

Best regards,
--  
Potin Lai <potin.lai.pt@gmail.com>


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

* [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
  2026-06-15  3:07 [PATCH v2 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai
@ 2026-06-15  3:07 ` Potin Lai
  2026-06-15  3:16   ` sashiko-bot
  2026-06-15  6:08   ` Krzysztof Kozlowski
  2026-06-15  3:07 ` [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support Potin Lai
  1 sibling, 2 replies; 8+ messages in thread
From: Potin Lai @ 2026-06-15  3:07 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Zev Weiss
  Cc: linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh,
	Potin Lai, Potin Lai

Add a 'ti,current-limit' string property to configure the device's Current
Limit (CL) behavior to "high" or "low".

LM25056 does not support setting the current limit via software, so
disallow this property for it.

Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
---
 .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml    | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
index a20f140dc79a..53ee98e871ff 100644
--- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
+++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
@@ -46,12 +46,30 @@ properties:
 
     additionalProperties: false
 
+  ti,current-limit:
+    description: |
+      Configure the current limit setting. When present, this property
+      overrides the hardware setting of the physical CL pin by configuring
+      the register.
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - low
+      - high
+
 required:
   - compatible
   - reg
 
 allOf:
   - $ref: /schemas/hwmon/hwmon-common.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: ti,lm25056
+    then:
+      properties:
+        ti,current-limit: false
 
 unevaluatedProperties: false
 

-- 
2.52.0


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

* [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support
  2026-06-15  3:07 [PATCH v2 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai
  2026-06-15  3:07 ` [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
@ 2026-06-15  3:07 ` Potin Lai
  2026-06-15  3:21   ` sashiko-bot
  1 sibling, 1 reply; 8+ messages in thread
From: Potin Lai @ 2026-06-15  3:07 UTC (permalink / raw)
  To: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Zev Weiss
  Cc: linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh,
	Potin Lai, Potin Lai

Add support for the 'ti,current-limit' devicetree property. When
present, this property overrides the hardware configuration pins via the
DEVICE_SETUP (0xD9) register to set the Current Limit Configuration bit
(bit 2) and Current Limit Setting bit (bit 4) to "high" or "low".

The Bit 4 mapping to High/Low current limit is handled dynamically on
probe because it is swapped for lm25066 compared to other supported
chips (lm5064, lm5066, and lm5066i).

LM25056 is excluded since it does not support configuring the current
limit via software (bit 2 of DEVICE_SETUP register is reserved).

Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
---
 drivers/hwmon/pmbus/lm25066.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
index dd7275a67a0a..c8e7aa7c3acd 100644
--- a/drivers/hwmon/pmbus/lm25066.c
+++ b/drivers/hwmon/pmbus/lm25066.c
@@ -34,6 +34,7 @@ enum chips { lm25056, lm25066, lm5064, lm5066, lm5066i };
 #define LM25066_READ_AVG_PIN		0xdf
 
 #define LM25066_DEV_SETUP_CL		BIT(4)	/* Current limit */
+#define LM25066_DEV_SETUP_CL_CFG	BIT(2)	/* Current limit configuration */
 
 #define LM25066_SAMPLES_FOR_AVG_MAX	4096
 
@@ -484,6 +485,42 @@ static int lm25066_probe(struct i2c_client *client)
 
 	data->id = (enum chips)(unsigned long)i2c_get_match_data(client);
 
+	if (data->id != lm25056) {
+		int config_new = config;
+		const char *cl_setting;
+		int ret;
+
+		if (!of_property_read_string(client->dev.of_node,
+					     "ti,current-limit", &cl_setting)) {
+			config_new |= LM25066_DEV_SETUP_CL_CFG;
+			if (strcmp(cl_setting, "high") == 0) {
+				if (data->id == lm25066)
+					config_new |= LM25066_DEV_SETUP_CL;
+				else
+					config_new &= ~LM25066_DEV_SETUP_CL;
+			} else if (strcmp(cl_setting, "low") == 0) {
+				if (data->id == lm25066)
+					config_new &= ~LM25066_DEV_SETUP_CL;
+				else
+					config_new |= LM25066_DEV_SETUP_CL;
+			} else {
+				dev_err(&client->dev,
+					"invalid current-limit setting: %s\n",
+					cl_setting);
+				return -EINVAL;
+			}
+		}
+
+		if (config_new != config) {
+			ret = i2c_smbus_write_byte_data(client,
+							LM25066_DEVICE_SETUP,
+							config_new);
+			if (ret < 0)
+				return ret;
+			config = config_new;
+		}
+	}
+
 	info = &data->info;
 
 	info->pages = 1;

-- 
2.52.0


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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
  2026-06-15  3:07 ` [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
@ 2026-06-15  3:16   ` sashiko-bot
  2026-06-15  3:28     ` Guenter Roeck
  2026-06-15  6:09     ` Krzysztof Kozlowski
  2026-06-15  6:08   ` Krzysztof Kozlowski
  1 sibling, 2 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-15  3:16 UTC (permalink / raw)
  To: Potin Lai; +Cc: conor+dt, linux-hwmon, robh, devicetree

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Device Tree Schema Type Conflict: The new `ti,current-limit` property is defined as a string, which conflicts with its existing global definition as a `uint32`.
--

commit 2d0e259557c772018da03b728b427c7fa9a267ca
Author: Potin Lai <potin.lai.pt@gmail.com>

dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties

This commit adds a 'ti,current-limit' string property to configure the
device's Current Limit (CL) behavior to "high" or "low". It also disables
this property for the LM25056 variant, which does not support software
configuration for the current limit.

> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> index a20f140dc79a6..53ee98e871ffc 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> @@ -46,12 +46,30 @@ properties:
>  
>      additionalProperties: false
>  
> +  ti,current-limit:
> +    description: |
> +      Configure the current limit setting. When present, this property
> +      overrides the hardware setting of the physical CL pin by configuring
> +      the register.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - low
> +      - high

[Severity: Low]
Does this introduce a global schema type conflict?

Device Tree schema tooling requires property names to have consistent primitive
types globally. The property ti,current-limit is already defined as a uint32 in
other Texas Instruments bindings (such as power/supply/bq2415x.yaml).

Redefining it here as a string breaks make dt_binding_check validation. Would
it be possible to rename this property (for example, ti,current-limit-setting)
to resolve the collision?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-lm25066-cl-config-v2-0-59be46e67d5a@gmail.com?part=1

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

* Re: [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support
  2026-06-15  3:07 ` [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support Potin Lai
@ 2026-06-15  3:21   ` sashiko-bot
  0 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-15  3:21 UTC (permalink / raw)
  To: Potin Lai; +Cc: linux-hwmon, robh, devicetree, conor+dt

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Telemetry coefficient selection mismatch due to unhandled hardware bit swap for current limit configuration.
--

commit 7163f92448b58cfee498d1fa9089543f75ba9068
Author: Potin Lai <potin.lai.pt@gmail.com>

hwmon: (pmbus/lm25066) add current limit configuration support

This patch adds support for the ti,current-limit devicetree property
to configure the current limit via the DEVICE_SETUP register. It dynamically
handles the Bit 4 mapping for High/Low current limits, which is swapped
for the lm25066 compared to other chips in the family.

> diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c
> index dd7275a67a0ab..c8e7aa7c3acd3 100644
> --- a/drivers/hwmon/pmbus/lm25066.c
> +++ b/drivers/hwmon/pmbus/lm25066.c

[ ... ]

> @@ -484,6 +485,42 @@ static int lm25066_probe(struct i2c_client *client)
>  
>  	data->id = (enum chips)(unsigned long)i2c_get_match_data(client);
>  
> +	if (data->id != lm25056) {
> +		int config_new = config;
> +		const char *cl_setting;
> +		int ret;
> +
> +		if (!of_property_read_string(client->dev.of_node,
> +					     "ti,current-limit", &cl_setting)) {
> +			config_new |= LM25066_DEV_SETUP_CL_CFG;
> +			if (strcmp(cl_setting, "high") == 0) {
> +				if (data->id == lm25066)
> +					config_new |= LM25066_DEV_SETUP_CL;
> +				else
> +					config_new &= ~LM25066_DEV_SETUP_CL;
> +			} else if (strcmp(cl_setting, "low") == 0) {
> +				if (data->id == lm25066)
> +					config_new &= ~LM25066_DEV_SETUP_CL;
> +				else
> +					config_new |= LM25066_DEV_SETUP_CL;

[Severity: High]
This isn't a bug introduced by this patch, but does the telemetry coefficient
selection further down in lm25066_probe() also need to account for this
hardware bit logic swap?

Later in lm25066_probe(), the coefficient selection unconditionally assumes
Bit 4 (LM25066_DEV_SETUP_CL) being set means the High limit
(coeff[PSC_CURRENT_IN_L]) for all chips:

	if (config & LM25066_DEV_SETUP_CL) {
		info->m[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN_L].m;
		info->b[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN_L].b;
		info->m[PSC_POWER] = coeff[PSC_POWER_L].m;
		info->b[PSC_POWER] = coeff[PSC_POWER_L].b;
	} else {
		info->m[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].m;
		info->b[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].b;
		...
	}

For chips like the LM5064, if they are configured for the High current limit,
Bit 4 will be 0 based on the DT parsing logic above.

Could this cause the driver to erroneously select the normal coefficients
(Low current limit) instead of the _L coefficients, potentially throwing the
reported current and power telemetry off by a factor of ~2x?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-lm25066-cl-config-v2-0-59be46e67d5a@gmail.com?part=2

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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
  2026-06-15  3:16   ` sashiko-bot
@ 2026-06-15  3:28     ` Guenter Roeck
  2026-06-15  6:09     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2026-06-15  3:28 UTC (permalink / raw)
  To: sashiko-reviews, Potin Lai; +Cc: conor+dt, linux-hwmon, robh, devicetree

On 6/14/26 20:16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Device Tree Schema Type Conflict: The new `ti,current-limit` property is defined as a string, which conflicts with its existing global definition as a `uint32`.
> --
> 
> commit 2d0e259557c772018da03b728b427c7fa9a267ca
> Author: Potin Lai <potin.lai.pt@gmail.com>
> 
> dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
> 
> This commit adds a 'ti,current-limit' string property to configure the
> device's Current Limit (CL) behavior to "high" or "low". It also disables
> this property for the LM25056 variant, which does not support software
> configuration for the current limit.
> 
>> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
>> index a20f140dc79a6..53ee98e871ffc 100644
>> --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
>> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
>> @@ -46,12 +46,30 @@ properties:
>>   
>>       additionalProperties: false
>>   
>> +  ti,current-limit:
>> +    description: |
>> +      Configure the current limit setting. When present, this property
>> +      overrides the hardware setting of the physical CL pin by configuring
>> +      the register.
>> +    $ref: /schemas/types.yaml#/definitions/string
>> +    enum:
>> +      - low
>> +      - high
> 
> [Severity: Low]
> Does this introduce a global schema type conflict?
> 
> Device Tree schema tooling requires property names to have consistent primitive
> types globally. The property ti,current-limit is already defined as a uint32 in
> other Texas Instruments bindings (such as power/supply/bq2415x.yaml).
> 
> Redefining it here as a string breaks make dt_binding_check validation. Would
> it be possible to rename this property (for example, ti,current-limit-setting)
> to resolve the collision?
> 

ti,current-scale or ti,current-range might be other possibilities.

Guenter


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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
  2026-06-15  3:07 ` [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
  2026-06-15  3:16   ` sashiko-bot
@ 2026-06-15  6:08   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-06-15  6:08 UTC (permalink / raw)
  To: Potin Lai
  Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Zev Weiss, linux-hwmon, devicetree, linux-kernel, Cosmo Chou,
	Mike Hsieh, Potin Lai

On Mon, Jun 15, 2026 at 11:07:13AM +0800, Potin Lai wrote:
> Add a 'ti,current-limit' string property to configure the device's Current
> Limit (CL) behavior to "high" or "low".
> 
> LM25056 does not support setting the current limit via software, so
> disallow this property for it.

Then which device supports it? Your commit msg should explain WHY you
are doing this, not WHAT you are doing (unless that is not obvious). The
reason WHY is for some device. You just explainde WHY NOT doing that...

> 
> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com>
> ---
>  .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml    | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> index a20f140dc79a..53ee98e871ff 100644
> --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml
> @@ -46,12 +46,30 @@ properties:
>  
>      additionalProperties: false
>  
> +  ti,current-limit:
> +    description: |
> +      Configure the current limit setting. When present, this property
> +      overrides the hardware setting of the physical CL pin by configuring
> +      the register.
> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - low
> +      - high

What is the meaning of low and high? Maybe these map to specific values?
Docs are saying 25 or 46 mV, so maybe there is no direct mapping. Anyway
commit msg could also explain that.

Best regards,
Krzysztof


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

* Re: [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties
  2026-06-15  3:16   ` sashiko-bot
  2026-06-15  3:28     ` Guenter Roeck
@ 2026-06-15  6:09     ` Krzysztof Kozlowski
  1 sibling, 0 replies; 8+ messages in thread
From: Krzysztof Kozlowski @ 2026-06-15  6:09 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: Potin Lai, conor+dt, linux-hwmon, robh, devicetree

On Mon, Jun 15, 2026 at 03:16:28AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Device Tree Schema Type Conflict: The new `ti,current-limit` property is defined as a string, which conflicts with its existing global definition as a `uint32`.
> --

Actually HIGH.

Properties can have only one type in general.

Best regards,
Krzysztof


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

end of thread, other threads:[~2026-06-15  6:09 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-15  3:07 [PATCH v2 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai
2026-06-15  3:07 ` [PATCH v2 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
2026-06-15  3:16   ` sashiko-bot
2026-06-15  3:28     ` Guenter Roeck
2026-06-15  6:09     ` Krzysztof Kozlowski
2026-06-15  6:08   ` Krzysztof Kozlowski
2026-06-15  3:07 ` [PATCH v2 2/2] hwmon: (pmbus/lm25066) add current limit configuration support Potin Lai
2026-06-15  3:21   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox