| Message ID | 20230322071959.9101-2-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | smsc911x: fix issues when interface is not up yet | expand |
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* ?
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
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; }
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(-)