Re: [PATCH 1/9] i2c: xiic: Do not continue in case of errors in Rx

2015-06-17 Thread Wolfram Sang

> > Your patch descriptions need improvement. They describe what you do, but
> > this can be seen in the patch as well. What you need to describe is WHY you
> > need this change.
> 
> In case  the slave nacks the transaction in the middle of the transfer The 
> driver continues.
> 
> > What was wrong before,
> 
> The usual expectation is that in case of error / nack the transaction is 
> halted.

Yeah, this is going into the right direction. Please note, that I meant
that all 9 patch descriptions need rework. I know this is extra work,
but really needed to keep drivers maintainable. Please resend the series
when you are done.

> > Because this driver is long in use, we must be very careful about 
> > regressions
> > and every change needs a good reason.
> 
> I agree.

Great! :)



signature.asc
Description: Digital signature


RE: [PATCH 1/9] i2c: xiic: Do not continue in case of errors in Rx

2015-06-17 Thread Shubhrajyoti Datta


> -Original Message-
> From: Wolfram Sang [mailto:w...@the-dreams.de]
> Sent: Wednesday, June 17, 2015 5:12 PM
> To: Shubhrajyoti Datta
> Cc: linux-i2c@vger.kernel.org; Shubhrajyoti Datta
> Subject: Re: [PATCH 1/9] i2c: xiic: Do not continue in case of errors in Rx
>
> Hi,
>
> On Mon, Jun 15, 2015 at 08:47:52PM +0530, Shubhrajyoti Datta wrote:
> > Handle error cases in the Rx path
> >
> > Signed-off-by: Shubhrajyoti Datta 
>
> Your patch descriptions need improvement. They describe what you do, but
> this can be seen in the patch as well. What you need to describe is WHY you
> need this change.

In case  the slave nacks the transaction in the middle of the transfer The 
driver continues.

> What was wrong before,

The usual expectation is that in case of error / nack the transaction is halted.

>what is better now.

There are 2 things my series does

1 . The interrupts were disabled in the start of the transfer and re-enabled 
again.
So if there are any error interrupts this will not be handled.

2. Secondly during the isr also the interrupts were disabled.
So if there is any nack in between that may not be respected.

3. I felt in case of all  errors we should
Call xiic_wakeup(i2c, STATE_ERROR); -> wake_up(&i2c->wait)
And come out of wait.


I made the patches speculatively after reading a bug report that says nacks are 
not
Respected in the middle of a transfer.

However do not have a peripheral that will nack in the middle of a transfer.
Could not test it in that sense.

> Because this driver is long in use, we must be very careful about regressions
> and every change needs a good reason.

I agree.

>
> So, please rework your patch descriptions.



This email and any attachments are intended for the sole use of the named 
recipient(s) and contain(s) confidential information that may be proprietary, 
privileged or copyrighted under applicable law. If you are not the intended 
recipient, do not read, copy, or forward this email message or any attachments. 
Delete this email message and any attachments immediately.

--
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 1/9] i2c: xiic: Do not continue in case of errors in Rx

2015-06-17 Thread Wolfram Sang
Hi,

On Mon, Jun 15, 2015 at 08:47:52PM +0530, Shubhrajyoti Datta wrote:
> Handle error cases in the Rx path
> 
> Signed-off-by: Shubhrajyoti Datta 

Your patch descriptions need improvement. They describe what you do,
but this can be seen in the patch as well. What you need to describe is
WHY you need this change. What was wrong before, what is better now.
Because this driver is long in use, we must be very careful about
regressions and every change needs a good reason.

So, please rework your patch descriptions.



signature.asc
Description: Digital signature