* [PATCH 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration
@ 2026-06-11 9:58 Potin Lai
2026-06-11 9:58 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai
2026-06-11 9:58 ` [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support Potin Lai
0 siblings, 2 replies; 11+ messages in thread
From: Potin Lai @ 2026-06-11 9:58 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 patch series adds support for configuring the Current Limit (CL)
behavior of the TI LM25066 and compatible devices (LM25056, LM5064,
LM5066, LM5066I) using SMBus settings instead of physical hardware pins.
The first patch documents the mutually exclusive Devicetree properties
'ti,cl-smbus-high' and 'ti,cl-smbus-low'.
The second patch implements the driver changes to configure the DEVICE_SETUP
(0xD9) register based on these DT properties. It handles the dynamic differences
in the CL bit (bit 4) mapping where LM25056 and LM25066 have swapped logic for
High/Low settings compared to LM5064, LM5066, and LM5066i.
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 SMBus current limit configuration support
.../bindings/hwmon/pmbus/ti,lm25066.yaml | 20 +++++++++++++++++
drivers/hwmon/pmbus/lm25066.c | 25 ++++++++++++++++++++++
2 files changed, 45 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] 11+ messages in thread* [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-11 9:58 [PATCH 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai @ 2026-06-11 9:58 ` Potin Lai 2026-06-11 10:05 ` sashiko-bot 2026-06-11 17:27 ` Conor Dooley 2026-06-11 9:58 ` [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support Potin Lai 1 sibling, 2 replies; 11+ messages in thread From: Potin Lai @ 2026-06-11 9:58 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 mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean properties to configure the device's Current Limit (CL) behavior using SMBus settings instead of physical pins. Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> --- .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml index a20f140dc79a..95ea7c26dec2 100644 --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml @@ -46,6 +46,26 @@ properties: additionalProperties: false + ti,cl-smbus-high: + description: | + Configure the Current Limit (CL) to use the SMBus high setting. + type: boolean + + ti,cl-smbus-low: + description: | + Configure the Current Limit (CL) to use the SMBus low setting. + type: boolean + +dependencies: + ti,cl-smbus-high: + not: + required: + - ti,cl-smbus-low + ti,cl-smbus-low: + not: + required: + - ti,cl-smbus-high + required: - compatible - reg -- 2.52.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-11 9:58 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai @ 2026-06-11 10:05 ` sashiko-bot 2026-06-11 17:27 ` Conor Dooley 1 sibling, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-06-11 10:05 UTC (permalink / raw) To: Potin Lai; +Cc: linux-hwmon, conor+dt, robh, devicetree > Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean > properties to configure the device's Current Limit (CL) behavior using > SMBus settings instead of physical pins. > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> Sashiko has reviewed this patch and found no issues. It looks great! -- Sashiko AI review · https://sashiko.dev/#/patchset/20260611-lm25066-cl-config-v1-0-02e567bf3d91@gmail.com?part=1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-11 9:58 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai 2026-06-11 10:05 ` sashiko-bot @ 2026-06-11 17:27 ` Conor Dooley 2026-06-12 9:10 ` Potin Lai 1 sibling, 1 reply; 11+ messages in thread From: Conor Dooley @ 2026-06-11 17:27 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 [-- Attachment #1: Type: text/plain, Size: 1664 bytes --] On Thu, Jun 11, 2026 at 05:58:44PM +0800, Potin Lai wrote: > Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean > properties to configure the device's Current Limit (CL) behavior using > SMBus settings instead of physical pins. > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> > --- > .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ > 1 file changed, 20 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > index a20f140dc79a..95ea7c26dec2 100644 > --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > @@ -46,6 +46,26 @@ properties: > > additionalProperties: false > > + ti,cl-smbus-high: > + description: | > + Configure the Current Limit (CL) to use the SMBus high setting. > + type: boolean > + > + ti,cl-smbus-low: > + description: | > + Configure the Current Limit (CL) to use the SMBus low setting. > + type: boolean What's smbus specific about this? If the pin was connected to a GPIO, you'd then need to have different properties or use these ones with an inaccurate name. Please also spell out "current-limit". pw-bot: changes-requested Thanks, Conor. > + > +dependencies: > + ti,cl-smbus-high: > + not: > + required: > + - ti,cl-smbus-low > + ti,cl-smbus-low: > + not: > + required: > + - ti,cl-smbus-high > + > required: > - compatible > - reg > > -- > 2.52.0 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-11 17:27 ` Conor Dooley @ 2026-06-12 9:10 ` Potin Lai 2026-06-12 16:12 ` Conor Dooley 0 siblings, 1 reply; 11+ messages in thread From: Potin Lai @ 2026-06-12 9:10 UTC (permalink / raw) To: Conor Dooley Cc: Guenter Roeck, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Zev Weiss, linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh, Potin Lai On Fri, Jun 12, 2026 at 1:27 AM Conor Dooley <conor@kernel.org> wrote: > > On Thu, Jun 11, 2026 at 05:58:44PM +0800, Potin Lai wrote: > > Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean > > properties to configure the device's Current Limit (CL) behavior using > > SMBus settings instead of physical pins. > > > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> > > --- > > .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > index a20f140dc79a..95ea7c26dec2 100644 > > --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > @@ -46,6 +46,26 @@ properties: > > > > additionalProperties: false > > > > + ti,cl-smbus-high: > > + description: | > > + Configure the Current Limit (CL) to use the SMBus high setting. > > + type: boolean > > + > > + ti,cl-smbus-low: > > + description: | > > + Configure the Current Limit (CL) to use the SMBus low setting. > > + type: boolean > > What's smbus specific about this? If the pin was connected to a GPIO, > you'd then need to have different properties or use these ones with an > inaccurate name. > The "smbus" in the property name was originally meant to indicate that the setting is configured via the internal register over the SMBus (I2C) interface, rather than physical pins. > Please also spell out "current-limit". > I will rename the properties to "ti,current-limit-high" and "ti,current-limit-low" in the next version. Thanks, Potin > pw-bot: changes-requested > > Thanks, > Conor. > > > + > > +dependencies: > > + ti,cl-smbus-high: > > + not: > > + required: > > + - ti,cl-smbus-low > > + ti,cl-smbus-low: > > + not: > > + required: > > + - ti,cl-smbus-high > > + > > required: > > - compatible > > - reg > > > > -- > > 2.52.0 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-12 9:10 ` Potin Lai @ 2026-06-12 16:12 ` Conor Dooley 2026-06-12 17:19 ` Guenter Roeck 0 siblings, 1 reply; 11+ messages in thread From: Conor Dooley @ 2026-06-12 16:12 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 [-- Attachment #1: Type: text/plain, Size: 2224 bytes --] On Fri, Jun 12, 2026 at 05:10:38PM +0800, Potin Lai wrote: > On Fri, Jun 12, 2026 at 1:27 AM Conor Dooley <conor@kernel.org> wrote: > > > > On Thu, Jun 11, 2026 at 05:58:44PM +0800, Potin Lai wrote: > > > Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean > > > properties to configure the device's Current Limit (CL) behavior using > > > SMBus settings instead of physical pins. > > > > > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> > > > --- > > > .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ > > > 1 file changed, 20 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > index a20f140dc79a..95ea7c26dec2 100644 > > > --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > @@ -46,6 +46,26 @@ properties: > > > > > > additionalProperties: false > > > > > > + ti,cl-smbus-high: > > > + description: | > > > + Configure the Current Limit (CL) to use the SMBus high setting. > > > + type: boolean > > > + > > > + ti,cl-smbus-low: > > > + description: | > > > + Configure the Current Limit (CL) to use the SMBus low setting. > > > + type: boolean > > > > What's smbus specific about this? If the pin was connected to a GPIO, > > you'd then need to have different properties or use these ones with an > > inaccurate name. > > > > The "smbus" in the property name was originally meant to indicate > that the setting is configured via the internal register over the SMBus (I2C) > interface, rather than physical pins. Right, but if you do it via the physical pins using a gpio, you still need a way to say what limit is. The status quo only works if the limit pin is tied high or low. > > > Please also spell out "current-limit". > > > > I will rename the properties to "ti,current-limit-high" and > "ti,current-limit-low" > in the next version. Other thing that might be worth doing is making the property a string and just having `ti,current-limit = "low"` etc. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-12 16:12 ` Conor Dooley @ 2026-06-12 17:19 ` Guenter Roeck 2026-06-12 21:13 ` Conor Dooley 0 siblings, 1 reply; 11+ messages in thread From: Guenter Roeck @ 2026-06-12 17:19 UTC (permalink / raw) To: Conor Dooley, Potin Lai Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Zev Weiss, linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh, Potin Lai On 6/12/26 09:12, Conor Dooley wrote: > On Fri, Jun 12, 2026 at 05:10:38PM +0800, Potin Lai wrote: >> On Fri, Jun 12, 2026 at 1:27 AM Conor Dooley <conor@kernel.org> wrote: >>> >>> On Thu, Jun 11, 2026 at 05:58:44PM +0800, Potin Lai wrote: >>>> Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean >>>> properties to configure the device's Current Limit (CL) behavior using >>>> SMBus settings instead of physical pins. >>>> >>>> Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> >>>> --- >>>> .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ >>>> 1 file changed, 20 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml >>>> index a20f140dc79a..95ea7c26dec2 100644 >>>> --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml >>>> +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml >>>> @@ -46,6 +46,26 @@ properties: >>>> >>>> additionalProperties: false >>>> >>>> + ti,cl-smbus-high: >>>> + description: | >>>> + Configure the Current Limit (CL) to use the SMBus high setting. >>>> + type: boolean >>>> + >>>> + ti,cl-smbus-low: >>>> + description: | >>>> + Configure the Current Limit (CL) to use the SMBus low setting. >>>> + type: boolean >>> >>> What's smbus specific about this? If the pin was connected to a GPIO, >>> you'd then need to have different properties or use these ones with an >>> inaccurate name. >>> >> >> The "smbus" in the property name was originally meant to indicate >> that the setting is configured via the internal register over the SMBus (I2C) >> interface, rather than physical pins. > > Right, but if you do it via the physical pins using a gpio, you still > need a way to say what limit is. The status quo only works if the limit > pin is tied high or low. > The physical pin is supposed to be connected to ground or left floating. It seems unlikely that anyone would ever have the idea of connecting it to a GPIO pin, and doing so would for sure mess up the driver because its state is only read in the probe function. The configuration here is for setting the limit range (scale) with a configuration register to override the configuration obtained from the external pin. Either case, even _if_ the CL pin is connected to a GPIO pin, the status of that pin would be read from the configuration register. A devicetree property is not needed for that. The properties are to _override_ the pin configuration, not to reflect it. Thanks, Guenter ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties 2026-06-12 17:19 ` Guenter Roeck @ 2026-06-12 21:13 ` Conor Dooley 0 siblings, 0 replies; 11+ messages in thread From: Conor Dooley @ 2026-06-12 21:13 UTC (permalink / raw) To: Guenter Roeck Cc: Potin Lai, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Zev Weiss, linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh, Potin Lai [-- Attachment #1: Type: text/plain, Size: 3689 bytes --] On Fri, Jun 12, 2026 at 10:19:14AM -0700, Guenter Roeck wrote: > On 6/12/26 09:12, Conor Dooley wrote: > > On Fri, Jun 12, 2026 at 05:10:38PM +0800, Potin Lai wrote: > > > On Fri, Jun 12, 2026 at 1:27 AM Conor Dooley <conor@kernel.org> wrote: > > > > > > > > On Thu, Jun 11, 2026 at 05:58:44PM +0800, Potin Lai wrote: > > > > > Add mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' boolean > > > > > properties to configure the device's Current Limit (CL) behavior using > > > > > SMBus settings instead of physical pins. > > > > > > > > > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> > > > > > --- > > > > > .../devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml | 20 ++++++++++++++++++++ > > > > > 1 file changed, 20 insertions(+) > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > > > index a20f140dc79a..95ea7c26dec2 100644 > > > > > --- a/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > > > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/ti,lm25066.yaml > > > > > @@ -46,6 +46,26 @@ properties: > > > > > > > > > > additionalProperties: false > > > > > > > > > > + ti,cl-smbus-high: > > > > > + description: | > > > > > + Configure the Current Limit (CL) to use the SMBus high setting. > > > > > + type: boolean > > > > > + > > > > > + ti,cl-smbus-low: > > > > > + description: | > > > > > + Configure the Current Limit (CL) to use the SMBus low setting. > > > > > + type: boolean > > > > > > > > What's smbus specific about this? If the pin was connected to a GPIO, > > > > you'd then need to have different properties or use these ones with an > > > > inaccurate name. > > > > > > > > > > The "smbus" in the property name was originally meant to indicate > > > that the setting is configured via the internal register over the SMBus (I2C) > > > interface, rather than physical pins. > > > > Right, but if you do it via the physical pins using a gpio, you still > > need a way to say what limit is. The status quo only works if the limit > > pin is tied high or low. > > > > The physical pin is supposed to be connected to ground or left floating. > It seems unlikely that anyone would ever have the idea of connecting it > to a GPIO pin, and doing so would for sure mess up the driver because > its state is only read in the probe function. The configuration here Well yeah, "obviously" if someone wanted to use a GPIO the driver would have to change to handle that - but probably not that much since it'd be a static setting that could be done at probe. I get that it may be unlikely, but it seems like a reasonable thing that someone might want to do, and renaming the property to not exclude that usecase seems to be "free". > is for setting the limit range (scale) with a configuration register > to override the configuration obtained from the external pin. > Either case, even _if_ the CL pin is connected to a GPIO pin, the status > of that pin would be read from the configuration register. A devicetree This isn't true, I don't think, unless you're using GPIO hogs? The GPIO state when linux comes up could be the reset value of the controller, rather than the correct configuration. You'd need a way to tell the driver what way to drive it. > property is not needed for that. The properties are to _override_ the pin > configuration, not to reflect it. TL;DR, I think the removal of smbus from the property name (I'm not asking for it to be taken out of the description) is reasonable. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support 2026-06-11 9:58 [PATCH 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai 2026-06-11 9:58 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai @ 2026-06-11 9:58 ` Potin Lai 2026-06-11 10:10 ` sashiko-bot 2026-06-11 12:20 ` Guenter Roeck 1 sibling, 2 replies; 11+ messages in thread From: Potin Lai @ 2026-06-11 9:58 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 mutually exclusive 'ti,cl-smbus-high' and 'ti,cl-smbus-low' devicetree properties. When present, these properties override 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 SMBus settings. The Bit 4 mapping to High/Low current limit is handled dynamically on probe because it is swapped for lm25056 and lm25066 compared to other supported chips (lm5064, lm5066, and lm5066i). Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> --- drivers/hwmon/pmbus/lm25066.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c index dd7275a67a0a..20e114bdc882 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 @@ -464,6 +465,8 @@ MODULE_DEVICE_TABLE(of, lm25066_of_match); static int lm25066_probe(struct i2c_client *client) { int config; + int config_new; + int ret; u32 shunt; struct lm25066_data *data; struct pmbus_driver_info *info; @@ -484,6 +487,28 @@ static int lm25066_probe(struct i2c_client *client) data->id = (enum chips)(unsigned long)i2c_get_match_data(client); + config_new = config; + if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-high")) { + config_new |= LM25066_DEV_SETUP_CL_CFG; + if (data->id == lm25056 || data->id == lm25066) + config_new |= LM25066_DEV_SETUP_CL; + else + config_new &= ~LM25066_DEV_SETUP_CL; + } else if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-low")) { + config_new |= LM25066_DEV_SETUP_CL_CFG; + if (data->id == lm25056 || data->id == lm25066) + config_new &= ~LM25066_DEV_SETUP_CL; + else + config_new |= LM25066_DEV_SETUP_CL; + } + + 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] 11+ messages in thread
* Re: [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support 2026-06-11 9:58 ` [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support Potin Lai @ 2026-06-11 10:10 ` sashiko-bot 2026-06-11 12:20 ` Guenter Roeck 1 sibling, 0 replies; 11+ messages in thread From: sashiko-bot @ 2026-06-11 10:10 UTC (permalink / raw) To: Potin Lai; +Cc: robh, devicetree, linux-hwmon, conor+dt Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] The driver incorrectly scales current and power telemetry for LM5064/LM5066/LM5066i chips by a factor of 2 because it fails to account for a swapped hardware bit mapping when selecting PMBus coefficients. -- commit c83de5ba055b8d019bcdbf2292f57ea088b2924b Author: Potin Lai <potin.lai.pt@gmail.com> hwmon: (pmbus/lm25066) add SMBus current limit configuration support This commit adds support for the 'ti,cl-smbus-high' and 'ti,cl-smbus-low' devicetree properties to configure the current limit via SMBus. It sets the Current Limit Configuration and Current Limit Setting bits, accounting for swapped bit mapping on certain chips. > diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c > index dd7275a67a0ab..20e114bdc882d 100644 > --- a/drivers/hwmon/pmbus/lm25066.c > +++ b/drivers/hwmon/pmbus/lm25066.c > @@ -464,6 +465,8 @@ MODULE_DEVICE_TABLE(of, lm25066_of_match); > static int lm25066_probe(struct i2c_client *client) > { > int config; > + int config_new; > + int ret; > u32 shunt; > struct lm25066_data *data; > struct pmbus_driver_info *info; [ ... ] > @@ -484,6 +487,28 @@ static int lm25066_probe(struct i2c_client *client) > > data->id = (enum chips)(unsigned long)i2c_get_match_data(client); > > + config_new = config; > + if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-high")) { > + config_new |= LM25066_DEV_SETUP_CL_CFG; > + if (data->id == lm25056 || data->id == lm25066) > + config_new |= LM25066_DEV_SETUP_CL; > + else > + config_new &= ~LM25066_DEV_SETUP_CL; > + } else if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-low")) { > + config_new |= LM25066_DEV_SETUP_CL_CFG; > + if (data->id == lm25056 || data->id == lm25066) > + config_new &= ~LM25066_DEV_SETUP_CL; > + else > + config_new |= LM25066_DEV_SETUP_CL; > + } > + > + if (config_new != config) { > + ret = i2c_smbus_write_byte_data(client, LM25066_DEVICE_SETUP, config_new); > + if (ret < 0) > + return ret; > + config = config_new; > + } > + [Severity: High] This is a pre-existing issue, but does this patch leave the new explicit device tree configuration feature broken for some chips? The new code correctly identifies that the DEVICE_SETUP Current Limit bit (Bit 4) mapping is swapped between the LM25056/LM25066 chips (1 = High limit) and LM5064/LM5066/LM5066i chips (0 = High limit). However, further down in lm25066_probe(), the coefficient selection logic fails to account for this chip-specific swap: 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; ... } else { info->m[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].m; info->b[PSC_CURRENT_IN] = coeff[PSC_CURRENT_IN].b; ... } When an LM5066 is configured to the high limit via ti,cl-smbus-high, the driver clears Bit 4. The coefficient logic then sees Bit 4 is 0 and incorrectly selects the PSC_CURRENT_IN (Low limit) coefficients. Could this cause the calculated current and power telemetry for the LM5064, LM5066, and LM5066i chips to be under-reported by roughly 2x? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260611-lm25066-cl-config-v1-0-02e567bf3d91@gmail.com?part=2 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support 2026-06-11 9:58 ` [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support Potin Lai 2026-06-11 10:10 ` sashiko-bot @ 2026-06-11 12:20 ` Guenter Roeck 1 sibling, 0 replies; 11+ messages in thread From: Guenter Roeck @ 2026-06-11 12:20 UTC (permalink / raw) To: Potin Lai, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Zev Weiss Cc: linux-hwmon, devicetree, linux-kernel, Cosmo Chou, Mike Hsieh, Potin Lai On 6/11/26 02:58, Potin Lai wrote: > Add support for the mutually exclusive 'ti,cl-smbus-high' and > 'ti,cl-smbus-low' devicetree properties. When present, these properties > override 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 SMBus settings. > > The Bit 4 mapping to High/Low current limit is handled dynamically on > probe because it is swapped for lm25056 and lm25066 compared to other > supported chips (lm5064, lm5066, and lm5066i). > > Signed-off-by: Potin Lai <potin.lai.pt@gmail.com> > --- > drivers/hwmon/pmbus/lm25066.c | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > > diff --git a/drivers/hwmon/pmbus/lm25066.c b/drivers/hwmon/pmbus/lm25066.c > index dd7275a67a0a..20e114bdc882 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 > > @@ -464,6 +465,8 @@ MODULE_DEVICE_TABLE(of, lm25066_of_match); > static int lm25066_probe(struct i2c_client *client) > { > int config; > + int config_new; > + int ret; > u32 shunt; > struct lm25066_data *data; > struct pmbus_driver_info *info; > @@ -484,6 +487,28 @@ static int lm25066_probe(struct i2c_client *client) > > data->id = (enum chips)(unsigned long)i2c_get_match_data(client); > > + config_new = config; > + if (of_property_read_bool(client->dev.of_node, "ti,cl-smbus-high")) { > + config_new |= LM25066_DEV_SETUP_CL_CFG; > + if (data->id == lm25056 || data->id == lm25066) LM25056 does not support setting the gain via software, and bit 2 of this register is reserved. These properties need to be disabled for that chip. That will have to be reflected both here and in the devicetree file. Thanks, Guenter ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-12 21:13 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-06-11 9:58 [PATCH 0/2] hwmon: (pmbus/lm25066) Support SMBus Current Limit configuration Potin Lai 2026-06-11 9:58 ` [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties Potin Lai 2026-06-11 10:05 ` sashiko-bot 2026-06-11 17:27 ` Conor Dooley 2026-06-12 9:10 ` Potin Lai 2026-06-12 16:12 ` Conor Dooley 2026-06-12 17:19 ` Guenter Roeck 2026-06-12 21:13 ` Conor Dooley 2026-06-11 9:58 ` [PATCH 2/2] hwmon: (pmbus/lm25066) add SMBus current limit configuration support Potin Lai 2026-06-11 10:10 ` sashiko-bot 2026-06-11 12:20 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox