Re: [PATCH v5 08/10] i2c: npcm: Remove own slave addresses 2:10

Wolfram Sang
2022-05-21T05:59:06Z

[-- Attachment #1: Type: text/plain, Size: 2079 bytes --]

> NPCM can support up to 10 own slave addresses. In practice, only one
> address is actually being used. In order to access addresses 2 and above,
> need to switch register banks. The switch needs spinlock.
> To avoid using spinlock for this useless feature removed support of SA >=
> 2. Also fix returned slave event enum.

Is the spinlock contention so high? The code paths do not really look
like hot paths to me. A bit sad to see this feature go.

> static const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = {
> NPCM_I2CADDR1, NPCM_I2CADDR2, NPCM_I2CADDR3, NPCM_I2CADDR4,
> NPCM_I2CADDR5, NPCM_I2CADDR6, NPCM_I2CADDR7, NPCM_I2CADDR8,

Why do we keep this array if we drop the support?

> @@ -604,8 +602,7 @@ static int npcm_i2c_slave_enable(struct npcm_i2c *bus, enum i2c_addr addr_type,
> i2cctl1 &= ~NPCM_I2CCTL1_GCMEN;
> iowrite8(i2cctl1, bus->reg + NPCM_I2CCTL1);
> return 0;
> - }
> - if (addr_type == I2C_ARP_ADDR) {
> + } else if (addr_type == I2C_ARP_ADDR) {

I might be wrong but this looks like a seperate change?

> @@ -924,11 +918,15 @@ static int npcm_i2c_slave_get_wr_buf(struct npcm_i2c *bus)
> for (i = 0; i < I2C_HW_FIFO_SIZE; i++) {
> if (bus->slv_wr_size >= I2C_HW_FIFO_SIZE)
> break;
> - i2c_slave_event(bus->slave, I2C_SLAVE_READ_REQUESTED, &value);
> + if (bus->state == I2C_SLAVE_MATCH) {
> + i2c_slave_event(bus->slave, I2C_SLAVE_READ_REQUESTED, &value);
> + bus->state = I2C_OPER_STARTED;
> + } else {
> + i2c_slave_event(bus->slave, I2C_SLAVE_READ_PROCESSED, &value);
> + }
> ind = (bus->slv_wr_ind + bus->slv_wr_size) % I2C_HW_FIFO_SIZE;
> bus->slv_wr_buf[ind] = value;
> bus->slv_wr_size++;
> - i2c_slave_event(bus->slave, I2C_SLAVE_READ_PROCESSED, &value);
> }
> return I2C_HW_FIFO_SIZE - ret;
> }
> @@ -976,7 +974,6 @@ static void npcm_i2c_slave_xmit(struct npcm_i2c *bus, u16 nwrite,
> if (nwrite == 0)
> return;
>
> - bus->state = I2C_OPER_STARTED;
> bus->operation = I2C_WRITE_OPER;

This is definately a seperate change!

All the best!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]