* [PATCH 0/2] omap i2c interrupt handler fixes @ 2009-12-16 13:43 Alexander Shishkin 2009-12-16 13:43 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin 0 siblings, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 13:43 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Hi, This is the second version of the patch that I've sent to linux-omap to address this issue. This time I've moved the whole errata workaround bit to a separate function to get rid of too long lines and a couple of extra levels of indentation. The actual fix is the same as the first time, it adds a timeout to a busy loop which happens to take place in an interrupt handler and is capable of hanging the kernel. Regards, -- Alex ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function 2009-12-16 13:43 [PATCH 0/2] omap i2c interrupt handler fixes Alexander Shishkin @ 2009-12-16 13:43 ` Alexander Shishkin 2009-12-16 13:43 ` Alexander Shishkin 0 siblings, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 13:43 UTC (permalink / raw) To: ben-linux; +Cc: linux-omap, linux-i2c, Alexander Shishkin This is to avoid insanely long lines and levels of indentation. Signed-off-by: Alexander Shishkin <virtuoso@slind.org> CC: linux-i2c@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) +{ + 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; +} + 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); } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function 2009-12-16 13:43 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin @ 2009-12-16 13:43 ` Alexander Shishkin [not found] ` <1260970986-26613-3-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 2009-12-16 13:57 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin 0 siblings, 2 replies; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 13:43 UTC (permalink / raw) To: ben-linux; +Cc: linux-omap, linux-i2c, Alexander Shishkin From: Alexander Shishkin <ext-alexander.shishkin@nokia.com> This is to avoid insanely long lines and levels of indentation. Signed-off-by: Alexander Shishkin <ext-alexander.shishkin@nokia.com> --- 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) +{ + 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; +} + 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); } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <1260970986-26613-3-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org>]
* [PATCH 2/2] omap i2c: add a timeout to the busy waiting [not found] ` <1260970986-26613-3-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> @ 2009-12-16 13:46 ` Alexander Shishkin 0 siblings, 0 replies; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 13:46 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Alexander Shishkin The errata 1.153 workaround is busy waiting on XUDF bit in interrupt context, which may lead to kernel hangs. The problem can be reproduced by running the bus with wrong (too high) speed. Signed-off-by: Alexander Shishkin <virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> CC: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org CC: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org --- drivers/i2c/busses/i2c-omap.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index ad8242a..b474c20 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -678,7 +678,9 @@ omap_i2c_rev1_isr(int this_irq, void *dev_id) */ static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err) { - while (!(*stat & OMAP_I2C_STAT_XUDF)) { + unsigned long timeout = 10000; + + while (!(*stat & OMAP_I2C_STAT_XUDF && --timeout)) { if (*stat & (OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) { omap_i2c_ack_stat(dev, *stat & (OMAP_I2C_STAT_XRDY | OMAP_I2C_STAT_XDR)); @@ -689,6 +691,9 @@ static int omap3430_workaround(struct omap_i2c_dev *dev, u16 *stat, int *err) *stat = omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); } + if (!timeout) + dev_err(dev->dev, "timeout waiting on XUDF bit\n"); + return 0; } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function 2009-12-16 13:43 ` Alexander Shishkin [not found] ` <1260970986-26613-3-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> @ 2009-12-16 13:57 ` Alexander Shishkin 1 sibling, 0 replies; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 13:57 UTC (permalink / raw) To: ben-linux; +Cc: linux-omap, linux-i2c, Alexander Shishkin On Wed, Dec 16, 2009 at 03:43:04 +0200, Alexander Shishkin wrote: > From: Alexander Shishkin <ext-alexander.shishkin@nokia.com> Please disregard this, I've got the emails wrong here. I'll resend shortly. Regards, -- Alex ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 0/2][RESEND] omap i2c interrupt handler fixes @ 2009-12-16 14:02 Alexander Shishkin [not found] ` <1260972144-31593-1-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 14:02 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Hi, This is the second version of the patch that I've sent to linux-omap to address this issue. This time I've moved the whole errata workaround bit to a separate function to get rid of too long lines and a couple of extra levels of indentation. The actual fix is the same as the first time, it adds a timeout to a busy loop which happens to take place in an interrupt handler and is capable of hanging the kernel. Regards, -- Alex ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <1260972144-31593-1-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org>]
* [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <1260972144-31593-1-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> @ 2009-12-16 14:02 ` Alexander Shishkin [not found] ` <1260972144-31593-2-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2009-12-16 14:02 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Alexander Shishkin This is to avoid insanely long lines and levels of indentation. Signed-off-by: Alexander Shishkin <virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> CC: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org CC: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.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) +{ + 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; +} + 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); } -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 12+ messages in thread
[parent not found: <1260972144-31593-2-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org>]
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <1260972144-31593-2-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> @ 2009-12-17 3:06 ` Menon, Nishanth [not found] ` <4B29A036.2040807-l0cyMroinI0@public.gmane.org> 2010-03-16 11:27 ` Alexander Shishkin 1 sibling, 1 reply; 12+ messages in thread From: Menon, Nishanth @ 2009-12-17 3:06 UTC (permalink / raw) To: Alexander Shishkin Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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 <virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> > CC: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > CC: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <4B29A036.2040807-l0cyMroinI0@public.gmane.org>]
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <4B29A036.2040807-l0cyMroinI0@public.gmane.org> @ 2009-12-17 12:48 ` Alexander Shishkin 2009-12-17 13:18 ` Menon, Nishanth 0 siblings, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2009-12-17 12:48 UTC (permalink / raw) To: Menon, Nishanth Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org, linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 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 <virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> > >CC: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > >CC: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function 2009-12-17 12:48 ` Alexander Shishkin @ 2009-12-17 13:18 ` Menon, Nishanth 0 siblings, 0 replies; 12+ messages in thread From: Menon, Nishanth @ 2009-12-17 13:18 UTC (permalink / raw) To: Menon, Nishanth, ben-linux@fluff.org, linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org 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 <virtuoso@slind.org> >>> CC: linux-i2c@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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <1260972144-31593-2-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 2009-12-17 3:06 ` Menon, Nishanth @ 2010-03-16 11:27 ` Alexander Shishkin [not found] ` <20100316112741.GA13389-rKUxRSusx2MF9cI+BDt40OTW4wlIGRCZ@public.gmane.org> 1 sibling, 1 reply; 12+ messages in thread From: Alexander Shishkin @ 2010-03-16 11:27 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, tony-4v6yS6AI5VpBDgjK7y7TUQ 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20100316112741.GA13389-rKUxRSusx2MF9cI+BDt40OTW4wlIGRCZ@public.gmane.org>]
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <20100316112741.GA13389-rKUxRSusx2MF9cI+BDt40OTW4wlIGRCZ@public.gmane.org> @ 2010-03-16 14:30 ` Tony Lindgren [not found] ` <20100316143025.GR2900-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 0 siblings, 1 reply; 12+ messages in thread From: Tony Lindgren @ 2010-03-16 14:30 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA * Alexander Shishkin <virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <20100316143025.GR2900-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>]
* Re: [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function [not found] ` <20100316143025.GR2900-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> @ 2010-03-25 9:52 ` Alexander Shishkin 0 siblings, 0 replies; 12+ messages in thread From: Alexander Shishkin @ 2010-03-25 9:52 UTC (permalink / raw) To: ben-linux-elnMNo+KYs3YtjvyW6yDsg Cc: nm-l0cyMroinI0, linux-omap-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Tony Lindgren > 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-03-25 9:52 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-16 13:43 [PATCH 0/2] omap i2c interrupt handler fixes Alexander Shishkin 2009-12-16 13:43 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin 2009-12-16 13:43 ` Alexander Shishkin [not found] ` <1260970986-26613-3-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 2009-12-16 13:46 ` [PATCH 2/2] omap i2c: add a timeout to the busy waiting Alexander Shishkin 2009-12-16 13:57 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin -- strict thread matches above, loose matches on Subject: below -- 2009-12-16 14:02 [PATCH 0/2][RESEND] omap i2c interrupt handler fixes Alexander Shishkin [not found] ` <1260972144-31593-1-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 2009-12-16 14:02 ` [PATCH 1/2] omap i2c: make errata 1.153 workaround a separate function Alexander Shishkin [not found] ` <1260972144-31593-2-git-send-email-virtuoso-0lOfPCoBze7YtjvyW6yDsg@public.gmane.org> 2009-12-17 3:06 ` Menon, Nishanth [not found] ` <4B29A036.2040807-l0cyMroinI0@public.gmane.org> 2009-12-17 12:48 ` Alexander Shishkin 2009-12-17 13:18 ` Menon, Nishanth 2010-03-16 11:27 ` Alexander Shishkin [not found] ` <20100316112741.GA13389-rKUxRSusx2MF9cI+BDt40OTW4wlIGRCZ@public.gmane.org> 2010-03-16 14:30 ` Tony Lindgren [not found] ` <20100316143025.GR2900-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> 2010-03-25 9:52 ` Alexander Shishkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).