[Xenomai-core] Re: Move rtdm_irq_enable close to rtdm_irq_request

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1.1: Type: text/plain, Size: 2737 bytes --]

Dmitry Adamushko wrote:
> On 05/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>>
>>
>> @Dmitry: What happens under CONFIG_XENO_OPT_SHIRQ_LEVEL &&
>> !CONFIG_XENO_OPT_SHIRQ_EDGE when someone comes along with
>> XN_ISR_SHARED|XN_ISR_EDGE? Looks like the level-triggered shared handler
>> gets installed. Should we catch this? At Kconfig or at nucleus level?
> 
> 
> Currently XN_ISR_SHARED and XN_ISR_EDGE are unconditionally available for
> any configuration. So nothing stops a user from calling rtdm_irq_request(
> ... , RTDM_IRQTYPE_SHARED | RTDM_IRQTYPE_EDGE) even when IRQ shared
> infrastructure is off.
> (the second call gives -EBUSY and a "bug" report is sent. I mean, if the
> user has overlooked "shared irq" section on the configure step and consider
> it a default behavior).
> 
> Probably the clean solution would be to export XN_ISR_SHARED and
> XN_ISR_EDGE
> only when their corresponding CONFIG_* parameters are defined.
> 
> Then e.g. rtdm skin should do :
> 
> #ifdef XN_ISR_SHARED
> #define RTDM_IRQTYPE_SHARED XN_ISR_SHARED
> #endif
> 
> and let a compiler complain on undefined symbol when a user tries to use
> SHARED/EDGE without proper CONFIG options. Although, I'm not sure it would
> be clear for all users.
> 
> Another approach,
> 
> in xnintr.h
> 
> #ifdef CONFIG_XENO_OPT_SHIRQ_LAYER
> #define XN_ISR_SHARED   1
> #else
> #define XN_ISR_SHARED    XN_ISR_WARNING
> #endif
> 
> and then catch all wrong use cases in one place :
> 
> int xnintr_attach(xnintr_t *intr, void *cookie)
> {
>        intr->hits = 0;
>        intr->cookie = cookie;
> 
> +      if (intr->flags & XN_ISR_WARNING) {
> +                   xnlogerr("blablabla...\n");
> +                   return -EINVAL;
> +      }
> 
> #if defined(CONFIG_XENO_OPT_SHIRQ_LEVEL) ||
> defined(CONFIG_XENO_OPT_SHIRQ_EDGE)
>        return xnintr_shirq_attach(intr, cookie);
> #else /* !CONFIG_XENO_OPT_SHIRQ_LEVEL && !CONFIG_XENO_OPT_SHIRQ_EDGE */
>        return xnarch_hook_irq(intr->irq, &xnintr_irq_handler, intr->iack,
>                               intr);
> #endif /* CONFIG_XENO_OPT_SHIRQ_LEVEL || CONFIG_XENO_OPT_SHIRQ_EDGE */
> }
> 
> 
> ?
> 

Sharing itself is no problem: if your request an IRQ with XN_ISR_SHARED
set but the nucleus is not able to actually assign it to more than one
driver, the second request will simply fail. I see no need to deny the
first request or even break the driver build.

Problematic is only the handling of edge-triggered shared IRQs with the
level-triggered handler (can cause lost IRQs). Probably a runtime check
is best as we cannot control the configuration of, e.g., external RTDM
drivers. What about the attached patch?

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: catch-unsupported-XN_ISR_EDGE.patch --]
[-- Type: text/x-patch; name="catch-unsupported-XN_ISR_EDGE.patch", Size: 661 bytes --]

Index: ksrc/nucleus/intr.c
===================================================================
--- ksrc/nucleus/intr.c	(Revision 1560)
+++ ksrc/nucleus/intr.c	(Arbeitskopie)
@@ -638,10 +638,14 @@ static int xnintr_shirq_attach(xnintr_t 
 			handler = &xnintr_shirq_handler;
 #endif /* CONFIG_XENO_OPT_SHIRQ_LEVEL */
 
+			if (intr->flags & XN_ISR_EDGE) {
 #if defined(CONFIG_XENO_OPT_SHIRQ_EDGE)
-			if (intr->flags & XN_ISR_EDGE)
 				handler = &xnintr_edge_shirq_handler;
+#else /* !CONFIG_XENO_OPT_SHIRQ_EDGE */
+				err = -ENOSYS;
+				goto unlock_and_exit;
 #endif /* CONFIG_XENO_OPT_SHIRQ_EDGE */
+			}
 		}
 		shirq->unhandled = 0;
 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]