[PATCH 2/4] irqchip: irq-mvebu-pic: new driver for Marvell Armada 7K/8K PIC

Jason Cooper
2016-08-05T16:27:03Z

Hi Thomas,

On Fri, Aug 05, 2016 at 05:58:12PM +0200, Thomas Petazzoni wrote:
> On Fri, 5 Aug 2016 15:31:13 +0000, Jason Cooper wrote:
>
> > > +config MVEBU_PIC
> > > + bool
> >
> > tri-state? Is there anything else attached to the PIC besides the PMU?
>
> tri-state would be fine I believe, it's indeed a secondary interrupt
> controller, not essential for booting the platform.
>
> But then I probably need to rework PATCH 3/4 and not have it
> unconditionally selected by the platform Kconfig option, right?

meh. I have no preference either way. It's what works best for your
platform. I've just seen one or two people on a tear lately regarding
module.h/MODULE_* and being boolean. I figured I'd address it while I
was here. :-)

> Regarding what else is attached to the PIC, I have no idea, I don't
> have this information.

Ok, then which ever way you go is fine by me.

> > > +static const struct of_device_id mvebu_pic_of_match[] = {
> > > + { .compatible = "marvell,armada-8k-pic", },
> >
> > You mention 7k in $subject, should you use that here as the youngest
> > compatible SoC generation?
>
> There isn't anything youngest or oldest between 7K and 8K, they both
> got released at the same time. They are really the same family of SoCs,
> the 7K having only one CP110, the 8K having two of them, which provides
> more I/Os.
>
> For several other IPs, we're using armada-8k as the compatible string:
>
> * marvell,armada8k-pcie
> * marvell,armada-8k-xhci

Ok, sure. That was just a nit. I know human nature, despite logic,
will assume 8k is newer that 7k, like SSLv3 being better than TLS v1.x
because the number is bigger. :-/

Consistency is better at this point.

The rest of it looks fine.

thx,

Jason.