于 2020年3月25日 GMT+08:00 下午9:57:49, Marc Zyngier <maz@xxxxxxxxxx> 写到: >On 2020-03-25 13:07, Jiaxun Yang wrote: >> 于 2020年3月25日 GMT+08:00 下午9:00:01, Marc Zyngier <maz@xxxxxxxxxx> 写到: >>> On 2020-03-25 12:37, Thomas Bogendoerfer wrote: >>>> On Wed, Mar 25, 2020 at 11:54:59AM +0800, Jiaxun Yang wrote: >>>>> The old code is using legacy domain to setup irq_domain for CPU >>>>> interrupts >>>>> which requires irq_desc to be preallocated. >>>>> >>>>> However, when MIPS_CPU_IRQ_BASE >= 16, irq_desc for CPU IRQs may >end >>> >>>>> up >>>>> unallocated and lead to incorrect behavior. >>>>> >>>>> Thus we convert the legacy domain to simple domain which can >>> allocate >>>>> irq_desc during initialization. >>>>> >>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> >>>>> Co-developed-by: Huacai Chen <chenhc@xxxxxxxxxx> >>>>> Signed-off-by: Huacai Chen <chenhc@xxxxxxxxxx> >>>>> Reviewed-by: Marc Zyngier <maz@xxxxxxxxxx> >>>>> --- >>>>> drivers/irqchip/irq-mips-cpu.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/irqchip/irq-mips-cpu.c >>>>> b/drivers/irqchip/irq-mips-cpu.c >>>>> index 95d4fd8f7a96..c3cf7fa76424 100644 >>>>> --- a/drivers/irqchip/irq-mips-cpu.c >>>>> +++ b/drivers/irqchip/irq-mips-cpu.c >>>>> @@ -251,7 +251,7 @@ static void __init __mips_cpu_irq_init(struct >>>>> device_node *of_node) >>>>> clear_c0_status(ST0_IM); >>>>> clear_c0_cause(CAUSEF_IP); >>>>> >>>>> - irq_domain = irq_domain_add_legacy(of_node, 8, >MIPS_CPU_IRQ_BASE, >>> 0, >>>>> + irq_domain = irq_domain_add_simple(of_node, 8, >MIPS_CPU_IRQ_BASE, >>>>> &mips_cpu_intc_irq_domain_ops, >>>>> NULL); >>>> >>>> this breaks at least IP30 and guess it will break every platform >>> where >>>> MIPS_CPU_IRQ_BASE == 0. add_legacy will always do >>>> irq_domain_associate_many(), >>>> while add_simple doesn't do it, if first_irq == 0. >>>> >>>> Marc, what is the reason not doing it all the time ? What's the >>> correct >>>> way here to work with irq_domain_add_simple() in this case ? >>> >>> On a fully DT-ified platform, using non-legacy irqdomains, virtual >>> interrupts >>> are allocated as a "random" number, depending on the order of >>> allocation, >>> and on demand. >>> >>> The first_irq hack in irq_domain_add_simple() is just a way to still >>> allocate >>> descriptors upfront (and I wish we could drop it...). >>> >>> If you have legacy code that "knows" about the relationship between >>> Linux's >>> virtual interrupt and the hwirq (that is only meaningful to the >>> interrupt >>> controller), you're screwed, and need to stick to the legacy >>> irqdomain. >>> >>> It feels like the MIPS code is squarely in the latter case, so I >guess >>> this >>> patch is probably the wrong thing to do for this architecture. >> >> So probably we can use legacy domain when MIPS IRQ BASE is in the >> range of legacy IRQ >> and switch to simple domain when it's not in that range? > >No, see below. > >> Here in Loongson systems IRQ 0-15 is occupied by I8259 so I did this >> hack. > >Well, if you have to consider which Linux IRQ gets assigned, >then your platform is definitely not ready for non-legacy >irqdomains. Just stick to legacy for now until you have removed >all the code that knows the hwirq mapping. Thanks. So I have to allocate irq_desc here in driver manually? > > M. -- Jiaxun Yang