* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-16 19:17 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-16 19:17 UTC (permalink / raw)
To: linux-arm-kernel
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 <balbi@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 86 ++++++++++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4be54fa32839..01e343bd6d97 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -29,6 +29,7 @@
*/
#include <linux/module.h>
+#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/err.h>
@@ -44,6 +45,8 @@
#include <linux/pm_runtime.h>
#include <linux/pinctrl/consumer.h>
+#define NSECS_PER_SEC 1000000000
+
/* I2C controller revisions */
#define OMAP_I2C_OMAP1_REV_2 0x20
@@ -347,6 +350,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) {
@@ -383,52 +388,73 @@ 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 = 1920000;
+ internal_clk_period = NSECS_PER_SEC /
+ internal_clk; /* ns */
+ } else {
+ internal_clk = 12000000;
+ internal_clk_period = NSECS_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 = NSECS_PER_SEC / dev->speed;
+
/* 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 = NSECS_PER_SEC / 400;
/* 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;
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-16 19:17 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-16 19:17 UTC (permalink / raw)
To: wsa
Cc: Tony Lindgren, Nishanth Menon, Dave Gerlach, linux-i2c,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List,
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%.
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 <balbi@ti.com>
---
drivers/i2c/busses/i2c-omap.c | 86 ++++++++++++++++++++++++++++---------------
1 file changed, 56 insertions(+), 30 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 4be54fa32839..01e343bd6d97 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -29,6 +29,7 @@
*/
#include <linux/module.h>
+#include <linux/kernel.h>
#include <linux/delay.h>
#include <linux/i2c.h>
#include <linux/err.h>
@@ -44,6 +45,8 @@
#include <linux/pm_runtime.h>
#include <linux/pinctrl/consumer.h>
+#define NSECS_PER_SEC 1000000000
+
/* I2C controller revisions */
#define OMAP_I2C_OMAP1_REV_2 0x20
@@ -347,6 +350,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) {
@@ -383,52 +388,73 @@ 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 = 1920000;
+ internal_clk_period = NSECS_PER_SEC /
+ internal_clk; /* ns */
+ } else {
+ internal_clk = 12000000;
+ internal_clk_period = NSECS_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 = NSECS_PER_SEC / dev->speed;
+
/* 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 = NSECS_PER_SEC / 400;
/* 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;
--
2.4.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
2015-06-16 19:17 ` Felipe Balbi
@ 2015-06-16 19:19 ` Felipe Balbi
-1 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-16 19:19 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jun 16, 2015 at 02:17:56PM -0500, Felipe Balbi wrote:
> 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 <balbi@ti.com>
the patch is correct, but I sent the version written on top of v3.14,
I'll resend correct version, sorry.
--
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/20150616/e6e85e1f/attachment.sig>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-16 19:19 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-16 19:19 UTC (permalink / raw)
To: Felipe Balbi
Cc: wsa, Tony Lindgren, Nishanth Menon, Dave Gerlach, linux-i2c,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 957 bytes --]
On Tue, Jun 16, 2015 at 02:17:56PM -0500, Felipe Balbi wrote:
> 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 <balbi@ti.com>
the patch is correct, but I sent the version written on top of v3.14,
I'll resend correct version, sorry.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
2015-06-16 19:17 ` Felipe Balbi
@ 2015-06-17 11:09 ` Michael Lawnick
-1 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-17 11:09 UTC (permalink / raw)
To: linux-arm-kernel
Am 16.06.2015 um 21:17 schrieb 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%.
>
> The idea of the patch is simple:
>
> calculate desired scl_period from requested scl
> and use 50% for tLow and 50% for tHigh.
...
Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> MIN. MAX. MIN. MAX.
> LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
> HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
Your signal is in spec (0.85 ?s high, 1,65 low).
Maybe your TPS65218 is just buggy or signals are bad?
--
KR
Michael
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-17 11:09 ` Michael Lawnick
0 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-17 11:09 UTC (permalink / raw)
To: Felipe Balbi, wsa
Cc: Tony Lindgren, Nishanth Menon, Dave Gerlach, linux-i2c,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
Am 16.06.2015 um 21:17 schrieb 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%.
>
> The idea of the patch is simple:
>
> calculate desired scl_period from requested scl
> and use 50% for tLow and 50% for tHigh.
...
Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> MIN. MAX. MIN. MAX.
> LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
> HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
Your signal is in spec (0.85 µs high, 1,65 low).
Maybe your TPS65218 is just buggy or signals are bad?
--
KR
Michael
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-17 15:38 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-17 15:38 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> Am 16.06.2015 um 21:17 schrieb 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%.
> >
> >The idea of the patch is simple:
> >
> >calculate desired scl_period from requested scl
> >and use 50% for tLow and 50% for tHigh.
> ...
> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>
> >PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> > MIN. MAX. MIN. MAX.
> >LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
> >HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
>
> Your signal is in spec (0.85 ?s high, 1,65 low).
> Maybe your TPS65218 is just buggy or signals are bad?
yes, tps is buggy, it's written in the commit log itself.
--
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/20150617/0f2305fc/attachment.sig>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-17 15:38 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-17 15:38 UTC (permalink / raw)
To: Michael Lawnick
Cc: Felipe Balbi, wsa-z923LK4zBo2bacvFa/9K2g, Tony Lindgren,
Nishanth Menon, Dave Gerlach, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]
Hi,
On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> Am 16.06.2015 um 21:17 schrieb 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%.
> >
> >The idea of the patch is simple:
> >
> >calculate desired scl_period from requested scl
> >and use 50% for tLow and 50% for tHigh.
> ...
> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>
> >PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> > MIN. MAX. MIN. MAX.
> >LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
> >HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
>
> Your signal is in spec (0.85 µs high, 1,65 low).
> Maybe your TPS65218 is just buggy or signals are bad?
yes, tps is buggy, it's written in the commit log itself.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-18 6:39 ` Michael Lawnick
0 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-18 6:39 UTC (permalink / raw)
To: linux-arm-kernel
Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> Hi,
>
> On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
>> Am 16.06.2015 um 21:17 schrieb 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%.
>>>
>>> The idea of the patch is simple:
>>>
>>> calculate desired scl_period from requested scl
>>> and use 50% for tLow and 50% for tHigh.
>> ...
>> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>>
>>> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
>>> MIN. MAX. MIN. MAX.
>>> LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
>>> HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
>>
>> Your signal is in spec (0.85 ?s high, 1,65 low).
>> Maybe your TPS65218 is just buggy or signals are bad?
>
> yes, tps is buggy, it's written in the commit log itself.
>
So I think it is unacceptable to change the adapters code violating
specification because some buggy device doesn't work properly.
This change for your device has chance to blow up many correctly working
ones.
--
KR
Michael
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-18 6:39 ` Michael Lawnick
0 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-18 6:39 UTC (permalink / raw)
To: balbi-l0cyMroinI0
Cc: wsa-z923LK4zBo2bacvFa/9K2g, Tony Lindgren, Nishanth Menon,
Dave Gerlach, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> Hi,
>
> On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
>> Am 16.06.2015 um 21:17 schrieb 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%.
>>>
>>> The idea of the patch is simple:
>>>
>>> calculate desired scl_period from requested scl
>>> and use 50% for tLow and 50% for tHigh.
>> ...
>> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>>
>>> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
>>> MIN. MAX. MIN. MAX.
>>> LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
>>> HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
>>
>> Your signal is in spec (0.85 µs high, 1,65 low).
>> Maybe your TPS65218 is just buggy or signals are bad?
>
> yes, tps is buggy, it's written in the commit log itself.
>
So I think it is unacceptable to change the adapters code violating
specification because some buggy device doesn't work properly.
This change for your device has chance to blow up many correctly working
ones.
--
KR
Michael
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
2015-06-18 6:39 ` Michael Lawnick
@ 2015-06-18 17:24 ` Felipe Balbi
-1 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-18 17:24 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
> Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> >Hi,
> >
> >On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> >>Am 16.06.2015 um 21:17 schrieb 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%.
> >>>
> >>>The idea of the patch is simple:
> >>>
> >>>calculate desired scl_period from requested scl
> >>>and use 50% for tLow and 50% for tHigh.
> >>...
> >>Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> >>
> >>>PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> >>> MIN. MAX. MIN. MAX.
> >>>LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
> >>>HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
> >>
> >>Your signal is in spec (0.85 ?s high, 1,65 low).
> >>Maybe your TPS65218 is just buggy or signals are bad?
> >
> >yes, tps is buggy, it's written in the commit log itself.
> >
>
> So I think it is unacceptable to change the adapters code violating
> specification because some buggy device doesn't work properly.
read the other thread and you'll see that it's not violating jack
> This change for your device has chance to blow up many correctly
> working ones.
How ?
--
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/2359f2bf/attachment.sig>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-18 17:24 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-18 17:24 UTC (permalink / raw)
To: Michael Lawnick
Cc: balbi, wsa, Tony Lindgren, Nishanth Menon, Dave Gerlach,
linux-i2c, Linux OMAP Mailing List, Linux ARM Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]
On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
> Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> >Hi,
> >
> >On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> >>Am 16.06.2015 um 21:17 schrieb 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%.
> >>>
> >>>The idea of the patch is simple:
> >>>
> >>>calculate desired scl_period from requested scl
> >>>and use 50% for tLow and 50% for tHigh.
> >>...
> >>Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> >>
> >>>PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> >>> MIN. MAX. MIN. MAX.
> >>>LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
> >>>HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
> >>
> >>Your signal is in spec (0.85 µs high, 1,65 low).
> >>Maybe your TPS65218 is just buggy or signals are bad?
> >
> >yes, tps is buggy, it's written in the commit log itself.
> >
>
> So I think it is unacceptable to change the adapters code violating
> specification because some buggy device doesn't work properly.
read the other thread and you'll see that it's not violating jack
> This change for your device has chance to blow up many correctly
> working ones.
How ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-19 5:41 ` Michael Lawnick
0 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-19 5:41 UTC (permalink / raw)
To: linux-arm-kernel
Am 18.06.2015 um 19:24 schrieb Felipe Balbi:
> On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
>> Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
>>> Hi,
>>>
>>> On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
>>>> Am 16.06.2015 um 21:17 schrieb 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%.
>>>>>
>>>>> The idea of the patch is simple:
>>>>>
>>>>> calculate desired scl_period from requested scl
>>>>> and use 50% for tLow and 50% for tHigh.
>>>> ...
>>>> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>>>>
>>>>> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
>>>>> MIN. MAX. MIN. MAX.
>>>>> LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
>>>>> HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
>>>>
>>>> Your signal is in spec (0.85 ?s high, 1,65 low).
>>>> Maybe your TPS65218 is just buggy or signals are bad?
>>>
>>> yes, tps is buggy, it's written in the commit log itself.
>>>
>>
>> So I think it is unacceptable to change the adapters code violating
>> specification because some buggy device doesn't work properly.
>
> read the other thread and you'll see that it's not violating jack
>
>> This change for your device has chance to blow up many correctly
>> working ones.
>
> How ?
>
The answer is so obvious that I'm a bit irritated.
Your patch description tells: 'and use 50% for tLow and 50% for tHigh'
For 400kHz this means 1.25 us for high and low. This clearly violates
the specification for minimum low period and will not work with any
device that relies on it.
In the other thread it is discussed that your patch does effectively not
do what you describe but this is something completely independent.
--
KR
Michael
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-19 5:41 ` Michael Lawnick
0 siblings, 0 replies; 16+ messages in thread
From: Michael Lawnick @ 2015-06-19 5:41 UTC (permalink / raw)
To: balbi-l0cyMroinI0
Cc: wsa-z923LK4zBo2bacvFa/9K2g, Tony Lindgren, Nishanth Menon,
Dave Gerlach, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
Am 18.06.2015 um 19:24 schrieb Felipe Balbi:
> On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
>> Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
>>> Hi,
>>>
>>> On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
>>>> Am 16.06.2015 um 21:17 schrieb 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%.
>>>>>
>>>>> The idea of the patch is simple:
>>>>>
>>>>> calculate desired scl_period from requested scl
>>>>> and use 50% for tLow and 50% for tHigh.
>>>> ...
>>>> Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
>>>>
>>>>> PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
>>>>> MIN. MAX. MIN. MAX.
>>>>> LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
>>>>> HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
>>>>
>>>> Your signal is in spec (0.85 µs high, 1,65 low).
>>>> Maybe your TPS65218 is just buggy or signals are bad?
>>>
>>> yes, tps is buggy, it's written in the commit log itself.
>>>
>>
>> So I think it is unacceptable to change the adapters code violating
>> specification because some buggy device doesn't work properly.
>
> read the other thread and you'll see that it's not violating jack
>
>> This change for your device has chance to blow up many correctly
>> working ones.
>
> How ?
>
The answer is so obvious that I'm a bit irritated.
Your patch description tells: 'and use 50% for tLow and 50% for tHigh'
For 400kHz this means 1.25 us for high and low. This clearly violates
the specification for minimum low period and will not work with any
device that relies on it.
In the other thread it is discussed that your patch does effectively not
do what you describe but this is something completely independent.
--
KR
Michael
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-19 15:30 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-19 15:30 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 19, 2015 at 07:41:49AM +0200, Michael Lawnick wrote:
> Am 18.06.2015 um 19:24 schrieb Felipe Balbi:
> >On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
> >>Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> >>>Hi,
> >>>
> >>>On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> >>>>Am 16.06.2015 um 21:17 schrieb 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%.
> >>>>>
> >>>>>The idea of the patch is simple:
> >>>>>
> >>>>>calculate desired scl_period from requested scl
> >>>>>and use 50% for tLow and 50% for tHigh.
> >>>>...
> >>>>Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> >>>>
> >>>>>PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> >>>>> MIN. MAX. MIN. MAX.
> >>>>>LOW period of the SCL clock tLOW 4.7 ? 1.3 ? ?s
> >>>>>HIGH period of the SCL clock tHIGH 4.0 ? 0.6 ? ?s
> >>>>
> >>>>Your signal is in spec (0.85 ?s high, 1,65 low).
> >>>>Maybe your TPS65218 is just buggy or signals are bad?
> >>>
> >>>yes, tps is buggy, it's written in the commit log itself.
> >>>
> >>
> >>So I think it is unacceptable to change the adapters code violating
> >>specification because some buggy device doesn't work properly.
> >
> >read the other thread and you'll see that it's not violating jack
> >
> >>This change for your device has chance to blow up many correctly
> >>working ones.
> >
> >How ?
> >
> The answer is so obvious that I'm a bit irritated.
> Your patch description tells: 'and use 50% for tLow and 50% for tHigh'
another one who can't do simple algebra.
http://marc.info/?l=linux-i2c&m=143456423512634&w=2
http://marc.info/?l=linux-i2c&m=143456444212698&w=2
http://marc.info/?l=linux-omap&m=143456762413953&w=2
> For 400kHz this means 1.25 us for high and low. This clearly violates the
> specification for minimum low period and will not work with any device that
> relies on it.
> In the other thread it is discussed that your patch does effectively not do
> what you describe but this is something completely independent.
Read the comment where the calculation goes, it states that we try to
get as close to 50% duty cycle while making sure we're within spec.
Also, commit log is saying that we're using 50% of SCL period for tLow
and tHigh calculation, not that duty cycle will be 50%, which it isn't.
--
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/20150619/a9f6121b/attachment.sig>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] i2c: omap: improve duty cycle on SCL
@ 2015-06-19 15:30 ` Felipe Balbi
0 siblings, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2015-06-19 15:30 UTC (permalink / raw)
To: Michael Lawnick
Cc: balbi-l0cyMroinI0, wsa-z923LK4zBo2bacvFa/9K2g, Tony Lindgren,
Nishanth Menon, Dave Gerlach, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
Linux OMAP Mailing List, Linux ARM Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 2812 bytes --]
On Fri, Jun 19, 2015 at 07:41:49AM +0200, Michael Lawnick wrote:
> Am 18.06.2015 um 19:24 schrieb Felipe Balbi:
> >On Thu, Jun 18, 2015 at 08:39:11AM +0200, Michael Lawnick wrote:
> >>Am 17.06.2015 um 17:38 schrieb Felipe Balbi:
> >>>Hi,
> >>>
> >>>On Wed, Jun 17, 2015 at 01:09:53PM +0200, Michael Lawnick wrote:
> >>>>Am 16.06.2015 um 21:17 schrieb 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%.
> >>>>>
> >>>>>The idea of the patch is simple:
> >>>>>
> >>>>>calculate desired scl_period from requested scl
> >>>>>and use 50% for tLow and 50% for tHigh.
> >>>>...
> >>>>Hmm, and what's about Philips I2C specification 2.1, Jan 2000, Table 5?
> >>>>
> >>>>>PARAMETER SYMBOL STANDARD-MODE FAST-MODE UNIT
> >>>>> MIN. MAX. MIN. MAX.
> >>>>>LOW period of the SCL clock tLOW 4.7 – 1.3 – µs
> >>>>>HIGH period of the SCL clock tHIGH 4.0 – 0.6 – µs
> >>>>
> >>>>Your signal is in spec (0.85 µs high, 1,65 low).
> >>>>Maybe your TPS65218 is just buggy or signals are bad?
> >>>
> >>>yes, tps is buggy, it's written in the commit log itself.
> >>>
> >>
> >>So I think it is unacceptable to change the adapters code violating
> >>specification because some buggy device doesn't work properly.
> >
> >read the other thread and you'll see that it's not violating jack
> >
> >>This change for your device has chance to blow up many correctly
> >>working ones.
> >
> >How ?
> >
> The answer is so obvious that I'm a bit irritated.
> Your patch description tells: 'and use 50% for tLow and 50% for tHigh'
another one who can't do simple algebra.
http://marc.info/?l=linux-i2c&m=143456423512634&w=2
http://marc.info/?l=linux-i2c&m=143456444212698&w=2
http://marc.info/?l=linux-omap&m=143456762413953&w=2
> For 400kHz this means 1.25 us for high and low. This clearly violates the
> specification for minimum low period and will not work with any device that
> relies on it.
> In the other thread it is discussed that your patch does effectively not do
> what you describe but this is something completely independent.
Read the comment where the calculation goes, it states that we try to
get as close to 50% duty cycle while making sure we're within spec.
Also, commit log is saying that we're using 50% of SCL period for tLow
and tHigh calculation, not that duty cycle will be 50%, which it isn't.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-06-19 15:30 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-16 19:17 [PATCH] i2c: omap: improve duty cycle on SCL Felipe Balbi
2015-06-16 19:17 ` Felipe Balbi
2015-06-16 19:19 ` Felipe Balbi
2015-06-16 19:19 ` Felipe Balbi
2015-06-17 11:09 ` Michael Lawnick
2015-06-17 11:09 ` Michael Lawnick
2015-06-17 15:38 ` Felipe Balbi
2015-06-17 15:38 ` Felipe Balbi
2015-06-18 6:39 ` Michael Lawnick
2015-06-18 6:39 ` Michael Lawnick
2015-06-18 17:24 ` Felipe Balbi
2015-06-18 17:24 ` Felipe Balbi
2015-06-19 5:41 ` Michael Lawnick
2015-06-19 5:41 ` Michael Lawnick
2015-06-19 15:30 ` Felipe Balbi
2015-06-19 15:30 ` Felipe Balbi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.