Re: [PATCH 1/1] i2c: add slave testunit driver

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

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

Hi Alain,

> great that you published your slave-unittest driver.
> I gave it a try rapidly and can confirm it works as expected on the stm32
> platform. I tested with a loopback between 2 i2c controllers.

Thanks, much appreciated! I read this as Tested-by :)

> > +config I2C_SLAVE_TESTUNIT
> > +	tristate "I2C eeprom testunit driver"
> 
> Should be tristate "I2C testunit driver" I believe (no eeprom)

Oops, yes.

> > +	switch (tu->regs[TU_REG_CMD]) {
> > +	case TU_CMD_READ_BYTES:
> > +		msg.addr = tu->regs[TU_REG_DATAL];
> > +		msg.flags = I2C_M_RD;
> > +		msg.len = tu->regs[TU_REG_DATAH];
> 
> Some protection might be needed here since the size is coming straight from
> the i2cset command while the buffer is limited to 256 bytes.

Well, the size is not a problem because all the regs are u8. But for the
address, we should mask the lower 7 bits, in deed. We could use the free
8th bit to double the length, but we probably better keep it reserved for
something yet unknown.

Thanks for the review!

   Wolfram


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