On Wed, Dec 02, 2020 at 01:07:12PM +0100, Oleksij Rempel wrote: > Add stats support for the ar9331 switch. > > Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> > --- > /* Warning: switch reset will reset last AR9331_SW_MDIO_PHY_MODE_PAGE request > @@ -422,6 +527,7 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > phy_interface_t interface) > { > struct ar9331_sw_priv *priv = (struct ar9331_sw_priv *)ds->priv; > + struct ar9331_sw_port *p = &priv->port[port]; > struct regmap *regmap = priv->regmap; > int ret; > > @@ -429,6 +535,8 @@ static void ar9331_sw_phylink_mac_link_down(struct dsa_switch *ds, int port, > AR9331_SW_PORT_STATUS_MAC_MASK, 0); > if (ret) > dev_err_ratelimited(priv->dev, "%s: %i\n", __func__, ret); > + > + cancel_delayed_work_sync(&p->mib_read); Is this sufficient? Do you get a guaranteed .phylink_mac_link_down event on unbind? How do you ensure you don't race with the stats worker there? > +static void ar9331_stats_update(struct ar9331_sw_port *port, > + struct rtnl_link_stats64 *stats) > +{ > + struct ar9331_sw_stats *s = &port->stats; > + > + stats->rx_packets = s->rxbroad + s->rxmulti + s->rx64byte + > + s->rx128byte + s->rx256byte + s->rx512byte + s->rx1024byte + > + s->rx1518byte + s->rxmaxbyte; > + stats->tx_packets = s->txbroad + s->txmulti + s->tx64byte + > + s->tx128byte + s->tx256byte + s->tx512byte + s->tx1024byte + > + s->tx1518byte + s->txmaxbyte; > + stats->rx_bytes = s->rxgoodbyte; > + stats->tx_bytes = s->txbyte; > + stats->rx_errors = s->rxfcserr + s->rxalignerr + s->rxrunt + > + s->rxfragment + s->rxoverflow; > + stats->tx_errors = s->txoversize; > + stats->multicast = s->rxmulti; > + stats->collisions = s->txcollision; > + stats->rx_length_errors = s->rxrunt * s->rxfragment + s->rxtoolong; Multiplication? Is this right? > + stats->rx_crc_errors = s->rxfcserr + s->rxalignerr + s->rxfragment; > + stats->rx_frame_errors = s->rxalignerr; > + stats->rx_missed_errors = s->rxoverflow; > + stats->tx_aborted_errors = s->txabortcol; > + stats->tx_fifo_errors = s->txunderrun; > + stats->tx_window_errors = s->txlatecol; > + stats->rx_nohandler = s->filtered; > +} > + > +static void ar9331_do_stats_poll(struct work_struct *work) > +{ > + Could you remove this empty line.