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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 Aug 2016 19:14:30 +0200
From:   Wolfram Sang <wsa@...-dreams.de>
To:     Ravikumar Kattekola <rk@...com>
Cc:     tony@...mide.com, linux-omap@...r.kernel.org,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC 1/1] drivers: i2c: omap: Add slave support

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

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)