| 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
| ||
|
Message-ID: <20141124190937.GF25712@saruman>
Date: Mon, 24 Nov 2014 13:09:37 -0600
From: Felipe Balbi <balbi@...com>
To: Alexander Kochetkov <al.kochet@...il.com>
CC: <linux-omap@...r.kernel.org>, <linux-i2c@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, Wolfram Sang <wsa@...-dreams.de>,
Tony Lindgren <tony@...mide.com>, Felipe Balbi <balbi@...com>,
Kevin Hilman <khilman@...aro.org>
Subject: Re: [PATCH 2/4] i2c: omap: implement workaround for handling invalid
BB-bit values
Hi,
On Fri, Nov 21, 2014 at 01:28:43AM +0400, Alexander Kochetkov wrote:
> In a multimaster environment, after IP software reset, BB-bit value doesn't
> correspond to the current bus state. It may happen what BB-bit will be 0,
> while the bus is busy due to another I2C master activity.
>
> Any transfer started when BB=0 and bus is busy wouldn't be completed by IP
> and results in controller timeout. More over, in some cases IP could
> interrupt another master's transfer and corrupt data on wire.
>
> The commit implement method allowing to prevent IP from entering into
> "controller timeout" state and from "data corruption" state.
>
> The one drawback is the need to wait for 10ms before the first transfer.
>
> Tested on Beagleboard XM C.
>
> Signed-off-by: Alexander Kochetkov <al.kochet@...il.com>
we have a report from Kevin Hilman that this commit breaks OMAP3530, see
[1]
[1] http://storage.armcloud.us/kernel-ci/next/next-20141124/arm-omap2plus_defconfig/boot-omap3-overo-tobi.log
> ---
> drivers/i2c/busses/i2c-omap.c | 103 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
> index a021733..3ffb9c0 100644
> --- a/drivers/i2c/busses/i2c-omap.c
> +++ b/drivers/i2c/busses/i2c-omap.c
> @@ -58,6 +58,9 @@
> /* timeout for pm runtime autosuspend */
> #define OMAP_I2C_PM_TIMEOUT 1000 /* ms */
>
> +/* timeout for making decision on bus free status */
> +#define OMAP_I2C_BUS_FREE_TIMEOUT (msecs_to_jiffies(10))
> +
> /* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */
> enum {
> OMAP_I2C_REV_REG = 0,
> @@ -210,6 +213,9 @@ struct omap_i2c_dev {
> */
> u32 rev;
> unsigned b_hw:1; /* bad h/w fixes */
> + unsigned bb_valid:1; /* true when BB-bit reflects
> + * the I2C bus state
> + */
> unsigned receiver:1; /* true when we're in receiver mode */
> u16 iestate; /* Saved interrupt register */
> u16 pscstate;
> @@ -336,7 +342,10 @@ static int omap_i2c_reset(struct omap_i2c_dev *dev)
> /* SYSC register is cleared by the reset; rewrite it */
> omap_i2c_write_reg(dev, OMAP_I2C_SYSC_REG, sysc);
>
> + /* Schedule I2C-bus monitoring on the next transfer */
> + dev->bb_valid = 0;
> }
> +
> return 0;
> }
>
> @@ -449,6 +458,11 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
> dev->scllstate = scll;
> dev->sclhstate = sclh;
>
> + if (dev->rev < OMAP_I2C_OMAP1_REV_2) {
> + /* Not implemented */
> + dev->bb_valid = 1;
> + }
> +
> __omap_i2c_init(dev);
>
> return 0;
> @@ -473,6 +487,91 @@ static int omap_i2c_wait_for_bb(struct omap_i2c_dev *dev)
> return 0;
> }
>
> +/*
> + * Wait while BB-bit doesn't reflect the I2C bus state
> + *
> + * In a multimaster environment, after IP software reset, BB-bit value doesn't
> + * correspond to the current bus state. It may happen what BB-bit will be 0,
> + * while the bus is busy due to another I2C master activity.
> + * Here are BB-bit values after reset:
> + * SDA SCL BB NOTES
> + * 0 0 0 1, 2
> + * 1 0 0 1, 2
> + * 0 1 1
> + * 1 1 0 3
> + * Later, if IP detect SDA=0 and SCL=1 (ACK) or SDA 1->0 while SCL=1 (START)
> + * combinations on the bus, it set BB-bit to 1.
> + * If IP detect SDA 0->1 while SCL=1 (STOP) combination on the bus,
> + * it set BB-bit to 0 and BF to 1.
> + * BB and BF bits correctly tracks the bus state while IP is suspended
> + * BB bit became valid on the next FCLK clock after CON_EN bit set
> + *
> + * NOTES:
> + * 1. Any transfer started when BB=0 and bus is busy wouldn't be
> + * completed by IP and results in controller timeout.
> + * 2. Any transfer started when BB=0 and SCL=0 results in IP
> + * starting to drive SDA low. In that case IP corrupt data
> + * on the bus.
> + * 3. Any transfer started in the middle of another master's transfer
> + * results in unpredictable results and data corruption
> + */
> +static int omap_i2c_wait_for_bb_valid(struct omap_i2c_dev *dev)
> +{
> + unsigned long bus_free_timeout = 0;
> + unsigned long timeout;
> + int bus_free = 0;
> + u16 stat, systest;
> +
> + if (dev->bb_valid)
> + return 0;
> +
> + timeout = jiffies + OMAP_I2C_TIMEOUT;
> + while (1) {
> + stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
> + /*
> + * We will see BB or BF event in a case IP had detected any
> + * activity on the I2C bus. Now IP correctly tracks the bus
> + * state. BB-bit value is valid.
> + */
> + if (stat & (OMAP_I2C_STAT_BB | OMAP_I2C_STAT_BF))
> + break;
> +
> + /*
> + * Otherwise, we must look signals on the bus to make
> + * the right decision.
> + */
> + systest = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
> + if ((systest & OMAP_I2C_SYSTEST_SCL_I_FUNC) &&
> + (systest & OMAP_I2C_SYSTEST_SDA_I_FUNC)) {
> + if (!bus_free) {
> + bus_free_timeout = jiffies +
> + OMAP_I2C_BUS_FREE_TIMEOUT;
> + bus_free = 1;
> + }
> +
> + /*
> + * SDA and SCL lines was high for 10 ms without bus
> + * activity detected. The bus is free. Consider
> + * BB-bit value is valid.
> + */
> + if (time_after(jiffies, bus_free_timeout))
> + break;
> + } else {
> + bus_free = 0;
> + }
> +
> + if (time_after(jiffies, timeout)) {
> + dev_warn(dev->dev, "timeout waiting for bus ready\n");
> + return -ETIMEDOUT;
> + }
> +
> + msleep(1);
> + }
> +
> + dev->bb_valid = 1;
> + return 0;
> +}
> +
> static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx)
> {
> u16 buf;
> @@ -643,6 +742,10 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> if (IS_ERR_VALUE(r))
> goto out;
>
> + r = omap_i2c_wait_for_bb_valid(dev);
> + if (r < 0)
> + goto out;
> +
> r = omap_i2c_wait_for_bb(dev);
> if (r < 0)
> goto out;
> --
> 1.7.9.5
>
--
balbi
Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)
Powered by blists - more mailing lists