Re: Using irq-crossbar.c

[Marc Zyngier]

[been away for a while, catching up...]

On 16/06/16 13:39, Sebastian Frias wrote:
> Hi Marc,
> 
> On 06/14/2016 06:39 PM, Sebastian Frias wrote:
>> On 06/14/2016 06:37 PM, Sebastian Frias wrote:
>>>>>> Also, without seeing the code,
>>>>>> it is pretty difficult to make any meaningful comment...
>>>>>
>>>>> Base code is either 4.7rc1 or 4.4.
>>>>> The irq-crossbar code is not much different from TI, but you can find it attached.
>>>>
>>>> Please post it separately (and inline), the email client I have here
>>>> makes it hard to review attached patches.
>>>
>>> Ok, I'll post it in a separate email and inline.
>>>
>>
>> Here it goes:
>>
>>
> 
>  <snipped code>
> 
>> IRQCHIP_DECLARE(tangox_intc, "sigma,smp-irq-mux", tangox_of_irq_mux_init);
>>
>>
> 
> I have tested the code, and aside from the missing #interrupt-cells
> in the DT that you pointed out, it seems it is working (devices using
> IRQ appear functional), here's some log:
> 
> tangox_irq_mux_domain_translate(): domain 0xcf805000
> tangox_irq_mux_domain_translate(): hwirq 1 (0x1) type 4 (0x4)
> tangox_irq_mux_domain_alloc(): domain 0xcf805000, virq 18 (0x12) nr_irqs 1
> tangox_allocate_gic_irq(): domain 0xcf805000, virq 18 (0x12) hwirq 1 (0x1)
> tangox_setup_irq_route(): route irq  1 (@ 0xf006f804) => irq 23
> tangox_irq_mux_domain_translate(): domain 0xcf805000
> tangox_irq_mux_domain_translate(): hwirq 38 (0x26) type 4 (0x4)
> tangox_irq_mux_domain_alloc(): domain 0xcf805000, virq 19 (0x13) nr_irqs 1
> tangox_allocate_gic_irq(): domain 0xcf805000, virq 19 (0x13) hwirq 38 (0x26)
> tangox_setup_irq_route(): route irq 38 (@ 0xf006f898) => irq 22
> tangox_irq_mux_domain_translate(): domain 0xcf805000
> tangox_irq_mux_domain_translate(): hwirq 67 (0x43) type 4 (0x4)
> tangox_irq_mux_domain_alloc(): domain 0xcf805000, virq 20 (0x14) nr_irqs 1
> tangox_allocate_gic_irq(): domain 0xcf805000, virq 20 (0x14) hwirq 67 (0x43)
> tangox_setup_irq_route(): route irq 67 (@ 0xf006f90c) => irq 21
> 
> Since irq-tango_v2.c is similar to irq-crossbar.c from TI (since it
> is based on it), I was wondering what is the policy or recommendation
> in such cases?
> Should I attempt to merge the code (mainly the way to set up the
> registers) on irq-crossbar.c or should we consider irq-tango_v2.c to
> live its own life?

If the HW is significantly different, I'd rather have a separate driver.
We can always share some things later on by having a small library of
"stuff".

> NOTE: IMHO, irq-crossbar.c could benefit from clearer names for some
> DT properties, because "max_irqs" and "max-crossbar-sources" are not
> straight forward names for "mux_outputs" and "mux_inputs"
> (respectively)

Maybe, but this ship has sailed a long time ago. This is an ABI now, and
it is not going to change unless proven to be broken. On the other hand,
you can name your own properties as you see fit.

> NOTE2: current irq-tango_v2.c code still has a 24 IRQ limitation
> since it is not using any API that would allow it to setup IRQ
> sharing.

Unless you limit your mux level interrupts only, I cannot see how you
could deal with cascaded interrupts. By the time you receive an edge,
the line will have dropped, and you won't be able to identify the source
interrupt.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...