On 7/11/19 5:09 PM, Jan Kiszka wrote:
> On 11.07.19 16:40, Philippe Gerum wrote:
>> On 7/5/19 9:38 AM, Jan Kiszka wrote:
>>
>>> This addresses it on x86 for me:
>>>
>>> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
>>> index 6c279e065879..d503b875f086 100644
>>> --- a/kernel/irq/chip.c
>>> +++ b/kernel/irq/chip.c
>>> @@ -1099,7 +1099,8 @@ void ipipe_enable_irq(unsigned int irq)
>>> ipipe_root_only();
>>>
>>> raw_spin_lock_irqsave(&desc->lock, flags);
>>> - if (desc->istate & IPIPE_IRQS_NEEDS_STARTUP) {
>>> + if (desc->istate & IPIPE_IRQS_NEEDS_STARTUP &&
>>> + !WARN_ON(irq_activate(desc))) {
>>> desc->istate &= ~IPIPE_IRQS_NEEDS_STARTUP;
>>> chip->irq_startup(&desc->irq_data);
>>> }
>>>
>>> Probably upstream commit c942cee46bba (genirq: Separate activation and
>>> startup) makes this necessary.
>>>
>>> Philippe, I suppose this is either not essential on arm, or external
>>> interrupts weren't tested yet, like I missed on x86. Fine to make this a
>>> noarch patch?
>>
>> No issue. I've not been working on/with the I-pipe but Dovetail instead
>> in the past weeks, so testing of 4.19 is still very limited on my end. I
>> have several full-fledged real world ARM*-based application systems to
>> improve this, just need to find a way to squeeze this work in.
>>
>
> One question remains, though: Should we just do the WARN_ON() thing within the
> existing API or change that API to return a potential error? Variant one I
> have
> ready, but I have no feeling for the risk that there is actually an error.
>
> The same goes for ipipe_set_irq_affinity that will require the activation as
> well but cannot return an error so far.
>
Moving from void to non-void would be backward-compatible provided we
don't tag these services as __must_check, so propagating the status
would make sense.
--
Philippe.
