[PATCH v2 0/3] net: dp83867: add new output-impedance DT binding

[PATCH v2 0/3] net: dp83867: add new output-impedance DT binding

[Steffen Trumtrar]

Currently the dp83867 supports setting the io impedance to minimum,
maximum, default or a value from a nvmem cell. In situations where there
is no backend for a nvmem cell, the value can not be set to anything but
minimum, maximum or default.

Add support for a ti,output-impedance-ohms binding, allowing to specify
the impedance via the devicetree. The existing boolean bindings for
minimum and maximum take precedence over this new binding. The nvmem
consumer still takes lowest precedence.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
Changes in v2:
- change 'u8 *val' to 'u32 *val'
- fix logic in driver
- update description of new binding
- rename ti,output-impedance -> ti,output-impedance-ohms
- make mutual exclusivity explicit in binding
- Link to v1: https://patch.msgid.link/20260804-v7-2-topic-dp83867-v1-0-1ca79eb54eef@pengutronix.de

To: Andrew Lunn <andrew+netdev@lunn.ch>
To: "David S. Miller" <davem@davemloft.net>
To: Eric Dumazet <edumazet@google.com>
To: Jakub Kicinski <kuba@kernel.org>
To: Paolo Abeni <pabeni@redhat.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Andrew Davis <afd@ti.com>
To: Andrew Lunn <andrew@lunn.ch>
To: Heiner Kallweit <hkallweit1@gmail.com>
To: Russell King <linux@armlinux.org.uk>
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Steffen Trumtrar (3):
      dt-bindings: net: dp83867: make exclusivity explicit
      dt-bindings: dp83867: add binding for output-impedance
      net: phy: dp83867: support setting output-impedance

 .../devicetree/bindings/net/ti,dp83867.yaml        | 26 +++++++++++++++++++---
 drivers/net/phy/dp83867.c                          | 18 +++++++++++----
 2 files changed, 37 insertions(+), 7 deletions(-)
---
base-commit: 495561a136eaf585a305d36fc3affdba94aaf28e
change-id: 20260804-v7-2-topic-dp83867-e73a84c37016

Best regards,
--  
Steffen Trumtrar <s.trumtrar@pengutronix.de>

[PATCH v2 1/3] dt-bindings: net: dp83867: make exclusivity explicit

[Steffen Trumtrar]

The properties nvmem-cells and ti,{min,max}-output-impedance are
mutually exclusive as already noted in the ti,max-output-impedance
description.

As this is only stated in the description and not made explicit in the
schema, the binding checker will not catch if this condition is ignored.

Make it explicit in the schema, that nmvem-cells can not be used with
ti,min-output-impedance or ti,max-output-impedance.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 Documentation/devicetree/bindings/net/ti,dp83867.yaml | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
index 4bc1f98fd9fe5..7a6fe516bab27 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
@@ -7,9 +7,6 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
 
 title: TI DP83867 ethernet PHY
 
-allOf:
-  - $ref: ethernet-controller.yaml#
-
 maintainers:
   - Andrew Davis <afd@ti.com>
 
@@ -118,6 +115,13 @@ properties:
       Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h for applicable
       values.
 
+allOf:
+  - $ref: ethernet-controller.yaml#
+  - not:
+      required: [nvmem-cells, "ti,min-output-impedance"]
+  - not:
+      required: [nvmem-cells, "ti,max-output-impedance"]
+
 required:
   - reg
 

-- 
2.54.0

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

[Steffen Trumtrar]

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
+
   ti,min-output-impedance:
     type: boolean
     description: |
@@ -121,6 +131,12 @@ allOf:
       required: [nvmem-cells, "ti,min-output-impedance"]
   - not:
       required: [nvmem-cells, "ti,max-output-impedance"]
+  - not:
+      required: [nvmem-cells, "ti,output-impedance-ohms"]
+  - not:
+      required: ["ti,output-impedance-ohms", "ti,min-output-impedance"]
+  - not:
+      required: ["ti,output-impedance-ohms", "ti,max-output-impedance"]
 
 required:
   - reg

-- 
2.54.0

[PATCH v2 3/3] net: phy: dp83867: support setting output-impedance

[Steffen Trumtrar]

Currently the dp83867 supports setting the io impedance to minimum,
maximum, default or a value from a nvmem cell. In situations where there
is no backend for a nvmem cell, the value can not be set to anything but
minimum, maximum or default.

Add support for the ti,output-impedance-ohms binding, allowing to
specify the impedance via the devicetree. The existing boolean bindings
for minimum and maximum take precedence over this new binding. The nvmem
consumer still takes lowest precedence.

Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
---
 drivers/net/phy/dp83867.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 88255e92b4cdb..973b70f11c45a 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -531,7 +531,8 @@ static int dp83867_of_init_io_impedance(struct phy_device *phydev)
 	struct device *dev = &phydev->mdio.dev;
 	struct device_node *of_node = dev->of_node;
 	struct nvmem_cell *cell;
-	u8 *buf, val;
+	u8 *buf;
+	u32 val;
 	int ret;
 
 	cell = of_nvmem_cell_get(of_node, "io_impedance_ctrl");
@@ -542,12 +543,21 @@ static int dp83867_of_init_io_impedance(struct phy_device *phydev)
 						"failed to get nvmem cell io_impedance_ctrl\n");
 
 		/* If no nvmem cell, check for the boolean properties. */
-		if (of_property_read_bool(of_node, "ti,max-output-impedance"))
+		if (of_property_read_bool(of_node, "ti,max-output-impedance")) {
 			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
-		else if (of_property_read_bool(of_node, "ti,min-output-impedance"))
+		} else if (of_property_read_bool(of_node, "ti,min-output-impedance")) {
 			dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
-		else
+		} else if (of_property_read_u32(of_node, "ti,output-impedance-ohms", &val) == 0) {
+			val = val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK;
+			if (val > DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN ||
+			    val < DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX) {
+				phydev_err(phydev, "'ti,output-impedance-ohms' contents out of range\n");
+				return -ERANGE;
+			}
+			dp83867->io_impedance = val;
+		} else {
 			dp83867->io_impedance = -1; /* leave at default */
+		}
 
 		return 0;
 	}

-- 
2.54.0

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

[Conor Dooley]

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

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.
pw-bot: changes-requested

Can't you just achieve the same result using the existing max property?
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?

Thanks,
Conor.

> +
>    ti,min-output-impedance:
>      type: boolean
>      description: |
> @@ -121,6 +131,12 @@ allOf:
>        required: [nvmem-cells, "ti,min-output-impedance"]
>    - not:
>        required: [nvmem-cells, "ti,max-output-impedance"]
> +  - not:
> +      required: [nvmem-cells, "ti,output-impedance-ohms"]
> +  - not:
> +      required: ["ti,output-impedance-ohms", "ti,min-output-impedance"]
> +  - not:
> +      required: ["ti,output-impedance-ohms", "ti,max-output-impedance"]
>  
>  required:
>    - reg
> 
> -- 
> 2.54.0
> 

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

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

[Steffen Trumtrar]

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

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".

> 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?

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.

Best regards,
Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

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

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

[Andrew Lunn]

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?

No. DT uses real unit, Ohms, Meters, Volts, Degrees Centigrade, Seconds
etc.

Have the driver do the conversion from Ohms to the magical register
value.

	Andrew

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

[Steffen Trumtrar]

On 2026-08-27 at 14:47 +02, Andrew Lunn <andrew@lunn.ch> wrote:

> 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?
> 
> No. DT uses real unit, Ohms, Meters, Volts, Degrees Centigrade, Seconds
> etc.
> 
> Have the driver do the conversion from Ohms to the magical register
> value.

Fair enough and I agree, but the datasheet says:

Output impedance approximate range from 35-70 Ohm in 32 steps. Lowest being 11111 and highest being 00000. Range and step size will vary with process.

I mean, the difference will be miniscule, but the binding can't specify the exact ohm value, because it is not guaranteed to be correct. There are more values than steps and even the range is only an approximate range.

So, just allow values between 35 and 70 in the binding and then convert that value to something near the truth in the driver?


Best regards,
Steffen

-- 
Pengutronix e.K.                | Dipl.-Inform. Steffen Trumtrar |
Steuerwalder Str. 21            | https://www.pengutronix.de/    |
31137 Hildesheim, Germany       | Phone: +49-5121-206917-0       |
Amtsgericht Hildesheim, HRA 2686| Fax:   +49-5121-206917-5555    |

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

[Andrew Lunn]

> So, just allow values between 35 and 70 in the binding and then
> convert that value to something near the truth in the driver?

Yes. And you can add a comment that the values are not exact, due to
process issues.

	Andrew

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

[Conor Dooley]

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