Hi! > From: Tristram Ha <tristram...@microchip.com> > > Add MIB counter reading support. > Rename ksz_9477_reg.h to ksz9477_reg.h for consistency as the product > name is always KSZ####. > Header file ksz_priv.h no longer contains any chip specific data.
Nothing obviously wrong here, but if you are doing another iteration,
it would be nice to separate "Rename ksz_9477_reg.h to ksz9477_reg.h"
from the code changes.
Best regards,
> + 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);
1000 iterations, 1usec each, so 1msec, but you allow up to 10 msec. Interesting.
> + /* failed to read MIB. get out of loop */
> + if (!timeout) {
> + dev_dbg(dev->dev, "Failed to get MIB\n");
> + return;
> + }
Are you sure this works? AFAICT "timeout" will underflow, so !timeout
will not trigger.
> -static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
> - uint64_t *buf)
> -{
> - struct ksz_device *dev = ds->priv;
> - int i;
> - u32 data;
> - int timeout;
> -
> - mutex_lock(&dev->stats_mutex);
> -
> - for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
> - data = MIB_COUNTER_READ;
> - data |= ((mib_names[i].index & 0xFF) << 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);
> -
> - /* failed to read MIB. get out of loop */
> - if (!timeout) {
> - dev_dbg(dev->dev, "Failed to get MIB\n");
> - break;
> - }
Hmm. Bug was there already...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
