i2c: omap: fix bus recovery setup

PATCH New Jan Luebbe 4 participants 8 messages Jul 8, 2015 – Jul 31, 2015

Subsystems (4)

I2C subsystem host drivers OMAP2+ support OMAP1 support OMAP I2C driver

lore

Share & tools

lore

Jan Luebbe

author

2015-07-08 14:35 UTC #0 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to allow direct access to the SCL and SDA pins. In addition to ST_EN, we need to set the TMODE to 0b11 (Loop back & SDA/SCL IO mode select). Also, as the reset values of SCL_O and SDA_O are 0 (which means "drive low level"), we need to set them to 1 (which means "high-impedance") to avoid unwanted changes on the pins.

As a precaution, reset all these bits to their default values after recovery is complete.

Signed-off-by Jan Luebbe <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/i2c/busses/i2c-omap.c +11 -0

index d1c22e3fdd14..fc9bf7f30e35 100644
@@ -1247,7 +1247,14 @@ static void omap_i2c_prepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* enable test mode */
 	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	/* select SDA/SCL IO mode */
+	reg |= 3 << OMAP_I2C_SYSTEST_TMODE_SHIFT;
+	/* set SCL to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SCL_O;
+	/* set SDA to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

@@ -1257,7 +1264,11 @@ static void omap_i2c_unprepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* restore reset values */
 	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	reg &= ~OMAP_I2C_SYSTEST_TMODE_MASK;
+	reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+	reg &= ~OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

Signature · 1 line

2.1.4

Alexander Sverdlin

2015-07-12 15:32 UTC #1 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

Quoted 11 lines · Jan Luebbe

At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to allow direct access to the SCL and SDA pins. In addition to ST_EN, we need to set the TMODE to 0b11 (Loop back & SDA/SCL IO mode select). Also, as the reset values of SCL_O and SDA_O are 0 (which means "drive low level"), we need to set them to 1 (which means "high-impedance") to avoid unwanted changes on the pins.
As a precaution, reset all these bits to their default values after recovery is complete.
Signed-off-by Jan Luebbe <[email protected]>

Tested with oscilloscope on Beagle Bone Black, without the patch nothing meningful appears on SCL during recovery, with the patch I'm able to see the recovery pulses.

Tested-by Alexander Sverdlin <[email protected]>

Quoted 34 lines

---
 drivers/i2c/busses/i2c-omap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/i2c/busses/i2c-omap.c +11 -0

index d1c22e3fdd14..fc9bf7f30e35 100644
@@ -1247,7 +1247,14 @@ static void omap_i2c_prepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* enable test mode */
 	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	/* select SDA/SCL IO mode */
+	reg |= 3 << OMAP_I2C_SYSTEST_TMODE_SHIFT;
+	/* set SCL to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SCL_O;
+	/* set SDA to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

@@ -1257,7 +1264,11 @@ static void omap_i2c_unprepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* restore reset values */
 	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	reg &= ~OMAP_I2C_SYSTEST_TMODE_MASK;
+	reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+	reg &= ~OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

Wolfram Sang

2015-07-14 11:10 UTC #2 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

In reply to Jan Luebbe

Quoted 1 line · Jan Luebbe

At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to

Felipe: it seems you did not need this; is this AM335x specific?

Quoted 48 lines

allow direct access to the SCL and SDA pins. In addition to ST_EN, we need to set the TMODE to 0b11 (Loop back & SDA/SCL IO mode select). Also, as the reset values of SCL_O and SDA_O are 0 (which means "drive low level"), we need to set them to 1 (which means "high-impedance") to avoid unwanted changes on the pins.

As a precaution, reset all these bits to their default values after recovery is complete.

Signed-off-by Jan Luebbe <[email protected]>

---
 drivers/i2c/busses/i2c-omap.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

drivers/i2c/busses/i2c-omap.c +11 -0

index d1c22e3fdd14..fc9bf7f30e35 100644
@@ -1247,7 +1247,14 @@ static void omap_i2c_prepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* enable test mode */
 	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	/* select SDA/SCL IO mode */
+	reg |= 3 << OMAP_I2C_SYSTEST_TMODE_SHIFT;
+	/* set SCL to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SCL_O;
+	/* set SDA to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

@@ -1257,7 +1264,11 @@ static void omap_i2c_unprepare_recovery(struct i2c_adapter *adap)
 	u32 reg;
 
 	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* restore reset values */
 	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	reg &= ~OMAP_I2C_SYSTEST_TMODE_MASK;
+	reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+	reg &= ~OMAP_I2C_SYSTEST_SDA_O;
 	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
 }

Signature · 1 line

2.1.4

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150714/d6115559/attachment.sig>

Grygorii Strashko

2015-07-14 12:10 UTC #3 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

Hi Wolfram,

Quoted 4 lines · Wolfram Sang

Quoted 1 line · Jan Luebbe
At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to
Felipe: it seems you did not need this; is this AM335x specific?

We need it (Felipe's reply can be delayed).

It's not AM335x specific. TMODE has to be configured for all OMAP4+ SoCs too.

Quoted 9 lines

Quoted 9 lines
allow direct access to the SCL and SDA pins. In addition to ST_EN, we need to set the TMODE to 0b11 (Loop back & SDA/SCL IO mode select). Also, as the reset values of SCL_O and SDA_O are 0 (which means "drive low level"), we need to set them to 1 (which means "high-impedance") to avoid unwanted changes on the pins.
As a precaution, reset all these bits to their default values after recovery is complete.

Reviewed-by Grygorii Strashko <[email protected]>

Quoted 40 lines

Signed-off-by Jan Luebbe <[email protected]>

---
  drivers/i2c/busses/i2c-omap.c | 11 +++++++++++
  1 file changed, 11 insertions(+)

drivers/i2c/busses/i2c-omap.c +11 -0

index d1c22e3fdd14..fc9bf7f30e35 100644
@@ -1247,7 +1247,14 @@ static void omap_i2c_prepare_recovery(struct i2c_adapter *adap)
  	u32 reg;
 
  	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* enable test mode */
  	reg |= OMAP_I2C_SYSTEST_ST_EN;
+	/* select SDA/SCL IO mode */
+	reg |= 3 << OMAP_I2C_SYSTEST_TMODE_SHIFT;
+	/* set SCL to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SCL_O;
+	/* set SDA to high-impedance state (reset value is 0) */
+	reg |= OMAP_I2C_SYSTEST_SDA_O;
  	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
  }

@@ -1257,7 +1264,11 @@ static void omap_i2c_unprepare_recovery(struct i2c_adapter *adap)
  	u32 reg;
 
  	reg = omap_i2c_read_reg(dev, OMAP_I2C_SYSTEST_REG);
+	/* restore reset values */
  	reg &= ~OMAP_I2C_SYSTEST_ST_EN;
+	reg &= ~OMAP_I2C_SYSTEST_TMODE_MASK;
+	reg &= ~OMAP_I2C_SYSTEST_SCL_O;
+	reg &= ~OMAP_I2C_SYSTEST_SDA_O;
  	omap_i2c_write_reg(dev, OMAP_I2C_SYSTEST_REG, reg);
  }

Signature · 1 line

2.1.4

Signature · 2 lines

regards,
-grygorii

Wolfram Sang

2015-07-14 12:12 UTC #4 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

Quoted 9 lines · Grygorii Strashko

Hi Wolfram,
Quoted 4 lines · Wolfram Sang
Quoted 1 line · Jan Luebbe
At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to
Felipe: it seems you did not need this; is this AM335x specific?
We need it (Felipe's reply can be delayed).

So, how did it work for him?

Quoted 1 line

It's not AM335x specific. TMODE has to be configured for all OMAP4+ SoCs too.

Okay, thanks for the stepping in!

Jan Lübbe

author

2015-07-29 10:27 UTC #5 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

Quoted 9 lines · Wolfram Sang

Quoted 8 lines · Grygorii Strashko
Hi Wolfram,
Quoted 4 lines · Wolfram Sang
Quoted 1 line · Jan Luebbe
At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to
Felipe: it seems you did not need this; is this AM335x specific?

Quoted 7 lines

Quoted 1 line
We need it (Felipe's reply can be delayed).
So, how did it work for him?
Quoted 1 line
It's not AM335x specific. TMODE has to be configured for all OMAP4+ SoCs too.
Okay, thanks for the stepping in!

Wolfram, will you pick this as-is?

Regards,
Jan

Signature · 4 lines

Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Wolfram Sang

2015-07-31 10:49 UTC #6 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

Quoted 12 lines · Jan L?bbe

Quoted 11 lines · Wolfram Sang
Quoted 9 lines · Grygorii Strashko
Hi Wolfram,
Quoted 4 lines · Wolfram Sang
Quoted 1 line · Jan Luebbe
At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to
Felipe: it seems you did not need this; is this AM335x specific?
We need it (Felipe's reply can be delayed).

Quoted 7 lines

Quoted 5 lines
So, how did it work for him?
Quoted 1 line
It's not AM335x specific. TMODE has to be configured for all OMAP4+ SoCs too.
Okay, thanks for the stepping in!
Wolfram, will you pick this as-is?

Yes, I was hoping for an answer to my above question to get a better understanding, but we can still apply the patches meanwhile.

Wolfram Sang

Applied

2015-07-31 10:51 UTC #7 lore raw

PATCH 1/1 i2c: omap: fix bus recovery setup

In reply to Jan Luebbe

Quoted 11 lines · Jan Luebbe

At least on the AM335x, enabling OMAP_I2C_SYSTEST_ST_EN is not enough to allow direct access to the SCL and SDA pins. In addition to ST_EN, we need to set the TMODE to 0b11 (Loop back & SDA/SCL IO mode select). Also, as the reset values of SCL_O and SDA_O are 0 (which means "drive low level"), we need to set them to 1 (which means "high-impedance") to avoid unwanted changes on the pins.
As a precaution, reset all these bits to their default values after recovery is complete.
Signed-off-by Jan Luebbe <[email protected]>

Applied to for-current, thanks!