Skip to content

Fix grabbing lock from atomic context in i2c driver #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 6, 2015
Merged

Fix grabbing lock from atomic context in i2c driver #780

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 65 additions & 25 deletions drivers/i2c/busses/i2c-bcm2708.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -85,6 +86,7 @@ struct bcm2708_i2c {
void __iomem *base;
int irq;
struct clk *clk;
u32 cdiv;

struct completion done;

Expand All @@ -108,10 +110,10 @@ static void bcm2708_i2c_init_pinmode(int id)
int pin;
u32 *gpio = ioremap(GPIO_BASE, 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 */
}
Expand Down Expand Up @@ -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;
Expand All @@ -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++;
Expand All @@ -196,43 +206,61 @@ 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)
{
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);

if (s & (BSC_S_CLKT | BSC_S_ERR)) {
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);

if (bi->nmsgs) {
/* 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);
}
Expand Down Expand Up @@ -265,22 +293,33 @@ static int bcm2708_i2c_master_xfer(struct i2c_adapter *adap,
bi->nmsgs = num;
bi->error = false;

bcm2708_bsc_setup(bi);
ret = bcm2708_bsc_setup(bi);

/* unlockig _after_ the setup to avoid races with the interrupt routine */
spin_unlock_irqrestore(&bi->lock, flags);

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)
Expand Down Expand Up @@ -383,6 +422,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);
Expand Down