LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: JeffyChen <jeffy.chen@rock-chips.com>
To: Brian Norris <briannorris@chromium.org>
Cc: linux-kernel@vger.kernel.org, heiko@sntech.de,
	linux-rockchip@lists.infradead.org,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Doug Anderson <dianders@chromium.org>
Subject: Re: [RESEND PATCH] pinctrl: rockchip: Disable interrupt when changing it's capability
Date: Tue, 08 May 2018 10:56:48 +0800	[thread overview]
Message-ID: <5AF111F0.3030409@rock-chips.com> (raw)
In-Reply-To: <5AF10C10.1070003@rock-chips.com>

Hi Brian,

On 05/08/2018 10:31 AM, JeffyChen wrote:
> Hi Brian,
>
> On 05/08/2018 09:56 AM, Brian Norris wrote:
>> On Tue, May 08, 2018 at 09:36:24AM +0800, Jeffy Chen wrote:
>>> On 05/08/2018 06:15 AM, Brian Norris wrote:
>>>> On the other hand...this also implies there may be a race condition
>>>> there, where we might lose an interrupt if there is an edge between the
>>>> re-configuration of the polarity in rockchip_irq_demux() and the
>>>> clearing/handling of the interrupt (handle_edge_irq() ->
>>>> chip->irq_ack()). If we have an edge between there, then we might ack
>>>> it, but leave the polarity such that we aren't ready for the next
>>>> (inverted) edge.
>>>
>>> if let me guess, the unexpected irq we saw is the hardware trying to
>>> avoid
>>> losing irq? for example, we set a EDGE_RISING, and the hardware saw
>>> the gpio
>>> is already high, then though it might lost an irq, so fake one for safe?
>>
>> I won't pretend to know what the IC designers were doing, but I don't
>> think that would resolve the problem I'm talking about. The sequence is
>> something like:
>> 1. EDGE_BOTH IRQ occurs (e.g., low to high)
>> 2. reconfigure polarity in rockchip_irq_demux() (polarity=low)
>> 3. continue to handle_edge_irq()
>> 4. another HW edge occurs (e.g., high to low)
>> 5. handle_edge_irq() (from 3) acks (clears) IRQ (before a subsequent
>>     rockchip_irq_demux() gets a chance to run and flip the polarity)
>> ...
>>
>> Now the polarity is still low, but the next trigger should be a
>> low-to-high edge.
>
> oops, i see the problem.
>
> so what if we do these:
> 1/ edge irq triggered
> 2/ read gpio level
> 3/ handle irq(ack irq)
> 4/ toggle edge mode(with a while gpio level check)
>
> if the gpio changed in 2/ -> 3/, the 4/ will trigger an irq when writing
> GPIO_INT_POLARITY(which is what we are trying to avoid in the set_type
> case)
>
> but this would not work if i'm wrong about how the HW fake an irq when
> changing POLARITY...
>
>
> or maybe we could just check the gpio status again after
> handle_edge_irq, and correct the polarity in this case

i saw the pinctrl-msm.c do this in the ack(),

and also at the end of set_type(), which might avoid another race in the 
set_type()

>
>>
>>> i'll try to confirm it with IC guys.
>>
>> Brian
>>
>>
>>
>

  reply	other threads:[~2018-05-08  2:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-03  6:55 Jeffy Chen
2018-05-07 22:15 ` Brian Norris
2018-05-08  1:36   ` JeffyChen
2018-05-08  1:56     ` Brian Norris
2018-05-08  2:31       ` JeffyChen
2018-05-08  2:56         ` JeffyChen [this message]
2018-05-08 19:46       ` Doug Anderson
2018-05-09  2:21         ` JeffyChen
2018-05-09  5:18           ` Doug Anderson
2018-05-09  6:41             ` JeffyChen
2018-05-10 22:16             ` Brian Norris

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=5AF111F0.3030409@rock-chips.com \
    --to=jeffy.chen@rock-chips.com \
    --cc=briannorris@chromium.org \
    --cc=dianders@chromium.org \
    --cc=heiko@sntech.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --subject='Re: [RESEND PATCH] pinctrl: rockchip: Disable interrupt when changing it'\''s capability' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).