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

Paolo Abeni
2023-05-16T09:44:39Z

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