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

From	Wolfram Sang <wsa@the-dreams.de>
Newsgroups	linux.kernel
Subject	Re: [RFC 1/1] drivers: i2c: omap: Add slave support
Date	2016-08-25 19:20 +0200
Message-ID	<sa6tP-3Uf-5@gated-at.bofh.it> (permalink)
References	<rCHPb-1YV-9@gated-at.bofh.it> <rCHPb-1YV-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

[Multipart message — attachments visible in raw view] - view raw

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

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

Re: [RFC 1/1] drivers: i2c: omap: Add slave support Wolfram Sang <wsa@the-dreams.de> - 2016-08-25 19:20 +0200
  Re: [RFC 1/1] drivers: i2c: omap: Add slave support Matthijs van Duin <matthijsvanduin@gmail.com> - 2016-08-27 16:10 +0200
    Re: [RFC 1/1] drivers: i2c: omap: Add slave support Wolfram Sang <wsa@the-dreams.de> - 2016-08-27 19:30 +0200
      Re: [RFC 1/1] drivers: i2c: omap: Add slave support Matthijs van Duin <matthijsvanduin@gmail.com> - 2016-08-28 01:40 +0200
        Re: [RFC 1/1] drivers: i2c: omap: Add slave support Wolfram Sang <wsa@the-dreams.de> - 2016-08-28 07:40 +0200
          Re: [RFC 1/1] drivers: i2c: omap: Add slave support Matthijs van Duin <matthijsvanduin@gmail.com> - 2016-08-29 05:50 +0200

csiph-web