| Message ID | 1557983320-14461-4-git-send-email-sagar.kadam@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Extend dt bindings to support I2C on sifive devices and a fix broken IRQ in polling mode. | expand |
> @@ -682,13 +693,24 @@ static int ocores_i2c_probe(struct platform_device *pdev) > > irq = platform_get_irq(pdev, 0); > if (irq == -ENXIO) { > - i2c->flags |= OCORES_FLAG_POLL; > + /* > + * Set a OCORES_FLAG_BROKEN_IRQ to enable workaround for > + * FU540-C000 SoC in polling mode interface of i2c-ocore driver. > + * Else enable default polling mode interface for SIFIVE/OCORE > + * device types. > + */ > + match = of_match_node(ocores_i2c_match, pdev->dev.of_node); > + if (match && (long)match->data == > + (TYPE_SIFIVE_REV0 | OCORES_FLAG_BROKEN_IRQ)) This looks wrong. You added: + { + .compatible = "sifive,fu540-c000-i2c", + .data = (void *)TYPE_SIFIVE_REV0, + }, + { + .compatible = "sifive,i2c0", + .data = (void *)TYPE_SIFIVE_REV0, + }, So match->data just has TYPE_SIFIVE_REV0. > + i2c->flags |= OCORES_FLAG_BROKEN_IRQ; > + else > + i2c->flags |= OCORES_FLAG_POLL; These two don't need to be exclusive. It makes more sense to say SIFIVE needs to poll and it its IRQ is broken. A lot of your other changes then go away. Andrew
Hello Andrew, On Thu, May 16, 2019 at 6:01 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > @@ -682,13 +693,24 @@ static int ocores_i2c_probe(struct platform_device *pdev) > > > > irq = platform_get_irq(pdev, 0); > > if (irq == -ENXIO) { > > - i2c->flags |= OCORES_FLAG_POLL; > > + /* > > + * Set a OCORES_FLAG_BROKEN_IRQ to enable workaround for > > + * FU540-C000 SoC in polling mode interface of i2c-ocore driver. > > + * Else enable default polling mode interface for SIFIVE/OCORE > > + * device types. > > + */ > > + match = of_match_node(ocores_i2c_match, pdev->dev.of_node); > > + if (match && (long)match->data == > > + (TYPE_SIFIVE_REV0 | OCORES_FLAG_BROKEN_IRQ)) > > This looks wrong. You added: > > + { > + .compatible = "sifive,fu540-c000-i2c", > + .data = (void *)TYPE_SIFIVE_REV0, > + }, > + { > + .compatible = "sifive,i2c0", > + .data = (void *)TYPE_SIFIVE_REV0, > + }, > > So match->data just has TYPE_SIFIVE_REV0. I updated the device_id table into two logically separated patches as follows:- 1. Update device id table for Sifive devices [PATCH v3 2/3] i2c-ocore: .data for sifive,fu540-540-c000 and sifive,i2c0 both are for sifive devices hence TYPE_SIFIVE_REV0 2. Add polling mode workaround fix for fu540-c000 SoC [PATCH v3 3/3] i2c-ocores: .data for sifive,fu540-540-c000 is of TYPE_SIFIVE_REV0 and has a broken IRQ so the flag OCORES_FLAG_BROKEN_IRQ is OR'd to data into device id table. Please let me know if you feel patch 2 and patch 3 need to be squashed together into a single patch. > > > + i2c->flags |= OCORES_FLAG_BROKEN_IRQ; > > + else > > + i2c->flags |= OCORES_FLAG_POLL; > > These two don't need to be exclusive. It makes more sense to say > SIFIVE needs to poll and it its IRQ is broken. A lot of your other > changes then go away. > Other SiFive chip's with Ocore based I2C re-implementation might not need the broken IRQ workaround. and can use the the existing mainline polling mode interface, using OCORES_FLAG_POLL. Thanks & BR, Sagar Kadam > Andrew
> Other SiFive chip's with Ocore based I2C re-implementation might not > need the broken IRQ workaround. They then get a different compatible string, because they are not bug compatible. Andrew
On Thu, May 16, 2019 at 10:38:40AM +0530, Sagar Shrikant Kadam wrote: > The i2c-ocore driver already has a polling mode interface.But it needs > a workaround for FU540 Chipset on HiFive unleashed board (RevA00). > There is an erratum in FU540 chip that prevents interrupt driven i2c > transfers from working, and also the I2C controller's interrupt bit > cannot be cleared if set, due to this the existing i2c polling mode > interface added in mainline earlier doesn't work, and CPU stall's > infinitely, when-ever i2c transfer is initiated. > > Ref:previous polling mode support in mainline > > commit 69c8c0c0efa8 ("i2c: ocores: add polling interface") > > The workaround / fix under OCORES_FLAG_BROKEN_IRQ is particularly for > FU540-COOO SoC. > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com> > --- > drivers/i2c/busses/i2c-ocores.c | 34 ++++++++++++++++++++++++++++------ > 1 file changed, 28 insertions(+), 6 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c > index aee1d86..00ee45c 100644 > --- a/drivers/i2c/busses/i2c-ocores.c > +++ b/drivers/i2c/busses/i2c-ocores.c > @@ -27,6 +27,7 @@ > #include <linux/jiffies.h> > > #define OCORES_FLAG_POLL BIT(0) > +#define OCORES_FLAG_BROKEN_IRQ BIT(2) /* Broken IRQ in HiFive Unleashed */ Hi Sigar BIT(1). Don't leave a gap. Andrew
Hi Andrew, On Thu, May 16, 2019 at 6:37 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Thu, May 16, 2019 at 10:38:40AM +0530, Sagar Shrikant Kadam wrote: > > The i2c-ocore driver already has a polling mode interface.But it needs > > a workaround for FU540 Chipset on HiFive unleashed board (RevA00). > > There is an erratum in FU540 chip that prevents interrupt driven i2c > > transfers from working, and also the I2C controller's interrupt bit > > cannot be cleared if set, due to this the existing i2c polling mode > > interface added in mainline earlier doesn't work, and CPU stall's > > infinitely, when-ever i2c transfer is initiated. > > > > Ref:previous polling mode support in mainline > > > > commit 69c8c0c0efa8 ("i2c: ocores: add polling interface") > > > > The workaround / fix under OCORES_FLAG_BROKEN_IRQ is particularly for > > FU540-COOO SoC. > > > > Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com> > > --- > > drivers/i2c/busses/i2c-ocores.c | 34 ++++++++++++++++++++++++++++------ > > 1 file changed, 28 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c > > index aee1d86..00ee45c 100644 > > --- a/drivers/i2c/busses/i2c-ocores.c > > +++ b/drivers/i2c/busses/i2c-ocores.c > > @@ -27,6 +27,7 @@ > > #include <linux/jiffies.h> > > > > #define OCORES_FLAG_POLL BIT(0) > > +#define OCORES_FLAG_BROKEN_IRQ BIT(2) /* Broken IRQ in HiFive Unleashed */ > > Hi Sigar > > BIT(1). Don't leave a gap. I will remove the gap and update this in V4. Thanks, Sagar > > Andrew
diff --git a/drivers/i2c/busses/i2c-ocores.c b/drivers/i2c/busses/i2c-ocores.c index aee1d86..00ee45c 100644 --- a/drivers/i2c/busses/i2c-ocores.c +++ b/drivers/i2c/busses/i2c-ocores.c @@ -27,6 +27,7 @@ #include <linux/jiffies.h> #define OCORES_FLAG_POLL BIT(0) +#define OCORES_FLAG_BROKEN_IRQ BIT(2) /* Broken IRQ in HiFive Unleashed */ /* * 'process_lock' exists because ocores_process() and ocores_process_timeout() @@ -239,9 +240,13 @@ static irqreturn_t ocores_isr(int irq, void *dev_id) struct ocores_i2c *i2c = dev_id; u8 stat = oc_getreg(i2c, OCI2C_STATUS); - if (!(stat & OCI2C_STAT_IF)) + if (i2c->flags & OCORES_FLAG_BROKEN_IRQ) { + if (stat & OCI2C_STAT_IF) + if (!(stat & OCI2C_STAT_BUSY)) + return IRQ_NONE; + } else if (!(stat & OCI2C_STAT_IF)) { return IRQ_NONE; - + } ocores_process(i2c, stat); return IRQ_HANDLED; @@ -356,6 +361,11 @@ static void ocores_process_polling(struct ocores_i2c *i2c) ret = ocores_isr(-1, i2c); if (ret == IRQ_NONE) break; /* all messages have been transferred */ + else { + if (i2c->flags & OCORES_FLAG_BROKEN_IRQ) + if (i2c->state == STATE_DONE) + break; + } } } @@ -406,7 +416,7 @@ static int ocores_xfer(struct i2c_adapter *adap, { struct ocores_i2c *i2c = i2c_get_adapdata(adap); - if (i2c->flags & OCORES_FLAG_POLL) + if ((i2c->flags & (OCORES_FLAG_POLL | OCORES_FLAG_BROKEN_IRQ))) return ocores_xfer_polling(adap, msgs, num); return ocores_xfer_core(i2c, msgs, num, false); } @@ -471,7 +481,7 @@ static u32 ocores_func(struct i2c_adapter *adap) }, { .compatible = "sifive,fu540-c000-i2c", - .data = (void *)TYPE_SIFIVE_REV0, + .data = (void *)(TYPE_SIFIVE_REV0 | OCORES_FLAG_BROKEN_IRQ), }, { .compatible = "sifive,i2c0", @@ -601,6 +611,7 @@ static int ocores_i2c_probe(struct platform_device *pdev) { struct ocores_i2c *i2c; struct ocores_i2c_platform_data *pdata; + const struct of_device_id *match; struct resource *res; int irq; int ret; @@ -682,13 +693,24 @@ static int ocores_i2c_probe(struct platform_device *pdev) irq = platform_get_irq(pdev, 0); if (irq == -ENXIO) { - i2c->flags |= OCORES_FLAG_POLL; + /* + * Set a OCORES_FLAG_BROKEN_IRQ to enable workaround for + * FU540-C000 SoC in polling mode interface of i2c-ocore driver. + * Else enable default polling mode interface for SIFIVE/OCORE + * device types. + */ + match = of_match_node(ocores_i2c_match, pdev->dev.of_node); + if (match && (long)match->data == + (TYPE_SIFIVE_REV0 | OCORES_FLAG_BROKEN_IRQ)) + i2c->flags |= OCORES_FLAG_BROKEN_IRQ; + else + i2c->flags |= OCORES_FLAG_POLL; } else { if (irq < 0) return irq; } - if (!(i2c->flags & OCORES_FLAG_POLL)) { + if (!(i2c->flags & (OCORES_FLAG_POLL | OCORES_FLAG_BROKEN_IRQ))) { ret = devm_request_irq(&pdev->dev, irq, ocores_isr, 0, pdev->name, i2c); if (ret) {
The i2c-ocore driver already has a polling mode interface.But it needs a workaround for FU540 Chipset on HiFive unleashed board (RevA00). There is an erratum in FU540 chip that prevents interrupt driven i2c transfers from working, and also the I2C controller's interrupt bit cannot be cleared if set, due to this the existing i2c polling mode interface added in mainline earlier doesn't work, and CPU stall's infinitely, when-ever i2c transfer is initiated. Ref:previous polling mode support in mainline commit 69c8c0c0efa8 ("i2c: ocores: add polling interface") The workaround / fix under OCORES_FLAG_BROKEN_IRQ is particularly for FU540-COOO SoC. Signed-off-by: Sagar Shrikant Kadam <sagar.kadam@sifive.com> --- drivers/i2c/busses/i2c-ocores.c | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-)