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

Russell King (Oracle)
2023-04-11T10:45:26Z

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!