RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
Hi,
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of ext
Sonasath, Moiz
Sent: 23. marraskuuta 2009 18:10
To: Kalle Jokiniemi
Cc: khil...@deeprootsystems.com; linux-omap@vger.kernel.org;
Jarkko Nikula; Paul Walmsley; Menon, Nishanth; Pandita,
Vikram; Hogander Jouni (Nokia-D/Tampere)
Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up
latency constraint in i2c
snip-snip
Well, I don't completely agree with the necessity of preparing for
different rx/tx thresholds. For this to make sense, the i2c-omap
driver should first separate in it's code the use of rx and tx
thresholds. If someone is planning to do that, he/she should anyway
update the usage of fifo_size throughout the code,
including the wake up latency setting.
Anyways, attached a patch that separates the mpu wake up
latencies for
rx and tx. In case that is needed. Though I'm not for it, since it
adds unneeded complexity.
Yes Kalle, you are right! Not having different RX/TX wake-up
latencies will absolutely work fine with the in-place code as
we have both the RX/TX thresholds same.
Great :)
But, I think in
future we might have to play around with different RX/TX
thresholds and so from a conceptually right and generic point
of view it makes sense to incur the cost of the added complexity.
I agree. Once someone differentiates the thresholds in the driver, it very much
makes sense to also calculate different mpu wake up latencies for RX and TX.
The patch V4 looks perfect to me :)
Great :)
So, where do we push it?
- Kalle
- Moiz
- Kalle
/* reset ASAP, clearing any IRQs */ diff --git
a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h new file
mode 100644 index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct
device *dev, int set);
+};
+
+#endif
--
1.5.4.3
Regards
Moiz Sonasath
--
To unsubscribe from this list: send the line unsubscribe
linux-omap in the body of a message to
majord...@vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
kalle.jokini...@nokia.com writes: The patch V4 looks perfect to me :) Great :) So, where do we push it? I suggest to Ben Dooks (i2c maintainer for embedded platforms) and to linux-i2c list. Also Cc linux-omap. From MAINTAINERS: I2C SUBSYSTEM M: Jean Delvare (PC drivers, core) kh...@linux-fr.org M: Ben Dooks (embedded platforms) ben-li...@fluff.org L: linux-...@vger.kernel.org W: http://i2c.wiki.kernel.org/ T: quilt kernel.org/pub/linux/kernel/people/jdelvare/linux-2.6/jdelvare-i2c/ S: Maintained F: Documentation/i2c/ F: drivers/i2c/ F: include/linux/i2c.h F: include/linux/i2c-dev.h F: include/linux/i2c-id.h Also, FYI... in recent kernels there is a scripts/get_maintainer.pl script. If you run a patch through that script, it will spit out a list of folks to send the patch to. Kevin -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Tuesday, October 27, 2009 7:02 AM
To: Sonasath, Moiz
Cc: khil...@deeprootsystems.com; linux-omap@vger.kernel.org; Jarkko
Nikula; Paul Walmsley; Menon, Nishanth; Pandita, Vikram; jouni.hogander
Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint
in i2c
On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote:
Hello Jokiniemi!
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Wednesday, October 21, 2009 6:51 AM
To: khil...@deeprootsystems.com
Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz;
Jarkko
Nikula; Paul Walmsley; Menon, Nishanth
Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in
i2c
While waiting for completion of the i2c transfer, the
MPU could hit OFF mode and cause several msecs of
delay that made i2c transfers fail more often. The
extra delays and subsequent re-trys cause i2c clocks
to be active more often. This has also an negative
effect on power consumption.
Created a mechanism for passing and using the
constraint setting function in driver code. The used
mpu wake up latency constraints are now set individually
per bus, and they are calculated based on clock rate
and fifo size.
Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley,
and Nishanth Menon for tuning out the details of
this patch.
Cc: Moiz Sonasath m-sonas...@ti.com
Cc: Jarkko Nikula jhnik...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com
---
dev-speed = speed;
dev-idle = 1;
@@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev)
*/
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
+
+ /* calculate wakeup latency constraint for MPU */
+ if (dev-set_mpu_wkup_lat != NULL)
+ dev-latency = (100 * dev-fifo_size) /
+(1000 * speed / 8);
}
IMHO, here instead of using 'dev-fifo_size' for calculating the
'dev-latency', we need to use:
1. For RX case, to avoid Reciver overrun:
if (msg-flags I2C_M_RD)
Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in calculating
dev-latency
Because conceptually, RDR/RRDY interrupts are generated when RTRSH is
reached, so we want the MPU to be wake up within the time it takes to
fill the FIFO from RTRSH to FIFO Depth (FIFO full).
2. For TX case, to avoid Transmitter Underflow:
if (!(msg-flags I2C_M_RD))
Use (FIFO THRSH)bytes in calculating dev-latency
Because conceptually, XDR/XRDY interrupts are generated when XTRSH is
reached, so we want the MPU to be wake up within the time it takes to
drain the FIFO from XTRSH to Zero (FIFO empty).
Using, dev-fifo_size instead, works in the present code because we
have a RTRSH/XTRSH = dev-fifo_size/2 = 4 bytes
And therefore,
(FIFO Depth)bytes - (FIFO THRSH)bytes
= 8 - 4 = 4 bytes
But, to make it more generic in future and to make it independent of
any changes in the RTRSH/XTRSH values or FIFO depths in future, we
should use a generic code here.
Well, I don't completely agree with the necessity of preparing for
different rx/tx thresholds. For this to make sense, the i2c-omap driver
should first separate in it's code the use of rx and tx thresholds. If
someone is planning to do that, he/she should anyway update the usage of
fifo_size throughout the code, including the wake up latency setting.
Anyways, attached a patch that separates the mpu wake up latencies for
rx and tx. In case that is needed. Though I'm not for it, since it adds
unneeded complexity.
Yes Kalle, you are right! Not having different RX/TX wake-up latencies will
absolutely work fine with the in-place code as we have both the RX/TX
thresholds same. But, I think in future we might have to play around with
different RX/TX thresholds and so from a conceptually right and generic point
of view it makes sense to incur the cost of the added complexity.
The patch V4 looks perfect to me :)
- Moiz
- Kalle
/* reset ASAP, clearing any IRQs */
diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
new file mode 100644
index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct device *dev, int set);
+};
+
+#endif
--
1.5.4.3
Regards
Moiz Sonasath
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More
RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
-Original Message- From: ext Kevin Hilman [mailto:khil...@deeprootsystems.com] Sent: 20. marraskuuta 2009 18:28 To: Jokiniemi Kalle (Nokia-D/Tampere); Tony Lindgren Cc: linux-omap@vger.kernel.org; m-sonas...@ti.com; Hogander Jouni (Nokia-D/Tampere) Subject: Re: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c kalle.jokini...@nokia.com writes: Could you please take this patch in? Currently i2c is not working very well with cpuidle and off mode, and the patch is needed. I would go with V4 of the patch. Kalle, Last I saw with this thread, it didn't look like there was resolution. OK. Let's resolve it then :) Moiz, could you please comment on my last reply in the old thread: - (sent on 29th of October) Well, I don't completely agree with the necessity of preparing for different rx/tx thresholds. For this to make sense, the i2c-omap driver should first separate in it's code the use of rx and tx thresholds. If someone is planning to do that, he/she should anyway update the usage of fifo_size throughout the code, including the wake up latency setting. Anyways, attached a patch that separates the mpu wake up latencies for rx and tx. In case that is needed. Though I'm not for it, since it adds unneeded complexity. - (sent on 29th of October) Attached also the alternative patch I was referring to. If things are resolved, this shouldn't go upstream via PM branch, since it doesn't seem to have any dependencies on the PM branch. Ah, the pm interfaces are in mainline now. It should go upstream either via the I2C list, or via Tony. If it goes via i2c list, I can add it to PM branch while waiting for it to merge. Tony, would you take this? If not, what is the address for i2c mailing list, and who maintains i2c? - Kalle Kevin -Original Message- From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of ext Kalle Jokiniemi Sent: 29. lokakuuta 2009 10:55 To: Sonasath, Moiz Cc: khil...@deeprootsystems.com; linux-omap@vger.kernel.org Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c OK, let's try this once more, since my mail did not seem to go to linux-omap. Sorry for the spam. See my comments below: On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote: Hello Jokiniemi! -Original Message- From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com] Sent: Wednesday, October 21, 2009 6:51 AM To: khil...@deeprootsystems.com Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz; Jarkko Nikula; Paul Walmsley; Menon, Nishanth Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c While waiting for completion of the i2c transfer, the MPU could hit OFF mode and cause several msecs of delay that made i2c transfers fail more often. The extra delays and subsequent re-trys cause i2c clocks to be active more often. This has also an negative effect on power consumption. Created a mechanism for passing and using the constraint setting function in driver code. The used mpu wake up latency constraints are now set individually per bus, and they are calculated based on clock rate and fifo size. Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and Nishanth Menon for tuning out the details of this patch. Cc: Moiz Sonasath m-sonas...@ti.com Cc: Jarkko Nikula jhnik...@gmail.com Cc: Paul Walmsley p...@pwsan.com Cc: Nishanth Menon n...@ti.com Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com --- dev-speed = speed; dev-idle = 1; @@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev) */ dev-fifo_size = (dev-fifo_size / 2); dev-b_hw = 1; /* Enable hardware fixes */ + + /* calculate wakeup latency constraint for MPU */ + if (dev-set_mpu_wkup_lat != NULL) + dev-latency = (100 * dev-fifo_size) / + (1000 * speed / 8); } IMHO, here instead of using 'dev-fifo_size' for calculating the 'dev-latency', we need to use: 1. For RX case, to avoid Reciver overrun: if (msg-flags I2C_M_RD) Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in calculating dev-latency Because conceptually, RDR/RRDY interrupts are generated when RTRSH is reached, so we want the MPU to be wake up within the time it takes to fill the FIFO from RTRSH to FIFO Depth (FIFO full). 2. For TX case, to avoid Transmitter Underflow: if (!(msg-flags I2C_M_RD)) Use (FIFO THRSH)bytes in calculating dev-latency Because conceptually, XDR/XRDY interrupts are generated when XTRSH
RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
Kevin,
Could you please take this patch in? Currently i2c is not working very well
with cpuidle and off mode, and the patch is needed.
I would go with V4 of the patch.
- Kalle
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of ext
Kalle Jokiniemi
Sent: 29. lokakuuta 2009 10:55
To: Sonasath, Moiz
Cc: khil...@deeprootsystems.com; linux-omap@vger.kernel.org
Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in i2c
OK, let's try this once more, since my mail did not seem to go
to linux-omap.
Sorry for the spam.
See my comments below:
On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote:
Hello Jokiniemi!
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Wednesday, October 21, 2009 6:51 AM
To: khil...@deeprootsystems.com
Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz;
Jarkko
Nikula; Paul Walmsley; Menon, Nishanth
Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in
i2c
While waiting for completion of the i2c transfer, the MPU
could hit
OFF mode and cause several msecs of delay that made i2c transfers
fail more often. The extra delays and subsequent re-trys cause i2c
clocks to be active more often. This has also an negative
effect on
power consumption.
Created a mechanism for passing and using the constraint setting
function in driver code. The used mpu wake up latency constraints
are now set individually per bus, and they are calculated based on
clock rate and fifo size.
Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and
Nishanth
Menon for tuning out the details of this patch.
Cc: Moiz Sonasath m-sonas...@ti.com
Cc: Jarkko Nikula jhnik...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com
---
dev-speed = speed;
dev-idle = 1;
@@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev)
*/
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
+
+ /* calculate wakeup latency constraint for MPU */
+ if (dev-set_mpu_wkup_lat != NULL)
+ dev-latency = (100 * dev-fifo_size) /
+ (1000 * speed / 8);
}
IMHO, here instead of using 'dev-fifo_size' for calculating the
'dev-latency', we need to use:
1. For RX case, to avoid Reciver overrun:
if (msg-flags I2C_M_RD)
Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in
calculating
dev-latency
Because conceptually, RDR/RRDY interrupts are generated when
RTRSH is
reached, so we want the MPU to be wake up within the time it
takes to
fill the FIFO from RTRSH to FIFO Depth (FIFO full).
2. For TX case, to avoid Transmitter Underflow:
if (!(msg-flags I2C_M_RD))
Use (FIFO THRSH)bytes in calculating dev-latency
Because conceptually, XDR/XRDY interrupts are generated when
XTRSH is
reached, so we want the MPU to be wake up within the time it
takes to
drain the FIFO from XTRSH to Zero (FIFO empty).
Using, dev-fifo_size instead, works in the present code because we
have a RTRSH/XTRSH = dev-fifo_size/2 = 4 bytes And therefore, (FIFO
Depth)bytes - (FIFO THRSH)bytes = 8 - 4 = 4 bytes
But, to make it more generic in future and to make it independent of
any changes in the RTRSH/XTRSH values or FIFO depths in future, we
should use a generic code here.
Well, I don't completely agree with the necessity of preparing
for different rx/tx thresholds. For this to make sense, the
i2c-omap driver should first separate in it's code the use of
rx and tx thresholds. If someone is planning to do that,
he/she should anyway update the usage of fifo_size throughout
the code, including the wake up latency setting.
Anyways, attached a patch that separates the mpu wake up
latencies for rx and tx. In case that is needed. Though I'm
not for it, since it adds unneeded complexity.
- Kalle
/* reset ASAP, clearing any IRQs */ diff --git
a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h new
file mode
100644 index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct device *dev,
int set);
+};
+
+#endif
--
1.5.4.3
Regards
Moiz Sonasath
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
kalle.jokini...@nokia.com writes:
Could you please take this patch in? Currently i2c is not working
very well with cpuidle and off mode, and the patch is needed.
I would go with V4 of the patch.
Kalle,
Last I saw with this thread, it didn't look like there was resolution.
If things are resolved, this shouldn't go upstream via PM branch, since
it doesn't seem to have any dependencies on the PM branch.
It should go upstream either via the I2C list, or via Tony.
If it goes via i2c list, I can add it to PM branch while waiting for
it to merge.
Kevin
-Original Message-
From: linux-omap-ow...@vger.kernel.org
[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of ext
Kalle Jokiniemi
Sent: 29. lokakuuta 2009 10:55
To: Sonasath, Moiz
Cc: khil...@deeprootsystems.com; linux-omap@vger.kernel.org
Subject: RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in i2c
OK, let's try this once more, since my mail did not seem to go
to linux-omap.
Sorry for the spam.
See my comments below:
On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote:
Hello Jokiniemi!
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Wednesday, October 21, 2009 6:51 AM
To: khil...@deeprootsystems.com
Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz;
Jarkko
Nikula; Paul Walmsley; Menon, Nishanth
Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in
i2c
While waiting for completion of the i2c transfer, the MPU
could hit
OFF mode and cause several msecs of delay that made i2c transfers
fail more often. The extra delays and subsequent re-trys cause i2c
clocks to be active more often. This has also an negative
effect on
power consumption.
Created a mechanism for passing and using the constraint setting
function in driver code. The used mpu wake up latency constraints
are now set individually per bus, and they are calculated based on
clock rate and fifo size.
Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and
Nishanth
Menon for tuning out the details of this patch.
Cc: Moiz Sonasath m-sonas...@ti.com
Cc: Jarkko Nikula jhnik...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com
---
dev-speed = speed;
dev-idle = 1;
@@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev)
*/
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
+
+ /* calculate wakeup latency constraint for MPU */
+ if (dev-set_mpu_wkup_lat != NULL)
+ dev-latency = (100 * dev-fifo_size) /
+(1000 * speed / 8);
}
IMHO, here instead of using 'dev-fifo_size' for calculating the
'dev-latency', we need to use:
1. For RX case, to avoid Reciver overrun:
if (msg-flags I2C_M_RD)
Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in
calculating
dev-latency
Because conceptually, RDR/RRDY interrupts are generated when
RTRSH is
reached, so we want the MPU to be wake up within the time it
takes to
fill the FIFO from RTRSH to FIFO Depth (FIFO full).
2. For TX case, to avoid Transmitter Underflow:
if (!(msg-flags I2C_M_RD))
Use (FIFO THRSH)bytes in calculating dev-latency
Because conceptually, XDR/XRDY interrupts are generated when
XTRSH is
reached, so we want the MPU to be wake up within the time it
takes to
drain the FIFO from XTRSH to Zero (FIFO empty).
Using, dev-fifo_size instead, works in the present code because we
have a RTRSH/XTRSH = dev-fifo_size/2 = 4 bytes And therefore, (FIFO
Depth)bytes - (FIFO THRSH)bytes = 8 - 4 = 4 bytes
But, to make it more generic in future and to make it independent of
any changes in the RTRSH/XTRSH values or FIFO depths in future, we
should use a generic code here.
Well, I don't completely agree with the necessity of preparing
for different rx/tx thresholds. For this to make sense, the
i2c-omap driver should first separate in it's code the use of
rx and tx thresholds. If someone is planning to do that,
he/she should anyway update the usage of fifo_size throughout
the code, including the wake up latency setting.
Anyways, attached a patch that separates the mpu wake up
latencies for rx and tx. In case that is needed. Though I'm
not for it, since it adds unneeded complexity.
- Kalle
/* reset ASAP, clearing any IRQs */ diff --git
a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h new
file mode
100644 index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct device *dev,
int set);
+};
+
+#endif
--
1.5.4.3
RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
OK, let's try this once more, since my mail did not seem to go to
linux-omap.
Sorry for the spam.
See my comments below:
On Fri, 2009-10-23 at 18:53 +0300, Sonasath, Moiz wrote:
Hello Jokiniemi!
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Wednesday, October 21, 2009 6:51 AM
To: khil...@deeprootsystems.com
Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz;
Jarkko
Nikula; Paul Walmsley; Menon, Nishanth
Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency
constraint in
i2c
While waiting for completion of the i2c transfer, the
MPU could hit OFF mode and cause several msecs of
delay that made i2c transfers fail more often. The
extra delays and subsequent re-trys cause i2c clocks
to be active more often. This has also an negative
effect on power consumption.
Created a mechanism for passing and using the
constraint setting function in driver code. The used
mpu wake up latency constraints are now set individually
per bus, and they are calculated based on clock rate
and fifo size.
Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley,
and Nishanth Menon for tuning out the details of
this patch.
Cc: Moiz Sonasath m-sonas...@ti.com
Cc: Jarkko Nikula jhnik...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com
---
dev-speed = speed;
dev-idle = 1;
@@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev)
*/
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
+
+ /* calculate wakeup latency constraint for MPU */
+ if (dev-set_mpu_wkup_lat != NULL)
+ dev-latency = (100 * dev-fifo_size) /
+ (1000 * speed / 8);
}
IMHO, here instead of using 'dev-fifo_size' for calculating the
'dev-latency', we need to use:
1. For RX case, to avoid Reciver overrun:
if (msg-flags I2C_M_RD)
Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in calculating
dev-latency
Because conceptually, RDR/RRDY interrupts are generated when RTRSH is
reached, so we want the MPU to be wake up within the time it takes to
fill the FIFO from RTRSH to FIFO Depth (FIFO full).
2. For TX case, to avoid Transmitter Underflow:
if (!(msg-flags I2C_M_RD))
Use (FIFO THRSH)bytes in calculating dev-latency
Because conceptually, XDR/XRDY interrupts are generated when XTRSH is
reached, so we want the MPU to be wake up within the time it takes to
drain the FIFO from XTRSH to Zero (FIFO empty).
Using, dev-fifo_size instead, works in the present code because we
have a RTRSH/XTRSH = dev-fifo_size/2 = 4 bytes
And therefore,
(FIFO Depth)bytes - (FIFO THRSH)bytes
= 8 - 4 = 4 bytes
But, to make it more generic in future and to make it independent of
any changes in the RTRSH/XTRSH values or FIFO depths in future, we
should use a generic code here.
Well, I don't completely agree with the necessity of preparing for
different rx/tx thresholds. For this to make sense, the i2c-omap driver
should first separate in it's code the use of rx and tx thresholds. If
someone is planning to do that, he/she should anyway update the usage of
fifo_size throughout the code, including the wake up latency setting.
Anyways, attached a patch that separates the mpu wake up latencies for
rx and tx. In case that is needed. Though I'm not for it, since it adds
unneeded complexity.
- Kalle
/* reset ASAP, clearing any IRQs */
diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
new file mode 100644
index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct device *dev, int set);
+};
+
+#endif
--
1.5.4.3
Regards
Moiz Sonasath
0001-OMAP-I2C-Add-mpu-wake-up-latency-constraint-in-i2c.patch
Description: 0001-OMAP-I2C-Add-mpu-wake-up-latency-constraint-in-i2c.patch
RE: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in i2c
Hello Jokiniemi!
-Original Message-
From: Kalle Jokiniemi [mailto:kalle.jokini...@digia.com]
Sent: Wednesday, October 21, 2009 6:51 AM
To: khil...@deeprootsystems.com
Cc: linux-omap@vger.kernel.org; Kalle Jokiniemi; Sonasath, Moiz; Jarkko
Nikula; Paul Walmsley; Menon, Nishanth
Subject: [PATCH V4 3/3] OMAP: I2C: Add mpu wake up latency constraint in
i2c
While waiting for completion of the i2c transfer, the
MPU could hit OFF mode and cause several msecs of
delay that made i2c transfers fail more often. The
extra delays and subsequent re-trys cause i2c clocks
to be active more often. This has also an negative
effect on power consumption.
Created a mechanism for passing and using the
constraint setting function in driver code. The used
mpu wake up latency constraints are now set individually
per bus, and they are calculated based on clock rate
and fifo size.
Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley,
and Nishanth Menon for tuning out the details of
this patch.
Cc: Moiz Sonasath m-sonas...@ti.com
Cc: Jarkko Nikula jhnik...@gmail.com
Cc: Paul Walmsley p...@pwsan.com
Cc: Nishanth Menon n...@ti.com
Signed-off-by: Kalle Jokiniemi kalle.jokini...@digia.com
---
dev-speed = speed;
dev-idle = 1;
@@ -911,6 +923,11 @@ omap_i2c_probe(struct platform_device *pdev)
*/
dev-fifo_size = (dev-fifo_size / 2);
dev-b_hw = 1; /* Enable hardware fixes */
+
+ /* calculate wakeup latency constraint for MPU */
+ if (dev-set_mpu_wkup_lat != NULL)
+ dev-latency = (100 * dev-fifo_size) /
+(1000 * speed / 8);
}
IMHO, here instead of using 'dev-fifo_size' for calculating the
'dev-latency', we need to use:
1. For RX case, to avoid Reciver overrun:
if (msg-flags I2C_M_RD)
Use [(FIFO Depth)bytes - (FIFO THRSH)bytes] in calculating
dev-latency
Because conceptually, RDR/RRDY interrupts are generated when RTRSH is reached,
so we want the MPU to be wake up within the time it takes to fill the FIFO from
RTRSH to FIFO Depth (FIFO full).
2. For TX case, to avoid Transmitter Underflow:
if (!(msg-flags I2C_M_RD))
Use (FIFO THRSH)bytes in calculating dev-latency
Because conceptually, XDR/XRDY interrupts are generated when XTRSH is reached,
so we want the MPU to be wake up within the time it takes to drain the FIFO
from XTRSH to Zero (FIFO empty).
Using, dev-fifo_size instead, works in the present code because we have a
RTRSH/XTRSH = dev-fifo_size/2 = 4 bytes
And therefore,
(FIFO Depth)bytes - (FIFO THRSH)bytes
= 8 - 4 = 4 bytes
But, to make it more generic in future and to make it independent of any
changes in the RTRSH/XTRSH values or FIFO depths in future, we should use a
generic code here.
/* reset ASAP, clearing any IRQs */
diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h
new file mode 100644
index 000..1362fba
--- /dev/null
+++ b/include/linux/i2c-omap.h
@@ -0,0 +1,9 @@
+#ifndef __I2C_OMAP_H__
+#define __I2C_OMAP_H__
+
+struct omap_i2c_bus_platform_data {
+ u32 clkrate;
+ void(*set_mpu_wkup_lat)(struct device *dev, int set);
+};
+
+#endif
--
1.5.4.3
Regards
Moiz Sonasath
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
