From 2c84355bf200f4d19d7078dee2c63011ad715395 Mon Sep 17 00:00:00 2001 From: jeanleflambeur Date: Sun, 1 Feb 2015 12:35:38 +0100 Subject: [PATCH 2/2] Fix grabbing lock from atomic context in i2c driver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit (cherry-pick from 558d0bfc8fe80ccdccee7f03e881a80965ec987c) 2 main changes: - check for timeouts in the bcm2708_bsc_setup function as indicated by this comment: /* poll for transfer start bit (should only take 1-20 polls) */ This implies that the setup function can now fail so account for this everywhere it's called - Removed the clk_get_rate call from inside the setup function as it locks a mutex and that's not ok since we call it from under a spin lock. removed dead code and update comment fixed typo in comment Upstream-Status: Pending Signed-off-by: Petter Mabäcker Conflicts: drivers/i2c/busses/i2c-bcm2708.c --- drivers/i2c/busses/i2c-bcm2708.c | 88 +++++++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 23 deletions(-) diff --git a/drivers/i2c/busses/i2c-bcm2708.c b/drivers/i2c/busses/i2c-bcm2708.c index 05531db..886672c 100644 --- a/drivers/i2c/busses/i2c-bcm2708.c +++ b/drivers/i2c/busses/i2c-bcm2708.c @@ -67,6 +67,7 @@ #define BSC_S_TA 0x00000001 #define I2C_TIMEOUT_MS 150 +#define I2C_WAIT_LOOP_COUNT 40 #define DRV_NAME "bcm2708_i2c" @@ -85,6 +86,7 @@ struct bcm2708_i2c { void __iomem *base; int irq; struct clk *clk; + u32 cdiv; struct completion done; @@ -108,10 +110,10 @@ static void bcm2708_i2c_init_pinmode(int id) int pin; u32 *gpio = ioremap(0x20200000, SZ_16K); - BUG_ON(id != 0 && id != 1); + BUG_ON(id != 0 && id != 1); /* BSC0 is on GPIO 0 & 1, BSC1 is on GPIO 2 & 3 */ for (pin = id*2+0; pin <= id*2+1; pin++) { -printk("bcm2708_i2c_init_pinmode(%d,%d)\n", id, pin); + printk("bcm2708_i2c_init_pinmode(%d,%d)\n", id, pin); INP_GPIO(pin); /* set mode to GPIO input first */ SET_GPIO_ALT(pin, 0); /* set mode to ALT 0 */ } @@ -150,16 +152,16 @@ static inline void bcm2708_bsc_fifo_fill(struct bcm2708_i2c *bi) bcm2708_wr(bi, BSC_FIFO, bi->msg->buf[bi->pos++]); } -static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi) +static inline int bcm2708_bsc_setup(struct bcm2708_i2c *bi) { - unsigned long bus_hz; u32 cdiv, s; u32 c = BSC_C_I2CEN | BSC_C_INTD | BSC_C_ST | BSC_C_CLEAR_1; + int wait_loops = I2C_WAIT_LOOP_COUNT; - bus_hz = clk_get_rate(bi->clk); - cdiv = bus_hz / baudrate; - if (cdiv > 0xffff) - cdiv = 0xffff; + /* Can't call clk_get_rate as it locks a mutex and here we are spinlocked. + * Use the value that we cached in the probe. + */ + cdiv = bi->cdiv; if (bi->msg->flags & I2C_M_RD) c |= BSC_C_INTR | BSC_C_READ; @@ -176,17 +178,25 @@ static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi) - Both messages to same slave address - Write message can fit inside FIFO (16 bytes or less) */ if ( (bi->nmsgs > 1) && - !(bi->msg[0].flags & I2C_M_RD) && (bi->msg[1].flags & I2C_M_RD) && - (bi->msg[0].addr == bi->msg[1].addr) && (bi->msg[0].len <= 16)) { + !(bi->msg[0].flags & I2C_M_RD) && (bi->msg[1].flags & I2C_M_RD) && + (bi->msg[0].addr == bi->msg[1].addr) && (bi->msg[0].len <= 16)) { /* Fill FIFO with entire write message (16 byte FIFO) */ - while (bi->pos < bi->msg->len) + while (bi->pos < bi->msg->len) { bcm2708_wr(bi, BSC_FIFO, bi->msg->buf[bi->pos++]); + } /* Start write transfer (no interrupts, don't clear FIFO) */ bcm2708_wr(bi, BSC_C, BSC_C_I2CEN | BSC_C_ST); + /* poll for transfer start bit (should only take 1-20 polls) */ do { s = bcm2708_rd(bi, BSC_S); - } while (!(s & (BSC_S_TA | BSC_S_ERR | BSC_S_CLKT | BSC_S_DONE))); + } while (!(s & (BSC_S_TA | BSC_S_ERR | BSC_S_CLKT | BSC_S_DONE)) && --wait_loops >= 0); + + /* did we time out or some error occured? */ + if (wait_loops < 0 || (s & (BSC_S_ERR | BSC_S_CLKT))) { + return -1; + } + /* Send next read message before the write transfer finishes. */ bi->nmsgs--; bi->msg++; @@ -196,6 +206,8 @@ static inline void bcm2708_bsc_setup(struct bcm2708_i2c *bi) } } bcm2708_wr(bi, BSC_C, c); + + return 0; } static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id) @@ -203,13 +215,15 @@ static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id) struct bcm2708_i2c *bi = dev_id; bool handled = true; u32 s; + int ret; spin_lock(&bi->lock); /* we may see camera interrupts on the "other" I2C channel - Just return if we've not sent anything */ - if (!bi->nmsgs || !bi->msg ) + Just return if we've not sent anything */ + if (!bi->nmsgs || !bi->msg) { goto early_exit; + } s = bcm2708_rd(bi, BSC_S); @@ -217,13 +231,16 @@ static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id) bcm2708_bsc_reset(bi); bi->error = true; + bi->msg = 0; /* to inform the that all work is done */ + bi->nmsgs = 0; /* wake up our bh */ complete(&bi->done); } else if (s & BSC_S_DONE) { bi->nmsgs--; - if (bi->msg->flags & I2C_M_RD) + if (bi->msg->flags & I2C_M_RD) { bcm2708_bsc_fifo_drain(bi); + } bcm2708_bsc_reset(bi); @@ -231,8 +248,19 @@ static irqreturn_t bcm2708_i2c_interrupt(int irq, void *dev_id) /* advance to next message */ bi->msg++; bi->pos = 0; - bcm2708_bsc_setup(bi); + ret = bcm2708_bsc_setup(bi); + if (ret < 0) { + bcm2708_bsc_reset(bi); + bi->error = true; + bi->msg = 0; /* to inform the that all work is done */ + bi->nmsgs = 0; + /* wake up our bh */ + complete(&bi->done); + goto early_exit; + } } else { + bi->msg = 0; /* to inform the that all work is done */ + bi->nmsgs = 0; /* wake up our bh */ complete(&bi->done); } @@ -265,21 +293,34 @@ static int bcm2708_i2c_master_xfer(struct i2c_adapter *adap, bi->nmsgs = num; bi->error = false; + ret = bcm2708_bsc_setup(bi); spin_unlock_irqrestore(&bi->lock, flags); bcm2708_bsc_setup(bi); - ret = wait_for_completion_timeout(&bi->done, - msecs_to_jiffies(I2C_TIMEOUT_MS)); + /* check the result of the setup */ + if (ret < 0) + { + dev_err(&adap->dev, "transfer setup timed out\n"); + goto error_timeout; + } + + ret = wait_for_completion_timeout(&bi->done, msecs_to_jiffies(I2C_TIMEOUT_MS)); if (ret == 0) { dev_err(&adap->dev, "transfer timed out\n"); - spin_lock_irqsave(&bi->lock, flags); - bcm2708_bsc_reset(bi); - spin_unlock_irqrestore(&bi->lock, flags); - return -ETIMEDOUT; + goto error_timeout; } - return bi->error ? -EIO : num; + ret = bi->error ? -EIO : num; + return ret; + +error_timeout: + spin_lock_irqsave(&bi->lock, flags); + bcm2708_bsc_reset(bi); + bi->msg = 0; /* to inform the interrupt handler that there's nothing else to be done */ + bi->nmsgs = 0; + spin_unlock_irqrestore(&bi->lock, flags); + return -ETIMEDOUT; } static u32 bcm2708_i2c_functionality(struct i2c_adapter *adap) @@ -382,6 +423,7 @@ static int bcm2708_i2c_probe(struct platform_device *pdev) cdiv = 0xffff; baudrate = bus_hz / cdiv; } + bi->cdiv = cdiv; dev_info(&pdev->dev, "BSC%d Controller at 0x%08lx (irq %d) (baudrate %d)\n", pdev->id, (unsigned long)regs->start, irq, baudrate); -- 1.9.1