[PATCH net] net: txgbe: fix the issue of TX failture

[PATCH net] net: txgbe: fix the issue of TX failture

[Jiawen Wu]

There is a occasional problem that ping is failed between AML devices.
That is because the manual enablement of the security Tx path on the
hardware is missing, no matter what its previous state was.

Fixes: 6f8b4c01a8cd ("net: txgbe: Implement PHYLINK for AML 25G/10G devices")
Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
---
 drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
index 7dbcf41750c1..dc87ccad9652 100644
--- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
+++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
@@ -294,6 +294,7 @@ static void txgbe_mac_link_up_aml(struct phylink_config *config,
 	wx_fc_enable(wx, tx_pause, rx_pause);
 
 	txgbe_reconfig_mac(wx);
+	txgbe_enable_sec_tx_path(wx);
 
 	txcfg = rd32(wx, TXGBE_AML_MAC_TX_CFG);
 	txcfg &= ~TXGBE_AML_MAC_TX_CFG_SPEED_MASK;
-- 
2.48.1

Re: [PATCH net] net: txgbe: fix the issue of TX failture

[Simon Horman]

On Tue, Jun 24, 2025 at 05:30:21PM +0800, Jiawen Wu wrote:
> There is a occasional problem that ping is failed between AML devices.
> That is because the manual enablement of the security Tx path on the
> hardware is missing, no matter what its previous state was.
> 
> Fixes: 6f8b4c01a8cd ("net: txgbe: Implement PHYLINK for AML 25G/10G devices")
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

nit: failure is misspelt in the subject

> ---
>  drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> index 7dbcf41750c1..dc87ccad9652 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> @@ -294,6 +294,7 @@ static void txgbe_mac_link_up_aml(struct phylink_config *config,
>  	wx_fc_enable(wx, tx_pause, rx_pause);
>  
>  	txgbe_reconfig_mac(wx);
> +	txgbe_enable_sec_tx_path(wx);
>  
>  	txcfg = rd32(wx, TXGBE_AML_MAC_TX_CFG);
>  	txcfg &= ~TXGBE_AML_MAC_TX_CFG_SPEED_MASK;

Hi Jiawen,

I am unsure if it is important, but I notice that:

* txgbe_mac_link_up_aml is the mac_link_up callback for txgbe_aml

* Whereas for txgbe_phy txgbe_enable_sec_tx_path() is called in
  txgbe_mac_finish(), which is the mac_finish callback

Could you comment on this asymmetry?

RE: [PATCH net] net: txgbe: fix the issue of TX failture

[Jiawen Wu]

On Wed, Jun 25, 2025 5:36 PM, Simon Horman wrote:
> On Tue, Jun 24, 2025 at 05:30:21PM +0800, Jiawen Wu wrote:
> > There is a occasional problem that ping is failed between AML devices.
> > That is because the manual enablement of the security Tx path on the
> > hardware is missing, no matter what its previous state was.
> >
> > Fixes: 6f8b4c01a8cd ("net: txgbe: Implement PHYLINK for AML 25G/10G devices")
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 
> nit: failure is misspelt in the subject

Thanks. :)

> 
> > ---
> >  drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > index 7dbcf41750c1..dc87ccad9652 100644
> > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > @@ -294,6 +294,7 @@ static void txgbe_mac_link_up_aml(struct phylink_config *config,
> >  	wx_fc_enable(wx, tx_pause, rx_pause);
> >
> >  	txgbe_reconfig_mac(wx);
> > +	txgbe_enable_sec_tx_path(wx);
> >
> >  	txcfg = rd32(wx, TXGBE_AML_MAC_TX_CFG);
> >  	txcfg &= ~TXGBE_AML_MAC_TX_CFG_SPEED_MASK;
> 
> Hi Jiawen,
> 
> I am unsure if it is important, but I notice that:
> 
> * txgbe_mac_link_up_aml is the mac_link_up callback for txgbe_aml
> 
> * Whereas for txgbe_phy txgbe_enable_sec_tx_path() is called in
>   txgbe_mac_finish(), which is the mac_finish callback
> 
> Could you comment on this asymmetry?

For txgbe_sp, the configuration of PCS is completed in the driver.
Disable sec_tx_path -> configure PCS -> enable sec_tx_path, it is
the necessary sequence. So these MAC operations were added in
txgbe_mac_prepare() and txgbe_mac_finish().

For txgbe_aml, the configuration of PCS is completed in the firmware.
So I didn't implement .mac_prepare and .mac_finish.

Re: [PATCH net] net: txgbe: fix the issue of TX failture

[Simon Horman]

On Wed, Jun 25, 2025 at 05:51:19PM +0800, Jiawen Wu wrote:
> On Wed, Jun 25, 2025 5:36 PM, Simon Horman wrote:
> > On Tue, Jun 24, 2025 at 05:30:21PM +0800, Jiawen Wu wrote:
> > > There is a occasional problem that ping is failed between AML devices.
> > > That is because the manual enablement of the security Tx path on the
> > > hardware is missing, no matter what its previous state was.
> > >
> > > Fixes: 6f8b4c01a8cd ("net: txgbe: Implement PHYLINK for AML 25G/10G devices")
> > > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > 
> > nit: failure is misspelt in the subject
> 
> Thanks. :)
> 
> > 
> > > ---
> > >  drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > > index 7dbcf41750c1..dc87ccad9652 100644
> > > --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > > +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_aml.c
> > > @@ -294,6 +294,7 @@ static void txgbe_mac_link_up_aml(struct phylink_config *config,
> > >  	wx_fc_enable(wx, tx_pause, rx_pause);
> > >
> > >  	txgbe_reconfig_mac(wx);
> > > +	txgbe_enable_sec_tx_path(wx);
> > >
> > >  	txcfg = rd32(wx, TXGBE_AML_MAC_TX_CFG);
> > >  	txcfg &= ~TXGBE_AML_MAC_TX_CFG_SPEED_MASK;
> > 
> > Hi Jiawen,
> > 
> > I am unsure if it is important, but I notice that:
> > 
> > * txgbe_mac_link_up_aml is the mac_link_up callback for txgbe_aml
> > 
> > * Whereas for txgbe_phy txgbe_enable_sec_tx_path() is called in
> >   txgbe_mac_finish(), which is the mac_finish callback
> > 
> > Could you comment on this asymmetry?
> 
> For txgbe_sp, the configuration of PCS is completed in the driver.
> Disable sec_tx_path -> configure PCS -> enable sec_tx_path, it is
> the necessary sequence. So these MAC operations were added in
> txgbe_mac_prepare() and txgbe_mac_finish().
> 
> For txgbe_aml, the configuration of PCS is completed in the firmware.
> So I didn't implement .mac_prepare and .mac_finish.

Thanks, that seems reasonable to me.

Reviewed-by: Simon Horman <horms@kernel.org>