Hi Tristan
> +static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
> + u64 *cnt)
> +{
> + u32 data;
> + int timeout;
> + struct ksz_port *p = &dev->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,
> + &data);
> + 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 = &dev->ports[port].mib;
> +
> + /* freeze MIB counters if supported */
> + if (dev->dev_ops->freeze_mib)
> + dev->dev_ops->freeze_mib(dev, port, true);
> + mutex_lock(&mib->cnt_mutex);
> + port_r_cnt(dev, port);
> + mutex_unlock(&mib->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?
> +}
> +
> 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.
