Re: [PATCH 1/2] dt-bindings: hwmon: pmbus: ti,lm25066: add current limit properties

[Conor Dooley <conor@kernel.org>]

[-- 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 --]