On 06/03/2021 00.05, Andy Shevchenko wrote:

quoted

+#define pr_fmt(fmt) "%s: " fmt, __func__

This is not needed, really, if you have unique / distinguishable
messages in the first place.
Rather people include module names, which may be useful.

Makes sense, I'll switch to KBUILD_MODNAME.

quoted

+#define MASK_BIT(x)            BIT((x) & 0x1f)

GENMASK(4,0)

It's not really a register bitmask, but rather extracting the low bits 
of an index... but sure, GENMASK also expresses that. Changed.

quoted

+static atomic_t aic_vipi_flag[AIC_MAX_CPUS];
+static atomic_t aic_vipi_enable[AIC_MAX_CPUS];

Isn't it easier to handle these when they are full width, i.e. 32
items per the array?

I don't think so, it doesn't really buy us anything. It's just a maximum 
beyond which the driver doesn't work in its current state anyway (if the 
number were much larger it'd make sense to dynamically allocate these, 
but not at this point).

quoted

+static int aic_irq_set_affinity(struct irq_data *d,
+                               const struct cpumask *mask_val, bool force)
+{
+       irq_hw_number_t hwirq = irqd_to_hwirq(d);
+       struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d);
+       int cpu;
+
+       if (hwirq > ic->nr_hw)

quoted

= ?

Good catch, but this is actually obsolete. Higher IRQs go into the FIQ 
irqchip, so this should never happen (it's a leftover from when they 
were a single one). I'll remove it.

Ack on the other comments, thanks!

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub