[RFT PATCH v3 16/27] irqchip/apple-aic: Add support for the Apple Interrupt Controller

[Hector Martin]

On 06/03/2021 00.05, Andy Shevchenko wrote:
>> +#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.

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

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

>> +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)
> 
> >= ?

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 at marcan.st)
Public Key: https://mrcn.st/pub