On 11.07.19 18:48, Philippe Gerum wrote:
> 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.
>
Evaluating the error code of ipipe_enable_irq and ipipe_set_irq_affinity in
Xenomai will mean requiring a minimal ipipe core version for 4.19. So I will
push the burden of dealing with unprepared drivers to Xenomai (WARN_ON there)
and rather enforce that ipipe update. Luckily, we have no release out yet that
support 4.19.
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
