[PATCH] net: phy: dp83869: fix default value for tx-/rx-internal-delay

[PATCH] net: phy: dp83869: fix default value for tx-/rx-internal-delay

[Josua Mayer]

dp83869 internally uses a look-up table for mapping supported delays in
nanoseconds to register values.
When specific delays are defined in device-tree, phy_get_internal_delay
does the lookup automatically returning an index.

The default case wrongly assigns the nanoseconds value from the lookup
table, resulting in numeric value 2000 applied to delay configuration
register, rather than the expected index values 0-7 (7 for 2000).
Ultimately this issue broke RX for 1Gbps links.

Fix default delay configuration by assigning the intended index value
directly.

Co-developed-by: Yazan Shhady <yazan.shhady@solid-run.com>
Signed-off-by: Yazan Shhady <yazan.shhady@solid-run.com>
Signed-off-by: Josua Mayer <josua@solid-run.com>
---
 drivers/net/phy/dp83869.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
index b4ff9c5073a3c..9ab5eff502b71 100644
--- a/drivers/net/phy/dp83869.c
+++ b/drivers/net/phy/dp83869.c
@@ -588,15 +588,13 @@ static int dp83869_of_init(struct phy_device *phydev)
 						       &dp83869_internal_delay[0],
 						       delay_size, true);
 	if (dp83869->rx_int_delay < 0)
-		dp83869->rx_int_delay =
-				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
+		dp83869->rx_int_delay = DP83869_CLK_DELAY_DEF;
 
 	dp83869->tx_int_delay = phy_get_internal_delay(phydev, dev,
 						       &dp83869_internal_delay[0],
 						       delay_size, false);
 	if (dp83869->tx_int_delay < 0)
-		dp83869->tx_int_delay =
-				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
+		dp83869->tx_int_delay = DP83869_CLK_DELAY_DEF;
 
 	return ret;
 }
-- 
2.35.3

Re: [PATCH] net: phy: dp83869: fix default value for tx-/rx-internal-delay

[Simon Horman]

On Thu, Mar 23, 2023 at 12:25:36PM +0200, Josua Mayer wrote:
> dp83869 internally uses a look-up table for mapping supported delays in
> nanoseconds to register values.
> When specific delays are defined in device-tree, phy_get_internal_delay
> does the lookup automatically returning an index.
> 
> The default case wrongly assigns the nanoseconds value from the lookup
> table, resulting in numeric value 2000 applied to delay configuration
> register, rather than the expected index values 0-7 (7 for 2000).
> Ultimately this issue broke RX for 1Gbps links.
> 
> Fix default delay configuration by assigning the intended index value
> directly.
> 
> Co-developed-by: Yazan Shhady <yazan.shhady@solid-run.com>
> Signed-off-by: Yazan Shhady <yazan.shhady@solid-run.com>
> Signed-off-by: Josua Mayer <josua@solid-run.com>

I wonder if this warrants:

Cc: stable@vger.kernel.org
736b25afe284 ("net: dp83869: Add RGMII internal delay configuration")

And being targeted at 'net' ([PATCH net] in subject).

In any case, the fix seems correct to me.

Reviewed-by: Simon Horman <simon.horman@corigine.com>

> ---
>  drivers/net/phy/dp83869.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/phy/dp83869.c b/drivers/net/phy/dp83869.c
> index b4ff9c5073a3c..9ab5eff502b71 100644
> --- a/drivers/net/phy/dp83869.c
> +++ b/drivers/net/phy/dp83869.c
> @@ -588,15 +588,13 @@ static int dp83869_of_init(struct phy_device *phydev)
>  						       &dp83869_internal_delay[0],
>  						       delay_size, true);
>  	if (dp83869->rx_int_delay < 0)
> -		dp83869->rx_int_delay =
> -				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
> +		dp83869->rx_int_delay = DP83869_CLK_DELAY_DEF;
>  
>  	dp83869->tx_int_delay = phy_get_internal_delay(phydev, dev,
>  						       &dp83869_internal_delay[0],
>  						       delay_size, false);
>  	if (dp83869->tx_int_delay < 0)
> -		dp83869->tx_int_delay =
> -				dp83869_internal_delay[DP83869_CLK_DELAY_DEF];
> +		dp83869->tx_int_delay = DP83869_CLK_DELAY_DEF;
>  
>  	return ret;
>  }
> -- 
> 2.35.3
> 

Re: [PATCH] net: phy: dp83869: fix default value for tx-/rx-internal-delay

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 23 Mar 2023 12:25:36 +0200 you wrote:
> dp83869 internally uses a look-up table for mapping supported delays in
> nanoseconds to register values.
> When specific delays are defined in device-tree, phy_get_internal_delay
> does the lookup automatically returning an index.
> 
> The default case wrongly assigns the nanoseconds value from the lookup
> table, resulting in numeric value 2000 applied to delay configuration
> register, rather than the expected index values 0-7 (7 for 2000).
> Ultimately this issue broke RX for 1Gbps links.
> 
> [...]

Here is the summary with links:
  - net: phy: dp83869: fix default value for tx-/rx-internal-delay
    https://git.kernel.org/netdev/net/c/82e2c39f9ef7

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html