From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ Date: Tue, 01 Apr 2014 14:13:50 +0100 Message-ID: <533ABB8E.4070004@linaro.org> References: <1390581822-32624-1-git-send-email-julien.grall@linaro.org> <1390581822-32624-8-git-send-email-julien.grall@linaro.org> <1392810905.29739.19.camel@kazak.uk.xensource.com> <53398D84.3020901@linaro.org> <1396281181.8667.29.camel@kazak.uk.xensource.com> <5339919D.7090801@linaro.org> <1396355354.8667.147.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WUyVl-00069k-O6 for xen-devel@lists.xenproject.org; Tue, 01 Apr 2014 13:13:53 +0000 Received: by mail-wi0-f171.google.com with SMTP id q5so5124484wiv.10 for ; Tue, 01 Apr 2014 06:13:52 -0700 (PDT) In-Reply-To: <1396355354.8667.147.camel@kazak.uk.xensource.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: Ian Campbell Cc: xen-devel@lists.xenproject.org, tim@xen.org, Keir Fraser , stefano.stabellini@citrix.com, patches@linaro.org List-Id: xen-devel@lists.xenproject.org On 04/01/2014 01:29 PM, Ian Campbell wrote: > On Mon, 2014-03-31 at 17:02 +0100, Julien Grall wrote: >> On 03/31/2014 04:53 PM, Ian Campbell wrote: >>> On Mon, 2014-03-31 at 16:45 +0100, Julien Grall wrote: >>>> On 02/19/2014 11:55 AM, Ian Campbell wrote: >>>>> On Fri, 2014-01-24 at 16:43 +0000, Julien Grall wrote: >>>>>> On ARM, it may happen (eg ARM SMMU) to setup multiple handler for the same >>>>>> interrupt. >>>>> >>>>> Mention here that you are therefore creating a linked list of actions >>>>> for each interrupt. >>>>> >>>>> If you use xen/list.h for this then you get a load of helpers and >>>>> iterators which would save you open coding them. >>>> >>>> I've tried to use xen/list.h. The amount of code it's basically the same >>>> and we I have to write open code to get the first element of the list >>> >>> Why? Can you post your WIP patch please for comparison. >> >> Because: >> - there is no helper to get the first element (__setup_irq) > > Wrong function? I don't see any problem in __setup_irq. __setup_irq has to check if every action has a dev_id. After thinking, I don't need to get the first action as now we have IRQ_SHARED flags set. > >> - I need to use 2 variables to search for an element in a list as there is >> no way to know after the end of the loop if we found or not an element. > > You've written that a bit weirdly IMHO. > > list_for_each(...) > if (not the one we want) > continue > free the one we wanted > break; > > don't worry about warning on a non-existent IRQ, or set a simple > boolean. We have to worry about non-existent action otherwise Xen may segfault... > It really doesn't look that bad to me. Ok. I will continue on this way then. > [...] >> - struct irqaction *next; >> +#ifdef CONFIG_ARM >> + struct list_head next; >> +#endif > [...] >> +#ifdef CONFIG_ARM >> + struct list_head action; /* IRQ action list */ >> +#else >> struct irqaction *action; /* IRQ action list */ >> +#endif > > Now these might be a legitimate problem with this approach. At the very > least this should be CONFIG_IRQ_HAS_IRQ_ACTION_LIST or some more > suitable name. Ok. I will use it. Regards, -- Julien Grall