[PATCH] net: phy: dp83822: fix transmit amplitude if CONFIG_OF_MDIO not defined

[PATCH] net: phy: dp83822: fix transmit amplitude if CONFIG_OF_MDIO not defined

[Dimitri Fedrau via B4 Relay]

From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>

When CONFIG_OF_MDIO is not defined the index for selecting the transmit
amplitude voltage for 100BASE-TX is set to 0, but it should be -1, if there
is no need to modify the transmit amplitude voltage. Add a flag to make
sure there is a need to modify it.

Fixes: 4f3735e82d8a ("net: phy: dp83822: Add support for changing the transmit amplitude voltage")
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
 drivers/net/phy/dp83822.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 3662f3905d5ade8ad933608fcaeabb714a588418..d69000cb0ceff28e8288ba24e0af1c960ea9cc97 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -201,6 +201,7 @@ struct dp83822_private {
 	bool set_gpio2_clk_out;
 	u32 gpio2_clk_out;
 	bool led_pin_enable[DP83822_MAX_LED_PINS];
+	bool tx_amplitude_100base_tx_modify;
 	int tx_amplitude_100base_tx_index;
 };
 
@@ -527,7 +528,7 @@ static int dp83822_config_init(struct phy_device *phydev)
 			       FIELD_PREP(DP83822_IOCTRL2_GPIO2_CLK_SRC,
 					  dp83822->gpio2_clk_out));
 
-	if (dp83822->tx_amplitude_100base_tx_index >= 0)
+	if (dp83822->tx_amplitude_100base_tx_modify)
 		phy_modify_mmd(phydev, MDIO_MMD_VEND2, MII_DP83822_LDCTRL,
 			       DP83822_100BASE_TX_LINE_DRIVER_SWING,
 			       FIELD_PREP(DP83822_100BASE_TX_LINE_DRIVER_SWING,
@@ -851,6 +852,8 @@ static int dp83822_of_init(struct phy_device *phydev)
 				   val);
 			return -EINVAL;
 		}
+
+		dp83822->tx_amplitude_100base_tx_modify = true;
 	}
 
 	return dp83822_of_init_leds(phydev);

---
base-commit: c62e6f056ea308d6382450c1cb32e41727375885
change-id: 20250307-dp83822-fix-transceiver-mdio-ae27fed80699

Best regards,
-- 
Dimitri Fedrau <dimitri.fedrau@liebherr.com>

Re: [PATCH] net: phy: dp83822: fix transmit amplitude if CONFIG_OF_MDIO not defined

[Gerhard Engleder]

On 12.03.25 18:23, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> 
> When CONFIG_OF_MDIO is not defined the index for selecting the transmit
> amplitude voltage for 100BASE-TX is set to 0, but it should be -1, if there
> is no need to modify the transmit amplitude voltage. Add a flag to make
> sure there is a need to modify it.
> 
> Fixes: 4f3735e82d8a ("net: phy: dp83822: Add support for changing the transmit amplitude voltage")
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> ---
>   drivers/net/phy/dp83822.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index 3662f3905d5ade8ad933608fcaeabb714a588418..d69000cb0ceff28e8288ba24e0af1c960ea9cc97 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -201,6 +201,7 @@ struct dp83822_private {
>   	bool set_gpio2_clk_out;
>   	u32 gpio2_clk_out;
>   	bool led_pin_enable[DP83822_MAX_LED_PINS];
> +	bool tx_amplitude_100base_tx_modify;
>   	int tx_amplitude_100base_tx_index;
>   };

You could instead init tx_amplitude_100base_tx_index in
dp8382x_probe() to -1.

But functional it should be ok.

Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>

Gerhard

Re: [PATCH] net: phy: dp83822: fix transmit amplitude if CONFIG_OF_MDIO not defined

[Dimitri Fedrau]

Am Wed, Mar 12, 2025 at 08:53:29PM +0100 schrieb Gerhard Engleder:
> On 12.03.25 18:23, Dimitri Fedrau via B4 Relay wrote:
> > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > 
> > When CONFIG_OF_MDIO is not defined the index for selecting the transmit
> > amplitude voltage for 100BASE-TX is set to 0, but it should be -1, if there
> > is no need to modify the transmit amplitude voltage. Add a flag to make
> > sure there is a need to modify it.
> > 
> > Fixes: 4f3735e82d8a ("net: phy: dp83822: Add support for changing the transmit amplitude voltage")
> > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
> > ---
> >   drivers/net/phy/dp83822.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index 3662f3905d5ade8ad933608fcaeabb714a588418..d69000cb0ceff28e8288ba24e0af1c960ea9cc97 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -201,6 +201,7 @@ struct dp83822_private {
> >   	bool set_gpio2_clk_out;
> >   	u32 gpio2_clk_out;
> >   	bool led_pin_enable[DP83822_MAX_LED_PINS];
> > +	bool tx_amplitude_100base_tx_modify;
> >   	int tx_amplitude_100base_tx_index;
> >   };
> 
> You could instead init tx_amplitude_100base_tx_index in
> dp8382x_probe() to -1.
> 
> But functional it should be ok.
> 
> Reviewed-by: Gerhard Engleder <gerhard@engleder-embedded.com>
>
Hi Gerhard,

will send out an V2 implementing your proposal. Thanks for reviewing.

Best regards,
Dimitri Fedrau