On 7/11/19 6:34 PM, Jan Kiszka wrote: > On 11.07.19 18:00, Philippe Gerum wrote: >> 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. > > ...but it would also be risky as we then had no reporting of an error. If we > change the API, I would do that in way users (namely drivers) have a chance to > become aware of this change. >
Coupling error propagation and WARN_ON (e.g. in pipeline debug mode) should not be a problem. -- Philippe.