Messages in this thread | | | Date | Thu, 21 May 2020 17:36:35 +0300 | From | Andy Shevchenko <> | Subject | Re: [PATCH v12 3/3] i2c: npcm7xx: Add support for slave mode for Nuvoton |
| |
On Thu, May 21, 2020 at 02:09:10PM +0300, Tali Perry wrote: > Add support for slave mode for Nuvoton > NPCM BMC I2C controller driver.
...
> +#if IS_ENABLED(CONFIG_I2C_SLAVE) > +/* > + * npcm_i2caddr array: > + * The module supports having multiple own slave addresses. > + * Since the addr regs are sprinkled all over the address space, > + * use this array to get the address or each register. > + */ > +#define I2C_NUM_OWN_ADDR 10 > +const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = {NPCM_I2CADDR1, NPCM_I2CADDR2,
Extra spaces. On top. please start assignment from the new line.
> + NPCM_I2CADDR3, NPCM_I2CADDR4, > + NPCM_I2CADDR5, NPCM_I2CADDR6, > + NPCM_I2CADDR7, NPCM_I2CADDR8,
> + NPCM_I2CADDR9, NPCM_I2CADDR10};
Split }; to new line and leave comma with the last member.
> +#endif
...
> +static int npcm_i2c_slave_enable(struct npcm_i2c *bus, enum i2c_addr addr_type, > + u8 addr, bool enable)
Extra spaces. Check entire patch for that and fix accordingly.
> +#if IS_ENABLED(CONFIG_I2C_SLAVE) > + if (bus->slave)
> + npcm_i2c_slave_enable(bus, I2C_SLAVE_ADDR1, bus->slave->addr, > + true);
I would leave this on one line.
> +#endif
...
> +static void npcm_i2c_write_fifo_slave(struct npcm_i2c *bus, u16 max_bytes) > +{ > + u8 size_free_fifo;
+ blank line.
> + /* > + * Fill the FIFO, while the FIFO is not full and there are more bytes > + * to write > + */ > + npcm_i2c_clear_fifo_int(bus); > + npcm_i2c_clear_tx_fifo(bus); > + iowrite8(0, bus->reg + NPCM_I2CTXF_CTL);
> + size_free_fifo = I2C_HW_FIFO_SIZE - npcm_i2c_fifo_usage(bus);
Dup, move into loop.
> + while (max_bytes-- && size_free_fifo) { > + if (bus->slv_wr_size > 0) { > + bus->slv_wr_ind = bus->slv_wr_ind % I2C_HW_FIFO_SIZE; > + npcm_i2c_wr_byte(bus, bus->slv_wr_buf[bus->slv_wr_ind]); > + bus->slv_wr_ind++; > + bus->slv_wr_ind = bus->slv_wr_ind % I2C_HW_FIFO_SIZE; > + bus->slv_wr_size--; > + size_free_fifo = I2C_HW_FIFO_SIZE - > + npcm_i2c_fifo_usage(bus); > + } else { > + break; > + } > + }
while (...) { if (...) break; ... }
> +}
...
> +static int npcm_i2c_slave_get_wr_buf(struct npcm_i2c *bus) > +{ > + int i;
> + u8 value = 0;
Redundant assignment.
> + int ind; > + int ret = bus->slv_wr_ind; > + > + /* fill a cyclic buffer */ > + 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); > + 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; > +}
-- With Best Regards, Andy Shevchenko
| |