[PATCH net 0/3] amd-xgbe: fixes to handle corner-cases

[PATCH net 0/3] amd-xgbe: fixes to handle corner-cases

[Raju Rangoju]

This series include bug fixes to amd-xgbe driver.

Raju Rangoju (3):
  amd-xgbe: handle corner-case during sfp hotplug
  amd-xgbe: handle the corner-case during tx completion
  amd-xgbe: propagate the correct speed and duplex status

 drivers/net/ethernet/amd/xgbe/xgbe-drv.c     | 14 ++++++++++++++
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 11 ++++++++---
 drivers/net/ethernet/amd/xgbe/xgbe-mdio.c    | 14 +++++++++++++-
 3 files changed, 35 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH net 1/3] amd-xgbe: handle corner-case during sfp hotplug

[Raju Rangoju]

Force the mode change for SFI in Fixed PHY configurations. Fixed PHY
configurations needs PLL to be enabled while doing mode set. When the
SFP module isn't connected during boot, driver assumes AN is ON and
attempts auto-negotiation. However, if the connected SFP comes up in
Fixed PHY configuration the link will not come up as PLL isn't enabled
while the initial mode set command is issued. So, force the mode change
for SFI in Fixed PHY configuration to fix link issues.

Fixes: e57f7a3feaef ("amd-xgbe: Prepare for working with more than one type of phy")
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
index 32d2c6fac652..4a2dc705b528 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
@@ -1193,7 +1193,19 @@ static int xgbe_phy_config_fixed(struct xgbe_prv_data *pdata)
 	if (pdata->phy.duplex != DUPLEX_FULL)
 		return -EINVAL;
 
-	xgbe_set_mode(pdata, mode);
+	/* Force the mode change for SFI in Fixed PHY config.
+	 * Fixed PHY configs needs PLL to be enabled while doing mode set.
+	 * When the SFP module isn't connected during boot, driver assumes
+	 * AN is ON and attempts autonegotiation. However, if the connected
+	 * SFP comes up in Fixed PHY config, the link will not come up as
+	 * PLL isn't enabled while the initial mode set command is issued.
+	 * So, force the mode change for SFI in Fixed PHY configuration to
+	 * fix link issues.
+	 */
+	if (mode == XGBE_MODE_SFI)
+		xgbe_change_mode(pdata, mode);
+	else
+		xgbe_set_mode(pdata, mode);
 
 	return 0;
 }
-- 
2.25.1

[PATCH net 2/3] amd-xgbe: handle the corner-case during tx completion

[Raju Rangoju]

The existing implementation uses software logic to accumulate tx
completions until the specified time (1ms) is met and then poll them.
However, there exists a tiny gap which leads to a race between
resetting and checking the tx_activate flag. Due to this the tx
completions are not reported to upper layer and tx queue timeout
kicks-in restarting the device.

To address this, introduce a tx cleanup mechanism as part of the
periodic maintenance process.

Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 614c0278419b..6b73648b3779 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -682,10 +682,24 @@ static void xgbe_service(struct work_struct *work)
 static void xgbe_service_timer(struct timer_list *t)
 {
 	struct xgbe_prv_data *pdata = from_timer(pdata, t, service_timer);
+	struct xgbe_channel *channel;
+	unsigned int i;
 
 	queue_work(pdata->dev_workqueue, &pdata->service_work);
 
 	mod_timer(&pdata->service_timer, jiffies + HZ);
+
+	if (!pdata->tx_usecs)
+		return;
+
+	for (i = 0; i < pdata->channel_count; i++) {
+		channel = pdata->channel[i];
+		if (!channel->tx_ring || channel->tx_timer_active)
+			break;
+		channel->tx_timer_active = 1;
+		mod_timer(&channel->tx_timer,
+			  jiffies + usecs_to_jiffies(pdata->tx_usecs));
+	}
 }
 
 static void xgbe_init_timers(struct xgbe_prv_data *pdata)
-- 
2.25.1

[PATCH net 3/3] amd-xgbe: propagate the correct speed and duplex status

[Raju Rangoju]

xgbe_get_link_ksettings() does not propagate correct speed and duplex
information to ethtool during cable unplug. Due to which ethtool reports
incorrect values for speed and duplex.

Address this by propagating correct information.

Fixes: 7c12aa08779c ("amd-xgbe: Move the PHY support into amd-xgbe")
Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
index 6e83ff59172a..32fab5e77246 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
@@ -314,10 +314,15 @@ static int xgbe_get_link_ksettings(struct net_device *netdev,
 
 	cmd->base.phy_address = pdata->phy.address;
 
-	cmd->base.autoneg = pdata->phy.autoneg;
-	cmd->base.speed = pdata->phy.speed;
-	cmd->base.duplex = pdata->phy.duplex;
+	if (netif_carrier_ok(netdev)) {
+		cmd->base.speed = pdata->phy.speed;
+		cmd->base.duplex = pdata->phy.duplex;
+	} else {
+		cmd->base.speed = SPEED_UNKNOWN;
+		cmd->base.duplex = DUPLEX_UNKNOWN;
+	}
 
+	cmd->base.autoneg = pdata->phy.autoneg;
 	cmd->base.port = PORT_NONE;
 
 	XGBE_LM_COPY(cmd, supported, lks, supported);
-- 
2.25.1

Re: [PATCH net 1/3] amd-xgbe: handle corner-case during sfp hotplug

[Wojciech Drewek]

On 21.11.2023 20:14, Raju Rangoju wrote:
> Force the mode change for SFI in Fixed PHY configurations. Fixed PHY
> configurations needs PLL to be enabled while doing mode set. When the
> SFP module isn't connected during boot, driver assumes AN is ON and
> attempts auto-negotiation. However, if the connected SFP comes up in
> Fixed PHY configuration the link will not come up as PLL isn't enabled
> while the initial mode set command is issued. So, force the mode change
> for SFI in Fixed PHY configuration to fix link issues.
> 
> Fixes: e57f7a3feaef ("amd-xgbe: Prepare for working with more than one type of phy")
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---

Thanks for our patches!
Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/amd/xgbe/xgbe-mdio.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> index 32d2c6fac652..4a2dc705b528 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-mdio.c
> @@ -1193,7 +1193,19 @@ static int xgbe_phy_config_fixed(struct xgbe_prv_data *pdata)
>  	if (pdata->phy.duplex != DUPLEX_FULL)
>  		return -EINVAL;
>  
> -	xgbe_set_mode(pdata, mode);
> +	/* Force the mode change for SFI in Fixed PHY config.
> +	 * Fixed PHY configs needs PLL to be enabled while doing mode set.
> +	 * When the SFP module isn't connected during boot, driver assumes
> +	 * AN is ON and attempts autonegotiation. However, if the connected
> +	 * SFP comes up in Fixed PHY config, the link will not come up as
> +	 * PLL isn't enabled while the initial mode set command is issued.
> +	 * So, force the mode change for SFI in Fixed PHY configuration to
> +	 * fix link issues.
> +	 */
> +	if (mode == XGBE_MODE_SFI)
> +		xgbe_change_mode(pdata, mode);
> +	else
> +		xgbe_set_mode(pdata, mode);
>  
>  	return 0;
>  }

Re: [PATCH net 2/3] amd-xgbe: handle the corner-case during tx completion

[Wojciech Drewek]

On 21.11.2023 20:14, Raju Rangoju wrote:
> The existing implementation uses software logic to accumulate tx
> completions until the specified time (1ms) is met and then poll them.
> However, there exists a tiny gap which leads to a race between
> resetting and checking the tx_activate flag. Due to this the tx
> completions are not reported to upper layer and tx queue timeout
> kicks-in restarting the device.
> 
> To address this, introduce a tx cleanup mechanism as part of the
> periodic maintenance process.
> 
> Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 614c0278419b..6b73648b3779 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -682,10 +682,24 @@ static void xgbe_service(struct work_struct *work)
>  static void xgbe_service_timer(struct timer_list *t)
>  {
>  	struct xgbe_prv_data *pdata = from_timer(pdata, t, service_timer);
> +	struct xgbe_channel *channel;
> +	unsigned int i;
>  
>  	queue_work(pdata->dev_workqueue, &pdata->service_work);
>  
>  	mod_timer(&pdata->service_timer, jiffies + HZ);
> +
> +	if (!pdata->tx_usecs)
> +		return;
> +
> +	for (i = 0; i < pdata->channel_count; i++) {
> +		channel = pdata->channel[i];
> +		if (!channel->tx_ring || channel->tx_timer_active)
> +			break;
> +		channel->tx_timer_active = 1;
> +		mod_timer(&channel->tx_timer,
> +			  jiffies + usecs_to_jiffies(pdata->tx_usecs));
> +	}
>  }
>  
>  static void xgbe_init_timers(struct xgbe_prv_data *pdata)

Re: [PATCH net 3/3] amd-xgbe: propagate the correct speed and duplex status

[Wojciech Drewek]

On 21.11.2023 20:14, Raju Rangoju wrote:
> xgbe_get_link_ksettings() does not propagate correct speed and duplex
> information to ethtool during cable unplug. Due to which ethtool reports
> incorrect values for speed and duplex.
> 
> Address this by propagating correct information.
> 
> Fixes: 7c12aa08779c ("amd-xgbe: Move the PHY support into amd-xgbe")
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---

Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>

>  drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> index 6e83ff59172a..32fab5e77246 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c
> @@ -314,10 +314,15 @@ static int xgbe_get_link_ksettings(struct net_device *netdev,
>  
>  	cmd->base.phy_address = pdata->phy.address;
>  
> -	cmd->base.autoneg = pdata->phy.autoneg;
> -	cmd->base.speed = pdata->phy.speed;
> -	cmd->base.duplex = pdata->phy.duplex;
> +	if (netif_carrier_ok(netdev)) {
> +		cmd->base.speed = pdata->phy.speed;
> +		cmd->base.duplex = pdata->phy.duplex;
> +	} else {
> +		cmd->base.speed = SPEED_UNKNOWN;
> +		cmd->base.duplex = DUPLEX_UNKNOWN;
> +	}
>  
> +	cmd->base.autoneg = pdata->phy.autoneg;
>  	cmd->base.port = PORT_NONE;
>  
>  	XGBE_LM_COPY(cmd, supported, lks, supported);

Re: [PATCH net 0/3] amd-xgbe: fixes to handle corner-cases

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 22 Nov 2023 00:44:32 +0530 you wrote:
> This series include bug fixes to amd-xgbe driver.
> 
> Raju Rangoju (3):
>   amd-xgbe: handle corner-case during sfp hotplug
>   amd-xgbe: handle the corner-case during tx completion
>   amd-xgbe: propagate the correct speed and duplex status
> 
> [...]

Here is the summary with links:
  - [net,1/3] amd-xgbe: handle corner-case during sfp hotplug
    https://git.kernel.org/netdev/net/c/676ec53844cb
  - [net,2/3] amd-xgbe: handle the corner-case during tx completion
    https://git.kernel.org/netdev/net/c/7121205d5330
  - [net,3/3] amd-xgbe: propagate the correct speed and duplex status
    https://git.kernel.org/netdev/net/c/7a2323ac24a5

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

Re: [PATCH net 2/3] amd-xgbe: handle the corner-case during tx completion

[Tom Lendacky]

On 11/21/23 13:14, Raju Rangoju wrote:
> The existing implementation uses software logic to accumulate tx
> completions until the specified time (1ms) is met and then poll them.
> However, there exists a tiny gap which leads to a race between
> resetting and checking the tx_activate flag. Due to this the tx
> completions are not reported to upper layer and tx queue timeout
> kicks-in restarting the device.
> 
> To address this, introduce a tx cleanup mechanism as part of the
> periodic maintenance process.

This looks to just be a work-around that happens to work (for now) and the 
actual race condition should be fixed.

Thanks,
Tom

> 
> Fixes: c5aa9e3b8156 ("amd-xgbe: Initial AMD 10GbE platform driver")
> Acked-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> index 614c0278419b..6b73648b3779 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
> @@ -682,10 +682,24 @@ static void xgbe_service(struct work_struct *work)
>   static void xgbe_service_timer(struct timer_list *t)
>   {
>   	struct xgbe_prv_data *pdata = from_timer(pdata, t, service_timer);
> +	struct xgbe_channel *channel;
> +	unsigned int i;
>   
>   	queue_work(pdata->dev_workqueue, &pdata->service_work);
>   
>   	mod_timer(&pdata->service_timer, jiffies + HZ);
> +
> +	if (!pdata->tx_usecs)
> +		return;
> +
> +	for (i = 0; i < pdata->channel_count; i++) {
> +		channel = pdata->channel[i];
> +		if (!channel->tx_ring || channel->tx_timer_active)
> +			break;
> +		channel->tx_timer_active = 1;
> +		mod_timer(&channel->tx_timer,
> +			  jiffies + usecs_to_jiffies(pdata->tx_usecs));
> +	}
>   }
>   
>   static void xgbe_init_timers(struct xgbe_prv_data *pdata)