Thread (16 messages) 16 messages, 2 authors, 2016-07-28

Re: [PATCH v4 2/2] irqchip: add J-Core AIC driver

From: Mark Rutland <mark.rutland@arm.com>
Date: 2016-07-27 10:12:46
Also in: linux-sh, lkml

On Wed, Jul 27, 2016 at 05:35:09AM +0000, Rich Felker wrote:
+int __init aic_irq_of_init(struct device_node *node, struct device_node *parent)
+{
+	struct aic_data *aic = &aic_data;
+	unsigned min_irq = 64;
+
+	pr_info("Initializing J-Core AIC\n");
+
+	if (!of_device_is_compatible(node, "jcore,aic2")) {
+		unsigned cpu;
+		for_each_possible_cpu(cpu) {
+			void __iomem *base = of_iomap(node, cpu);
+			if (!base)
+				continue;
This sounds like it would be a critical error.

It would be best to at least pr_warn() if you can't map a CPU's AI
interface.
+			pr_info("Local AIC1 enable for cpu %u at %p\n",
+				cpu, base + AIC1_INTPRI);
+			__raw_writel(0xffffffff, base + AIC1_INTPRI);
+		}
Here base goes out of scope. If you don't need it, it would be best
practice to iounmap it (even if that happens to be a no-op on your
arch).
+		min_irq = 16;
+	}
+
+	aic->chip.name = node->name;
It's probably best to give the name explicitly in the driver (e.g.
"AIC"), rather than taknig whatever happens to be in the DT (which
should be 'interrupt-controller@<addr>'.
+	aic->chip.irq_mask = noop;
+	aic->chip.irq_unmask = noop;
If the core code wants to mask IRQs, how do you handle that? Can you
mask your CPU traps?

Thanks,
Mark.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help