All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
To: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
	Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>,
	Grygorii Strashko
	<grygorii.strashko-l0cyMroinI0@public.gmane.org>
Subject: Re: [PATCH] i2c: omap: fix NACK and Arbitration Lost irq handling
Date: Fri, 21 Nov 2014 07:52:20 +0100	[thread overview]
Message-ID: <20141121065220.GD1480@katana> (raw)
In-Reply-To: <1416330058-21473-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 1955 bytes --]

On Tue, Nov 18, 2014 at 09:00:58PM +0400, Alexander Kochetkov wrote:
> commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts)
> changed the interrupt handler to complete transfers without clearing
> XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupts will be
> fired again. As a result, ISR keep processing transfer after it was already
> complete (from the driver code point of view).
> 
> A didn't see real impacts of the 1d7afc9, but it is really bad idea to
> have ISR running on user data after transfer was complete.
> 
> It looks, what 1d7afc9 violate TI specs in what how AL and NACK should be
> handled (see Note 1, sprugn4r, Figure 17-31 and Figure 17-32).
> 
> According to specs (if I understood correctly), in case of NACK and AL driver
> must reset NACK, AL, ARDY, RDR, and RRDY (Master Receive Mode), and
> NACK, AL, ARDY, and XDR (Master Transmitter Mode).
> 
> All that is done down the code under the if condition:
> if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) ...
> 
> The patch restore pre 1d7afc9 logic of handling NACK and AL interrupts, so
> no interrupts is fired after ISR informs the rest of driver what transfer
> complete.
> 
> Note: instead of removing break under NACK case, we could just replace 'break'
> with 'continue' and allow NACK transfer to finish using ARDY event. I found
> that NACK and ARDY bits usually set together. That case confirm TI wiki:
> http://processors.wiki.ti.com/index.php/I2C_Tips#Detecting_and_handling_NACK
> 
> In order if someone interested in the event traces for NACK and AL cases,
> I sent them to mailing list.
> 
> Tested on Beagleboard XM C.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Fixes: 1d7afc9 i2c: omap: ack IRQ in parts
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v3.7+
> 

Applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Wolfram Sang <wsa@the-dreams.de>
To: Alexander Kochetkov <al.kochet@gmail.com>
Cc: linux-omap@vger.kernel.org, linux-i2c@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Tony Lindgren <tony@atomide.com>, Felipe Balbi <balbi@ti.com>,
	Grygorii Strashko <grygorii.strashko@ti.com>
Subject: Re: [PATCH] i2c: omap: fix NACK and Arbitration Lost irq handling
Date: Fri, 21 Nov 2014 07:52:20 +0100	[thread overview]
Message-ID: <20141121065220.GD1480@katana> (raw)
In-Reply-To: <1416330058-21473-1-git-send-email-al.kochet@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

On Tue, Nov 18, 2014 at 09:00:58PM +0400, Alexander Kochetkov wrote:
> commit 1d7afc95946487945cc7f5019b41255b72224b70 (i2c: omap: ack IRQ in parts)
> changed the interrupt handler to complete transfers without clearing
> XRDY (AL case) and ARDY (NACK case) flags. XRDY or ARDY interrupts will be
> fired again. As a result, ISR keep processing transfer after it was already
> complete (from the driver code point of view).
> 
> A didn't see real impacts of the 1d7afc9, but it is really bad idea to
> have ISR running on user data after transfer was complete.
> 
> It looks, what 1d7afc9 violate TI specs in what how AL and NACK should be
> handled (see Note 1, sprugn4r, Figure 17-31 and Figure 17-32).
> 
> According to specs (if I understood correctly), in case of NACK and AL driver
> must reset NACK, AL, ARDY, RDR, and RRDY (Master Receive Mode), and
> NACK, AL, ARDY, and XDR (Master Transmitter Mode).
> 
> All that is done down the code under the if condition:
> if (stat & (OMAP_I2C_STAT_ARDY | OMAP_I2C_STAT_NACK | OMAP_I2C_STAT_AL)) ...
> 
> The patch restore pre 1d7afc9 logic of handling NACK and AL interrupts, so
> no interrupts is fired after ISR informs the rest of driver what transfer
> complete.
> 
> Note: instead of removing break under NACK case, we could just replace 'break'
> with 'continue' and allow NACK transfer to finish using ARDY event. I found
> that NACK and ARDY bits usually set together. That case confirm TI wiki:
> http://processors.wiki.ti.com/index.php/I2C_Tips#Detecting_and_handling_NACK
> 
> In order if someone interested in the event traces for NACK and AL cases,
> I sent them to mailing list.
> 
> Tested on Beagleboard XM C.
> 
> Signed-off-by: Alexander Kochetkov <al.kochet@gmail.com>
> Fixes: 1d7afc9 i2c: omap: ack IRQ in parts
> Cc: <stable@vger.kernel.org> # v3.7+
> 

Applied to for-current, thanks!


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  parent reply	other threads:[~2014-11-21  6:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17 16:41 [PATCH] Fixes: 1d7afc9 i2c: omap: ack IRQ in parts Alexander Kochetkov
2014-11-17 16:41 ` Alexander Kochetkov
2014-11-17 17:01 ` Alexander Kochetkov
2014-11-17 17:36 ` Alexander Kochetkov
2014-11-17 17:36   ` Alexander Kochetkov
2014-11-18 16:39 ` Felipe Balbi
2014-11-18 16:39   ` Felipe Balbi
2014-11-21  6:48   ` Wolfram Sang
2014-11-21  6:48     ` Wolfram Sang
2014-11-18 17:00 ` [PATCH] i2c: omap: fix NACK and Arbitration Lost irq handling Alexander Kochetkov
2014-11-20 21:10   ` Aaro Koskinen
     [not found]   ` <1416330058-21473-1-git-send-email-al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-11-21  6:52     ` Wolfram Sang [this message]
2014-11-21  6:52       ` Wolfram Sang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141121065220.GD1480@katana \
    --to=wsa-z923lk4zbo2bacvfa/9k2g@public.gmane.org \
    --cc=al.kochet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=balbi-l0cyMroinI0@public.gmane.org \
    --cc=grygorii.strashko-l0cyMroinI0@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.