RE: [PATCH 7/8] i2c: img-scb: improve transaction complete handle
Hi James,
> -Original Message-
> From: James Hogan
> Sent: 29 July 2015 13:22
> To: Sifan Naeem; Wolfram Sang; linux-i2c@vger.kernel.org
> Cc: Stable kernel (v3.19+)
> Subject: Re: [PATCH 7/8] i2c: img-scb: improve transaction complete handle
>
> Hi Sifan,
>
> On 27/07/15 12:47, Sifan Naeem wrote:
> > Clear line status and all interrupts when transaction is complete, as
> > not doing so might leave unserviced interrupts that might be
>
> Do you have a specific example of when this might happen, and whether it
> could occur after img_i2c_complete_transaction()?
>
> I'm just wondering if it would be better to do this before starting every new
> message instead of after handling the last irq that is of interest (maybe
> somewhere in img_i2c_xfer).
>
Moved to happen before each transfer.
> > handled in the context of a new transfer. Soft reset if the the
> > transfer failed to bring back the i2c block to a reset state.
> >
> > Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB
> > driver")
> > Signed-off-by: Sifan Naeem
> > Cc: Stable kernel (v3.19+)
> > ---
> > drivers/i2c/busses/i2c-img-scb.c |5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-img-scb.c
> > b/drivers/i2c/busses/i2c-img-scb.c
> > index 341130e..bbfee33 100644
> > --- a/drivers/i2c/busses/i2c-img-scb.c
> > +++ b/drivers/i2c/busses/i2c-img-scb.c
> > @@ -626,7 +626,10 @@ static void img_i2c_complete_transaction(struct
> img_i2c *i2c, int status)
> > img_i2c_switch_mode(i2c, MODE_INACTIVE);
> > if (status) {
> > i2c->msg_status = status;
> > - img_i2c_transaction_halt(i2c, false);
> > + img_i2c_soft_reset(i2c);
>
> This seems like overkill. This will only happen in a couple of cases:
> (1) an automatic mode ack error, which is completely recoverable.
> (2) a fatal error (clock low timeout), which switches mode to MODE_FATAL
> anyway, preventing further transactions.
>
Removed.
Thanks,
Sifan
> Cheers
> James
>
> > + } else {
> > + img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> > + img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
> > }
> > complete(&i2c->msg_complete);
> > }
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] i2c: img-scb: improve transaction complete handle
Hi Sifan,
On 27/07/15 12:47, Sifan Naeem wrote:
> Clear line status and all interrupts when transaction is complete,
> as not doing so might leave unserviced interrupts that might be
Do you have a specific example of when this might happen, and whether it
could occur after img_i2c_complete_transaction()?
I'm just wondering if it would be better to do this before starting
every new message instead of after handling the last irq that is of
interest (maybe somewhere in img_i2c_xfer).
> handled in the context of a new transfer. Soft reset if the the
> transfer failed to bring back the i2c block to a reset state.
>
> Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
> Signed-off-by: Sifan Naeem
> Cc: Stable kernel (v3.19+)
> ---
> drivers/i2c/busses/i2c-img-scb.c |5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-img-scb.c
> b/drivers/i2c/busses/i2c-img-scb.c
> index 341130e..bbfee33 100644
> --- a/drivers/i2c/busses/i2c-img-scb.c
> +++ b/drivers/i2c/busses/i2c-img-scb.c
> @@ -626,7 +626,10 @@ static void img_i2c_complete_transaction(struct img_i2c
> *i2c, int status)
> img_i2c_switch_mode(i2c, MODE_INACTIVE);
> if (status) {
> i2c->msg_status = status;
> - img_i2c_transaction_halt(i2c, false);
> + img_i2c_soft_reset(i2c);
This seems like overkill. This will only happen in a couple of cases:
(1) an automatic mode ack error, which is completely recoverable.
(2) a fatal error (clock low timeout), which switches mode to MODE_FATAL
anyway, preventing further transactions.
Cheers
James
> + } else {
> + img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
> + img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
> }
> complete(&i2c->msg_complete);
> }
>
signature.asc
Description: OpenPGP digital signature
[PATCH 7/8] i2c: img-scb: improve transaction complete handle
Clear line status and all interrupts when transaction is complete,
as not doing so might leave unserviced interrupts that might be
handled in the context of a new transfer. Soft reset if the the
transfer failed to bring back the i2c block to a reset state.
Fixes: 27bce4 ("i2c: img-scb: Add Imagination Technologies I2C SCB driver")
Signed-off-by: Sifan Naeem
Cc: Stable kernel (v3.19+)
---
drivers/i2c/busses/i2c-img-scb.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-img-scb.c b/drivers/i2c/busses/i2c-img-scb.c
index 341130e..bbfee33 100644
--- a/drivers/i2c/busses/i2c-img-scb.c
+++ b/drivers/i2c/busses/i2c-img-scb.c
@@ -626,7 +626,10 @@ static void img_i2c_complete_transaction(struct img_i2c
*i2c, int status)
img_i2c_switch_mode(i2c, MODE_INACTIVE);
if (status) {
i2c->msg_status = status;
- img_i2c_transaction_halt(i2c, false);
+ img_i2c_soft_reset(i2c);
+ } else {
+ img_i2c_writel(i2c, SCB_INT_CLEAR_REG, ~0);
+ img_i2c_writel(i2c, SCB_CLEAR_REG, ~0);
}
complete(&i2c->msg_complete);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
