Messages in this thread | | | Date | Wed, 11 Oct 2017 23:03:29 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH RFC] Add Microchip KSZ8895 DSA driver |
| |
Hi!
> +static void ksz8895_set_prio_queue(struct ksz_device *dev, int port, int queue) > +{ > + u8 hi; > + u8 lo; > + > + /* Number of queues can only be 1, 2, or 4. */ > + switch (queue) { > + case 4: > + case 3: > + queue = PORT_QUEUE_SPLIT_4; > + break; > + case 2: > + queue = PORT_QUEUE_SPLIT_2; > + break; > + default: > + queue = PORT_QUEUE_SPLIT_1; > + } > + ksz_pread8(dev, port, REG_PORT_CTRL_0, &lo); > + ksz_pread8(dev, port, P_DROP_TAG_CTRL, &hi); > + lo &= ~PORT_QUEUE_SPLIT_L; > + if (queue & PORT_QUEUE_SPLIT_2) > + lo |= PORT_QUEUE_SPLIT_L; > + hi &= ~PORT_QUEUE_SPLIT_H; > + if (queue & PORT_QUEUE_SPLIT_4) > + hi |= PORT_QUEUE_SPLIT_H; > + ksz_pwrite8(dev, port, REG_PORT_CTRL_0, lo); > + ksz_pwrite8(dev, port, P_DROP_TAG_CTRL, hi); > + > + /* Default is port based for egress rate limit. */ > + if (queue != PORT_QUEUE_SPLIT_1) > + ksz_cfg(dev, REG_SW_CTRL_19, SW_OUT_RATE_LIMIT_QUEUE_BASED, > + true); > +}
This is same as the other driver, right? Same comments apply here, and please find a way to make it shared.
> +static void ksz8895_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, > + u64 *cnt) > +{ > + u32 data; > + u16 ctrl_addr; > + u8 check; > + int loop; > + > + ctrl_addr = addr + SWITCH_COUNTER_NUM * port; > + ctrl_addr |= IND_ACC_TABLE(TABLE_MIB | TABLE_READ); > + > + mutex_lock(&dev->alu_mutex); > + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr); > + > + /* It is almost guaranteed to always read the valid bit because of > + * slow SPI speed. > + */ > + for (loop = 2; loop > 0; loop--) { > + ksz_read8(dev, REG_IND_MIB_CHECK, &check); > + > + if (check & MIB_COUNTER_VALID) { > + ksz_read32(dev, REG_IND_DATA_LO, &data); > + if (check & MIB_COUNTER_OVERFLOW) > + *cnt += MIB_COUNTER_VALUE + 1; > + *cnt += data & MIB_COUNTER_VALUE; > + break; > + } > + } > + mutex_unlock(&dev->alu_mutex);
Again, same function, same review comments.
> +static void ksz8895_r_table(struct ksz_device *dev, int table, u16 addr, > + u64 *data) > +{ > + u16 ctrl_addr; > + > + ctrl_addr = IND_ACC_TABLE(table | TABLE_READ) | addr; > + > + mutex_lock(&dev->alu_mutex); > + ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr); > + ksz_get(dev, REG_IND_DATA_HI, data, sizeof(u64)); > + mutex_unlock(&dev->alu_mutex); > + *data = be64_to_cpu(*data); > +}
I've seen this before; this is duplicated code, it does not make sense to review.
-- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html [unhandled content-type:application/pgp-signature] | |