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