From 069a9502dd7c51cf2f9031cd86d88774c696101c Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Fri, 7 Feb 2014 14:24:09 +0530 Subject: i2c: s3c2410: Leave the bus disabled unless it is in use There is a rather odd feature of the exynos i2c controller that if it is left enabled, it can lock itself up with the clk line held low. This makes the bus unusable. Unfortunately, the s3c24xx_i2c_set_master() function does not notice this, and reports a timeout. From then on the bus cannot be used until the AP is rebooted. The problem happens when any sort of interrupt occurs (e.g. due to a bus transition) when we are not in the middle of a transaction. We have seen many instances of this when U-Boot leaves the bus apparently happy, but Linux cannot access it. The current code is therefore pretty fragile. This fixes things by leaving the bus disabled unless we are actually in a transaction. We enable the bus at the start of the transaction and disable it at the end. That way we won't get interrupts and will not lock up the bus. It might be possible to clear pending interrupts on start-up, but this seems to be a more robust solution. We can't service interrupts when we are not in a transaction, and anyway would rather not lock up the bus while we try. Signed-off-by: Simon Glass Signed-off-by: Naveen Krishna Chatradhi Acked-by: Kyungmin Park Signed-off-by: Wolfram Sang --- drivers/i2c/busses/i2c-s3c2410.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) (limited to 'drivers/i2c') diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c index 8162901ff753..ae4491062e41 100644 --- a/drivers/i2c/busses/i2c-s3c2410.c +++ b/drivers/i2c/busses/i2c-s3c2410.c @@ -601,6 +601,31 @@ static irqreturn_t s3c24xx_i2c_irq(int irqno, void *dev_id) return IRQ_HANDLED; } +/* + * Disable the bus so that we won't get any interrupts from now on, or try + * to drive any lines. This is the default state when we don't have + * anything to send/receive. + * + * If there is an event on the bus, or we have a pre-existing event at + * kernel boot time, we may not notice the event and the I2C controller + * will lock the bus with the I2C clock line low indefinitely. + */ +static inline void s3c24xx_i2c_disable_bus(struct s3c24xx_i2c *i2c) +{ + unsigned long tmp; + + /* Stop driving the I2C pins */ + tmp = readl(i2c->regs + S3C2410_IICSTAT); + tmp &= ~S3C2410_IICSTAT_TXRXEN; + writel(tmp, i2c->regs + S3C2410_IICSTAT); + + /* We don't expect any interrupts now, and don't want send acks */ + tmp = readl(i2c->regs + S3C2410_IICCON); + tmp &= ~(S3C2410_IICCON_IRQEN | S3C2410_IICCON_IRQPEND | + S3C2410_IICCON_ACKEN); + writel(tmp, i2c->regs + S3C2410_IICCON); +} + /* s3c24xx_i2c_set_master * @@ -735,7 +760,11 @@ static int s3c24xx_i2c_doxfer(struct s3c24xx_i2c *i2c, s3c24xx_i2c_wait_idle(i2c); + s3c24xx_i2c_disable_bus(i2c); + out: + i2c->state = STATE_IDLE; + return ret; } @@ -1004,7 +1033,6 @@ static void s3c24xx_i2c_dt_gpio_free(struct s3c24xx_i2c *i2c) static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) { - unsigned long iicon = S3C2410_IICCON_IRQEN | S3C2410_IICCON_ACKEN; struct s3c2410_platform_i2c *pdata; unsigned int freq; @@ -1018,12 +1046,12 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) dev_info(i2c->dev, "slave address 0x%02x\n", pdata->slave_addr); - writel(iicon, i2c->regs + S3C2410_IICCON); + writel(0, i2c->regs + S3C2410_IICCON); + writel(0, i2c->regs + S3C2410_IICSTAT); /* we need to work out the divisors for the clock... */ if (s3c24xx_i2c_clockrate(i2c, &freq) != 0) { - writel(0, i2c->regs + S3C2410_IICCON); dev_err(i2c->dev, "cannot meet bus frequency required\n"); return -EINVAL; } @@ -1031,7 +1059,8 @@ static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) /* todo - check that the i2c lines aren't being dragged anywhere */ dev_info(i2c->dev, "bus frequency set to %d KHz\n", freq); - dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02lx\n", iicon); + dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02x\n", + readl(i2c->regs + S3C2410_IICCON)); return 0; } -- cgit v1.2.3