Thanks for the review Andy. On Tue, Oct 01, 2019 at 07:29:13PM +0300, Andy Shevchenko wrote: > > > On Tue, Oct 01, 2019 at 10:59:59AM -0500, Patrick Williams wrote: > There are quite a few people in the Cc list. I'm not sure they all are > interested in this. I deliberately dropped few names, sorry, if I was mistaken. Agree it was kind of a big list. Just chose what was given to me by get_maintainer.pl. It seems like there isn't a direct identified maintainer of this file. > > > + if (isr & ISR_RWM) { > > + u8 byte = 0; > > + > > + i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED, > > + &byte); > > + writel(byte, _IDBR(i2c)); > > + } else { > > + i2c_slave_event(i2c->slave, I2C_SLAVE_WRITE_REQUESTED, > > + NULL); > > + } > > Hmm... Perhaps > > u8 byte = 0; > > i2c_slave_event(i2c->slave, I2C_SLAVE_READ_REQUESTED, &byte); > if (isr & ISR_RWM) > writel(byte, _IDBR(i2c)); > The two different paths also require READ_REQUEST vs WRITE_REQUESTED. I could do a ternary there but it seemed more obvious to just unroll the logic. -- - Patrick