* [PATCH net v3 0/2] smsc911x: fix issues when interface is not up yet
@ 2023-03-22 7:19 Wolfram Sang
2023-03-22 7:19 ` [PATCH net v3 1/2] smsc911x: only update stats when interface is up Wolfram Sang
2023-03-22 7:19 ` [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up Wolfram Sang
0 siblings, 2 replies; 6+ messages in thread
From: Wolfram Sang @ 2023-03-22 7:19 UTC (permalink / raw)
To: netdev; +Cc: linux-renesas-soc, linux-kernel, Wolfram Sang
Similar to upstream commit 7f5ebf5dae42 ("ravb: avoid PHY being resumed
when interface is not up"), here is the fix for SMSC911x (patch 2).
Also, I saw a splat running 'ifconfig' when interface was not up. Patch
1 fixes it.
Patches are based on v6.2-rc3 and tested on a Renesas APE6-EK.
Changes since v2:
* added tags for patch 1
* patch 2 has been refactored to be less intrusive for easier
backporting
Wolfram Sang (2):
smsc911x: only update stats when interface is up
smsc911x: avoid PHY being resumed when interface is not up
drivers/net/ethernet/smsc/smsc911x.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net v3 1/2] smsc911x: only update stats when interface is up 2023-03-22 7:19 [PATCH net v3 0/2] smsc911x: fix issues when interface is not up yet Wolfram Sang @ 2023-03-22 7:19 ` Wolfram Sang 2023-03-23 18:39 ` Jakub Kicinski 2023-03-22 7:19 ` [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up Wolfram Sang 1 sibling, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2023-03-22 7:19 UTC (permalink / raw) To: netdev Cc: linux-renesas-soc, linux-kernel, Wolfram Sang, Simon Horman, Florian Fainelli, Steve Glendinning, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Geert Uytterhoeven Otherwise the clocks are not enabled and reading registers will BUG. Fixes: 1e30b8d755b8 ("net: smsc911x: Make Runtime PM handling more fine-grained") Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> --- Changes since v2: * added tags drivers/net/ethernet/smsc/smsc911x.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index a2e511912e6a..67cb5eb9c716 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1838,8 +1838,12 @@ smsc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev) static struct net_device_stats *smsc911x_get_stats(struct net_device *dev) { struct smsc911x_data *pdata = netdev_priv(dev); - smsc911x_tx_update_txcounters(dev); - dev->stats.rx_dropped += smsc911x_reg_read(pdata, RX_DROP); + + if (netif_running(dev)) { + smsc911x_tx_update_txcounters(dev); + dev->stats.rx_dropped += smsc911x_reg_read(pdata, RX_DROP); + } + return &dev->stats; } -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 1/2] smsc911x: only update stats when interface is up 2023-03-22 7:19 ` [PATCH net v3 1/2] smsc911x: only update stats when interface is up Wolfram Sang @ 2023-03-23 18:39 ` Jakub Kicinski 2023-03-24 7:33 ` Wolfram Sang 0 siblings, 1 reply; 6+ messages in thread From: Jakub Kicinski @ 2023-03-23 18:39 UTC (permalink / raw) To: Wolfram Sang Cc: netdev, linux-renesas-soc, linux-kernel, Simon Horman, Florian Fainelli, Steve Glendinning, David S. Miller, Eric Dumazet, Paolo Abeni, Geert Uytterhoeven, Vladimir Oltean On Wed, 22 Mar 2023 08:19:58 +0100 Wolfram Sang wrote: > - smsc911x_tx_update_txcounters(dev); > - dev->stats.rx_dropped += smsc911x_reg_read(pdata, RX_DROP); > + > + if (netif_running(dev)) { > + smsc911x_tx_update_txcounters(dev); > + dev->stats.rx_dropped += smsc911x_reg_read(pdata, RX_DROP); > + } Same problem as on the renesas patch, netif_running() can return true before ndo->open() is called. And stats can be read with just the RCU lock (via procfs). Maybe we should add a false-negative version of netif_running() ? __LINK_STATE_START*ED* ? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 1/2] smsc911x: only update stats when interface is up 2023-03-23 18:39 ` Jakub Kicinski @ 2023-03-24 7:33 ` Wolfram Sang 0 siblings, 0 replies; 6+ messages in thread From: Wolfram Sang @ 2023-03-24 7:33 UTC (permalink / raw) To: Jakub Kicinski Cc: netdev, linux-renesas-soc, linux-kernel, Simon Horman, Florian Fainelli, Steve Glendinning, David S. Miller, Eric Dumazet, Paolo Abeni, Geert Uytterhoeven, Vladimir Oltean [-- Attachment #1: Type: text/plain, Size: 1277 bytes --] Hi Jakub, > Maybe we should add a false-negative version of netif_running() ? > __LINK_STATE_START*ED* ? Sounds very reasonable, yet I am missing subsystem details to really judge it. I just hacked a quick coccinelle script and 14 more drivers could be happy about such a change: drivers/net/ethernet/3com/3c515.c | 2 +- drivers/net/ethernet/8390/axnet_cs.c | 2 +- drivers/net/ethernet/8390/lib8390.c | 2 +- drivers/net/ethernet/dec/tulip/de2104x.c | 2 +- drivers/net/ethernet/dec/tulip/tulip_core.c | 2 +- drivers/net/ethernet/dec/tulip/winbond-840.c | 2 +- drivers/net/ethernet/fealnx.c | 2 +- drivers/net/ethernet/natsemi/natsemi.c | 2 +- drivers/net/ethernet/realtek/8139cp.c | 2 +- drivers/net/ethernet/silan/sc92031.c | 2 +- drivers/net/ethernet/smsc/epic100.c | 2 +- drivers/net/ethernet/sun/sungem.c | 2 +- drivers/net/ethernet/toshiba/tc35815.c | 2 +- drivers/net/ethernet/via/via-velocity.c | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) The script: @@ identifier name, args; @@ static struct net_device_stats *name(struct net_device *args) { ... - netif_running + netif_opened ... } Thanks and happy hacking, Wolfram [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up 2023-03-22 7:19 [PATCH net v3 0/2] smsc911x: fix issues when interface is not up yet Wolfram Sang 2023-03-22 7:19 ` [PATCH net v3 1/2] smsc911x: only update stats when interface is up Wolfram Sang @ 2023-03-22 7:19 ` Wolfram Sang 2023-03-22 20:20 ` Simon Horman 1 sibling, 1 reply; 6+ messages in thread From: Wolfram Sang @ 2023-03-22 7:19 UTC (permalink / raw) To: netdev Cc: linux-renesas-soc, linux-kernel, Wolfram Sang, Heiner Kallweit, Steve Glendinning, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Florian Fainelli SMSC911x doesn't need mdiobus suspend/resume, that's why it sets 'mac_managed_pm'. However, setting it needs to be moved from init to probe, so mdiobus PM functions will really never be called (e.g. when the interface is not up yet during suspend/resume). Fixes: 3ce9f2bef755 ("net: smsc911x: Stop and start PHY during suspend and resume") Suggested-by: Heiner Kallweit <hkallweit1@gmail.com> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- Changes since v2: * kept the original error handling when a PHY is not present in mii_probe() to avoid regressions. Because the patch is simpler now, it is also easier to backport. drivers/net/ethernet/smsc/smsc911x.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 67cb5eb9c716..01aac820ac30 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -1037,8 +1037,6 @@ static int smsc911x_mii_probe(struct net_device *dev) return ret; } - /* Indicate that the MAC is responsible for managing PHY PM */ - phydev->mac_managed_pm = true; phy_attached_info(phydev); phy_set_max_speed(phydev, SPEED_100); @@ -1066,6 +1064,7 @@ static int smsc911x_mii_init(struct platform_device *pdev, struct net_device *dev) { struct smsc911x_data *pdata = netdev_priv(dev); + struct phy_device *phydev; int err = -ENXIO; pdata->mii_bus = mdiobus_alloc(); @@ -1108,6 +1107,10 @@ static int smsc911x_mii_init(struct platform_device *pdev, goto err_out_free_bus_2; } + phydev = phy_find_first(pdata->mii_bus); + if (phydev) + phydev->mac_managed_pm = true; + return 0; err_out_free_bus_2: -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up 2023-03-22 7:19 ` [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up Wolfram Sang @ 2023-03-22 20:20 ` Simon Horman 0 siblings, 0 replies; 6+ messages in thread From: Simon Horman @ 2023-03-22 20:20 UTC (permalink / raw) To: Wolfram Sang Cc: netdev, linux-renesas-soc, linux-kernel, Heiner Kallweit, Steve Glendinning, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Florian Fainelli On Wed, Mar 22, 2023 at 08:19:59AM +0100, Wolfram Sang wrote: > SMSC911x doesn't need mdiobus suspend/resume, that's why it sets > 'mac_managed_pm'. However, setting it needs to be moved from init to > probe, so mdiobus PM functions will really never be called (e.g. when > the interface is not up yet during suspend/resume). > > Fixes: 3ce9f2bef755 ("net: smsc911x: Stop and start PHY during suspend and resume") > Suggested-by: Heiner Kallweit <hkallweit1@gmail.com> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Reviewed-by: Simon Horman <simon.horman@corigine.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-24 7:33 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-03-22 7:19 [PATCH net v3 0/2] smsc911x: fix issues when interface is not up yet Wolfram Sang 2023-03-22 7:19 ` [PATCH net v3 1/2] smsc911x: only update stats when interface is up Wolfram Sang 2023-03-23 18:39 ` Jakub Kicinski 2023-03-24 7:33 ` Wolfram Sang 2023-03-22 7:19 ` [PATCH net v3 2/2] smsc911x: avoid PHY being resumed when interface is not up Wolfram Sang 2023-03-22 20:20 ` Simon Horman
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.