From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2 16/16] xen/arm: IRQ: Handle multiple action per IRQ Date: Fri, 04 Apr 2014 09:52:41 +0100 Message-ID: <533E72D9.9030605@linaro.org> References: <1396557727-19102-1-git-send-email-julien.grall@linaro.org> <1396557727-19102-17-git-send-email-julien.grall@linaro.org> <533E82740200007800005828@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WVzrh-0005C0-B3 for xen-devel@lists.xenproject.org; Fri, 04 Apr 2014 08:52:45 +0000 Received: by mail-we0-f179.google.com with SMTP id x48so3116360wes.10 for ; Fri, 04 Apr 2014 01:52:43 -0700 (PDT) In-Reply-To: <533E82740200007800005828@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel@lists.xenproject.org, Keir Fraser , stefano.stabellini@citrix.com, ian.campbell@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 04/04/2014 08:59 AM, Jan Beulich wrote: >>>> On 03.04.14 at 22:42, wrote: >> --- a/xen/include/asm-arm/irq.h >> +++ b/xen/include/asm-arm/irq.h >> @@ -45,6 +45,13 @@ int route_dt_irq_to_guest(struct domain *d, const struct dt_irq *irq, >> unsigned int platform_get_irq(const struct dt_device_node *device, >> int index); >> >> +int request_irq_flags(unsigned int irq, >> + void (*handler)(int, void *, struct cpu_user_regs *), >> + unsigned int irqflags, >> + const char * devname, void *dev_id); >> +int setup_irq_flags(unsigned int irq, struct irqaction *new, >> + unsigned int irqflags); >> + > > I think it is a bad choice to have separate ..._flags() versions of these, > the normal functions should take flags at least when > CONFIG_IRQ_HAS_MULTIPLE_ACTION. Perhaps, with the expectation > of wanting to add further flags in the future, it would be reasonable to > have the functions always have the flags parameters, and assert the > value passed is zero on x86. I chose this solution because I didn't know if extending the prototype {request,setup}_irq would be acceptable. With this patch series I've tried to remove ARM specific functions (e.g {request,setup}_dt_irq) for IRQ management. So I would prefer to extend the both function prototypes unconditionally to avoid ifdery in serial drivers. > >> @@ -27,6 +30,7 @@ struct irqaction { >> #define IRQ_MOVE_PENDING (1u<<5) /* IRQ is migrating to another CPUs */ >> #define IRQ_PER_CPU (1u<<6) /* IRQ is per CPU */ >> #define IRQ_GUEST_EOI_PENDING (1u<<7) /* IRQ was disabled, pending a guest EOI */ >> +#define IRQ_SHARED (1<<8) /* IRQ is shared */ > > This is now given two meanings (input to above functions and status), > which in the long run can become problematic. Please follow Linux by > using two distinct flag sets. Ok, I will rename it Ito IRQF_SHARED and introduce a new field "flags" in irq_desc. Regards, -- Julien Grall