From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: <43FDA8E2.5010806@domain.hid> Date: Thu, 23 Feb 2006 13:21:54 +0100 From: Philippe Gerum MIME-Version: 1.0 Subject: Re: [Xenomai-core] Re: [PATCH] Shared interrupts (ready to merge) References: <43FAD322.4060001@domain.hid> <43FAF94C.4080709@domain.hid> <43FAFE58.5060201@domain.hid> <43FB480E.9020808@domain.hid> <43FB529B.3040207@domain.hid> <43FB6102.1070004@domain.hid> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: "Xenomai life and development \(bug reports, patches, discussions\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Dmitry Adamushko Cc: Jan Kiszka , xenomai@xenomai.org Dmitry Adamushko wrote: > > > For RTDM I'm now almost determined to rework the API in way that only > > HANDLED/UNHANDLED (or what ever their names will be) get exported, any > > additional guru features will remain excluded as long as we have no > > clean usage policy for them. > > Good. Then let's go for > > HANDLED, UNHANDLED - we may consider them even as 2 scalar values > > + > > NOENABLE, CHAINED - additional bits. > > They are not encouraged to be used with shared interrupts > (explained in docs + debug messages when XENO_OPT_DEBUG is on). > > Any ISR on the shared irq line should "understand" that it's > just one among the equals. That said, it should not do anything > that may affect other ISRs and not require any special treatment > (like CHAINED or NOENABLE). > If it wants it indeed, then don't declare itself as SHARED. > > We may come back to the topic about possible return values of ISRs > a bit later maybe having got more feedback (hm.. hopefully) > on shared irq support. > > > >>> > >>> But the later one is not only about enabling the line, but > >>> on some archs - about .end-ing it too (sending EOI). > >>> > >>> And to support HANDLED_NOENABLE properly, those 2 have to be > >>> decoupled, i.e. > >>> EOI should always be sent from xnintr_shirq_handler(). > >> But the one returning HANDLED_NOENABLE is likely to leave the interrupt > >> asserted, hence we can't EOI at this point (unless NO_ENABLE means > >> DISABLE). > > > >I guess this is what Dmitry meant: explicitly call disable() if one or > >more ISRs returned NOENABLE - at least on archs where end != enable. > >Will this work? We could then keep on using the existing IRQ-enable API > >from bottom-half IRQ tasks. > > Almost. > > Let's consider the following only as anorther way of doing some things; > I don't propose to implement it, it's just to illustrate my thoughts. > So one may simply ski-skip-skip it :o) > > Let's keep in mind that what is behind .end-ing the IRQ line depends on > archs. > Sometimes end == enable (EOI was sent on .ack step), while in other cases > end == send_EOI [+ re-enabling]. > > But anyway, all ISRs are running with a given IRQ line disabled. > > Supported values : HANDLED, UNHANDLED, PROPAGATE. > > nucleus :: xnintr_irq_handler() > { > ret = 0; > > ... > > for each isr in isr_list[ IRQ ] > { > temp = isr->handler(); > > if (temp > ret) > ret = temp; > } > > if (ret == PROPAGATE) > { > // quite dengerous with shared interrupts, be sure you understand > // what you are doing! > > xnarch_chain_irq(irq); // will be .end-ed in Linux domain > } > else > { > // HANDLED or UNHANDLED > > xnarch_end_irq(); > } > > ... > > } > > ENABLE or NOENABLE is missing? Nop. > > let's say we have 2 rt-ISRs : > > isr1 : HANDLED > isr2 : HANDLED + WISH > > WISH == I want the IRQ line remain disabled until later > (e.g. bottom half in rt_task will enable it) > > How may isr2 express this WISH? Simply, xnarch_irq_disable/enable() should > support an internal counter that allows them to be called in a nested way. > > So e.g. if there are 2 consecutive calls to disable_irq(), then > 2 calls to enable_irq() are needed to really enable the IRQ line. > > This way, the WISH is only about directly calling xnarch_irq_disable() > in isr2 > and there is no need in ENABLE or NOENABLE flags. > > This way, PROPAGATE really means NOEND - the IRQ will be .end-ed in > Linux domain; > while WISH==NOENABLE - has nothing to do with sending EOI, but only with > enabling/disabling the given IRQ line. > > Yes, if one isr (or a few) defers the IRQ line enabling until later, it > will affect > all others ISR => all interrupts are temporary not accepted on this line. > This scenario is possible under Linux, but should be used with even more > care in real-time system. At least, this way HANDLED_NOENABLE case works > and doesn't lead to lost interrupts on some archs. > > Moreover, it avoids the need for ENABLE flag even in non-shared > interrupt case. > > Ok, general bottom line regarding IRQ support (and the rest): 1- we want the rules applicable to the common case to be simple, well-defined and straightforward. This applies to the ISR return value as exposed. 2- some requirements might fall outside of the common case; to support them, we need to refrain from imposing a strong policy, but only provide sound mechanisms to implement that. Specifically, let's keep the basic ability to control the Adeos pipelining from Xenomai intact. 3- however, let's be true to the good old UNIX way-of-life: there is no point in penalizing 99% of the common user base to please only 1% of them who happen to have very specific needs. E.g., don't slow down the fast path everyone takes, don't impose hard rules most of the users won't benefit from. [I'm not referring to the enable/disable nesting count here: this _is_ correct, and is currently missing -- this should be done at Xeno's arch-level, not from the HAL which should be kept the way it is to have a direct, unmoderated access to the PIC handling]. IOW, the patch, the way you discussed it recently, is ok for me too. -- Philippe.