LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Jiaxun Yang <jiaxun.yang@flygoat.com>
To: Marc Zyngier <maz@kernel.org>, Paul Burton <paulburton@kernel.org>
Cc: linux-mips@vger.kernel.org, chenhc@lemote.com,
	paul.burton@mips.com, tglx@linutronix.de, jason@lakedaemon.net,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] irqchip: mips-cpu: Remove eoi operation
Date: Wed, 15 Jan 2020 22:23:29 +0800	[thread overview]
Message-ID: <C7C70199-38CC-473E-B20D-C1782F08CA2E@flygoat.com> (raw)
In-Reply-To: <9cd8df72fc3a7dfcdd88eb1fb56bbe35@kernel.org>



于 2020年1月15日 GMT+08:00 下午9:40:31, Marc Zyngier <maz@kernel.org> 写到:
>On 2020-01-14 23:30, Paul Burton wrote:
>> Hi Jiaxun,
>> 
>> On Mon, Jan 13, 2020 at 06:12:51PM +0800, Jiaxun Yang wrote:
>>> The eoi opreation in mips_cpu_irq_controller caused
>chained_irq_enter
>>> falsely consider CPU IP interrupt as a FastEOI type IRQ. So the 
>>> interrupt
>>> won't be masked during in handler. Which might lead to spurious 
>>> interrupt.
>>> 
>>> Thus we simply remove eoi operation for mips_cpu_irq_controller,
>>> 
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>  drivers/irqchip/irq-mips-cpu.c | 1 -
>>>  1 file changed, 1 deletion(-)
>>> 
>>> diff --git a/drivers/irqchip/irq-mips-cpu.c 
>>> b/drivers/irqchip/irq-mips-cpu.c
>>> index 95d4fd8f7a96..0ad7f1f9a58b 100644
>>> --- a/drivers/irqchip/irq-mips-cpu.c
>>> +++ b/drivers/irqchip/irq-mips-cpu.c
>>> @@ -55,7 +55,6 @@ static struct irq_chip mips_cpu_irq_controller = {
>>>  	.irq_mask	= mask_mips_irq,
>>>  	.irq_mask_ack	= mask_mips_irq,
>>>  	.irq_unmask	= unmask_mips_irq,
>>> -	.irq_eoi	= unmask_mips_irq,
>>>  	.irq_disable	= mask_mips_irq,
>>>  	.irq_enable	= unmask_mips_irq,
>>>  };
>> 
>> This one scares me; something doesn't seem right. The irq_eoi (née
>eoi)
>> callback was first added way back in commit 1417836e81c0 ("[MIPS] use
>> generic_handle_irq, handle_level_irq, handle_percpu_irq"). The commit
>> message there states that the motivation was to allow use of
>> handle_percpu_irq(), and indeed handle_percpu_irq() does:
>> 
>>     irq_ack() (ie. mask)
>>     invoke the handler(s)
>>     irq_eoi() (ie. unmask)
>> 
>> By removing the irq_eoi callback I don't see how we'd ever unmask the
>> interrupt again..?
>
>To be completely blunt, the fact that unmask and eoi are implemented
>the
>same way is a clear sign that this is a bit broken.
>
>irq_eoi is used if the irqchip tracks the IRQ life-cycle in HW, and
>it's
>not obvious that this is the case. The fact that ack is also mapped to 
>mask

It's just a kind of hack to workaround the fact that our current percpu irq handler assumed
all percpu irqs are edge triggered or fasteoi type.

However MIPS processor implemented it in level triggered way.

My solution would be add a check. If neither ack nor eoi exist for the chip,
than we assume it's level triggered and process precpu irq in mask/unmask way.

Could it be a possible option?

Thanks.

>just adds to my feeling...
>
>         M.

-- 
Jiaxun Yang

  reply	other threads:[~2020-01-15 14:23 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-13 10:12 Jiaxun Yang
2020-01-14 23:30 ` Paul Burton
2020-01-15  0:03   ` Jiaxun Yang
2020-01-15 13:40   ` Marc Zyngier
2020-01-15 14:23     ` Jiaxun Yang [this message]
2020-01-15 15:22       ` Marc Zyngier
2020-01-17  0:17 ` [PATCH v1 1/2] genirq: Check for level based percpu irq Jiaxun Yang
2020-01-17  0:17   ` [PATCH v1 2/2] irqchip: mips-cpu: Drop unnecessary ack/eoi operations Jiaxun Yang
2020-01-17  1:29   ` [PATCH v1 1/2] genirq: Check for level based percpu irq Thomas Gleixner
2020-01-17  2:23     ` Jiaxun Yang

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=C7C70199-38CC-473E-B20D-C1782F08CA2E@flygoat.com \
    --to=jiaxun.yang@flygoat.com \
    --cc=chenhc@lemote.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=paul.burton@mips.com \
    --cc=paulburton@kernel.org \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH] irqchip: mips-cpu: Remove eoi operation' \
    /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).