[Xenomai-core] Move rtdm_irq_enable close to rtdm_irq_request

[Xenomai-core] Move rtdm_irq_enable close to rtdm_irq_request

[Jan Kiszka]

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

Hi Wolfgang,

in the process of preparing to merge rtdm_irq_enable into
rtdm_irq_request I would like to check if the attached patch is ok, thus
we could finally drop rtdm_irq_enable once the API is refactored. Please
check carefully when the first IRQs may happen and what the handler
expects to be initialised! SJA1000 /should/ be ok as it works with
shared IRQs, but MSCAN does not (why, BTW?) and /may/ stumble.

Jan

[-- Attachment #1.2: reorder-rtdm_irq_enable.patch --]
[-- Type: text/plain, Size: 1375 bytes --]

Index: ksrc/drivers/can/mscan/rtcan_mscan.c
===================================================================
--- ksrc/drivers/can/mscan/rtcan_mscan.c	(revision 1559)
+++ ksrc/drivers/can/mscan/rtcan_mscan.c	(working copy)
@@ -801,7 +801,8 @@ int __init rtcan_mscan_init_one(int idx)
 	printk("ERROR! rtdm_irq_request for IRQ %d failed\n", irq);
 	goto out_dev_free;
     }
-    
+    rtdm_irq_enable(&dev->irq_handle);
+
     mscan_chip_config(regs);
     
 
@@ -814,8 +815,6 @@ int __init rtcan_mscan_init_one(int idx)
 
     rtcan_mscan_create_proc(dev);
 
-    rtdm_irq_enable(&dev->irq_handle);
-    
     /* Remember initialized devices */
     rtcan_mscan_devs[idx] = dev;
 
Index: ksrc/drivers/can/sja1000/rtcan_sja1000.c
===================================================================
--- ksrc/drivers/can/sja1000/rtcan_sja1000.c	(revision 1559)
+++ ksrc/drivers/can/sja1000/rtcan_sja1000.c	(working copy)
@@ -747,6 +747,7 @@ int rtcan_sja1000_register(struct rtcan_
 	printk("ERROR! IRQ %d busy or invalid (code=%d)!\n", chip->irq_num, ret);
 	return ret;
     }
+    rtdm_irq_enable(&dev->irq_handle);
 
     sja1000_chip_config(dev);
 
@@ -759,8 +760,6 @@ int rtcan_sja1000_register(struct rtcan_
 
     rtcan_sja_create_proc(dev);
 
-    rtdm_irq_enable(&dev->irq_handle);
-    
     return 0;
 
  out_irq_free:

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

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

[Wolfgang Grandegger]

Jan Kiszka wrote:
> Hi Wolfgang,
> 
> in the process of preparing to merge rtdm_irq_enable into
> rtdm_irq_request I would like to check if the attached patch is ok, thus
> we could finally drop rtdm_irq_enable once the API is refactored. Please
> check carefully when the first IRQs may happen and what the handler
> expects to be initialised! SJA1000 /should/ be ok as it works with

> shared IRQs, but MSCAN does not (why, BTW?) and /may/ stumble.

OK. Why should I use shared interrupts if there is no need? Most 
embedded PowerPC systems have a dedicated interrupt source.

Wolfgang.

> Jan
> 
> 
> ------------------------------------------------------------------------
> 
> Index: ksrc/drivers/can/mscan/rtcan_mscan.c
> ===================================================================
> --- ksrc/drivers/can/mscan/rtcan_mscan.c	(revision 1559)
> +++ ksrc/drivers/can/mscan/rtcan_mscan.c	(working copy)
> @@ -801,7 +801,8 @@ int __init rtcan_mscan_init_one(int idx)
>  	printk("ERROR! rtdm_irq_request for IRQ %d failed\n", irq);
>  	goto out_dev_free;
>      }
> -    
> +    rtdm_irq_enable(&dev->irq_handle);
> +
>      mscan_chip_config(regs);
>      
>  
> @@ -814,8 +815,6 @@ int __init rtcan_mscan_init_one(int idx)
>  
>      rtcan_mscan_create_proc(dev);
>  
> -    rtdm_irq_enable(&dev->irq_handle);
> -    
>      /* Remember initialized devices */
>      rtcan_mscan_devs[idx] = dev;
>  
> Index: ksrc/drivers/can/sja1000/rtcan_sja1000.c
> ===================================================================
> --- ksrc/drivers/can/sja1000/rtcan_sja1000.c	(revision 1559)
> +++ ksrc/drivers/can/sja1000/rtcan_sja1000.c	(working copy)
> @@ -747,6 +747,7 @@ int rtcan_sja1000_register(struct rtcan_
>  	printk("ERROR! IRQ %d busy or invalid (code=%d)!\n", chip->irq_num, ret);
>  	return ret;
>      }
> +    rtdm_irq_enable(&dev->irq_handle);
>  
>      sja1000_chip_config(dev);
>  
> @@ -759,8 +760,6 @@ int rtcan_sja1000_register(struct rtcan_
>  
>      rtcan_sja_create_proc(dev);
>  
> -    rtdm_irq_enable(&dev->irq_handle);
> -    
>      return 0;
>  
>   out_irq_free:

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1354 bytes --]

Wolfgang Grandegger wrote:
> Jan Kiszka wrote:
>> Hi Wolfgang,
>>
>> in the process of preparing to merge rtdm_irq_enable into
>> rtdm_irq_request I would like to check if the attached patch is ok, thus
>> we could finally drop rtdm_irq_enable once the API is refactored. Please
>> check carefully when the first IRQs may happen and what the handler
>> expects to be initialised! SJA1000 /should/ be ok as it works with
> 
>> shared IRQs, but MSCAN does not (why, BTW?) and /may/ stumble.
> 
> OK. Why should I use shared interrupts if there is no need? Most
> embedded PowerPC systems have a dedicated interrupt source.

Of course, /me forgot once again that not all the world is designed like
crappy x86. :)

At this chance I looked over the irq_flag mechanism of the CAN stack and
found a minor cleanup: this #ifdef [1] is not required. If there is no
sharing support, the subscriber will simply be redirected to the
non-shared handler.

@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?

Jan


[1]http://www.rts.uni-hannover.de/xenomai/lxr/source/ksrc/drivers/can/sja1000/rtcan_peak_pci.c?v=SVN-trunk#L233


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

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]

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 */
}


?

-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 3135 bytes --]

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

[Jan Kiszka]

[-- 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 --]

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

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?


It's ok with me.

I just thought maybe it's better to break a build and alert a user earlier
(although, it's kind of inderect alert indeed) when the host environment
(Xeno) doesn't support a requested mode (e.g. SHARED_EDGE).

If such a driver (that requires EDGE_SHARED) is a part of the mainline, then
we may use Kconfig features either (1) to make it "selectable" only when
XENO_OPT_SHIRQ_EDGE is on or (2) select XENO_OPT_SHIRQ_EDGE automatically
when the driver has been choosen.




Jan
>
>
>


-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 1771 bytes --]

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1682 bytes --]

Dmitry Adamushko wrote:
> 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?
> 
> 
> It's ok with me.
> 
> I just thought maybe it's better to break a build and alert a user earlier
> (although, it's kind of inderect alert indeed) when the host environment
> (Xeno) doesn't support a requested mode (e.g. SHARED_EDGE).
> 
> If such a driver (that requires EDGE_SHARED) is a part of the mainline,
> then
> we may use Kconfig features either (1) to make it "selectable" only when
> XENO_OPT_SHIRQ_EDGE is on or (2) select XENO_OPT_SHIRQ_EDGE automatically
> when the driver has been choosen.
> 

Let's do both, the runtime check + some Kconfig magic for mainline drivers.

For the latter we should reorganise the config options slightly.
XENO_OPT_SHIRQ_* may better depend on a new switch XENO_OPT_SHIRQ. Thus,
when the user enables IRQ sharing and some in-tree driver requires
edge-triggering support, XENO_OPT_SHIRQ_EDGE will be selected by the
driver's Kconfig: select XENO_OPT_SHIRQ_EDGE if XENO_OPT_SHIRQ. With the
current layout it would look like this: select XENO_OPT_SHIRQ_EDGE if
XENO_OPT_SHIRQ_LEVEL. That may appear illogical to the user.

Jan


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

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>
> > If such a driver (that requires EDGE_SHARED) is a part of the mainline,
> > then
> > we may use Kconfig features either (1) to make it "selectable" only when
> > XENO_OPT_SHIRQ_EDGE is on or (2) select XENO_OPT_SHIRQ_EDGE
> automatically
> > when the driver has been choosen.
> >
>
> Let's do both, the runtime check + some Kconfig magic for mainline
> drivers.
>
> For the latter we should reorganise the config options slightly.
> XENO_OPT_SHIRQ_* may better depend on a new switch XENO_OPT_SHIRQ. Thus,
> when the user enables IRQ sharing and some in-tree driver requires
> edge-triggering support, XENO_OPT_SHIRQ_EDGE will be selected by the
> driver's Kconfig: select XENO_OPT_SHIRQ_EDGE if XENO_OPT_SHIRQ. With the
> current layout it would look like this: select XENO_OPT_SHIRQ_EDGE if
> XENO_OPT_SHIRQ_LEVEL.


XENO_OPT_SHIRQ_LEVEL doesn't need to be on in order to use
XENO_OPT_SHIRQ_EDGE.

Or you probably mean the following behavior :

(1)
some driver with XN_ISR_EDGE is selected :

if XENO_OPT_IRQ
     select XENO_OPT_SHIRQ_EDGE

else
     "run-time" check in xnintr_attach() will report -EBUSY in case the line
is already busy

What I meant is

(2)
some driver with XN_ISR_EDGE is selected :

o   select XENO_OPT_SHIRQ_EDGE

So that shared irq support (only for edge-triggered interrupts) gets
unconditionally enabled.

in case of (1), XENO_OPT_SHIRQ can't be enabled on its own, i.e. without any
of XENO_OPT_SHIRQ_*.

Well, your proposal is probably better. With XN_ISR_EDGE a driver only
declares that it's ready to work in shared mode but it doesn't mean it can't
work in non-shared one.

If a user has a separate line for it, then the shared-IRQ infrastracture
adds just non-used overhead. Yep, you are right (heh... you are asking who
had doubts? :)




> Jan
>
>

-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 2561 bytes --]

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

[Jan Kiszka]

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

Dmitry Adamushko wrote:
> On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>>
>> > If such a driver (that requires EDGE_SHARED) is a part of the mainline,
>> > then
>> > we may use Kconfig features either (1) to make it "selectable" only
>> when
>> > XENO_OPT_SHIRQ_EDGE is on or (2) select XENO_OPT_SHIRQ_EDGE
>> automatically
>> > when the driver has been choosen.
>> >
>>
>> Let's do both, the runtime check + some Kconfig magic for mainline
>> drivers.
>>
>> For the latter we should reorganise the config options slightly.
>> XENO_OPT_SHIRQ_* may better depend on a new switch XENO_OPT_SHIRQ. Thus,
>> when the user enables IRQ sharing and some in-tree driver requires
>> edge-triggering support, XENO_OPT_SHIRQ_EDGE will be selected by the
>> driver's Kconfig: select XENO_OPT_SHIRQ_EDGE if XENO_OPT_SHIRQ. With the
>> current layout it would look like this: select XENO_OPT_SHIRQ_EDGE if
>> XENO_OPT_SHIRQ_LEVEL.
> 
> 
> XENO_OPT_SHIRQ_LEVEL doesn't need to be on in order to use
> XENO_OPT_SHIRQ_EDGE.
> 
> Or you probably mean the following behavior :
> 
> (1)
> some driver with XN_ISR_EDGE is selected :
> 
> if XENO_OPT_IRQ
>     select XENO_OPT_SHIRQ_EDGE
> 
> else
>     "run-time" check in xnintr_attach() will report -EBUSY in case the line
> is already busy
> 
> What I meant is
> 
> (2)
> some driver with XN_ISR_EDGE is selected :
> 
> o   select XENO_OPT_SHIRQ_EDGE
> 
> So that shared irq support (only for edge-triggered interrupts) gets
> unconditionally enabled.
> 
> in case of (1), XENO_OPT_SHIRQ can't be enabled on its own, i.e. without
> any
> of XENO_OPT_SHIRQ_*.

See attached patch: XENO_OPT_SHIRQ would just be a menu-enabler without
any affect outside kconfig. You could enabled it and leave the rest off
(makes no sense of course) - as long as there are no edge-triggered
users around.

Doesn't apply on 2.4, though, but we would still have the runtime test
in place. As long as catching luser mistakes is that simple, I'm
personally in favour of appropriate tests.

> 
> Well, your proposal is probably better. With XN_ISR_EDGE a driver only
> declares that it's ready to work in shared mode but it doesn't mean it
> can't
> work in non-shared one.
> 
> If a user has a separate line for it, then the shared-IRQ infrastracture
> adds just non-used overhead. Yep, you are right (heh... you are asking who
> had doubts? :)
> 

Jan

[-- Attachment #1.2: shirq-kconfig.patch --]
[-- Type: text/plain, Size: 2709 bytes --]

Index: ksrc/drivers/serial/Kconfig
===================================================================
--- ksrc/drivers/serial/Kconfig	(revision 1561)
+++ ksrc/drivers/serial/Kconfig	(working copy)
@@ -3,6 +3,7 @@ menu "Serial drivers"
 config XENO_DRIVERS_16550A
 	depends on XENO_SKIN_RTDM
 	tristate "16550A UART driver"
+	select XENO_OPT_SHIRQ_EDGE if XENO_OPT_SHIRQ
 	help
 	Real-time UART driver for 16550A compatible controllers. See
 	doc/txt/16550A-driver.txt for more details.
Index: ksrc/drivers/can/sja1000/Kconfig
===================================================================
--- ksrc/drivers/can/sja1000/Kconfig	(revision 1561)
+++ ksrc/drivers/can/sja1000/Kconfig	(working copy)
@@ -1,24 +1,21 @@
 config XENO_DRIVERS_RTCAN_SJA1000
        depends on XENO_DRIVERS_RTCAN
        tristate "Philips SJA1000 CAN controller"
-       default n
 
 config XENO_DRIVERS_RTCAN_SJA1000_ISA
        depends on XENO_DRIVERS_RTCAN_SJA1000
        tristate "Standard ISA devices"
-       default n
+       select XENO_OPT_SHIRQ_EDGE if XENO_OPT_SHIRQ
 
 config XENO_DRIVERS_RTCAN_SJA1000_ISA_MAX_DEV
        depends on XENO_DRIVERS_RTCAN_SJA1000_ISA
        int "Maximum number of ISA devices"
-       default 4       
+       default 4
 
 config XENO_DRIVERS_RTCAN_SJA1000_PEAK_PCI
        depends on XENO_DRIVERS_RTCAN_SJA1000
        tristate "PEAK PCI Card"
-       default n
 
 config XENO_DRIVERS_RTCAN_SJA1000_PEAK_DNG
        depends on XENO_DRIVERS_RTCAN_SJA1000
        tristate "PEAK Parallel Port Dongle"
-       default n
Index: ksrc/nucleus/Kconfig
===================================================================
--- ksrc/nucleus/Kconfig	(revision 1561)
+++ ksrc/nucleus/Kconfig	(working copy)
@@ -355,13 +355,15 @@ config XENO_OPT_TIMER_WHEEL_STEP
 endmenu
 
 
-menu "Shared interrupts"
+menuconfig XENO_OPT_SHIRQ
+	bool "Shared interrupts"
 
 config XENO_OPT_SHIRQ_LEVEL
 	bool "Level-triggered interrupts"
-	default n
+	depends on XENO_OPT_SHIRQ
+	default y
 	help
-	
+
 	Enables support for shared level-triggered interrupts, so that
 	multiple real-time interrupt handlers are allowed to control
 	dedicated hardware devices which are configured to share
@@ -369,7 +371,8 @@ config XENO_OPT_SHIRQ_LEVEL
 
 config XENO_OPT_SHIRQ_EDGE
 	bool "Edge-triggered interrupts"
-	default n
+	depends on XENO_OPT_SHIRQ
+	default y
 	help
 
 	Enables support for shared edge-triggered interrupts, so that
@@ -377,8 +380,6 @@ config XENO_OPT_SHIRQ_EDGE
 	dedicated hardware devices which are configured to share
 	the same interrupt channel.
 
-endmenu
-
 menu "LTT tracepoints filtering"
 
 	depends on LTT

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

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]

On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>
> See attached patch: XENO_OPT_SHIRQ would just be a menu-enabler without
> any affect outside kconfig. You could enabled it and leave the rest off
> (makes no sense of course) - as long as there are no edge-triggered
> users around.




-menu "Shared interrupts"
> +menuconfig XENO_OPT_SHIRQ
> +       bool "Shared interrupts"
>
> config XENO_OPT_SHIRQ_LEVEL
>         bool "Level-triggered interrupts"
> -       default n
> +       depends on XENO_OPT_SHIRQ
> +       default y
>         help
> -
> +
>         Enables support for shared level-triggered interrupts, so that
>         multiple real-time interrupt handlers are allowed to control
>         dedicated hardware devices which are configured to share
> @@ -369,7 +371,8 @@ config XENO_OPT_SHIRQ_LEVEL
>
> config XENO_OPT_SHIRQ_EDGE
>         bool "Edge-triggered interrupts"
> -       default n
> +       depends on XENO_OPT_SHIRQ
> +       default y
>         help


So a user may end up with XENO_OPT_SHIRQ being enabled while both LEVEL and
EDGE are disabled? Maybe it's worth to make LEVEL "y" by default as it's
likely to be a required option?




-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 2261 bytes --]

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1527 bytes --]

Dmitry Adamushko wrote:
> On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>>
>> See attached patch: XENO_OPT_SHIRQ would just be a menu-enabler without
>> any affect outside kconfig. You could enabled it and leave the rest off
>> (makes no sense of course) - as long as there are no edge-triggered
>> users around.
> 
> 
> 
> 
> -menu "Shared interrupts"
>> +menuconfig XENO_OPT_SHIRQ
>> +       bool "Shared interrupts"
>>
>> config XENO_OPT_SHIRQ_LEVEL
>>         bool "Level-triggered interrupts"
>> -       default n
>> +       depends on XENO_OPT_SHIRQ
>> +       default y
>>         help
>> -
>> +
>>         Enables support for shared level-triggered interrupts, so that
>>         multiple real-time interrupt handlers are allowed to control
>>         dedicated hardware devices which are configured to share
>> @@ -369,7 +371,8 @@ config XENO_OPT_SHIRQ_LEVEL
>>
>> config XENO_OPT_SHIRQ_EDGE
>>         bool "Edge-triggered interrupts"
>> -       default n
>> +       depends on XENO_OPT_SHIRQ
>> +       default y
>>         help
> 
> 
> So a user may end up with XENO_OPT_SHIRQ being enabled while both LEVEL and
> EDGE are disabled? Maybe it's worth to make LEVEL "y" by default as it's
> likely to be a required option?
> 

Do you see the "default y" above, no? :)

I thought about making only XENO_OPT_SHIRQ_LEVEL default y, but at least
for poor x86 users on legacy hardware (ISA) sharing takes at least as
often place with edge-triggered sources.

Jan


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

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

[Dmitry Adamushko]

[-- Attachment #1: Type: text/plain, Size: 1877 bytes --]

On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>
> Dmitry Adamushko wrote:




> -menu "Shared interrupts"
> >> +menuconfig XENO_OPT_SHIRQ
> >> +       bool "Shared interrupts"
> >>
> >> config XENO_OPT_SHIRQ_LEVEL
> >>         bool "Level-triggered interrupts"
> >> -       default n
> >> +       depends on XENO_OPT_SHIRQ
> >> +       default y
> >>         help
> >> -
> >> +
> >>         Enables support for shared level-triggered interrupts, so that
> >>         multiple real-time interrupt handlers are allowed to control
> >>         dedicated hardware devices which are configured to share
> >> @@ -369,7 +371,8 @@ config XENO_OPT_SHIRQ_LEVEL
> >>
> >> config XENO_OPT_SHIRQ_EDGE
> >>         bool "Edge-triggered interrupts"
> >> -       default n
> >> +       depends on XENO_OPT_SHIRQ
> >> +       default y
> >>         help
> >
> >
> > So a user may end up with XENO_OPT_SHIRQ being enabled while both LEVEL
> and
> > EDGE are disabled? Maybe it's worth to make LEVEL "y" by default as it's
> > likely to be a required option?
> >
>
> Do you see the "default y" above, no? :)


Arghhh, again... nop, I bet it was not there before! How did you manage to
hack my gmail account? :)


I thought about making only XENO_OPT_SHIRQ_LEVEL default y, but at least
> for poor x86 users on legacy hardware (ISA) sharing takes at least as
> often place with edge-triggered sources.


I thought it's level-triggered indeed. At least, judging by the fact that
linux provides a generic support only for level-triggered case.

e.g. cross-domain IRQ sharing (with you approach) would require only LEVEL
option (I actually wanted to port/rework, taking into account the
improvements we have discussed recently, your patch over some recent e.g.
e1000 driver + Xeno so to have an up-to-date example illustrating the
approach).




Jan
>
>


-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: Type: text/html, Size: 3431 bytes --]

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

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2427 bytes --]

Dmitry Adamushko wrote:
> On 06/09/06, Jan Kiszka <jan.kiszka@domain.hid> wrote:
>>
>> Dmitry Adamushko wrote:
> 
> 
> 
> 
>> -menu "Shared interrupts"
>> >> +menuconfig XENO_OPT_SHIRQ
>> >> +       bool "Shared interrupts"
>> >>
>> >> config XENO_OPT_SHIRQ_LEVEL
>> >>         bool "Level-triggered interrupts"
>> >> -       default n
>> >> +       depends on XENO_OPT_SHIRQ
>> >> +       default y
>> >>         help
>> >> -
>> >> +
>> >>         Enables support for shared level-triggered interrupts, so that
>> >>         multiple real-time interrupt handlers are allowed to control
>> >>         dedicated hardware devices which are configured to share
>> >> @@ -369,7 +371,8 @@ config XENO_OPT_SHIRQ_LEVEL
>> >>
>> >> config XENO_OPT_SHIRQ_EDGE
>> >>         bool "Edge-triggered interrupts"
>> >> -       default n
>> >> +       depends on XENO_OPT_SHIRQ
>> >> +       default y
>> >>         help
>> >
>> >
>> > So a user may end up with XENO_OPT_SHIRQ being enabled while both LEVEL
>> and
>> > EDGE are disabled? Maybe it's worth to make LEVEL "y" by default as
>> it's
>> > likely to be a required option?
>> >
>>
>> Do you see the "default y" above, no? :)
> 
> 
> Arghhh, again... nop, I bet it was not there before! How did you manage to
> hack my gmail account? :)

"Don't comment", my lawyer always says.

> 
> I thought about making only XENO_OPT_SHIRQ_LEVEL default y, but at least
>> for poor x86 users on legacy hardware (ISA) sharing takes at least as
>> often place with edge-triggered sources.
> 
> 
> I thought it's level-triggered indeed. At least, judging by the fact that
> linux provides a generic support only for level-triggered case.
> 
> e.g. cross-domain IRQ sharing (with you approach) would require only LEVEL
> option (I actually wanted to port/rework, taking into account the
> improvements we have discussed recently, your patch over some recent e.g.
> e1000 driver + Xeno so to have an up-to-date example illustrating the
> approach).
> 

Sounds good. Do you plan to use RTDM for the RT-stub? Then we would only
have to add the propagation flag to the return codes, right?

Anyway. So you have e1000 hardware to test it? [Mmh, then you could also
test our rt_e1000 in RTnet...] Another option would be the UHCI driver,
might be an even more common IRQ hog. But pick whatever is easier to
implement and test.

Jan


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