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

[PATCH 0/2] 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 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>
---
Steffen Trumtrar (2):
      dt-bindings: dp83867: add binding for output-impedance
      net: phy: dp83867: support setting output-impedance

 Documentation/devicetree/bindings/net/ti,dp83867.yaml | 13 ++++++++++---
 drivers/net/phy/dp83867.c                             | 13 ++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260804-v7-2-topic-dp83867-e73a84c37016

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

[PATCH 1/2] 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 for specifying the output-impedance via a fixed value.
The existing boolean values for minimum and maximum still take
precendance over the new value.

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

diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
index 4bc1f98fd9fe5..f43b4e44ef7ab 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
@@ -41,6 +41,12 @@ properties:
     items:
       - const: io_impedance_ctrl
 
+  ti,output-impedance:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      MAC Interface Impedance control to set the programmable output impedance
+      to a value between minimum (35 ohms) and maximum (70 ohms).
+
   ti,min-output-impedance:
     type: boolean
     description: |
@@ -53,10 +59,11 @@ properties:
       MAC Interface Impedance control to set the programmable output impedance
       to a maximum value (70 ohms).
       Note: Specifying an io_impedance_ctrl nvmem cell or one of the
-        ti,min-output-impedance, ti,max-output-impedance properties
-        are mutually exclusive. If more than one is present, an nvmem
+        ti,min-output-impedance, ti,max-output-impedance or ti,output-impedance
+        properties are mutually exclusive. If more than one is present, an nvmem
         cell takes precedence over ti,max-output-impedance, which in
-        turn takes precedence over ti,min-output-impedance.
+        turn takes precedence over ti,min-output-impedance. Both take precedence
+        over ti,output-impedance.
 
   tx-fifo-depth:
     $ref: /schemas/types.yaml#/definitions/uint32

-- 
2.54.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Missing schema constraints for numeric property bounds.
- [Low] Devicetree property naming convention violation (-ohms).
--

--- Patch [1]: [PATCH 1/2] dt-bindings: dp83867: add binding for output-impedance ---
commit 9c13d3ae2983ce2a58c2c0ce9fcf6fedc072b384
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

dt-bindings: dp83867: add binding for output-impedance

This commit adds a new devicetree property, ti,output-impedance, to configure 
the MAC interface output impedance to a fixed value. It also specifies 
precedence rules when used alongside existing impedance configuration properties.

> diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.yaml b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> --- a/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,dp83867.yaml
> @@ -41,6 +41,12 @@ properties:
>      items:
>        - const: io_impedance_ctrl
>  
> +  ti,output-impedance:

[Severity: Low]
This isn't a bug, but should this property name include a standard unit suffix?

According to dtschema conventions for physical units, resistance properties 
typically use a suffix like -ohms or -micro-ohms (e.g., ti,output-impedance-ohms).

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      MAC Interface Impedance control to set the programmable output impedance
> +      to a value between minimum (35 ohms) and maximum (70 ohms).

[Severity: Low]
Should this property definition include minimum: 35 and maximum: 70 constraints?

The documentation mentions the allowed range, but without explicit JSON schema 
constraints, dt_binding_check will silently allow out-of-bounds values in 
device trees.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804-v7-2-topic-dp83867-v1-0-1ca79eb54eef@pengutronix.de?part=1

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

[Andrew Lunn]

> +  ti,output-impedance:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      MAC Interface Impedance control to set the programmable output impedance
> +      to a value between minimum (35 ohms) and maximum (70 ohms).

Please could you express this min/max in yaml, so .dts files can be
verified.

>    ti,min-output-impedance:
>      type: boolean
>      description: |
> @@ -53,10 +59,11 @@ properties:
>        MAC Interface Impedance control to set the programmable output impedance
>        to a maximum value (70 ohms).
>        Note: Specifying an io_impedance_ctrl nvmem cell or one of the
> -        ti,min-output-impedance, ti,max-output-impedance properties
> -        are mutually exclusive. If more than one is present, an nvmem
> +        ti,min-output-impedance, ti,max-output-impedance or ti,output-impedance
> +        properties are mutually exclusive. If more than one is present, an nvmem
>          cell takes precedence over ti,max-output-impedance, which in
> -        turn takes precedence over ti,min-output-impedance.
> +        turn takes precedence over ti,min-output-impedance. Both take precedence
> +        over ti,output-impedance.

You should also be able to express this mutual exclusion in yaml.


    Andrew

---
pw-bot: cr

[PATCH 2/2] 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 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 | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 88255e92b4cdb..e98c5d940410e 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -542,12 +542,19 @@ 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", &val)) {
+			if ((val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK) != val) {
+				phydev_err(phydev, "'ti,output-impedance' 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 2/2] net: phy: dp83867: support setting output-impedance

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Inverted error check on `of_property_read_u32()` ignores the property when present and causes probe failure or garbage configuration when missing.
- [High] Passing a pointer to a 1-byte variable to a function expecting a 4-byte pointer causes a stack buffer overflow.
- [Medium] Meaningful code and commit message mismatch: nvmem takes highest precedence, not lowest.
--

--- Patch [2]: [PATCH 2/2] net: phy: dp83867: support setting output-impedance ---
commit 5d72fffedcf9773506f98861b65736e9f8be835a
Author: Steffen Trumtrar <s.trumtrar@pengutronix.de>

net: phy: dp83867: support setting output-impedance

This commit adds support for the ti,output-impedance devicetree binding to
configure the IO impedance of the DP83867 PHY. It introduces a fallback to
this new property when nvmem is unavailable.

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

[Severity: Medium]
Is this commit message accurate regarding the precedence?

Looking at dp83867_of_init_io_impedance(), it seems the code attempts to
retrieve the nvmem cell first. If that succeeds, the early return prevents
any device tree fallback logic from running:

dp83867_of_init_io_impedance() {
	cell = of_nvmem_cell_get(of_node, "io_impedance_ctrl");
	if (IS_ERR(cell)) {
		// DT fallback logic happens here
		return 0;
	}
	...
}

This would give the nvmem consumer the highest precedence rather than the
lowest. Could the commit message be clarified?

> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c

[ ... ]

> @@ -542,12 +542,19 @@ 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", &val)) {

[Severity: High]
Does this condition check for success properly?

The function of_property_read_u32() returns 0 on success and a negative error
code on failure. Because of this, the if condition evaluates to true when the
property is missing (returning a negative value), and false when it is
successfully read (returning 0).

This means the driver might use an uninitialized stack value for configuration
and fail the probe if the property is absent, while ignoring the property
entirely if it is present.

[Severity: High]
Can this cause a stack buffer overflow?

The variable val is declared as a u8 earlier in dp83867_of_init_io_impedance():

dp83867_of_init_io_impedance() {
	u8 *buf, val;
	...
}

However, of_property_read_u32() expects a u32 pointer. Passing a pointer to
a 1-byte allocation into a function that writes 4 bytes could corrupt adjacent
stack memory.

> +			if ((val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK) != val) {
> +				phydev_err(phydev, "'ti,output-impedance' contents out of range\n");
> +				return -ERANGE;
> +			}
> +			dp83867->io_impedance = val;
> +		} else {
>  			dp83867->io_impedance = -1; /* leave at default */
> +		}
>  
>  		return 0;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804-v7-2-topic-dp83867-v1-0-1ca79eb54eef@pengutronix.de?part=2

Re: [PATCH 2/2] net: phy: dp83867: support setting output-impedance

[Jakub Kicinski]

On Tue, 04 Aug 2026 14:46:34 +0200 Steffen Trumtrar wrote:
> 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 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.

Compiler says no:

drivers/net/phy/dp83867.c:549:67: error: incompatible pointer types passing 'u8 *' (aka 'unsigned char *') to parameter of type 'u32 *' (aka 'unsigned int *') [-Wincompatible-pointer-types]
  549 |                 } else if (of_property_read_u32(of_node, "ti,output-impedance", &val)) {
      |                                                                                 ^~~~
../include/linux/of.h:1491:17: note: passing argument to parameter 'out_value' here
 1491 |                                        u32 *out_value)
      |                                             ^
1 error generated.
make[6]: *** [../scripts/Makefile.build:289: drivers/net/phy/dp83867.o] Error 1
-- 
pw-bot: cr

Re: [PATCH 2/2] net: phy: dp83867: support setting output-impedance

[Andrew Lunn]

On Tue, Aug 04, 2026 at 02:46:34PM +0200, Steffen Trumtrar wrote:
> 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 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 | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 88255e92b4cdb..e98c5d940410e 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -542,12 +542,19 @@ 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", &val)) {
> +			if ((val & DP83867_IO_MUX_CFG_IO_IMPEDANCE_MASK) != val) {
> +				phydev_err(phydev, "'ti,output-impedance' contents out of range\n");

The binding says 35-70. A simple mask operation is not sufficient for
that.

Andrew