[Adeos-main] [PATCH] let ipipe_catch_event return old handler

[Adeos-main] [PATCH] let ipipe_catch_event return old handler

[Jan Kiszka]

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

Hi,

this patch extends ipipe_catch_event() so that the previously registered
event handler is returned. The usage scenario are event listeners that
hook onto an existing listener for a specific domain, maybe filtering
events or simply overruling the original handler.

Note that this patch effectively removes the error detection from
ipipe_catch_event(), i.e. requests for event >= IPIPE_NR_EVENTS will be
silently ignored now. Regarding Xenomai as Ipipe user, this is a
non-issue as return codes are not checked there and only valid events
are passed.

Jan

[-- Attachment #1.2: catch_event.patch --]
[-- Type: text/plain, Size: 2451 bytes --]

Index: linux-2.6.16.1/include/linux/ipipe.h
===================================================================
--- linux-2.6.16.1.orig/include/linux/ipipe.h
+++ linux-2.6.16.1/include/linux/ipipe.h
@@ -125,9 +125,13 @@
 typedef void (*ipipe_irq_handler_t)(unsigned irq,
 				    void *cookie);
 
+#define IPIPE_SAME_HANDLER	((ipipe_irq_handler_t)(-1))
+
 typedef int (*ipipe_irq_ackfn_t)(unsigned irq);
 
-#define IPIPE_SAME_HANDLER	((ipipe_irq_handler_t)(-1))
+typedef int (*ipipe_event_handler_t)(unsigned event,
+				     struct ipipe_domain *from,
+				     void *data);
 
 struct ipipe_domain {
 
@@ -150,9 +154,7 @@
 		void *cookie;
 	} ____cacheline_aligned irqs[IPIPE_NR_IRQS];
 
-	int (*evhand[IPIPE_NR_EVENTS])(unsigned event,
-				       struct ipipe_domain *from,
-				       void *data); /* Event handlers. */
+	ipipe_event_handler_t evhand[IPIPE_NR_EVENTS]; /* Event handlers. */
 	unsigned long long evself;	/* Self-monitored event bits. */
 
 #ifdef CONFIG_IPIPE_STATS
@@ -711,11 +713,9 @@
 	clear_bit(IPIPE_SPRINTK_FLAG, &ipd->flags);
 }
 
-int ipipe_catch_event(struct ipipe_domain *ipd,
-		      unsigned event,
-		      int (*handler)(unsigned event,
-				     struct ipipe_domain *ipd,
-				     void *data));
+ipipe_event_handler_t ipipe_catch_event(struct ipipe_domain *ipd,
+					unsigned event,
+					ipipe_event_handler_t handler);
 
 cpumask_t ipipe_set_irq_affinity(unsigned irq,
 				 cpumask_t cpumask);
Index: linux-2.6.16.1/kernel/ipipe/generic.c
===================================================================
--- linux-2.6.16.1.orig/kernel/ipipe/generic.c
+++ linux-2.6.16.1/kernel/ipipe/generic.c
@@ -266,9 +266,9 @@
  * ipipe_catch_event() -- Interpose or remove an event handler for a
  * given domain.
  */
-int ipipe_catch_event(struct ipipe_domain *ipd,
-		      unsigned event,
-		      int (*handler)(unsigned event, struct ipipe_domain *ipd, void *data))
+ipipe_event_handler_t ipipe_catch_event(struct ipipe_domain *ipd,
+					unsigned event,
+					ipipe_event_handler_t handler)
 {
 	int self = 0;
 
@@ -278,7 +278,7 @@
 	}
 
 	if (event >= IPIPE_NR_EVENTS)
-		return -EINVAL;
+		return NULL;
 
 	if (!xchg(&ipd->evhand[event],handler))	{
 		if (handler) {
@@ -301,7 +301,7 @@
 			ipd->evself |= (1LL << event);
 	}
 
-	return 0;
+	return handler;
 }
 
 cpumask_t ipipe_set_irq_affinity (unsigned irq, cpumask_t cpumask)

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

[Xenomai-core] Re: [Adeos-main] [PATCH] let ipipe_catch_event return old handler

[Philippe Gerum]

Jan Kiszka wrote:

<snip>

Don't you mean this instead?

> -int ipipe_catch_event(struct ipipe_domain *ipd,
> -		      unsigned event,
> -		      int (*handler)(unsigned event, struct ipipe_domain *ipd, void *data))
> +ipipe_event_handler_t ipipe_catch_event(struct ipipe_domain *ipd,
> +					unsigned event,
> +					ipipe_event_handler_t handler)
>  {
+       ipipe_event_handler_t old_handler;
>  	int self = 0;
>  
> @@ -278,7 +278,7 @@
>  	}
>  
>  	if (event >= IPIPE_NR_EVENTS)
> -		return -EINVAL;
> +		return NULL;
>  
-  	if (!xchg(&ipd->evhand[event],handler))	{
+  	if (!(old_handler = xchg(&ipd->evhand[event],handler)))	{
>  		if (handler) {
> @@ -301,7 +301,7 @@
>  			ipd->evself |= (1LL << event);
>  	}
>  
> -	return 0;
> +	return handler;
-	return handler;
+	return old_handler;
>  }
>  
>  cpumask_t ipipe_set_irq_affinity (unsigned irq, cpumask_t cpumask)
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Adeos-main mailing list
> Adeos-main@domain.hid
> https://mail.gna.org/listinfo/adeos-main


-- 

Philippe.

[Xenomai-core] Re: [Adeos-main] [PATCH] let ipipe_catch_event return old handler

[Jan Kiszka]

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

Philippe Gerum wrote:
> Jan Kiszka wrote:
> 
> <snip>
> 
> Don't you mean this instead?
> 
>> -int ipipe_catch_event(struct ipipe_domain *ipd,
>> -              unsigned event,
>> -              int (*handler)(unsigned event, struct ipipe_domain
>> *ipd, void *data))
>> +ipipe_event_handler_t ipipe_catch_event(struct ipipe_domain *ipd,
>> +                    unsigned event,
>> +                    ipipe_event_handler_t handler)
>>  {
> +       ipipe_event_handler_t old_handler;
>>      int self = 0;
>>  
>> @@ -278,7 +278,7 @@
>>      }
>>  
>>      if (event >= IPIPE_NR_EVENTS)
>> -        return -EINVAL;
>> +        return NULL;
>>  
> -      if (!xchg(&ipd->evhand[event],handler))    {
> +      if (!(old_handler = xchg(&ipd->evhand[event],handler)))    {
>>          if (handler) {
>> @@ -301,7 +301,7 @@
>>              ipd->evself |= (1LL << event);
>>      }
>>  
>> -    return 0;
>> +    return handler;
> -    return handler;
> +    return old_handler;
>>  }
>>  
>>  cpumask_t ipipe_set_irq_affinity (unsigned irq, cpumask_t cpumask)
>>
>>

Ouch, obviously. A quick hack which was only tested for not breaking
existing software.

Jan


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

[Xenomai-core] Re: [Adeos-main] [PATCH] let ipipe_catch_event return old handler

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
> 
>>Jan Kiszka wrote:
>>
>><snip>
>>
>>Don't you mean this instead?
>>
>>
>>>-int ipipe_catch_event(struct ipipe_domain *ipd,
>>>-              unsigned event,
>>>-              int (*handler)(unsigned event, struct ipipe_domain
>>>*ipd, void *data))
>>>+ipipe_event_handler_t ipipe_catch_event(struct ipipe_domain *ipd,
>>>+                    unsigned event,
>>>+                    ipipe_event_handler_t handler)
>>> {
>>
>>+       ipipe_event_handler_t old_handler;
>>
>>>     int self = 0;
>>> 
>>>@@ -278,7 +278,7 @@
>>>     }
>>> 
>>>     if (event >= IPIPE_NR_EVENTS)
>>>-        return -EINVAL;
>>>+        return NULL;
>>> 
>>
>>-      if (!xchg(&ipd->evhand[event],handler))    {
>>+      if (!(old_handler = xchg(&ipd->evhand[event],handler)))    {
>>
>>>         if (handler) {
>>>@@ -301,7 +301,7 @@
>>>             ipd->evself |= (1LL << event);
>>>     }
>>> 
>>>-    return 0;
>>>+    return handler;
>>
>>-    return handler;
>>+    return old_handler;
>>
>>> }
>>> 
>>> cpumask_t ipipe_set_irq_affinity (unsigned irq, cpumask_t cpumask)
>>>
>>>
> 
> 
> Ouch, obviously. A quick hack which was only tested for not breaking
> existing software.
> 

Ok, will merge. Thanks.

-- 

Philippe.