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

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:

quoted

quoted

+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.

quoted

quoted

+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.