lkml.org 
[lkml]   [2020]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] irqchip: mips-cpu: Remove eoi operation
From


于 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

\
 
 \ /
  Last update: 2020-01-15 15:24    [W:0.042 / U:0.576 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site