Re: [PATCH v4 05/19] irqchip: add nps Internal and external irqchips

From: Vineet Gupta
Date: Mon Jan 25 2016 - 08:09:06 EST

Hi Marc,

On Friday 18 December 2015 10:01 PM, Marc Zyngier wrote:
> On 18/12/15 14:29, Noam Camus wrote:
>>> From: Marc Zyngier [mailto:marc.zyngier@xxxxxxx]
>>> Sent: Friday, December 18, 2015 1:21 PM
>>>> I need this for my per CPU irqs such timer and IPI which do not come
>>>> from some external device but from CPUs. For these IRQs I am calling
>>>> to irq_create_mapping() from my platform at arch/arc and at that point
>>>> I got no irqdomain and using irq_find_host() is not good since I got
>>>> no device_node (at most I can have DT root).
>>> That's a problem. You should never do that for your timer (doing a request_irq will do the right thing, and that's what your timer driver already does).
>> Please be more specific, from all that I wrote what is the problem?
> Calling irq_create_mapping out of the blue like you do it here:
> +static void eznps_init_per_cpu(int cpu)
> +{
> + /* Create mapping for all per cpu IRQs */
> + if (cpu == 0) {
> + irq_create_mapping(NULL, TIMER0_IRQ);
> + irq_create_mapping(NULL, IPI_IRQ);
> + }
> is simply not acceptable.

I understand but... see below

>>> As for initializing your IPIs, they are usually outside of the IRQ
>>> space, so you should handle them separately (and get your irqchip
>>> to initialize them).
>> I am handling all my IRQs within same irqchip, which is the only one
>> I have. So I am not sure what you expect here. Please be more
>> elaborate.
> Do not create a mapping for IPIs. Full stop. Handle them independently
> from your normal IRQs.

why not ? IPI is a hardware construct afterall.

Anyhow I looked in arch/arm and do_IPI/handle_IPI are the handlers. do_IPI is
called from asm, irq-gic.c calls handle_IPI. I can't seem to find an explicit
request_irq / request_percpu_irq for IPI irq ?


>> Note that I am working with ARC (seem alike) here and we do not
>> define CONFIG_HANDLE_DOMAIN_IRQ and do not implement
>> set_handle_irq().
>> So for ARC this reverse mapping is something we can leave without
>> (maybe because we are kind of a legacy domain).
> Yeah, I just located the crap: arch_do_IRQ() happily takes a hwirq (the
> vector number), and uses that as a Linux IRQ. This looks a lot like ARM
> pre-DT, about 10 years ago.
> Well, time to meet the 21st century. If you intend to use DT, please fix
> your arch port. Otherwise, just hardcode everything in your platform and
> don't pretend to support device tree.

Following works for me, hopefully it is closer to 21st century code :-)