2020-02-26 05:48:24

by Oleksij Rempel

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

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

Signed-off-by: Oleksij Rempel <[email protected]>
---
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


2020-02-26 09:22:13

by Russell King (Oracle)

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

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 <[email protected]>
> ---
> 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

2020-02-28 11:48:27

by Russell King (Oracle)

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

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 <[email protected]>
> > ---
> > 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

2020-02-28 13:28:23

by Oleksij Rempel

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

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 <[email protected]>
> > > ---
> > > 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 |


Attachments:
(No filename) (1.58 kB)
signature.asc (849.00 B)
Download all attachments