Re: [PATCH v2 2/3] dt-bindings: dp83867: add binding for output-impedance

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2457 bytes --]

On Thu, Aug 27, 2026 at 10:04:07AM +0200, Steffen Trumtrar wrote:
> On 2026-08-26 at 17:01 +01, Conor Dooley <conor@kernel.org> wrote:
> 
> > On Wed, Aug 26, 2026 at 02:48:40PM +0200, Steffen Trumtrar wrote:
> > > Currently, the output-impedance can only be set to default, minimum or
> > > maximum or via a nvmem cell.
> > > In cases where there is no backend for a nvmem cell, a value can not be
> > > set.
> > > > Add a DT binding, 'ti,output-impedance-ohms', for specifying the
> > > output-impedance via a fixed value. The existing boolean values for
> > > minimum and maximum still take precedence over the new value.
> > > > Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> > > ---
> > >  Documentation/devicetree/bindings/net/ti,dp83867.yaml | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > > > diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> > b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> > > index 7a6fe516bab27..49fafe3ce6f47 100644
> > > --- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> > > +++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> > > @@ -38,6 +38,16 @@ properties:
> > >      items:
> > >        - const: io_impedance_ctrl
> > >  > +  ti,output-impedance-ohms:
> > > +    description: |
> > > +      MAC Interface Impedance control to set the programmable output impedance.
> > > +      Valid range is 0x00 to 0x1f (32 steps).
> > > +      - 0x1f = 35 ohms (minimum impedance)
> > > +      - 0x00 = 70 ohms (maximum impedance)
> > > +      - Step size varies with process.
> > > +    minimum: 0
> > > +    maximum: 31
> > 
> > Your property is named in ohms, but not implemented in ohms.
> 
> So without the -ohms?
> 
> > pw-bot: changes-requested
> > 
> > Can't you just achieve the same result using the existing max property?
> 
> The max property is a boolean which just says "use 70ohms".

Ah, I somehow missed that. I thought it was a uint32...

> 
> > I don't get the use case where you specify a max, but then don't want to
> > use that max. Just set the max to what you actually want?

And this question was based on that assumption, so while your answer
makes no sense that's because the question didn't. Sorry bout that.

> 
> That's why I added the exclusion in the binding, because having nvmem-cells
> and/or any of the other output-impedance bindings doesn't make any sense.

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