i2c: omap: improve duty cycle on SCL

PATCH Superseded Felipe Balbi 3 participants 7 messages Jun 17, 2015 – Aug 18, 2015 1 message not in the archive

Subsystems (4)

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

lore

Share & tools

lore

Felipe Balbi

author

2015-06-17 19:31 UTC #0 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.

One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.

The idea of the patch is simple:

calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.

tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.

Kudos to Nishanth Menon and Dave Gerlach for helping debugging the TPS65218 problem found on AM437x SK.

Signed-off-by Felipe Balbi <[email protected]>

---

Changes since v2:
- use NSEC_PER_SEC from time.h
- Fix 19.2MHz (it was missing one 0)

Changes since v1:
- Fix for dev->speed which is in kHz, rather than Hz

ps: if there are any extra comments, I'll wait a couple days before resending to avoid spamming the list.

 drivers/i2c/busses/i2c-omap.c | 87 ++++++++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 31 deletions(-)

drivers/i2c/busses/i2c-omap.c +56 ~~-31~~

index 0e894193accf..0e74bd4d6c82 100644
@@ -25,6 +25,8 @@
  */
 
 #include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/time.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/err.h>

@@ -359,6 +361,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
 	unsigned long fclk_rate = 12000000;
 	unsigned long internal_clk = 0;
+	unsigned long internal_clk_period = 0;
+	unsigned long scl_period = 0;
 	struct clk *fclk;
 
 	if (dev->rev >= OMAP_I2C_REV_ON_3430_3530) {

@@ -395,58 +399,79 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
 	}
 
 	if (!(dev->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) {
-
 		/*
 		 * HSI2C controller internal clk rate should be 19.2 Mhz for
-		 * HS and for all modes on 2430. On 34xx we can use lower rate
-		 * to get longer filter period for better noise suppression.
-		 * The filter is iclk (fclk for HS) period.
+		 * HS and for all modes on 2430. For all other devices and
+		 * speeds we will use a 12MHz internal clock.
 		 */
-		if (dev->speed > 400 ||
-			       dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK)
-			internal_clk = 19200;
-		else if (dev->speed > 100)
-			internal_clk = 9600;
-		else
-			internal_clk = 4000;
+		if (dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK ||
+				dev->speed > 400) {
+			internal_clk = 19200000;
+			internal_clk_period = NSEC_PER_SEC /
+				internal_clk; /* ns */
+		} else {
+			internal_clk = 12000000;
+			internal_clk_period = NSEC_PER_SEC /
+				internal_clk; /* ns */
+		}
+
 		fclk = clk_get(dev->dev, "fck");
-		fclk_rate = clk_get_rate(fclk) / 1000;
+		fclk_rate = clk_get_rate(fclk);
 		clk_put(fclk);
 
 		/* Compute prescaler divisor */
 		psc = fclk_rate / internal_clk;
 		psc = psc - 1;
 
+		/*
+		 * Here's the tricky part, we want to make sure our duty cycle
+		 * is as close to 50% as possible. In order to achieve that, we
+		 * will first figure out what's the period on chosen scl is,
+		 * then divide that by two and calculate SCLL and SCLH based on
+		 * that.
+		 *
+		 * SCLL and SCLH equations are as folows:
+		 *
+		 * SCLL = (tLow / iclk_period) - 7;
+		 * SCLH = (tHigh / iclk_period) - 5;
+		 *
+		 * Where iclk_period is period of Internal Clock.
+		 *
+		 * tLow and tHigh will be basically half of scl_period where
+		 * possible as long as we can match I2C spec's minimum limits
+		 * for them.
+		 */
+		scl_period = NSEC_PER_SEC / (dev->speed * 1000);
+
 		/* If configured for High Speed */
 		if (dev->speed > 400) {
-			unsigned long scl;
+			unsigned long fs_period;
+
+			/*
+			 * first phase of HS mode is up to
+			 * 400kHz so we will use that.
+			 */
+			fs_period = NSEC_PER_SEC / 400000;
 
 			/* For first phase of HS mode */
-			scl = internal_clk / 400;
-			fsscll = scl - (scl / 3) - 7;
-			fssclh = (scl / 3) - 5;
+			fsscll = DIV_ROUND_UP(fs_period >> 1,
+					internal_clk_period) - 7;
+			fssclh = (fs_period >> 1) / internal_clk_period - 5;
 
 			/* For second phase of HS mode */
-			scl = fclk_rate / dev->speed;
-			hsscll = scl - (scl / 3) - 7;
-			hssclh = (scl / 3) - 5;
-		} else if (dev->speed > 100) {
-			unsigned long scl;
-
-			/* Fast mode */
-			scl = internal_clk / dev->speed;
-			fsscll = scl - (scl / 3) - 7;
-			fssclh = (scl / 3) - 5;
-		} else {
-			/* Standard mode */
-			fsscll = internal_clk / (dev->speed * 2) - 7;
-			fssclh = internal_clk / (dev->speed * 2) - 5;
+			hsscll = DIV_ROUND_UP(scl_period >> 1,
+					internal_clk_period) - 7;
+			hssclh = (scl_period >> 1) / internal_clk_period - 5;
+		} else  {
+			fsscll = DIV_ROUND_UP(scl_period >> 1,
+					internal_clk_period) - 7;
+			fssclh = (scl_period >> 1) / internal_clk_period - 5;
 		}
 		scll = (hsscll << OMAP_I2C_SCLL_HSSCLL) | fsscll;
 		sclh = (hssclh << OMAP_I2C_SCLH_HSSCLH) | fssclh;
 	} else {
 		/* Program desired operating rate */
-		fclk_rate /= (psc + 1) * 1000;
+		fclk_rate /= (psc + 1);
 		if (psc > 2)
 			psc = 2;
 		scll = fclk_rate / (dev->speed * 2) - 7 + psc;

Signature · 1 line

2.4.3

Alexander Sverdlin

2015-06-18 08:09 UTC #1 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

Hello Felipe,

Quoted 9 lines · ext Felipe Balbi

With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.
One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.

Quoted 8 lines

The idea of the patch is simple:
calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.
tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.

if you refuse to change the calculations to achieve maximum possible bus rate (as I've shown you with SCLL=9 and SCLH=9), maybe you want to change the description? Because you are doing something else than is written here. You are only in spec because you are not doing 50% duty cycle. And you didn't mention here that you lower the bus speed below 400kHz to achieve this.

Signature · 2 lines

Best regards,
Alexander Sverdlin.

Felipe Balbi

author

2015-06-18 17:25 UTC #2 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

Quoted 21 lines · Alexander Sverdlin

Hello Felipe,
Quoted 9 lines · ext Felipe Balbi
With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.
One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.
Quoted 8 lines
The idea of the patch is simple:
calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.
tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.

Quoted 6 lines

if you refuse to change the calculations to achieve maximum possible bus rate (as I've shown you with SCLL=9 and SCLH=9), maybe you want to change the description? Because you are doing something else than is written here. You are only in spec because you are not doing 50% duty cycle. And you didn't mention here that you lower the bus speed below 400kHz to achieve this.

and there's a comment where the calculation goes which states "as close to 50% as possible but we make sure tLow is higher than tHigh so we're still within spec".

Signature · 8 lines

balbi
-------------- 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/20150618/4fb0ce66/attachment.sig>

Wolfram Sang

2015-07-09 19:42 UTC #3 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

Quoted 29 lines · Felipe Balbi

Quoted 21 lines · Alexander Sverdlin
Hello Felipe,
Quoted 9 lines · ext Felipe Balbi
With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.
One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.
Quoted 8 lines
The idea of the patch is simple:
calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.
tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.
Quoted 6 lines
if you refuse to change the calculations to achieve maximum possible bus rate (as I've shown you with SCLL=9 and SCLH=9), maybe you want to change the description? Because you are doing something else than is written here. You are only in spec because you are not doing 50% duty cycle. And you didn't mention here that you lower the bus speed below 400kHz to achieve this.

Quoted 3 lines

and there's a comment where the calculation goes which states "as close to 50% as possible but we make sure tLow is higher than tHigh so we're still within spec".

So, is that ready to go in for-next?

-------------- 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/20150709/0b1eaa85/attachment.sig>

Felipe Balbi

author

2015-07-10 17:27 UTC #4 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

Quoted 30 lines · Wolfram Sang

Quoted 29 lines · Felipe Balbi
Quoted 27 lines · Alexander Sverdlin
Hello Felipe,
Quoted 9 lines · ext Felipe Balbi
With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.
One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.
Quoted 8 lines
The idea of the patch is simple:
calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.
tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.
if you refuse to change the calculations to achieve maximum possible bus rate (as I've shown you with SCLL=9 and SCLH=9), maybe you want to change the description? Because you are doing something else than is written here. You are only in spec because you are not doing 50% duty cycle. And you didn't mention here that you lower the bus speed below 400kHz to achieve this.

Quoted 5 lines

Quoted 3 lines
and there's a comment where the calculation goes which states "as close to 50% as possible but we make sure tLow is higher than tHigh so we're still within spec".
So, is that ready to go in for-next?

should be.

Signature · 8 lines

balbi
-------------- 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/20150710/5969e077/attachment.sig>

Felipe Balbi

author

2015-08-13 14:36 UTC #5 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

Quoted 31 lines · Felipe Balbi

Quoted 30 lines · Wolfram Sang
Quoted 29 lines · Felipe Balbi
Quoted 27 lines · Alexander Sverdlin
Hello Felipe,
Quoted 9 lines · ext Felipe Balbi
With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles.
One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%.
Quoted 8 lines
The idea of the patch is simple:
calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh.
tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications.
if you refuse to change the calculations to achieve maximum possible bus rate (as I've shown you with SCLL=9 and SCLH=9), maybe you want to change the description? Because you are doing something else than is written here. You are only in spec because you are not doing 50% duty cycle. And you didn't mention here that you lower the bus speed below 400kHz to achieve this.

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
and there's a comment where the calculation goes which states "as close to 50% as possible but we make sure tLow is higher than tHigh so we're still within spec".
So, is that ready to go in for-next?
should be.

ping ?

Signature · 8 lines

balbi
-------------- 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/20150813/595fa791/attachment.sig>

A message that is not in the archive (sent off-list or not yet received): CAFqkuUbO1CZ8bu8g8O7SsLF-0nUV274GZ08=6J1sVS4Foy+_EA@mail.gmail.com

Felipe Balbi

author

2015-08-18 14:43 UTC #7 lore raw

PATCH v3 1/1 i2c: omap: improve duty cycle on SCL

HI,

Quoted 10 lines · Matt Reimer

This doesn't seem to work for higher speeds, like 2.6 MHz. hssclh and hsscll end up going negative.
My case is an OMAP3 trying to configure i2c1 at 2.6 MHz, on which it talks to TPS65950. I end up with the following values:
        SCLL 0x0000fd12 SCLH 0x0000fe13
The end result is that the high-speed part gets configured way too slow and so the OMAP3 can't talk to the TPS65950.

hmm, indeed. Seems like anything above 1MHz would have an issue. I'll see what can be done there.

Signature · 8 lines

balbi
-------------- 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/20150818/1ab4b88c/attachment.sig>