Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function
Can you please repost and I'll add those too into omap-testing first? Or maybe point to the right patchwork.kernel.org link. Resending. P.S. I think linux-i2c@ mailing list adds some vicious email headers that the email clients obey and don't include me in the to/cc in replies. I've only found Tony's reply in the archives because I suspected something wrong. Regards, -- Alex -- 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 1/2] omap i2c: make errata 1.153 workaround a separate function
On Wed, Dec 16, 2009 at 04:02:23 +0200, Alexander Shishkin wrote: This is to avoid insanely long lines and levels of indentation. These seem to be forgotten. Is there any problem with these I should address? Regards, -- Alex -- 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 1/2] omap i2c: make errata 1.153 workaround a separate function
* Alexander Shishkin virtu...@slind.org [100316 04:28]: On Wed, Dec 16, 2009 at 04:02:23 +0200, Alexander Shishkin wrote: This is to avoid insanely long lines and levels of indentation. These seem to be forgotten. Is there any problem with these I should address? Can you please repost and I'll add those too into omap-testing first? Or maybe point to the right patchwork.kernel.org link. Regards, Tony -- 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 1/2] omap i2c: make errata 1.153 workaround a separate function
On Thu, Dec 17, 2009 at 08:36:30 +0530, Menon, Nishanth wrote:
Alexander Shishkin said the following on 12/16/2009 07:32 PM:
This is to avoid insanely long lines and levels of indentation.
Signed-off-by: Alexander Shishkin virtu...@slind.org
CC: linux-...@vger.kernel.org
CC: linux-omap@vger.kernel.org
---
drivers/i2c/busses/i2c-omap.c | 43
++--
1 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 75bf3ad..ad8242a 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -671,6 +671,27 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
#define omap_i2c_rev1_isr NULL
#endif
+/*
+ * OMAP3430 Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before
writing
+ * data to DATA_REG. Otherwise some data bytes can be lost while
transferring
+ * them from the memory to the I2C interface.
+ */
+static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int
*err)
note, though this is identified as being part of 3430, it is not
really restricted to 3430 alone
we might want to rename this as errata_omap3_1p153() perhaps?
Ok, I don't see why not.
+{
+while (!(*stat OMAP_I2C_STAT_XUDF)) {
+if (*stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
+omap_i2c_ack_stat(dev, *stat (OMAP_I2C_STAT_XRDY |
+OMAP_I2C_STAT_XDR));
+*err |= OMAP_I2C_STAT_XUDF;
+return -1;
+}
+cpu_relax();
+*stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+}
+
+return 0;
+}
wonder if using an inline might help throw away the function call
overhead (considering it is used only once)?
objdump -S says it's implicitly inlined already. I actually had in mind
the conversation about generalizing the features/erratas for chips/IPs
and that somehow stopped me from explicitly inlining this. Do you think
it makes sense (for the next version of this patchset) to explicitly
inline this?
+
static irqreturn_t
omap_i2c_isr(int this_irq, void *dev_id)
{
@@ -794,25 +815,9 @@ complete:
break;
}
-/*
- * OMAP3430 Errata 1.153: When an XRDY/XDR
- * is hit, wait for XUDF before writing data
- * to DATA_REG. Otherwise some data bytes can
- * be lost while transferring them from the
- * memory to the I2C interface.
- */
-
-if (dev-rev = OMAP_I2C_REV_ON_3430) {
-while (!(stat
OMAP_I2C_STAT_XUDF)) {
-if (stat
(OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
-
omap_i2c_ack_stat(dev, stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
-err |=
OMAP_I2C_STAT_XUDF;
-goto complete;
-}
-cpu_relax();
-stat =
omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
-}
-}
+if (dev-rev = OMAP_I2C_REV_ON_3430
+omap3430_workaround(dev, stat, err))
+goto complete;
omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
}
Regards,
Nishanth Menon
--
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 1/2] omap i2c: make errata 1.153 workaround a separate function
Alexander Shishkin said the following on 12/17/2009 06:18 PM:
On Thu, Dec 17, 2009 at 08:36:30 +0530, Menon, Nishanth wrote:
Alexander Shishkin said the following on 12/16/2009 07:32 PM:
This is to avoid insanely long lines and levels of indentation.
Signed-off-by: Alexander Shishkin virtu...@slind.org
CC: linux-...@vger.kernel.org
CC: linux-omap@vger.kernel.org
---
drivers/i2c/busses/i2c-omap.c | 43 ++--
1 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 75bf3ad..ad8242a 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -671,6 +671,27 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
#define omap_i2c_rev1_isr NULL
#endif
+/*
+ * OMAP3430 Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before writing
+ * data to DATA_REG. Otherwise some data bytes can be lost while transferring
+ * them from the memory to the I2C interface.
+ */
+static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err)
note, though this is identified as being part of 3430, it is not
really restricted to 3430 alone
we might want to rename this as errata_omap3_1p153() perhaps?
Ok, I don't see why not.
Thanks..
+{
+ while (!(*stat OMAP_I2C_STAT_XUDF)) {
+ if (*stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
+ omap_i2c_ack_stat(dev, *stat (OMAP_I2C_STAT_XRDY |
+ OMAP_I2C_STAT_XDR));
+ *err |= OMAP_I2C_STAT_XUDF;
+ return -1;
+ }
+ cpu_relax();
+ *stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+ }
+
+ return 0;
+}
wonder if using an inline might help throw away the function call
overhead (considering it is used only once)?
objdump -S says it's implicitly inlined already. I actually had in mind
the conversation about generalizing the features/erratas for chips/IPs
and that somehow stopped me from explicitly inlining this. Do you think
it makes sense (for the next version of this patchset) to explicitly
inline this?
I guess that might allow folks to realize that without objdump -S ;)
[...]
regards,
Nishanth Menon
--
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 1/2] omap i2c: make errata 1.153 workaround a separate function
On Wed, Dec 16, 2009 at 03:43:04 +0200, Alexander Shishkin wrote: From: Alexander Shishkin ext-alexander.shish...@nokia.com Please disregard this, I've got the emails wrong here. I'll resend shortly. Regards, -- Alex -- 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 1/2] omap i2c: make errata 1.153 workaround a separate function
Alexander Shishkin said the following on 12/16/2009 07:32 PM:
This is to avoid insanely long lines and levels of indentation.
Signed-off-by: Alexander Shishkin virtu...@slind.org
CC: linux-...@vger.kernel.org
CC: linux-omap@vger.kernel.org
---
drivers/i2c/busses/i2c-omap.c | 43 ++--
1 files changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 75bf3ad..ad8242a 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -671,6 +671,27 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id)
#define omap_i2c_rev1_isr NULL
#endif
+/*
+ * OMAP3430 Errata 1.153: When an XRDY/XDR is hit, wait for XUDF before writing
+ * data to DATA_REG. Otherwise some data bytes can be lost while transferring
+ * them from the memory to the I2C interface.
+ */
+static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err)
note, though this is identified as being part of 3430, it is not really
restricted to 3430 alone
we might want to rename this as errata_omap3_1p153() perhaps?
+{
+ while (!(*stat OMAP_I2C_STAT_XUDF)) {
+ if (*stat (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
+ omap_i2c_ack_stat(dev, *stat (OMAP_I2C_STAT_XRDY |
+ OMAP_I2C_STAT_XDR));
+ *err |= OMAP_I2C_STAT_XUDF;
+ return -1;
+ }
+ cpu_relax();
+ *stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
+ }
+
+ return 0;
+}
wonder if using an inline might help throw away the function call
overhead (considering it is used only once)?
+
static irqreturn_t
omap_i2c_isr(int this_irq, void *dev_id)
{
@@ -794,25 +815,9 @@ complete:
break;
}
-/*
-* OMAP3430 Errata 1.153: When an XRDY/XDR
-* is hit, wait for XUDF before writing data
-* to DATA_REG. Otherwise some data bytes can
-* be lost while transferring them from the
-* memory to the I2C interface.
-*/
-
- if (dev-rev = OMAP_I2C_REV_ON_3430) {
- while (!(stat
OMAP_I2C_STAT_XUDF)) {
- if (stat
(OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) {
-
omap_i2c_ack_stat(dev, stat (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR));
- err |=
OMAP_I2C_STAT_XUDF;
- goto complete;
- }
- cpu_relax();
- stat =
omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG);
- }
- }
+ if (dev-rev = OMAP_I2C_REV_ON_3430
+ omap3430_workaround(dev, stat, err))
+ goto complete;
omap_i2c_write_reg(dev, OMAP_I2C_DATA_REG, w);
}
Regards,
Nishanth Menon
--
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
