: Correct i2c support on am33xx

PATCH New Tom Rini 2 participants 12 messages May 21, 2012 – May 22, 2012

Subsystems (2)

Arm (32-bit) I2C

lore

Share & tools

lore

Tom Rini

author

2012-05-21 16:46 UTC #0 lore raw

cover : Correct i2c support on am33xx

Hey all,

The following series corrects i2c support for the am33xx family of devices (including beaglebone) and has been tested on a beagleboard xM and pandaboard. I would like to take these changes in via the u-boot-ti tree as it's mostly changes to arch and board files but I've cc'd Heiko on the i2c related parts. The biggest change here is to revert a previous change to the omap24xx_i2c.c driver for the i2c probe function. In short, the change made before violated the TRM constraints but did no harm on the IP block found in omap3/related boards. Moving forward to the IP block found on omap4 and am33xx (and others) it still voilates the TRM and now leaves the bus in an unboundedly defined state like the TRM states. We must revert this change for both correctness and functionality. The rest of the series is minor corrections to structs/defines and adding CONFIG_AM33XX to the block of !CONFIG_OMAP2430. I've thought of, but think it should be separate to change omap24xx_i2c.c to test for CONFIG_OMAP2430 rather than every other present and future user in a few areas.

Signature · 1 line

Tom

Tom Rini

author

2012-05-21 16:46 UTC #1 lore raw

PATCH 1/5 Revert "I2C: OMAP: detect more devices when probing an i2c bus"

This reverts commit 0e57968a215d1b9d271f3fa5bebeddeaea0c8075.

The short version of the original commit is that some i2c devices cannot be probed via read as they NAK the first cycle, so try and probe via a write that we abort before it writes to the device. This however is not allowed by the TRM for any of these parts. The section on I2C_CON (table 17-35 I2C_CON for am/dm37x for example) says you must not change the register while STT has been set. On these parts, the unpredictable behavior that the chip exhibits is not problematic. On OMAP4 however it results in the chip being in a bad state:
Panda # i2c probe
Valid chip addresses: 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F 60 61 62 63 64 65 66 67 68 69 6A 6B 6C 6D 6E 6F 70 71 72 73 74 75 76 77 78 79 7A 7B 7C 7D 7E 7F
Panda # i2c md 50 0
timed out in wait_for_pin: I2C_STAT=0
I2C read: I/O error
Error reading the chip.

We must revert the original behavior to bring probe back into line with the TRM.

Cc Nick Thompson <[email protected]>

Cc Heiko Schocher <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/i2c/omap24xx_i2c.c |   42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

drivers/i2c/omap24xx_i2c.c +31 ~~-11~~

index a7ffd95..3a50417 100644
@@ -255,23 +255,43 @@ int i2c_probe(uchar chip)
 	/* wait until bus not busy */
 	wait_for_bb();
 
-	/* try to write one byte */
+	/* try to read one byte */
 	writew(1, &i2c_base->cnt);
 	/* set slave address */
 	writew(chip, &i2c_base->sa);
 	/* stop bit needed here */
-	writew(I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
-	       I2C_CON_STP, &i2c_base->con);
-
-	status = wait_for_pin();
+	writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, &i2c_base->con);
 
-	/* check for ACK (!NAK) */
-	if (!(status & I2C_STAT_NACK))
-		res = 0;
-
-	/* abort transfer (force idle state) */
-	writew(0, &i2c_base->con);
+	while (1) {
+		status = wait_for_pin();
+		if (status == 0 || status & I2C_STAT_AL) {
+			res = 1;
+			goto probe_exit;
+		}
+		if (status & I2C_STAT_NACK) {
+			res = 1;
+			writew(0xff, &i2c_base->stat);
+			writew (readw (&i2c_base->con) | I2C_CON_STP, &i2c_base->con);
+			wait_for_bb ();
+			break;
+		}
+		if (status & I2C_STAT_ARDY) {
+			writew(I2C_STAT_ARDY, &i2c_base->stat);
+			break;
+		}
+		if (status & I2C_STAT_RRDY) {
+			res = 0;
+#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
+    defined(CONFIG_OMAP44XX)
+			readb(&i2c_base->data);
+#else
+			readw(&i2c_base->data);
+#endif
+			writew(I2C_STAT_RRDY, &i2c_base->stat);
+		}
+	}
 
+probe_exit:
 	flush_fifo();
 	/* don't allow any more data in... we don't want it. */
 	writew(0, &i2c_base->cnt);

Signature · 1 line

1.7.9.5

Heiko Schocher

2012-05-22 12:11 UTC #2 lore raw

PATCH 1/5 Revert "I2C: OMAP: detect more devices when probing an i2c bus"

Hello Tom,

Quoted 25 lines · Tom Rini

This reverts commit 0e57968a215d1b9d271f3fa5bebeddeaea0c8075.
The short version of the original commit is that some i2c devices cannot be probed via read as they NAK the first cycle, so try and probe via a write that we abort before it writes to the device. This however is not allowed by the TRM for any of these parts. The section on I2C_CON (table 17-35 I2C_CON for am/dm37x for example) says you must not change the register while STT has been set. On these parts, the unpredictable behavior that the chip exhibits is not problematic. On OMAP4 however it results in the chip being in a bad state:
Panda # i2c probe
Valid chip addresses: 02 03 04 05 06 07 08 09 0A 0B 0C 0D 0E 0F 10 11 12 13 14 15 16 17 18 19 1A 1B 1C 1D 1E 1F 20 21 22 23 24 25 26 27 28 29 2A 2B 2C 2D 2E 2F 30 31 32 33 34 35 36 37 38 39 3A 3B 3C 3D 3E 3F 40 41 42 43 44 45 46 47 48 49 4A 4B 4C 4D 4E 4F 50 51 52 53 54 55 56 57 58 59 5A 5B 5C 5D 5E 5F 60 61 62 63 64 65 66 67 68 69 6A 6B 6C 6D 6E 6F 70 71 72 73 74 75 76 77 78 79 7A 7B 7C 7D 7E 7F
Panda # i2c md 50 0
timed out in wait_for_pin: I2C_STAT=0
I2C read: I/O error
Error reading the chip.
We must revert the original behavior to bring probe back into line with the TRM.

Quoted 6 lines

Cc Nick Thompson <[email protected]>
Cc Heiko Schocher <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 drivers/i2c/omap24xx_i2c.c |   42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

Acked-by Heiko Schocher <[email protected]>

Thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Tom Rini

author

2012-05-21 16:46 UTC #3 lore raw

PATCH 2/5 am335x: Correct i2c sysc offset

In reply to Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/i2c.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

arch/arm/include/asm/arch-am33xx/i2c.h +3 -3

index 366e2bb..dcb294c 100644
@@ -34,9 +34,9 @@ struct i2c {
 	unsigned short revnb_lo;        /* 0x00 */
 	unsigned short res1;
 	unsigned short revnb_hi;        /* 0x04 */
-	unsigned short res2[13];
-	unsigned short sysc;            /* 0x20 */
-	unsigned short res3;
+	unsigned short res2[5];
+	unsigned short sysc;		/* 0x10 */
+	unsigned short res3[9];
 	unsigned short irqstatus_raw;   /* 0x24 */
 	unsigned short res4;
 	unsigned short stat;            /* 0x28 */

Signature · 1 line

1.7.9.5

Heiko Schocher

2012-05-22 12:11 UTC #4 lore raw

PATCH 2/5 am335x: Correct i2c sysc offset

Hello Tom,

Quoted 4 lines · Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/i2c.h |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by Heiko Schocher <[email protected]>

Thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Tom Rini

author

2012-05-21 16:46 UTC #5 lore raw

PATCH 3/5 am33xx: Fill in more cm_wkuppll / cm_perpll

In reply to Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/cpu.h |   40 ++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 10 deletions(-)

arch/arm/include/asm/arch-am33xx/cpu.h +30 ~~-10~~

index 8760cc0..5a6534e 100644
@@ -62,7 +62,7 @@
 struct cm_wkuppll {
 	unsigned int wkclkstctrl;	/* offset 0x00 */
 	unsigned int wkctrlclkctrl;	/* offset 0x04 */
-	unsigned int resv1[1];
+	unsigned int wkgpio0clkctrl;	/* offset 0x08 */
 	unsigned int wkl4wkclkctrl;	/* offset 0x0c */
 	unsigned int resv2[4];
 	unsigned int idlestdpllmpu;	/* offset 0x20 */

@@ -111,34 +111,54 @@ struct cm_perpll {
 	unsigned int l3clkstctrl;	/* offset 0x0c */
 	unsigned int resv1;
 	unsigned int cpgmac0clkctrl;	/* offset 0x14 */
-	unsigned int resv2[4];
+	unsigned int lcdclkctrl;	/* offset 0x18 */
+	unsigned int usb0clkctrl;	/* offset 0x1C */
+	unsigned int resv2;
+	unsigned int tptc0clkctrl;	/* offset 0x24 */
 	unsigned int emifclkctrl;	/* offset 0x28 */
 	unsigned int ocmcramclkctrl;	/* offset 0x2c */
 	unsigned int gpmcclkctrl;	/* offset 0x30 */
-	unsigned int resv3[2];
+	unsigned int mcasp0clkctrl;	/* offset 0x34 */
+	unsigned int uart5clkctrl;	/* offset 0x38 */
 	unsigned int mmc0clkctrl;	/* offset 0x3C */
 	unsigned int elmclkctrl;	/* offset 0x40 */
 	unsigned int i2c2clkctrl;	/* offset 0x44 */
 	unsigned int i2c1clkctrl;	/* offset 0x48 */
 	unsigned int spi0clkctrl;	/* offset 0x4C */
 	unsigned int spi1clkctrl;	/* offset 0x50 */
-	unsigned int resv4[3];
+	unsigned int resv3[3];
 	unsigned int l4lsclkctrl;	/* offset 0x60 */
 	unsigned int l4fwclkctrl;	/* offset 0x64 */
-	unsigned int resv5[6];
+	unsigned int mcasp1clkctrl;	/* offset 0x68 */
+	unsigned int uart1clkctrl;	/* offset 0x6C */
+	unsigned int uart2clkctrl;	/* offset 0x70 */
+	unsigned int uart3clkctrl;	/* offset 0x74 */
+	unsigned int uart4clkctrl;	/* offset 0x78 */
+	unsigned int timer7clkctrl;	/* offset 0x7C */
 	unsigned int timer2clkctrl;	/* offset 0x80 */
-	unsigned int resv6[11];
+	unsigned int timer3clkctrl;	/* offset 0x84 */
+	unsigned int timer4clkctrl;	/* offset 0x88 */
+	unsigned int resv4[8];
+	unsigned int gpio1clkctrl;	/* offset 0xAC */
 	unsigned int gpio2clkctrl;	/* offset 0xB0 */
-	unsigned int resv7[7];
+	unsigned int gpio3clkctrl;	/* offset 0xB4 */
+	unsigned int resv5;
+	unsigned int tpccclkctrl;	/* offset 0xBC */
+	unsigned int dcan0clkctrl;	/* offset 0xC0 */
+	unsigned int dcan1clkctrl;	/* offset 0xC4 */
+	unsigned int resv6[2];
 	unsigned int emiffwclkctrl;	/* offset 0xD0 */
-	unsigned int resv8[2];
+	unsigned int resv7[2];
 	unsigned int l3instrclkctrl;	/* offset 0xDC */
 	unsigned int l3clkctrl;		/* Offset 0xE0 */
-	unsigned int resv9[14];
+	unsigned int resv8[4];
+	unsigned int mmc1clkctrl;	/* offset 0xF4 */
+	unsigned int mmc2clkctrl;	/* offset 0xF8 */
+	unsigned int resv9[8];
 	unsigned int l4hsclkstctrl;	/* offset 0x11C */
 	unsigned int l4hsclkctrl;	/* offset 0x120 */
 	unsigned int resv10[8];
-	unsigned int cpswclkctrl;	/* offset 0x144 */
+	unsigned int cpswclkstctrl;	/* offset 0x144 */
 };
 
 /* Encapsulating Display pll registers */

Signature · 1 line

1.7.9.5

Heiko Schocher

2012-05-22 12:12 UTC #6 lore raw

PATCH 3/5 am33xx: Fill in more cm_wkuppll / cm_perpll

Hello Tom,

Quoted 4 lines · Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/cpu.h |   40 ++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 10 deletions(-)

Acked-by Heiko Schocher <[email protected]>

Thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Tom Rini

author

2012-05-21 16:46 UTC #7 lore raw

PATCH 4/5 am33xx: Fix i2c sampling rate typo

In reply to Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/i2c.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/include/asm/arch-am33xx/i2c.h +1 -1

index dcb294c..32b2258 100644
@@ -76,6 +76,6 @@ struct i2c {
 };
 
 #define I2C_IP_CLK			48000000
-#define I2C_INTERNAL_SAMLPING_CLK	12000000
+#define I2C_INTERNAL_SAMPLING_CLK	12000000
 
 #endif /* _I2C_H_ */

Signature · 1 line

1.7.9.5

Heiko Schocher

2012-05-22 12:12 UTC #8 lore raw

PATCH 4/5 am33xx: Fix i2c sampling rate typo

Hello Tom,

Quoted 4 lines · Tom Rini

Signed-off-by Tom Rini <[email protected]>

---
 arch/arm/include/asm/arch-am33xx/i2c.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by Heiko Schocher <[email protected]>

Thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Tom Rini

author

2012-05-21 16:46 UTC #9 lore raw

PATCH 5/5 omap24xx_i2c: Add AM33XX support

In reply to Tom Rini

The same places that check for CONFIG_OMAP44XX need to check for CONFIG_AM33XX as we share the same i2c block.

Cc Heiko Schocher <[email protected]>

Signed-off-by Tom Rini <[email protected]>

---
 drivers/i2c/omap24xx_i2c.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/i2c/omap24xx_i2c.c +3 -3

index 3a50417..81193b0 100644
@@ -202,7 +202,7 @@ static int i2c_read_byte(u8 devaddr, u8 regoffset, u8 *value)
 		}
 		if (status & I2C_STAT_RRDY) {
 #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-	defined(CONFIG_OMAP44XX)
+	defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
 			*value = readb(&i2c_base->data);
 #else
 			*value = readw(&i2c_base->data);

@@ -232,7 +232,7 @@ static void flush_fifo(void)
 		stat = readw(&i2c_base->stat);
 		if (stat == I2C_STAT_RRDY) {
 #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-	defined(CONFIG_OMAP44XX)
+	defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
 			readb(&i2c_base->data);
 #else
 			readw(&i2c_base->data);

@@ -282,7 +282,7 @@ int i2c_probe(uchar chip)
 		if (status & I2C_STAT_RRDY) {
 			res = 0;
 #if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-    defined(CONFIG_OMAP44XX)
+    defined(CONFIG_OMAP44XX) || defined(CONFIG_AM33XX)
 			readb(&i2c_base->data);
 #else
 			readw(&i2c_base->data);

Signature · 1 line

1.7.9.5

Heiko Schocher

2012-05-22 12:12 UTC #10 lore raw

PATCH 5/5 omap24xx_i2c: Add AM33XX support

Hello Tom,

Quoted 8 lines · Tom Rini

The same places that check for CONFIG_OMAP44XX need to check for CONFIG_AM33XX as we share the same i2c block.
Cc Heiko Schocher <[email protected]>
Signed-off-by Tom Rini <[email protected]>
---
 drivers/i2c/omap24xx_i2c.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Acked-by Heiko Schocher <[email protected]>

Thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de

Heiko Schocher

2012-05-22 07:29 UTC #11 lore raw

cover : Correct i2c support on am33xx

In reply to Tom Rini

Hello Tom,

Quoted 7 lines · Tom Rini

The following series corrects i2c support for the am33xx family of devices (including beaglebone) and has been tested on a beagleboard xM and pandaboard. I would like to take these changes in via the u-boot-ti tree as it's mostly changes to arch and board files but I've cc'd Heiko on the i2c related parts. The biggest change here is to revert a previous change to the omap24xx_i2c.c driver for the i2c probe function. In short, the change made before violated the TRM constraints but did no harm on the IP

Quoted 8 lines

block found in omap3/related boards. Moving forward to the IP block found on omap4 and am33xx (and others) it still voilates the TRM and now leaves the bus in an unboundedly defined state like the TRM states. We must revert this change for both correctness and functionality. The rest of the series is minor corrections to structs/defines and adding CONFIG_AM33XX to the block of !CONFIG_OMAP2430. I've thought of, but think it should be separate to change omap24xx_i2c.c to test for CONFIG_OMAP2430 rather than every other present and future user in a few areas.

Ok, just saw, that you delegate this patches in patchwork to you. I am fine with that, but want to ack this patches, so please wait until I sent my Ack ... thanks!

bye,
Heiko

Signature · 3 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de