Re: [PATCH net-next v2 6/6] net: txgbe: Support phylink MAC layer

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Apr 11, 2023 at 05:27:25PM +0800, Jiawen Wu wrote:
> Add phylink support to Wangxun 10Gb Ethernet controller, for the 10GBASE-R
> and 1000BASE-X interfaces.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> ---
>  .../ethernet/wangxun/txgbe/txgbe_ethtool.c    |  34 ++++++
>  .../net/ethernet/wangxun/txgbe/txgbe_main.c   |  19 ++-
>  .../net/ethernet/wangxun/txgbe/txgbe_phy.c    | 111 +++++++++++++++++-
>  .../net/ethernet/wangxun/txgbe/txgbe_type.h   |   5 +
>  4 files changed, 156 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> index d914e9a05404..43ca84c90637 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_ethtool.c
> @@ -6,11 +6,45 @@
>  #include <linux/netdevice.h>
>  
>  #include "../libwx/wx_ethtool.h"
> +#include "../libwx/wx_type.h"
> +#include "txgbe_type.h"
>  #include "txgbe_ethtool.h"

I wonder if a small helper would be useful in txgbe_type.h:

static inline struct txgbe *netdev_to_txgbe(struct net_device *netdev)
{
	struct wx *wx = netdev_priv(netdev);

	return wx->priv;
}

> +static int txgbe_nway_reset(struct net_device *netdev)
> +{
> +	struct wx *wx = netdev_priv(netdev);
> +	struct txgbe *txgbe;
> +
> +	txgbe = (struct txgbe *)wx->priv;

Then all of these can be simply:

	struct txgbe *txgbe = netdev_to_txgbe(netdev);

> +	return phylink_ethtool_nway_reset(txgbe->phylink);
> +}
> +
> +static int txgbe_get_link_ksettings(struct net_device *netdev,
> +				    struct ethtool_link_ksettings *cmd)
> +{
> +	struct wx *wx = netdev_priv(netdev);
> +	struct txgbe *txgbe;
> +
> +	txgbe = (struct txgbe *)wx->priv;
> +	return phylink_ethtool_ksettings_get(txgbe->phylink, cmd);
> +}
> +
> +static int txgbe_set_link_ksettings(struct net_device *netdev,
> +				    const struct ethtool_link_ksettings *cmd)
> +{
> +	struct wx *wx = netdev_priv(netdev);
> +	struct txgbe *txgbe;
> +
> +	txgbe = (struct txgbe *)wx->priv;
> +	return phylink_ethtool_ksettings_set(txgbe->phylink, cmd);
> +}
> +
>  static const struct ethtool_ops txgbe_ethtool_ops = {
>  	.get_drvinfo		= wx_get_drvinfo,
> +	.nway_reset		= txgbe_nway_reset,
>  	.get_link		= ethtool_op_get_link,
> +	.get_link_ksettings	= txgbe_get_link_ksettings,
> +	.set_link_ksettings	= txgbe_set_link_ksettings,
>  };
>  
>  void txgbe_set_ethtool_ops(struct net_device *netdev)
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c
> index d8108ab30818..f640ff1a084e 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,7 @@ static int txgbe_request_irq(struct wx *wx)
>  
>  static void txgbe_up_complete(struct wx *wx)
>  {
> -	u32 reg;
> +	struct txgbe *txgbe = (struct txgbe *)wx->priv;

Personal choice I guess, but normally we tend to rely on compilers
accepting the implicit cast from void * to whatever struct pointer
in the kernel.

>  
>  	wx_control_hw(wx, true);
>  	wx_configure_vectors(wx);
> @@ -213,24 +214,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);
>  }
>  
>  static void txgbe_reset(struct wx *wx)
> @@ -264,7 +257,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 +287,11 @@ static void txgbe_disable_device(struct wx *wx)
>  
>  static void txgbe_down(struct wx *wx)
>  {
> +	struct txgbe *txgbe = (struct txgbe *)wx->priv;
> +
>  	txgbe_disable_device(wx);
>  	txgbe_reset(wx);
> +	phylink_stop(txgbe->phylink);
>  
>  	wx_clean_all_tx_rings(wx);
>  	wx_clean_all_rx_rings(wx);
> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> index 123fa7ed9039..84dc3e850036 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_phy.c
> @@ -13,6 +13,7 @@
>  #include <linux/pci.h>
>  
>  #include "../libwx/wx_type.h"
> +#include "../libwx/wx_lib.h"
>  #include "../libwx/wx_hw.h"
>  #include "txgbe_type.h"
>  #include "txgbe_phy.h"
> @@ -445,6 +446,98 @@ static int txgbe_mdio_pcs_init(struct txgbe *txgbe)
>  	return 0;
>  }
>  
> +static struct phylink_pcs *txgbe_phylink_mac_select(struct phylink_config *config,
> +						    phy_interface_t interface)
> +{
> +	struct wx *wx = netdev_priv(to_net_dev(config->dev));
> +	struct txgbe *txgbe = (struct txgbe *)wx->priv;

	struct txgbr *txgbe = netdev_to_txgbe(to_net_dev(config->dev));

> +
> +	return &txgbe->pcs;
> +}
> +

Thanks!

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!