| Message ID | 849513d9-c981-ec20-5a10-08c663d0aa37@free.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/24/2017 08:01 AM, Mason wrote: > On 24/07/2017 13:07, Mason wrote: > >> When I set the link down via 'ip link set eth0 down' >> (as opposed to pulling the Ethernet cable) things don't happen as expected: >> >> The driver's adjust_link() callback is never called, and doesn't >> get a chance make some required changes. And when I set the link >> up again, there is no network connectivity. >> >> I get this problem only if I enable interrupts on my PHY. >> If I use polling, things work as expected. >> >> >> When I set the link down, devinet_ioctl() eventually calls >> ndo_set_rx_mode() and ndo_stop() >> >> In ndo_stop() the driver calls >> phy_stop(phydev); >> which disables interrupts and sets the state to HALTED. >> >> In phy_state_machine() >> the PHY_HALTED case does call the adjust_link() callback: >> >> if (phydev->link) { >> phydev->link = 0; >> netif_carrier_off(phydev->attached_dev); >> phy_adjust_link(phydev); >> do_suspend = true; >> } >> >> But it's not called when I use interrupts... >> >> Perhaps because there are no interrupts generated? >> Or even if there were, they have been turned off by phy_stop? >> >> Basically, it seems like when I use interrupts, >> the phy_state_machine() is not called on link down, >> which breaks the MAC driver's expectations. >> >> Am I barking up the wrong tree? > > FWIW, the patch below solves my issue. > Basically, we reset the MAC in open(), instead of probe(). > > I also had to solve the issue of adjust_link() not being > called by calling it explicitly in stop() instead of > relying on phy_stop() to do it indirectly. Which is of course absolutely not how it is intended to be used. phy_stop() does the following: - if the PHY was already HALTED do nothing and exit - if it was not and an interrupt is valid for this PHY: disable and clear these interrupts - set state to PHY_HALTED somehow an interrupt should be generated from doing this such that phy_change(), invoked from phy_interrupt() should have a chance to run and make the PHY state machine transition properly to PHY_HALTED. So from there can you check a few things: - is such an interrupt actually generated? - if you turn on dynamic debug prints for drivers/net/phy/phy.c where do we leave the PHY state machine and what state is it in when you call ifconfig up again? > > With this code, I think it is easy to handle suspend/resume: > on suspend, I will stop() and on resume, I will start(), > and everything should work as expected. > > I'd like to hear comments on the patch, so I can turn it > into a formal submission.
On 24/07/2017 18:49, Florian Fainelli wrote: > On 07/24/2017 08:01 AM, Mason wrote: > >>> When I set the link down via 'ip link set eth0 down' >>> (as opposed to pulling the Ethernet cable) things don't happen as expected: >>> >>> The driver's adjust_link() callback is never called, and doesn't >>> get a chance make some required changes. And when I set the link >>> up again, there is no network connectivity. >>> >>> I get this problem only if I enable interrupts on my PHY. >>> If I use polling, things work as expected. >>> >>> >>> When I set the link down, devinet_ioctl() eventually calls >>> ndo_set_rx_mode() and ndo_stop() >>> >>> In ndo_stop() the driver calls >>> phy_stop(phydev); >>> which disables interrupts and sets the state to HALTED. >>> >>> In phy_state_machine() >>> the PHY_HALTED case does call the adjust_link() callback: >>> >>> if (phydev->link) { >>> phydev->link = 0; >>> netif_carrier_off(phydev->attached_dev); >>> phy_adjust_link(phydev); >>> do_suspend = true; >>> } >>> >>> But it's not called when I use interrupts... >>> >>> Perhaps because there are no interrupts generated? >>> Or even if there were, they have been turned off by phy_stop? >>> >>> Basically, it seems like when I use interrupts, >>> the phy_state_machine() is not called on link down, >>> which breaks the MAC driver's expectations. >>> >>> Am I barking up the wrong tree? >> >> FWIW, the patch below solves my issue. >> Basically, we reset the MAC in open(), instead of probe(). >> >> I also had to solve the issue of adjust_link() not being >> called by calling it explicitly in stop() instead of >> relying on phy_stop() to do it indirectly. > > Which is of course absolutely not how it is intended to be used. > phy_stop() does the following: > > - if the PHY was already HALTED do nothing and exit > - if it was not and an interrupt is valid for this PHY: disable and > clear these interrupts > - set state to PHY_HALTED > > somehow an interrupt should be generated from doing this such that > phy_change(), invoked from phy_interrupt() should have a chance to run > and make the PHY state machine transition properly to PHY_HALTED. I'm totally confused. Are you saying that phy_stop itself should trigger an interrupt, or that the process of setting the link down should generate an interrupt *before* we reach phy_stop? I'm also perplex over this synchronous IRQ business. Should I be looking for a way to trigger an IRQ in software in the Atheros PHY? Before I forget: there is also an issue when using the PHY in polling mode. The ndo_stop callback runs through phy_stop and phy_disconnect too fast for the adjust_link() callback to be called. My patch fixed that too, by calling nb8800_link_reconfigure() explicitly. > So from there can you check a few things: > > - is such an interrupt actually generated? > - if you turn on dynamic debug prints for drivers/net/phy/phy.c where do > we leave the PHY state machine and what state is it in when you call > ifconfig up again? The only interrupts I've ever seen the PHY generate are on plugging/unplugging the Ethernet cable. Looking at the driver and datasheet... http://elixir.free-electrons.com/linux/v4.13-rc2/source/drivers/net/phy/at803x.c#L312 value |= AT803X_INTR_ENABLE_AUTONEG_ERR; value |= AT803X_INTR_ENABLE_SPEED_CHANGED; value |= AT803X_INTR_ENABLE_DUPLEX_CHANGED; value |= AT803X_INTR_ENABLE_LINK_FAIL; value |= AT803X_INTR_ENABLE_LINK_SUCCESS; And the interrupts reasons supported by the PHY are: #define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15) #define AT803X_INTR_ENABLE_SPEED_CHANGED BIT(14) #define AT803X_INTR_ENABLE_DUPLEX_CHANGED BIT(13) #define AT803X_INTR_ENABLE_PAGE_RECEIVED BIT(12) #define AT803X_INTR_ENABLE_LINK_FAIL BIT(11) #define AT803X_INTR_ENABLE_LINK_SUCCESS BIT(10) #define AT803X_INTR_ENABLE_WIRESPEED_DOWNGRADE BIT(5) #define AT803X_INTR_ENABLE_POLARITY_CHANGED BIT(1) #define AT803X_INTR_ENABLE_WOL BIT(0) These all seem to be external reasons (from the peer). I did enable debug logs in drivers/net/phy/phy.c to trace the state machine, and it is not called at all on set link down, so it remains in state RUNNING (both in polling and interrupt modes). IIRC, this used to work on the 2-core board, but breaks on the 4-core board. Could this be some kind of race? Regards.
On 07/24/2017 12:13 PM, Mason wrote: > On 24/07/2017 18:49, Florian Fainelli wrote: > >> On 07/24/2017 08:01 AM, Mason wrote: >> >>>> When I set the link down via 'ip link set eth0 down' >>>> (as opposed to pulling the Ethernet cable) things don't happen as expected: >>>> >>>> The driver's adjust_link() callback is never called, and doesn't >>>> get a chance make some required changes. And when I set the link >>>> up again, there is no network connectivity. >>>> >>>> I get this problem only if I enable interrupts on my PHY. >>>> If I use polling, things work as expected. >>>> >>>> >>>> When I set the link down, devinet_ioctl() eventually calls >>>> ndo_set_rx_mode() and ndo_stop() >>>> >>>> In ndo_stop() the driver calls >>>> phy_stop(phydev); >>>> which disables interrupts and sets the state to HALTED. >>>> >>>> In phy_state_machine() >>>> the PHY_HALTED case does call the adjust_link() callback: >>>> >>>> if (phydev->link) { >>>> phydev->link = 0; >>>> netif_carrier_off(phydev->attached_dev); >>>> phy_adjust_link(phydev); >>>> do_suspend = true; >>>> } >>>> >>>> But it's not called when I use interrupts... >>>> >>>> Perhaps because there are no interrupts generated? >>>> Or even if there were, they have been turned off by phy_stop? >>>> >>>> Basically, it seems like when I use interrupts, >>>> the phy_state_machine() is not called on link down, >>>> which breaks the MAC driver's expectations. >>>> >>>> Am I barking up the wrong tree? >>> >>> FWIW, the patch below solves my issue. >>> Basically, we reset the MAC in open(), instead of probe(). >>> >>> I also had to solve the issue of adjust_link() not being >>> called by calling it explicitly in stop() instead of >>> relying on phy_stop() to do it indirectly. >> >> Which is of course absolutely not how it is intended to be used. >> phy_stop() does the following: >> >> - if the PHY was already HALTED do nothing and exit >> - if it was not and an interrupt is valid for this PHY: disable and >> clear these interrupts >> - set state to PHY_HALTED >> >> somehow an interrupt should be generated from doing this such that >> phy_change(), invoked from phy_interrupt() should have a chance to run >> and make the PHY state machine transition properly to PHY_HALTED. > > I'm totally confused. Are you saying that phy_stop itself > should trigger an interrupt, or that the process of setting > the link down should generate an interrupt *before* we reach > phy_stop? My reading of the code, and because I don't actually have a system where PHY interrupts proper are used (only polling or PHY_IGNORE INTERRUPT) is that, yes, somehow calling phy_stop() should result in a PHY interrupt to be generated making the state machine move to PHY_HALTED. > > I'm also perplex over this synchronous IRQ business. > Should I be looking for a way to trigger an IRQ in > software in the Atheros PHY? No, first understand the problem and what is going on before trying to workaround things in the PHY driver, there were questions for you as to what state the PHY state machine is left in we need to see that to understand how to possibly fix what you are seeing. > > Before I forget: there is also an issue when using the PHY > in polling mode. The ndo_stop callback runs through phy_stop > and phy_disconnect too fast for the adjust_link() callback > to be called. My patch fixed that too, by calling > nb8800_link_reconfigure() explicitly. Most, if not all drivers should have this: ndo_open() calls phy_connect() or phy_attach() + phy_start() because that allows you to properly manage the PHY's power state and the state machine, the reciprocal is to have ndo_stop() call phy_disconnect() (and just that) which properly waits for the PHY state machine to be fully stopped. phy_stop() returns immediately but the PHY state machine only gets stopped asynchronously at a later time, either with an interrupt or with an explicit work queue scheduling. If you call phy_disconnect() right after, this cancels the work queue and it may not have run the adjust_link callback yet. > > >> So from there can you check a few things: >> >> - is such an interrupt actually generated? >> - if you turn on dynamic debug prints for drivers/net/phy/phy.c where do >> we leave the PHY state machine and what state is it in when you call >> ifconfig up again? > > The only interrupts I've ever seen the PHY generate are > on plugging/unplugging the Ethernet cable. > > Looking at the driver and datasheet... > http://elixir.free-electrons.com/linux/v4.13-rc2/source/drivers/net/phy/at803x.c#L312 > value |= AT803X_INTR_ENABLE_AUTONEG_ERR; > value |= AT803X_INTR_ENABLE_SPEED_CHANGED; > value |= AT803X_INTR_ENABLE_DUPLEX_CHANGED; > value |= AT803X_INTR_ENABLE_LINK_FAIL; > value |= AT803X_INTR_ENABLE_LINK_SUCCESS; > > And the interrupts reasons supported by the PHY are: > #define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15) > #define AT803X_INTR_ENABLE_SPEED_CHANGED BIT(14) > #define AT803X_INTR_ENABLE_DUPLEX_CHANGED BIT(13) > #define AT803X_INTR_ENABLE_PAGE_RECEIVED BIT(12) > #define AT803X_INTR_ENABLE_LINK_FAIL BIT(11) > #define AT803X_INTR_ENABLE_LINK_SUCCESS BIT(10) > #define AT803X_INTR_ENABLE_WIRESPEED_DOWNGRADE BIT(5) > #define AT803X_INTR_ENABLE_POLARITY_CHANGED BIT(1) > #define AT803X_INTR_ENABLE_WOL BIT(0) > > These all seem to be external reasons (from the peer). > > I did enable debug logs in drivers/net/phy/phy.c > to trace the state machine, and it is not called > at all on set link down, so it remains in state > RUNNING (both in polling and interrupt modes). > > IIRC, this used to work on the 2-core board, but breaks > on the 4-core board. Could this be some kind of race? See what I just replied, phy_stop() then phy_disconnect() is "racy" because there is a work queue involved. If I can read the code so can you.
On 07/24/2017 12:32 PM, Florian Fainelli wrote: > On 07/24/2017 12:13 PM, Mason wrote: >> On 24/07/2017 18:49, Florian Fainelli wrote: >> >>> On 07/24/2017 08:01 AM, Mason wrote: >>> >>>>> When I set the link down via 'ip link set eth0 down' >>>>> (as opposed to pulling the Ethernet cable) things don't happen as expected: >>>>> >>>>> The driver's adjust_link() callback is never called, and doesn't >>>>> get a chance make some required changes. And when I set the link >>>>> up again, there is no network connectivity. >>>>> >>>>> I get this problem only if I enable interrupts on my PHY. >>>>> If I use polling, things work as expected. >>>>> >>>>> >>>>> When I set the link down, devinet_ioctl() eventually calls >>>>> ndo_set_rx_mode() and ndo_stop() >>>>> >>>>> In ndo_stop() the driver calls >>>>> phy_stop(phydev); >>>>> which disables interrupts and sets the state to HALTED. >>>>> >>>>> In phy_state_machine() >>>>> the PHY_HALTED case does call the adjust_link() callback: >>>>> >>>>> if (phydev->link) { >>>>> phydev->link = 0; >>>>> netif_carrier_off(phydev->attached_dev); >>>>> phy_adjust_link(phydev); >>>>> do_suspend = true; >>>>> } >>>>> >>>>> But it's not called when I use interrupts... >>>>> >>>>> Perhaps because there are no interrupts generated? >>>>> Or even if there were, they have been turned off by phy_stop? >>>>> >>>>> Basically, it seems like when I use interrupts, >>>>> the phy_state_machine() is not called on link down, >>>>> which breaks the MAC driver's expectations. >>>>> >>>>> Am I barking up the wrong tree? >>>> >>>> FWIW, the patch below solves my issue. >>>> Basically, we reset the MAC in open(), instead of probe(). >>>> >>>> I also had to solve the issue of adjust_link() not being >>>> called by calling it explicitly in stop() instead of >>>> relying on phy_stop() to do it indirectly. >>> >>> Which is of course absolutely not how it is intended to be used. >>> phy_stop() does the following: >>> >>> - if the PHY was already HALTED do nothing and exit >>> - if it was not and an interrupt is valid for this PHY: disable and >>> clear these interrupts >>> - set state to PHY_HALTED >>> >>> somehow an interrupt should be generated from doing this such that >>> phy_change(), invoked from phy_interrupt() should have a chance to run >>> and make the PHY state machine transition properly to PHY_HALTED. >> >> I'm totally confused. Are you saying that phy_stop itself >> should trigger an interrupt, or that the process of setting >> the link down should generate an interrupt *before* we reach >> phy_stop? > > My reading of the code, and because I don't actually have a system where > PHY interrupts proper are used (only polling or PHY_IGNORE INTERRUPT) is > that, yes, somehow calling phy_stop() should result in a PHY interrupt > to be generated making the state machine move to PHY_HALTED. > >> >> I'm also perplex over this synchronous IRQ business. >> Should I be looking for a way to trigger an IRQ in >> software in the Atheros PHY? > > No, first understand the problem and what is going on before trying to > workaround things in the PHY driver, there were questions for you as to > what state the PHY state machine is left in we need to see that to > understand how to possibly fix what you are seeing. > >> >> Before I forget: there is also an issue when using the PHY >> in polling mode. The ndo_stop callback runs through phy_stop >> and phy_disconnect too fast for the adjust_link() callback >> to be called. My patch fixed that too, by calling >> nb8800_link_reconfigure() explicitly. > > Most, if not all drivers should have this: > > ndo_open() calls phy_connect() or phy_attach() + phy_start() because > that allows you to properly manage the PHY's power state and the state > machine, the reciprocal is to have ndo_stop() call phy_disconnect() (and > just that) which properly waits for the PHY state machine to be fully > stopped. > > phy_stop() returns immediately but the PHY state machine only gets > stopped asynchronously at a later time, either with an interrupt or with > an explicit work queue scheduling. If you call phy_disconnect() right > after, this cancels the work queue and it may not have run the > adjust_link callback yet. > >> >> >>> So from there can you check a few things: >>> >>> - is such an interrupt actually generated? >>> - if you turn on dynamic debug prints for drivers/net/phy/phy.c where do >>> we leave the PHY state machine and what state is it in when you call >>> ifconfig up again? >> >> The only interrupts I've ever seen the PHY generate are >> on plugging/unplugging the Ethernet cable. >> >> Looking at the driver and datasheet... >> http://elixir.free-electrons.com/linux/v4.13-rc2/source/drivers/net/phy/at803x.c#L312 >> value |= AT803X_INTR_ENABLE_AUTONEG_ERR; >> value |= AT803X_INTR_ENABLE_SPEED_CHANGED; >> value |= AT803X_INTR_ENABLE_DUPLEX_CHANGED; >> value |= AT803X_INTR_ENABLE_LINK_FAIL; >> value |= AT803X_INTR_ENABLE_LINK_SUCCESS; >> >> And the interrupts reasons supported by the PHY are: >> #define AT803X_INTR_ENABLE_AUTONEG_ERR BIT(15) >> #define AT803X_INTR_ENABLE_SPEED_CHANGED BIT(14) >> #define AT803X_INTR_ENABLE_DUPLEX_CHANGED BIT(13) >> #define AT803X_INTR_ENABLE_PAGE_RECEIVED BIT(12) >> #define AT803X_INTR_ENABLE_LINK_FAIL BIT(11) >> #define AT803X_INTR_ENABLE_LINK_SUCCESS BIT(10) >> #define AT803X_INTR_ENABLE_WIRESPEED_DOWNGRADE BIT(5) >> #define AT803X_INTR_ENABLE_POLARITY_CHANGED BIT(1) >> #define AT803X_INTR_ENABLE_WOL BIT(0) >> >> These all seem to be external reasons (from the peer). >> >> I did enable debug logs in drivers/net/phy/phy.c >> to trace the state machine, and it is not called >> at all on set link down, so it remains in state >> RUNNING (both in polling and interrupt modes). Well now that I see the possible interrupts generated, I indeed don't see how you can get a link down notification unless you somehow force the link down yourself, which would certainly happen in phy_suspend() when we set BMCR.pwrdwn, but that may be too late. You should still expect the adjust_link() function to be called though with PHY_HALTED being set and that takes care of doing phydev->link = 0 and netif_carrier_off(). If that still does not work, then see whether removing the call to phy_stop() does help (it really should).
On 24/07/2017 21:53, Florian Fainelli wrote: > Well now that I see the possible interrupts generated, I indeed don't > see how you can get a link down notification unless you somehow force > the link down yourself, which would certainly happen in phy_suspend() > when we set BMCR.pwrdwn, but that may be too late. > > You should still expect the adjust_link() function to be called though > with PHY_HALTED being set and that takes care of doing phydev->link = 0 > and netif_carrier_off(). If that still does not work, then see whether > removing the call to phy_stop() does help (it really should). The only functions setting phydev->state to PHY_HALTED are phy_error() and phy_stop() AFAICT. I am aware that when phy_state_machine() handles the PHY_HALTED state, it will set phydev->link = 0; and call netif_carrier_off() -- because that's where I copied that code from. My issue is that phy_state_machine() does not run when I run 'ip set link dev eth0 down' from the command line. If I'm reading the code right, phy_disconnect() actually stops the state machine. In interrupt mode, phy_state_machine() doesn't run because no interrupt is generated. In polling mode, phy_state_machine() doesn't run because phy_disconnect() stops the state machine. Introducing a sleep before phy_disconnect() gives the state machine a chance to run in polling mode, but it doesn't feel right, and doesn't fix the other mode, which I'm using. Looking at bcm_enet_stop() it calls phy_stop() and phy_disconnect() just like the nb8800 driver... I'm stumped. Regards.
diff --git a/drivers/net/ethernet/aurora/nb8800.c b/drivers/net/ethernet/aurora/nb8800.c index e94159507847..22e1dd41962d 100644 --- a/drivers/net/ethernet/aurora/nb8800.c +++ b/drivers/net/ethernet/aurora/nb8800.c @@ -41,6 +41,7 @@ static void nb8800_tx_done(struct net_device *dev); static int nb8800_dma_stop(struct net_device *dev); +static int mac_init(struct net_device *dev); static inline u8 nb8800_readb(struct nb8800_priv *priv, int reg) { @@ -54,16 +55,20 @@ static inline u32 nb8800_readl(struct nb8800_priv *priv, int reg) static inline void nb8800_writeb(struct nb8800_priv *priv, int reg, u8 val) { + printk("++ETH++ gw8 reg=%p val=%02x\n", priv->base + reg, val); writeb_relaxed(val, priv->base + reg); } static inline void nb8800_writew(struct nb8800_priv *priv, int reg, u16 val) { + printk("++ETH++ gw16 reg=%p val=%04x\n", priv->base + reg, val); writew_relaxed(val, priv->base + reg); } static inline void nb8800_writel(struct nb8800_priv *priv, int reg, u32 val) { + if (reg != 0x20 && reg < 0x100) + printk("++ETH++ gw32 reg=%p val=%08x\n", priv->base + reg, val); writel_relaxed(val, priv->base + reg); } @@ -605,6 +610,7 @@ static void nb8800_mac_config(struct net_device *dev) u32 phy_clk; u32 ict; + printk("ENTER %s\n", __func__); if (!priv->duplex) mac_mode |= HALF_DUPLEX; @@ -635,6 +641,7 @@ static void nb8800_pause_config(struct net_device *dev) struct phy_device *phydev = dev->phydev; u32 rxcr; + printk("ENTER %s\n", __func__); if (priv->pause_aneg) { if (!phydev || !phydev->link) return; @@ -668,6 +675,11 @@ static void nb8800_link_reconfigure(struct net_device *dev) struct phy_device *phydev = dev->phydev; int change = 0; + printk("ENTER %s\n", __func__); + printk("PRIV link=%d speed=%d duplex=%d\n", + priv->link, priv->speed, priv->duplex); + printk("PHYDEV link=%d speed=%d duplex=%d\n", + phydev->link, phydev->speed, phydev->duplex); if (phydev->link) { if (phydev->speed != priv->speed) { priv->speed = phydev->speed; @@ -699,6 +711,7 @@ static void nb8800_update_mac_addr(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); int i; + printk("ENTER %s\n", __func__); for (i = 0; i < ETH_ALEN; i++) nb8800_writeb(priv, NB8800_SRC_ADDR(i), dev->dev_addr[i]); @@ -710,6 +723,7 @@ static int nb8800_set_mac_address(struct net_device *dev, void *addr) { struct sockaddr *sock = addr; + printk("ENTER %s\n", __func__); if (netif_running(dev)) return -EBUSY; @@ -723,6 +737,7 @@ static void nb8800_mc_init(struct net_device *dev, int val) { struct nb8800_priv *priv = netdev_priv(dev); + printk("ENTER %s\n", __func__); nb8800_writeb(priv, NB8800_MC_INIT, val); readb_poll_timeout_atomic(priv->base + NB8800_MC_INIT, val, !val, 1, 1000); @@ -734,6 +749,7 @@ static void nb8800_set_rx_mode(struct net_device *dev) struct netdev_hw_addr *ha; int i; + printk("ENTER %s\n", __func__); if (dev->flags & (IFF_PROMISC | IFF_ALLMULTI)) { nb8800_mac_af(dev, false); return; @@ -840,6 +856,7 @@ static int nb8800_dma_init(struct net_device *dev) unsigned int i; int err; + printk("ENTER %s\n", __func__); priv->rx_descs = dma_alloc_coherent(dev->dev.parent, RX_DESC_SIZE, &priv->rx_desc_dma, GFP_KERNEL); if (!priv->rx_descs) @@ -957,6 +974,9 @@ static int nb8800_open(struct net_device *dev) struct phy_device *phydev; int err; + mac_init(dev); + + printk("ENTER %s\n", __func__); /* clear any pending interrupts */ nb8800_writel(priv, NB8800_RXC_SR, 0xf); nb8800_writel(priv, NB8800_TXC_SR, 0xf); @@ -1004,7 +1024,7 @@ static int nb8800_stop(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); struct phy_device *phydev = dev->phydev; - phy_stop(phydev); + printk("ENTER %s\n", __func__); netif_stop_queue(dev); napi_disable(&priv->napi); @@ -1013,7 +1033,11 @@ static int nb8800_stop(struct net_device *dev) nb8800_mac_rx(dev, false); nb8800_mac_tx(dev, false); + phydev->link = 0; + netif_carrier_off(dev); + nb8800_link_reconfigure(dev); phy_disconnect(phydev); + priv->duplex = priv->speed = 0; free_irq(dev->irq, dev); @@ -1171,6 +1195,7 @@ static int nb8800_hw_init(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); u32 val; + printk("ENTER %s\n", __func__); val = TX_RETRY_EN | TX_PAD_EN | TX_APPEND_FCS; nb8800_writeb(priv, NB8800_TX_CTL1, val); @@ -1261,6 +1286,7 @@ static int nb8800_tangox_init(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); u32 pad_mode = PAD_MODE_MII; + printk("ENTER %s\n", __func__); switch (priv->phy_mode) { case PHY_INTERFACE_MODE_MII: case PHY_INTERFACE_MODE_GMII: @@ -1290,6 +1316,7 @@ static int nb8800_tangox_reset(struct net_device *dev) struct nb8800_priv *priv = netdev_priv(dev); int clk_div; + printk("ENTER %s\n", __func__); nb8800_writeb(priv, NB8800_TANGOX_RESET, 0); usleep_range(1000, 10000); nb8800_writeb(priv, NB8800_TANGOX_RESET, 1); @@ -1316,6 +1343,7 @@ static int nb8800_tango4_init(struct net_device *dev) if (err) return err; + printk("ENTER %s\n", __func__); /* On tango4 interrupt on DMA completion per frame works and gives * better performance despite generating more rx interrupts. */ @@ -1350,6 +1378,21 @@ static int nb8800_tango4_init(struct net_device *dev) }; MODULE_DEVICE_TABLE(of, nb8800_dt_ids); +static int mac_init(struct net_device *dev) +{ +#ifndef RESET_IN_PROBE + struct nb8800_priv *priv = netdev_priv(dev); + const struct nb8800_ops *ops = priv->ops; + + ops->reset(dev); + nb8800_hw_init(dev); + ops->init(dev); + nb8800_update_mac_addr(dev); +#endif + + return 0; +} + static int nb8800_probe(struct platform_device *pdev) { const struct of_device_id *match; @@ -1363,6 +1406,7 @@ static int nb8800_probe(struct platform_device *pdev) int irq; int ret; + printk("ENTER %s\n", __func__); match = of_match_device(nb8800_dt_ids, &pdev->dev); if (match) ops = match->data; @@ -1389,6 +1433,7 @@ static int nb8800_probe(struct platform_device *pdev) priv = netdev_priv(dev); priv->base = base; + priv->ops = ops; priv->phy_mode = of_get_phy_mode(pdev->dev.of_node); if (priv->phy_mode < 0) @@ -1407,11 +1452,13 @@ static int nb8800_probe(struct platform_device *pdev) spin_lock_init(&priv->tx_lock); +#ifdef RESET_IN_PROBE if (ops && ops->reset) { ret = ops->reset(dev); if (ret) goto err_disable_clk; } +#endif bus = devm_mdiobus_alloc(&pdev->dev); if (!bus) { @@ -1454,6 +1501,7 @@ static int nb8800_probe(struct platform_device *pdev) priv->mii_bus = bus; +#ifdef RESET_IN_PROBE ret = nb8800_hw_init(dev); if (ret) goto err_deregister_fixed_link; @@ -1463,6 +1511,7 @@ static int nb8800_probe(struct platform_device *pdev) if (ret) goto err_deregister_fixed_link; } +#endif dev->netdev_ops = &nb8800_netdev_ops; dev->ethtool_ops = &nb8800_ethtool_ops; @@ -1476,7 +1525,9 @@ static int nb8800_probe(struct platform_device *pdev) if (!is_valid_ether_addr(dev->dev_addr)) eth_hw_addr_random(dev); +#ifdef RESET_IN_PROBE nb8800_update_mac_addr(dev); +#endif netif_carrier_off(dev); diff --git a/drivers/net/ethernet/aurora/nb8800.h b/drivers/net/ethernet/aurora/nb8800.h index 6ec4a956e1e5..d5f4481a2c7b 100644 --- a/drivers/net/ethernet/aurora/nb8800.h +++ b/drivers/net/ethernet/aurora/nb8800.h @@ -305,6 +305,7 @@ struct nb8800_priv { dma_addr_t tx_desc_dma; struct clk *clk; + const struct nb8800_ops *ops; }; struct nb8800_ops {