> > +static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
> > + u64 *cnt)
> > +{
> > + u32 data;
> > + int timeout;
> > + struct ksz_port *p = >ports[port];
> > +
> > + /* retain the flush/freeze bit */
> > + data = p->freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
> > + data |= MIB_COUNTER_READ;
> > + data |= (addr << MIB_COUNTER_INDEX_S);
> > + ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
> > +
> > + timeout = 1000;
> > + do {
> > + ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
> > + );
> > + usleep_range(1, 10);
> > + if (!(data & MIB_COUNTER_READ))
> > + break;
> > + } while (timeout-- > 0);
>
> Could you use readx_poll_timeout() here?
>
> > +void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
> > +{
> > + struct ksz_device *dev = ds->priv;
> > + struct ksz_port_mib *mib;
> > +
> > + mib = >ports[port].mib;
> > +
> > + /* freeze MIB counters if supported */
> > + if (dev->dev_ops->freeze_mib)
> > + dev->dev_ops->freeze_mib(dev, port, true);
> > + mutex_lock(>cnt_mutex);
> > + port_r_cnt(dev, port);
> > + mutex_unlock(>cnt_mutex);
> > + if (dev->dev_ops->freeze_mib)
> > + dev->dev_ops->freeze_mib(dev, port, false);
>
> Should the freeze be protected by the mutex as well?
>
> > + memcpy(buf, mib->counters, dev->mib_cnt * sizeof(u64));
>
> I wonder if this memcpy should also be protected by the mutex. As soon
> as the mutex is dropped, the scheduled work could start updating
> mib->counters in non-atomic ways?
>
I will update as suggested.
> > +}
> > +
> > int ksz_port_bridge_join(struct dsa_switch *ds, int port,
> > struct net_device *br)
> > {
> > @@ -255,6 +349,7 @@ int ksz_enable_port(struct dsa_switch *ds, int port,
> struct phy_device *phy)
> > /* setup slave port */
> > dev->dev_ops->port_setup(dev, port, false);
> > dev->dev_ops->phy_setup(dev, port, phy);
> > + dev->dev_ops->port_init_cnt(dev, port);
>
> This is probably not the correct place to do this. MIB counters should
> not be cleared by an ifdown/ifup cycle. They should only be cleared
> when the driver is probed.
I wonder what is the official way to clear the counters. For network debugging
It is good to clear the counters to start fresh to see which frame is not
being sent or received. Typically the device is reset when it is shutdown as
there
are hardware problems. I would think it is the job of applications like SNMP
Manager
to keep track of MIB counters throughout the life of a running system.
