Messages in this thread | | | From | Hector Martin <> | Subject | Re: [RFT PATCH v3 16/27] irqchip/apple-aic: Add support for the Apple Interrupt Controller | Date | Fri, 26 Mar 2021 22:40:26 +0900 |
| |
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@marcan.st) Public Key: https://mrcn.st/pub
| |