Messages in this thread Patch in this message | | | Subject | Re: [RFC/PATCH] net: nixge: Add PHYLINK support | From | Florian Fainelli <> | Date | Tue, 4 Sep 2018 17:27:54 -0700 |
| |
On 09/04/2018 05:15 PM, Moritz Fischer wrote: > Add basic PHYLINK support to driver. > > Suggested-by: Andrew Lunn <andrew@lunn.ch> > Signed-off-by: Moritz Fischer <mdf@kernel.org> > --- > > Hi all, > > as Andrew suggested in order to enable SFP as > well as fixed-link support add PHYLINK support. > > A couple of questions are still open (hence the RFC): > > 1) It seems odd to implement PHYLINK callbacks that > are all empty? If so, should we have generic empty > ones in drivers/net/phy/phylink.c like we have for > genphys?
Yes it is odd, the validate callback most certainly should not be empty, neither should the mac_config and mac_link_{up,down}, but, with some luck, you can get things to just work, typically with MDIO PHYs, since a large amount of what they can do is discoverable.
If you had an existing adjust_link callback from PHYLIB, it's really about breaking it down such that the MAC configuration of speed/duplex/pause happens in mac_config, and the link setting (if necessary), happens in mac_link_{up,down}, and that's pretty much it for MLO_AN_PHY cases.
> > 2) If this is ok, then I'll go ahead rework this with > a DT binding update to deprecate the non-'mdio'-subnode > case (since there are no in-tree users we might just > change the binding)? > > 3) I'm again not sure about the 'select PHYLINK', wouldn't > wanna break the build again... > > Thanks again for your time! > > Moritz > > --- > drivers/net/ethernet/ni/Kconfig | 1 + > drivers/net/ethernet/ni/nixge.c | 115 +++++++++++++++++++++++--------- > 2 files changed, 83 insertions(+), 33 deletions(-) > > diff --git a/drivers/net/ethernet/ni/Kconfig b/drivers/net/ethernet/ni/Kconfig > index c73978474c4b..80cd72948551 100644 > --- a/drivers/net/ethernet/ni/Kconfig > +++ b/drivers/net/ethernet/ni/Kconfig > @@ -21,6 +21,7 @@ config NI_XGE_MANAGEMENT_ENET > depends on HAS_IOMEM && HAS_DMA > select PHYLIB > select OF_MDIO if OF > + select PHYLINK > help > Simple LAN device for debug or management purposes. Can > support either 10G or 1G PHYs via SFP+ ports. > diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c > index 74cf52e3fb09..a0e790d07b1c 100644 > --- a/drivers/net/ethernet/ni/nixge.c > +++ b/drivers/net/ethernet/ni/nixge.c > @@ -11,6 +11,7 @@ > #include <linux/of_mdio.h> > #include <linux/of_net.h> > #include <linux/of_platform.h> > +#include <linux/phylink.h> > #include <linux/of_irq.h> > #include <linux/skbuff.h> > #include <linux/phy.h> > @@ -165,7 +166,7 @@ struct nixge_priv { > struct device *dev; > > /* Connection to PHY device */ > - struct device_node *phy_node; > + struct phylink *phylink; > phy_interface_t phy_mode; > > int link; > @@ -416,20 +417,6 @@ static void nixge_device_reset(struct net_device *ndev) > netif_trans_update(ndev); > } > > -static void nixge_handle_link_change(struct net_device *ndev) > -{ > - struct nixge_priv *priv = netdev_priv(ndev); > - struct phy_device *phydev = ndev->phydev; > - > - if (phydev->link != priv->link || phydev->speed != priv->speed || > - phydev->duplex != priv->duplex) { > - priv->link = phydev->link; > - priv->speed = phydev->speed; > - priv->duplex = phydev->duplex; > - phy_print_status(phydev); > - } > -} > - > static void nixge_tx_skb_unmap(struct nixge_priv *priv, > struct nixge_tx_skb *tx_skb) > { > @@ -859,17 +846,15 @@ static void nixge_dma_err_handler(unsigned long data) > static int nixge_open(struct net_device *ndev) > { > struct nixge_priv *priv = netdev_priv(ndev); > - struct phy_device *phy; > int ret; > > nixge_device_reset(ndev); > > - phy = of_phy_connect(ndev, priv->phy_node, > - &nixge_handle_link_change, 0, priv->phy_mode); > - if (!phy) > - return -ENODEV; > + ret = phylink_of_phy_connect(priv->phylink, priv->dev->of_node, 0); > + if (ret < 0) > + return ret; > > - phy_start(phy); > + phylink_start(priv->phylink); > > /* Enable tasklets for Axi DMA error handling */ > tasklet_init(&priv->dma_err_tasklet, nixge_dma_err_handler, > @@ -893,8 +878,7 @@ static int nixge_open(struct net_device *ndev) > err_rx_irq: > free_irq(priv->tx_irq, ndev); > err_tx_irq: > - phy_stop(phy); > - phy_disconnect(phy); > + phylink_disconnect_phy(priv->phylink); > tasklet_kill(&priv->dma_err_tasklet); > netdev_err(ndev, "request_irq() failed\n"); > return ret; > @@ -908,9 +892,9 @@ static int nixge_stop(struct net_device *ndev) > netif_stop_queue(ndev); > napi_disable(&priv->napi); > > - if (ndev->phydev) { > - phy_stop(ndev->phydev); > - phy_disconnect(ndev->phydev); > + if (priv->phylink) { > + phylink_stop(priv->phylink); > + phylink_disconnect_phy(priv->phylink); > } > > cr = nixge_dma_read_reg(priv, XAXIDMA_RX_CR_OFFSET); > @@ -1076,13 +1060,31 @@ static int nixge_ethtools_set_phys_id(struct net_device *ndev, > return 0; > } > > +static int > +nixge_ethtool_set_link_ksettings(struct net_device *ndev, > + const struct ethtool_link_ksettings *cmd) > +{ > + struct nixge_priv *priv = netdev_priv(ndev); > + > + return phylink_ethtool_ksettings_set(priv->phylink, cmd); > +} > + > +static int > +nixge_ethtool_get_link_ksettings(struct net_device *ndev, > + struct ethtool_link_ksettings *cmd) > +{ > + struct nixge_priv *priv = netdev_priv(ndev); > + > + return phylink_ethtool_ksettings_get(priv->phylink, cmd); > +} > + > static const struct ethtool_ops nixge_ethtool_ops = { > .get_drvinfo = nixge_ethtools_get_drvinfo, > .get_coalesce = nixge_ethtools_get_coalesce, > .set_coalesce = nixge_ethtools_set_coalesce, > .set_phys_id = nixge_ethtools_set_phys_id, > - .get_link_ksettings = phy_ethtool_get_link_ksettings, > - .set_link_ksettings = phy_ethtool_set_link_ksettings, > + .get_link_ksettings = nixge_ethtool_get_link_ksettings, > + .set_link_ksettings = nixge_ethtool_set_link_ksettings, > .get_link = ethtool_op_get_link, > }; > > @@ -1225,11 +1227,52 @@ static void *nixge_get_nvmem_address(struct device *dev) > return mac; > } > > +static void nixge_validate(struct net_device *ndev, unsigned long *supported, > + struct phylink_link_state *state) > +{ > +} > + > +static int nixge_mac_link_state(struct net_device *ndev, > + struct phylink_link_state *state) > +{ > + return 0; > +} > + > +static void nixge_mac_config(struct net_device *ndev, unsigned int mode, > + const struct phylink_link_state *state) > +{ > +} > + > +static void nixge_mac_an_restart(struct net_device *ndev) > +{ > +} > + > +static void nixge_mac_link_down(struct net_device *ndev, unsigned int mode, > + phy_interface_t interface) > +{ > +} > + > +static void nixge_mac_link_up(struct net_device *ndev, unsigned int mode, > + phy_interface_t interface, > + struct phy_device *phy) > +{ > +} > + > +static const struct phylink_mac_ops nixge_phylink_ops = { > + .validate = nixge_validate, > + .mac_link_state = nixge_mac_link_state, > + .mac_an_restart = nixge_mac_an_restart, > + .mac_config = nixge_mac_config, > + .mac_link_down = nixge_mac_link_down, > + .mac_link_up = nixge_mac_link_up, > +}; > + > static int nixge_probe(struct platform_device *pdev) > { > struct nixge_priv *priv; > struct net_device *ndev; > struct resource *dmares; > + struct device_node *mn; > const u8 *mac_addr; > int err; > > @@ -1286,7 +1329,13 @@ static int nixge_probe(struct platform_device *pdev) > priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD; > priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD; > > - err = nixge_mdio_setup(priv, pdev->dev.of_node); > + mn = of_get_child_by_name(pdev->dev.of_node, "mdio"); > + if (!mn) { > + dev_warn(&pdev->dev, "No \"mdio\" subnode found, defaulting to legacy\n"); > + mn = pdev->dev.of_node; > + } > + > + err = nixge_mdio_setup(priv, mn); > if (err) { > netdev_err(ndev, "error registering mdio bus"); > goto free_netdev; > @@ -1299,10 +1348,10 @@ static int nixge_probe(struct platform_device *pdev) > goto unregister_mdio; > } > > - priv->phy_node = of_parse_phandle(pdev->dev.of_node, "phy-handle", 0); > - if (!priv->phy_node) { > - netdev_err(ndev, "not find \"phy-handle\" property\n"); > - err = -EINVAL; > + priv->phylink = phylink_create(ndev, pdev->dev.fwnode, priv->phy_mode, > + &nixge_phylink_ops); > + if (IS_ERR(priv->phylink)) { > + err = PTR_ERR(priv->phylink); > goto unregister_mdio; > } > >
-- Florian
| |