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: 3827 bytes --]

On Sat, Jun 13, 2026 at 06:05:28AM -0700, Guenter Roeck wrote:
> 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@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".
> > 
> 
> 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.

Okay, if you think noone will do it, then I will cede to your more
relevant expertise.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]