Re: [RFC 1/1] drivers: i2c: omap: Add slave support

[Wolfram Sang <wsa@the-dreams.de>]

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

Hi,

> The omap i2c controller (at least on dra7x devices)
> doesn't have  start/stop (STT/STP) support for slave mode
> so event  #5 is not implemented in the driver.

I think you can deduce that. If a new {READ|WRITE}_REQUESTED slave event
comes in when you had *_PROCESSED events before, there must have been a
STOP on the bus inbetween.

> +		if (stat & OMAP_I2C_STAT_XRDY) {
> +			i2c_slave_event(omap->slave, I2C_SLAVE_READ_REQUESTED,
> +					&value);
> +			omap_i2c_write_reg(omap, OMAP_I2C_DATA_REG, value);
> +			i2c_slave_event(omap->slave, I2C_SLAVE_READ_PROCESSED,
> +					&value);

This looks fishy. READ_REQUESTED is only sent after the address phase.
Have you read the documentation (Documentation/i2c/slave-interface)?
Please say if it was unclear.

> +	/* As of now, We dont need all interrupts be enabled */
> +	omap->iestate = OMAP_I2C_IE_AAS | OMAP_I2C_IE_XRDY | OMAP_I2C_IE_RRDY;

This looks even more fishy. Are you disabling the master interrupts?
That's a no (unless there are HW constraints). Your driver should be
able to switch between master and slave depending on what happens on the
bus.

For more guidance, here is my talk at ELCE 2015:
https://www.youtube.com/watch?v=JdQ21jlwb58

Regards,

   Wolfram


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