Re: [PATCH v3 1/2] irqchip: Add driver for Loongson-1 interrupt controller

From: Marc Zyngier
Date: Sat Jan 26 2019 - 07:09:25 EST


On Fri, 25 Jan 2019 10:56:33 +0000,
Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
>
> Hi Marc
>
> Thanks for your suggestions, I'm working on v4 and I would like to
> ask if it is better to have a driver for only one irqchip and create
> dt nodes for each chip, or just register all the chips in a single
> driver with only one dt node.

It would make more sense to have a node per chip, meaning that you
end-up with one instance per chip as well. It won't make the driver
much more complicated.

[...]

> >> + domain = irq_domain_add_legacy(node, num_chips * 32, LS1X_IRQ_BASE, 0,
> >> + &irq_domain_simple_ops, NULL);
> > Why a legacy domain? This is usually reserved to old drivers that are
> > converted to a new infrastructure, while needing some form of platform
> > hacks. I don't see this being the case here.
> >
> > It is also worrying that although you have up to 5 irqchips, they all
> > share a single domain. What does this mean? each irqchip is expected
> > to have its own domain.
>
> Yes, I do like this for backward compatible reason. I'm turning
> a legacy platform device mach(arch/mips/loongson32) in to
> dt based generic mach and I would like to do it step by step rather
> than one time.
>
> So I use legacy domain in order to keep IRQ same with the
> old driver exist on arch/mips/loongson32/common/irq.c

OK, it would have been good to make a note of that in the cover
letter, which is a bit empty at the moment.

Thanks,

M.

--
Jazz is not dead, it just smell funny.