Re: [PATCH net-next v8 9/9] net: txgbe: Support phylink MAC layer

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2023-05-15 at 14:32 +0800, Jiawen Wu wrote:
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index ded04e9e136f..bdf735e863eb 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> @@ -7,6 +7,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/string.h>
>  #include <linux/etherdevice.h>
> +#include <linux/phylink.h>
>  #include <net/ip.h>
>  #include <linux/if_vlan.h>
>  
> @@ -204,7 +205,8 @@ static int txgbe_request_irq(struct wx *wx)
>  
>  static void txgbe_up_complete(struct wx *wx)
>  {
> -	u32 reg;
> +	struct net_device *netdev = wx->netdev;
> +	struct txgbe *txgbe = netdev_to_txgbe(netdev);

Please respect the reverse x-mass tree order. Either avoid 'netdev'
declaration or split txgbe declaration and assignment.

>  
>  	wx_control_hw(wx, true);
>  	wx_configure_vectors(wx);
> @@ -213,24 +215,16 @@ static void txgbe_up_complete(struct wx *wx)
>  	smp_mb__before_atomic();
>  	wx_napi_enable_all(wx);
>  
> +	phylink_start(txgbe->phylink);
> +
>  	/* clear any pending interrupts, may auto mask */
>  	rd32(wx, WX_PX_IC(0));
>  	rd32(wx, WX_PX_IC(1));
>  	rd32(wx, WX_PX_MISC_IC);
>  	txgbe_irq_enable(wx, true);
>  
> -	/* Configure MAC Rx and Tx when link is up */
> -	reg = rd32(wx, WX_MAC_RX_CFG);
> -	wr32(wx, WX_MAC_RX_CFG, reg);
> -	wr32(wx, WX_MAC_PKT_FLT, WX_MAC_PKT_FLT_PR);
> -	reg = rd32(wx, WX_MAC_WDG_TIMEOUT);
> -	wr32(wx, WX_MAC_WDG_TIMEOUT, reg);
> -	reg = rd32(wx, WX_MAC_TX_CFG);
> -	wr32(wx, WX_MAC_TX_CFG, (reg & ~WX_MAC_TX_CFG_SPEED_MASK) | WX_MAC_TX_CFG_SPEED_10G);
> -
>  	/* enable transmits */
> -	netif_tx_start_all_queues(wx->netdev);
> -	netif_carrier_on(wx->netdev);
> +	netif_tx_start_all_queues(netdev);
>  }
>  
>  static void txgbe_reset(struct wx *wx)
> @@ -264,7 +258,6 @@ static void txgbe_disable_device(struct wx *wx)
>  		wx_disable_rx_queue(wx, wx->rx_ring[i]);
>  
>  	netif_tx_stop_all_queues(netdev);
> -	netif_carrier_off(netdev);
>  	netif_tx_disable(netdev);
>  
>  	wx_irq_disable(wx);
> @@ -295,8 +288,12 @@ static void txgbe_disable_device(struct wx *wx)
>  
>  static void txgbe_down(struct wx *wx)
>  {
> +	struct net_device *netdev = wx->netdev;
> +	struct txgbe *txgbe = netdev_to_txgbe(netdev);

You can replace the above 2 lines with:

	struct txgbe *txgbe = netdev_to_txgbe(wx->netdev);


Cheers,

Paolo