[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.5 7/8] xen/irq: Handle multiple action per IRQ
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 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |