[PATCH v8 1/1] net: ag71xx: port to phylink

[PATCH v8 1/1] net: ag71xx: port to phylink

[Oleksij Rempel]

The port to phylink was done as close as possible to initial
functionality.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v8:
- set the autoneg bit
- provide implementations for the mac_pcs_get_state and mac_an_restart
  methods
- do phylink_disconnect_phy() on _stop()
- rename ag71xx_phy_setup() to ag71xx_phylink_setup() 

 drivers/net/ethernet/atheros/Kconfig  |   2 +-
 drivers/net/ethernet/atheros/ag71xx.c | 150 +++++++++++++++++---------
 2 files changed, 98 insertions(+), 54 deletions(-)

diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig
index 0058051ba925..2720bde5034e 100644
--- a/drivers/net/ethernet/atheros/Kconfig
+++ b/drivers/net/ethernet/atheros/Kconfig
@@ -20,7 +20,7 @@ if NET_VENDOR_ATHEROS
 config AG71XX
 	tristate "Atheros AR7XXX/AR9XXX built-in ethernet mac support"
 	depends on ATH79
-	select PHYLIB
+	select PHYLINK
 	help
 	  If you wish to compile a kernel for AR7XXX/91XXX and enable
 	  ethernet support, then you should always answer Y to this.
diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index e95687a780fb..9692ae1734a8 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -32,6 +32,7 @@
 #include <linux/of_mdio.h>
 #include <linux/of_net.h>
 #include <linux/of_platform.h>
+#include <linux/phylink.h>
 #include <linux/regmap.h>
 #include <linux/reset.h>
 #include <linux/clk.h>
@@ -314,6 +315,8 @@ struct ag71xx {
 	dma_addr_t stop_desc_dma;
 
 	phy_interface_t phy_if_mode;
+	struct phylink *phylink;
+	struct phylink_config phylink_config;
 
 	struct delayed_work restart_work;
 	struct timer_list oom_timer;
@@ -845,24 +848,23 @@ static void ag71xx_hw_start(struct ag71xx *ag)
 	netif_wake_queue(ag->ndev);
 }
 
-static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
+static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
+			      const struct phylink_link_state *state)
 {
-	struct phy_device *phydev = ag->ndev->phydev;
+	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
 	u32 cfg2;
 	u32 ifctl;
 	u32 fifo5;
 
-	if (!phydev->link && update) {
-		ag71xx_hw_stop(ag);
+	if (phylink_autoneg_inband(mode))
 		return;
-	}
 
 	if (!ag71xx_is(ag, AR7100) && !ag71xx_is(ag, AR9130))
 		ag71xx_fast_reset(ag);
 
 	cfg2 = ag71xx_rr(ag, AG71XX_REG_MAC_CFG2);
 	cfg2 &= ~(MAC_CFG2_IF_1000 | MAC_CFG2_IF_10_100 | MAC_CFG2_FDX);
-	cfg2 |= (phydev->duplex) ? MAC_CFG2_FDX : 0;
+	cfg2 |= (state->duplex) ? MAC_CFG2_FDX : 0;
 
 	ifctl = ag71xx_rr(ag, AG71XX_REG_MAC_IFCTL);
 	ifctl &= ~(MAC_IFCTL_SPEED);
@@ -870,7 +872,7 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
 	fifo5 = ag71xx_rr(ag, AG71XX_REG_FIFO_CFG5);
 	fifo5 &= ~FIFO_CFG5_BM;
 
-	switch (phydev->speed) {
+	switch (state->speed) {
 	case SPEED_1000:
 		cfg2 |= MAC_CFG2_IF_1000;
 		fifo5 |= FIFO_CFG5_BM;
@@ -883,7 +885,6 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
 		cfg2 |= MAC_CFG2_IF_10_100;
 		break;
 	default:
-		WARN(1, "not supported speed %i\n", phydev->speed);
 		return;
 	}
 
@@ -897,58 +898,91 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
 	ag71xx_wr(ag, AG71XX_REG_MAC_CFG2, cfg2);
 	ag71xx_wr(ag, AG71XX_REG_FIFO_CFG5, fifo5);
 	ag71xx_wr(ag, AG71XX_REG_MAC_IFCTL, ifctl);
+}
 
-	ag71xx_hw_start(ag);
+static void ag71xx_mac_validate(struct phylink_config *config,
+			    unsigned long *supported,
+			    struct phylink_link_state *state)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	if (state->interface != PHY_INTERFACE_MODE_NA &&
+	    state->interface != PHY_INTERFACE_MODE_GMII &&
+	    state->interface != PHY_INTERFACE_MODE_MII) {
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		return;
+	}
+
+	phylink_set(mask, MII);
+
+	phylink_set(mask, Autoneg);
+	phylink_set(mask, 10baseT_Half);
+	phylink_set(mask, 10baseT_Full);
+	phylink_set(mask, 100baseT_Half);
+	phylink_set(mask, 100baseT_Full);
+
+	if (state->interface == PHY_INTERFACE_MODE_NA ||
+	    state->interface == PHY_INTERFACE_MODE_GMII) {
+		phylink_set(mask, 1000baseT_Full);
+		phylink_set(mask, 1000baseX_Full);
+	}
 
-	if (update)
-		phy_print_status(phydev);
+	bitmap_and(supported, supported, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(state->advertising, state->advertising, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 }
 
-static void ag71xx_phy_link_adjust(struct net_device *ndev)
+static void ag71xx_mac_pcs_get_state(struct phylink_config *config,
+				     struct phylink_link_state *state)
 {
-	struct ag71xx *ag = netdev_priv(ndev);
+	state->link = 0;
+}
 
-	ag71xx_link_adjust(ag, true);
+static void ag71xx_mac_an_restart(struct phylink_config *config)
+{
+	/* Not Supported */
 }
 
-static int ag71xx_phy_connect(struct ag71xx *ag)
+static void ag71xx_mac_link_down(struct phylink_config *config,
+				 unsigned int mode, phy_interface_t interface)
 {
-	struct device_node *np = ag->pdev->dev.of_node;
-	struct net_device *ndev = ag->ndev;
-	struct device_node *phy_node;
-	struct phy_device *phydev;
-	int ret;
+	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
 
-	if (of_phy_is_fixed_link(np)) {
-		ret = of_phy_register_fixed_link(np);
-		if (ret < 0) {
-			netif_err(ag, probe, ndev, "Failed to register fixed PHY link: %d\n",
-				  ret);
-			return ret;
-		}
+	ag71xx_hw_stop(ag);
+}
 
-		phy_node = of_node_get(np);
-	} else {
-		phy_node = of_parse_phandle(np, "phy-handle", 0);
-	}
+static void ag71xx_mac_link_up(struct phylink_config *config, unsigned int mode,
+			       phy_interface_t interface,
+			       struct phy_device *phy)
+{
+	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
 
-	if (!phy_node) {
-		netif_err(ag, probe, ndev, "Could not find valid phy node\n");
-		return -ENODEV;
-	}
+	ag71xx_hw_start(ag);
+}
 
-	phydev = of_phy_connect(ag->ndev, phy_node, ag71xx_phy_link_adjust,
-				0, ag->phy_if_mode);
+static const struct phylink_mac_ops ag71xx_phylink_mac_ops = {
+	.validate = ag71xx_mac_validate,
+	.mac_pcs_get_state = ag71xx_mac_pcs_get_state,
+	.mac_an_restart = ag71xx_mac_an_restart,
+	.mac_config = ag71xx_mac_config,
+	.mac_link_down = ag71xx_mac_link_down,
+	.mac_link_up = ag71xx_mac_link_up,
+};
 
-	of_node_put(phy_node);
+static int ag71xx_phylink_setup(struct ag71xx *ag)
+{
+	struct phylink *phylink;
 
-	if (!phydev) {
-		netif_err(ag, probe, ndev, "Could not connect to PHY device\n");
-		return -ENODEV;
-	}
+	ag->phylink_config.dev = &ag->ndev->dev;
+	ag->phylink_config.type = PHYLINK_NETDEV;
 
-	phy_attached_info(phydev);
+	phylink = phylink_create(&ag->phylink_config, ag->pdev->dev.fwnode,
+				 ag->phy_if_mode, &ag71xx_phylink_mac_ops);
+	if (IS_ERR(phylink))
+		return PTR_ERR(phylink);
 
+	ag->phylink = phylink;
 	return 0;
 }
 
@@ -1239,6 +1273,13 @@ static int ag71xx_open(struct net_device *ndev)
 	unsigned int max_frame_len;
 	int ret;
 
+	ret = phylink_of_phy_connect(ag->phylink, ag->pdev->dev.of_node, 0);
+	if (ret) {
+		netif_err(ag, link, ndev, "phylink_of_phy_connect filed with err: %i\n",
+			  ret);
+		goto err;
+	}
+
 	max_frame_len = ag71xx_max_frame_len(ndev->mtu);
 	ag->rx_buf_size =
 		SKB_DATA_ALIGN(max_frame_len + NET_SKB_PAD + NET_IP_ALIGN);
@@ -1251,11 +1292,7 @@ static int ag71xx_open(struct net_device *ndev)
 	if (ret)
 		goto err;
 
-	ret = ag71xx_phy_connect(ag);
-	if (ret)
-		goto err;
-
-	phy_start(ndev->phydev);
+	phylink_start(ag->phylink);
 
 	return 0;
 
@@ -1268,8 +1305,8 @@ static int ag71xx_stop(struct net_device *ndev)
 {
 	struct ag71xx *ag = netdev_priv(ndev);
 
-	phy_stop(ndev->phydev);
-	phy_disconnect(ndev->phydev);
+	phylink_stop(ag->phylink);
+	phylink_disconnect_phy(ag->phylink);
 	ag71xx_hw_disable(ag);
 
 	return 0;
@@ -1414,13 +1451,14 @@ static void ag71xx_restart_work_func(struct work_struct *work)
 {
 	struct ag71xx *ag = container_of(work, struct ag71xx,
 					 restart_work.work);
-	struct net_device *ndev = ag->ndev;
 
 	rtnl_lock();
 	ag71xx_hw_disable(ag);
 	ag71xx_hw_enable(ag);
-	if (ndev->phydev->link)
-		ag71xx_link_adjust(ag, false);
+
+	phylink_stop(ag->phylink);
+	phylink_start(ag->phylink);
+
 	rtnl_unlock();
 }
 
@@ -1759,6 +1797,12 @@ static int ag71xx_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, ndev);
 
+	err = ag71xx_phylink_setup(ag);
+	if (err) {
+		netif_err(ag, probe, ndev, "failed to setup phylink (%d)\n", err);
+		goto err_mdio_remove;
+	}
+
 	err = register_netdev(ndev);
 	if (err) {
 		netif_err(ag, probe, ndev, "unable to register net device\n");
-- 
2.25.0

Re: [PATCH v8 1/1] net: ag71xx: port to phylink

[Russell King - ARM Linux admin]

On Wed, Feb 26, 2020 at 06:46:24AM +0100, Oleksij Rempel wrote:
> The port to phylink was done as close as possible to initial
> functionality.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v8:
> - set the autoneg bit
> - provide implementations for the mac_pcs_get_state and mac_an_restart
>   methods
> - do phylink_disconnect_phy() on _stop()
> - rename ag71xx_phy_setup() to ag71xx_phylink_setup() 

There will be one more change required; I'm changing the prototype for
the mac_link_up() function, and I suggest as you don't support in-band
AN that most of the setup for speed and duplex gets moved out of your
mac_config() implementation to mac_link_up().

The patches have been available on netdev for just over a week now.

> 
>  drivers/net/ethernet/atheros/Kconfig  |   2 +-
>  drivers/net/ethernet/atheros/ag71xx.c | 150 +++++++++++++++++---------
>  2 files changed, 98 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/Kconfig b/drivers/net/ethernet/atheros/Kconfig
> index 0058051ba925..2720bde5034e 100644
> --- a/drivers/net/ethernet/atheros/Kconfig
> +++ b/drivers/net/ethernet/atheros/Kconfig
> @@ -20,7 +20,7 @@ if NET_VENDOR_ATHEROS
>  config AG71XX
>  	tristate "Atheros AR7XXX/AR9XXX built-in ethernet mac support"
>  	depends on ATH79
> -	select PHYLIB
> +	select PHYLINK
>  	help
>  	  If you wish to compile a kernel for AR7XXX/91XXX and enable
>  	  ethernet support, then you should always answer Y to this.
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index e95687a780fb..9692ae1734a8 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -32,6 +32,7 @@
>  #include <linux/of_mdio.h>
>  #include <linux/of_net.h>
>  #include <linux/of_platform.h>
> +#include <linux/phylink.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
>  #include <linux/clk.h>
> @@ -314,6 +315,8 @@ struct ag71xx {
>  	dma_addr_t stop_desc_dma;
>  
>  	phy_interface_t phy_if_mode;
> +	struct phylink *phylink;
> +	struct phylink_config phylink_config;
>  
>  	struct delayed_work restart_work;
>  	struct timer_list oom_timer;
> @@ -845,24 +848,23 @@ static void ag71xx_hw_start(struct ag71xx *ag)
>  	netif_wake_queue(ag->ndev);
>  }
>  
> -static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
> +static void ag71xx_mac_config(struct phylink_config *config, unsigned int mode,
> +			      const struct phylink_link_state *state)
>  {
> -	struct phy_device *phydev = ag->ndev->phydev;
> +	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
>  	u32 cfg2;
>  	u32 ifctl;
>  	u32 fifo5;
>  
> -	if (!phydev->link && update) {
> -		ag71xx_hw_stop(ag);
> +	if (phylink_autoneg_inband(mode))
>  		return;
> -	}
>  
>  	if (!ag71xx_is(ag, AR7100) && !ag71xx_is(ag, AR9130))
>  		ag71xx_fast_reset(ag);
>  
>  	cfg2 = ag71xx_rr(ag, AG71XX_REG_MAC_CFG2);
>  	cfg2 &= ~(MAC_CFG2_IF_1000 | MAC_CFG2_IF_10_100 | MAC_CFG2_FDX);
> -	cfg2 |= (phydev->duplex) ? MAC_CFG2_FDX : 0;
> +	cfg2 |= (state->duplex) ? MAC_CFG2_FDX : 0;
>  
>  	ifctl = ag71xx_rr(ag, AG71XX_REG_MAC_IFCTL);
>  	ifctl &= ~(MAC_IFCTL_SPEED);
> @@ -870,7 +872,7 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
>  	fifo5 = ag71xx_rr(ag, AG71XX_REG_FIFO_CFG5);
>  	fifo5 &= ~FIFO_CFG5_BM;
>  
> -	switch (phydev->speed) {
> +	switch (state->speed) {
>  	case SPEED_1000:
>  		cfg2 |= MAC_CFG2_IF_1000;
>  		fifo5 |= FIFO_CFG5_BM;
> @@ -883,7 +885,6 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
>  		cfg2 |= MAC_CFG2_IF_10_100;
>  		break;
>  	default:
> -		WARN(1, "not supported speed %i\n", phydev->speed);
>  		return;
>  	}
>  
> @@ -897,58 +898,91 @@ static void ag71xx_link_adjust(struct ag71xx *ag, bool update)
>  	ag71xx_wr(ag, AG71XX_REG_MAC_CFG2, cfg2);
>  	ag71xx_wr(ag, AG71XX_REG_FIFO_CFG5, fifo5);
>  	ag71xx_wr(ag, AG71XX_REG_MAC_IFCTL, ifctl);
> +}
>  
> -	ag71xx_hw_start(ag);
> +static void ag71xx_mac_validate(struct phylink_config *config,
> +			    unsigned long *supported,
> +			    struct phylink_link_state *state)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> +	if (state->interface != PHY_INTERFACE_MODE_NA &&
> +	    state->interface != PHY_INTERFACE_MODE_GMII &&
> +	    state->interface != PHY_INTERFACE_MODE_MII) {
> +		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +		return;
> +	}
> +
> +	phylink_set(mask, MII);
> +
> +	phylink_set(mask, Autoneg);
> +	phylink_set(mask, 10baseT_Half);
> +	phylink_set(mask, 10baseT_Full);
> +	phylink_set(mask, 100baseT_Half);
> +	phylink_set(mask, 100baseT_Full);
> +
> +	if (state->interface == PHY_INTERFACE_MODE_NA ||
> +	    state->interface == PHY_INTERFACE_MODE_GMII) {
> +		phylink_set(mask, 1000baseT_Full);
> +		phylink_set(mask, 1000baseX_Full);
> +	}
>  
> -	if (update)
> -		phy_print_status(phydev);
> +	bitmap_and(supported, supported, mask,
> +		   __ETHTOOL_LINK_MODE_MASK_NBITS);
> +	bitmap_and(state->advertising, state->advertising, mask,
> +		   __ETHTOOL_LINK_MODE_MASK_NBITS);
>  }
>  
> -static void ag71xx_phy_link_adjust(struct net_device *ndev)
> +static void ag71xx_mac_pcs_get_state(struct phylink_config *config,
> +				     struct phylink_link_state *state)
>  {
> -	struct ag71xx *ag = netdev_priv(ndev);
> +	state->link = 0;
> +}
>  
> -	ag71xx_link_adjust(ag, true);
> +static void ag71xx_mac_an_restart(struct phylink_config *config)
> +{
> +	/* Not Supported */
>  }
>  
> -static int ag71xx_phy_connect(struct ag71xx *ag)
> +static void ag71xx_mac_link_down(struct phylink_config *config,
> +				 unsigned int mode, phy_interface_t interface)
>  {
> -	struct device_node *np = ag->pdev->dev.of_node;
> -	struct net_device *ndev = ag->ndev;
> -	struct device_node *phy_node;
> -	struct phy_device *phydev;
> -	int ret;
> +	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
>  
> -	if (of_phy_is_fixed_link(np)) {
> -		ret = of_phy_register_fixed_link(np);
> -		if (ret < 0) {
> -			netif_err(ag, probe, ndev, "Failed to register fixed PHY link: %d\n",
> -				  ret);
> -			return ret;
> -		}
> +	ag71xx_hw_stop(ag);
> +}
>  
> -		phy_node = of_node_get(np);
> -	} else {
> -		phy_node = of_parse_phandle(np, "phy-handle", 0);
> -	}
> +static void ag71xx_mac_link_up(struct phylink_config *config, unsigned int mode,
> +			       phy_interface_t interface,
> +			       struct phy_device *phy)
> +{
> +	struct ag71xx *ag = netdev_priv(to_net_dev(config->dev));
>  
> -	if (!phy_node) {
> -		netif_err(ag, probe, ndev, "Could not find valid phy node\n");
> -		return -ENODEV;
> -	}
> +	ag71xx_hw_start(ag);
> +}
>  
> -	phydev = of_phy_connect(ag->ndev, phy_node, ag71xx_phy_link_adjust,
> -				0, ag->phy_if_mode);
> +static const struct phylink_mac_ops ag71xx_phylink_mac_ops = {
> +	.validate = ag71xx_mac_validate,
> +	.mac_pcs_get_state = ag71xx_mac_pcs_get_state,
> +	.mac_an_restart = ag71xx_mac_an_restart,
> +	.mac_config = ag71xx_mac_config,
> +	.mac_link_down = ag71xx_mac_link_down,
> +	.mac_link_up = ag71xx_mac_link_up,
> +};
>  
> -	of_node_put(phy_node);
> +static int ag71xx_phylink_setup(struct ag71xx *ag)
> +{
> +	struct phylink *phylink;
>  
> -	if (!phydev) {
> -		netif_err(ag, probe, ndev, "Could not connect to PHY device\n");
> -		return -ENODEV;
> -	}
> +	ag->phylink_config.dev = &ag->ndev->dev;
> +	ag->phylink_config.type = PHYLINK_NETDEV;
>  
> -	phy_attached_info(phydev);
> +	phylink = phylink_create(&ag->phylink_config, ag->pdev->dev.fwnode,
> +				 ag->phy_if_mode, &ag71xx_phylink_mac_ops);
> +	if (IS_ERR(phylink))
> +		return PTR_ERR(phylink);
>  
> +	ag->phylink = phylink;
>  	return 0;
>  }
>  
> @@ -1239,6 +1273,13 @@ static int ag71xx_open(struct net_device *ndev)
>  	unsigned int max_frame_len;
>  	int ret;
>  
> +	ret = phylink_of_phy_connect(ag->phylink, ag->pdev->dev.of_node, 0);
> +	if (ret) {
> +		netif_err(ag, link, ndev, "phylink_of_phy_connect filed with err: %i\n",
> +			  ret);
> +		goto err;
> +	}
> +
>  	max_frame_len = ag71xx_max_frame_len(ndev->mtu);
>  	ag->rx_buf_size =
>  		SKB_DATA_ALIGN(max_frame_len + NET_SKB_PAD + NET_IP_ALIGN);
> @@ -1251,11 +1292,7 @@ static int ag71xx_open(struct net_device *ndev)
>  	if (ret)
>  		goto err;
>  
> -	ret = ag71xx_phy_connect(ag);
> -	if (ret)
> -		goto err;
> -
> -	phy_start(ndev->phydev);
> +	phylink_start(ag->phylink);
>  
>  	return 0;
>  
> @@ -1268,8 +1305,8 @@ static int ag71xx_stop(struct net_device *ndev)
>  {
>  	struct ag71xx *ag = netdev_priv(ndev);
>  
> -	phy_stop(ndev->phydev);
> -	phy_disconnect(ndev->phydev);
> +	phylink_stop(ag->phylink);
> +	phylink_disconnect_phy(ag->phylink);
>  	ag71xx_hw_disable(ag);
>  
>  	return 0;
> @@ -1414,13 +1451,14 @@ static void ag71xx_restart_work_func(struct work_struct *work)
>  {
>  	struct ag71xx *ag = container_of(work, struct ag71xx,
>  					 restart_work.work);
> -	struct net_device *ndev = ag->ndev;
>  
>  	rtnl_lock();
>  	ag71xx_hw_disable(ag);
>  	ag71xx_hw_enable(ag);
> -	if (ndev->phydev->link)
> -		ag71xx_link_adjust(ag, false);
> +
> +	phylink_stop(ag->phylink);
> +	phylink_start(ag->phylink);
> +
>  	rtnl_unlock();
>  }
>  
> @@ -1759,6 +1797,12 @@ static int ag71xx_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, ndev);
>  
> +	err = ag71xx_phylink_setup(ag);
> +	if (err) {
> +		netif_err(ag, probe, ndev, "failed to setup phylink (%d)\n", err);
> +		goto err_mdio_remove;
> +	}
> +
>  	err = register_netdev(ndev);
>  	if (err) {
>  		netif_err(ag, probe, ndev, "unable to register net device\n");
> -- 
> 2.25.0
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v8 1/1] net: ag71xx: port to phylink

[Russell King - ARM Linux admin]

On Wed, Feb 26, 2020 at 09:21:38AM +0000, Russell King - ARM Linux admin wrote:
> On Wed, Feb 26, 2020 at 06:46:24AM +0100, Oleksij Rempel wrote:
> > The port to phylink was done as close as possible to initial
> > functionality.
> > 
> > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > ---
> > changes v8:
> > - set the autoneg bit
> > - provide implementations for the mac_pcs_get_state and mac_an_restart
> >   methods
> > - do phylink_disconnect_phy() on _stop()
> > - rename ag71xx_phy_setup() to ag71xx_phylink_setup() 
> 
> There will be one more change required; I'm changing the prototype for
> the mac_link_up() function, and I suggest as you don't support in-band
> AN that most of the setup for speed and duplex gets moved out of your
> mac_config() implementation to mac_link_up().
> 
> The patches have been available on netdev for just over a week now.

The patches are now in net-next.  Please respin your patch against these
changes, which basically means the code which programs the speed and
duplex in ag71xx_mac_config() needs to be moved to ag71xx_mac_link_up().

Thanks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH v8 1/1] net: ag71xx: port to phylink

[Oleksij Rempel]

[-- Attachment #1: Type: text/plain, Size: 1617 bytes --]

On Fri, Feb 28, 2020 at 11:47:53AM +0000, Russell King - ARM Linux admin wrote:
> On Wed, Feb 26, 2020 at 09:21:38AM +0000, Russell King - ARM Linux admin wrote:
> > On Wed, Feb 26, 2020 at 06:46:24AM +0100, Oleksij Rempel wrote:
> > > The port to phylink was done as close as possible to initial
> > > functionality.
> > > 
> > > Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> > > ---
> > > changes v8:
> > > - set the autoneg bit
> > > - provide implementations for the mac_pcs_get_state and mac_an_restart
> > >   methods
> > > - do phylink_disconnect_phy() on _stop()
> > > - rename ag71xx_phy_setup() to ag71xx_phylink_setup() 
> > 
> > There will be one more change required; I'm changing the prototype for
> > the mac_link_up() function, and I suggest as you don't support in-band
> > AN that most of the setup for speed and duplex gets moved out of your
> > mac_config() implementation to mac_link_up().
> > 
> > The patches have been available on netdev for just over a week now.
> 
> The patches are now in net-next.  Please respin your patch against these
> changes, which basically means the code which programs the speed and
> duplex in ag71xx_mac_config() needs to be moved to ag71xx_mac_link_up().

OK, Thank you!
I'll  update it.

Regards,
Oleksij

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]