Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <sjg@chromium.org>
Signed-off-by: Naveen Krishna Chatradhi <ch.naveen@samsung.com>
Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>

authored by

Simon Glass and committed by
Wolfram Sang
069a9502 5304032c

+33 -4
+33 -4
drivers/i2c/busses/i2c-s3c2410.c
··· 601 601 return IRQ_HANDLED; 602 602 } 603 603 604 + /* 605 + * Disable the bus so that we won't get any interrupts from now on, or try 606 + * to drive any lines. This is the default state when we don't have 607 + * anything to send/receive. 608 + * 609 + * If there is an event on the bus, or we have a pre-existing event at 610 + * kernel boot time, we may not notice the event and the I2C controller 611 + * will lock the bus with the I2C clock line low indefinitely. 612 + */ 613 + static inline void s3c24xx_i2c_disable_bus(struct s3c24xx_i2c *i2c) 614 + { 615 + unsigned long tmp; 616 + 617 + /* Stop driving the I2C pins */ 618 + tmp = readl(i2c->regs + S3C2410_IICSTAT); 619 + tmp &= ~S3C2410_IICSTAT_TXRXEN; 620 + writel(tmp, i2c->regs + S3C2410_IICSTAT); 621 + 622 + /* We don't expect any interrupts now, and don't want send acks */ 623 + tmp = readl(i2c->regs + S3C2410_IICCON); 624 + tmp &= ~(S3C2410_IICCON_IRQEN | S3C2410_IICCON_IRQPEND | 625 + S3C2410_IICCON_ACKEN); 626 + writel(tmp, i2c->regs + S3C2410_IICCON); 627 + } 628 + 604 629 605 630 /* s3c24xx_i2c_set_master 606 631 * ··· 760 735 761 736 s3c24xx_i2c_wait_idle(i2c); 762 737 738 + s3c24xx_i2c_disable_bus(i2c); 739 + 763 740 out: 741 + i2c->state = STATE_IDLE; 742 + 764 743 return ret; 765 744 } 766 745 ··· 1033 1004 1034 1005 static int s3c24xx_i2c_init(struct s3c24xx_i2c *i2c) 1035 1006 { 1036 - unsigned long iicon = S3C2410_IICCON_IRQEN | S3C2410_IICCON_ACKEN; 1037 1007 struct s3c2410_platform_i2c *pdata; 1038 1008 unsigned int freq; 1039 1009 ··· 1046 1018 1047 1019 dev_info(i2c->dev, "slave address 0x%02x\n", pdata->slave_addr); 1048 1020 1049 - writel(iicon, i2c->regs + S3C2410_IICCON); 1021 + writel(0, i2c->regs + S3C2410_IICCON); 1022 + writel(0, i2c->regs + S3C2410_IICSTAT); 1050 1023 1051 1024 /* we need to work out the divisors for the clock... */ 1052 1025 1053 1026 if (s3c24xx_i2c_clockrate(i2c, &freq) != 0) { 1054 - writel(0, i2c->regs + S3C2410_IICCON); 1055 1027 dev_err(i2c->dev, "cannot meet bus frequency required\n"); 1056 1028 return -EINVAL; 1057 1029 } ··· 1059 1031 /* todo - check that the i2c lines aren't being dragged anywhere */ 1060 1032 1061 1033 dev_info(i2c->dev, "bus frequency set to %d KHz\n", freq); 1062 - dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02lx\n", iicon); 1034 + dev_dbg(i2c->dev, "S3C2410_IICCON=0x%02x\n", 1035 + readl(i2c->regs + S3C2410_IICCON)); 1063 1036 1064 1037 return 0; 1065 1038 }