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

[Guenter Roeck]

On 6/12/26 14:13, Conor Dooley wrote:
> 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@xxxxxxxxxx> 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@xxxxxxxxx>
> > > > > > ---
> > > > > >    .../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".

It is not only unlikely, it would be risky and potentially result in
undefined behavior. The pin is supposed to be static. It is undefined
if the hardware evaluates it once after power-up, after "power good"
was detected (if the specific chip supports it), or continuously.
Making the pin run-time configurable would be a risky endeavor with
zero gain since the value can be set by software configuration,
while at the same time adding complexity to the hardware.

Guenter