2020-11-26 14:58:40

by Robert Foss

[permalink] [raw]
Subject: [PATCH v1] i2c: qcom: Fix IRQ error misassignement

During cci_isr() errors read from register fields belonging to
i2c master1 are currently assigned to the status field belonging to
i2c master0. This patch corrects this error, and always assigns
master1 errors to the status field of master1.

Suggested-by: Loic Poulain <[email protected]>
Signed-off-by: Robert Foss <[email protected]>
---
drivers/i2c/busses/i2c-qcom-cci.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
index f13735beca58..1c259b5188de 100644
--- a/drivers/i2c/busses/i2c-qcom-cci.c
+++ b/drivers/i2c/busses/i2c-qcom-cci.c
@@ -194,9 +194,9 @@ static irqreturn_t cci_isr(int irq, void *dev)
if (unlikely(val & CCI_IRQ_STATUS_0_I2C_M1_ERROR)) {
if (val & CCI_IRQ_STATUS_0_I2C_M1_Q0_NACK_ERR ||
val & CCI_IRQ_STATUS_0_I2C_M1_Q1_NACK_ERR)
- cci->master[0].status = -ENXIO;
+ cci->master[1].status = -ENXIO;
else
- cci->master[0].status = -EIO;
+ cci->master[1].status = -EIO;

writel(CCI_HALT_REQ_I2C_M1_Q0Q1, cci->base + CCI_HALT_REQ);
ret = IRQ_HANDLED;
--
2.27.0


2020-11-26 15:07:20

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v1] i2c: qcom: Fix IRQ error misassignement

On Thu, Nov 26, 2020 at 03:53:21PM +0100, Robert Foss wrote:
> During cci_isr() errors read from register fields belonging to
> i2c master1 are currently assigned to the status field belonging to

s/correctly/incorrectly

> i2c master0. This patch corrects this error, and always assigns
> master1 errors to the status field of master1.
>

This patch fixes a legitimate bug. So there should be a fixes tag!

> Suggested-by: Loic Poulain <[email protected]>

Reported-by?

Thanks,
Mani

> Signed-off-by: Robert Foss <[email protected]>
> ---
> drivers/i2c/busses/i2c-qcom-cci.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qcom-cci.c b/drivers/i2c/busses/i2c-qcom-cci.c
> index f13735beca58..1c259b5188de 100644
> --- a/drivers/i2c/busses/i2c-qcom-cci.c
> +++ b/drivers/i2c/busses/i2c-qcom-cci.c
> @@ -194,9 +194,9 @@ static irqreturn_t cci_isr(int irq, void *dev)
> if (unlikely(val & CCI_IRQ_STATUS_0_I2C_M1_ERROR)) {
> if (val & CCI_IRQ_STATUS_0_I2C_M1_Q0_NACK_ERR ||
> val & CCI_IRQ_STATUS_0_I2C_M1_Q1_NACK_ERR)
> - cci->master[0].status = -ENXIO;
> + cci->master[1].status = -ENXIO;
> else
> - cci->master[0].status = -EIO;
> + cci->master[1].status = -EIO;
>
> writel(CCI_HALT_REQ_I2C_M1_Q0Q1, cci->base + CCI_HALT_REQ);
> ret = IRQ_HANDLED;
> --
> 2.27.0
>

2020-11-26 16:00:40

by Robert Foss

[permalink] [raw]
Subject: Re: [PATCH v1] i2c: qcom: Fix IRQ error misassignement

Thanks for the review Mani.

On Thu, 26 Nov 2020 at 16:03, Manivannan Sadhasivam
<[email protected]> wrote:
>
> On Thu, Nov 26, 2020 at 03:53:21PM +0100, Robert Foss wrote:
> > During cci_isr() errors read from register fields belonging to
> > i2c master1 are currently assigned to the status field belonging to
>
> s/correctly/incorrectly

I don't think there actually is an error in the comment.

>
> > i2c master0. This patch corrects this error, and always assigns
> > master1 errors to the status field of master1.
> >
>
> This patch fixes a legitimate bug. So there should be a fixes tag!

On it, fixed in v2

>
> > Suggested-by: Loic Poulain <[email protected]>
>
> Reported-by?

I'll add both in v2

2020-11-26 16:14:30

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v1] i2c: qcom: Fix IRQ error misassignement

On Thu, Nov 26, 2020 at 04:58:48PM +0100, Robert Foss wrote:
> Thanks for the review Mani.
>
> On Thu, 26 Nov 2020 at 16:03, Manivannan Sadhasivam
> <[email protected]> wrote:
> >
> > On Thu, Nov 26, 2020 at 03:53:21PM +0100, Robert Foss wrote:
> > > During cci_isr() errors read from register fields belonging to
> > > i2c master1 are currently assigned to the status field belonging to
> >
> > s/correctly/incorrectly
>
> I don't think there actually is an error in the comment.
>

Yeah its an error in my comment ;) Please ignore that.

Thanks,
Mani

> >
> > > i2c master0. This patch corrects this error, and always assigns
> > > master1 errors to the status field of master1.
> > >
> >
> > This patch fixes a legitimate bug. So there should be a fixes tag!
>
> On it, fixed in v2
>
> >
> > > Suggested-by: Loic Poulain <[email protected]>
> >
> > Reported-by?
>
> I'll add both in v2