On Fri, 05 Mar 2021 15:05:08 +0000, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote: [...] > > +#define TIMER_FIRING(x) \ > > + (((x) & (ARCH_TIMER_CTRL_ENABLE | ARCH_TIMER_CTRL_IT_MASK | \ > > + ARCH_TIMER_CTRL_IT_STAT)) == \ > > + (ARCH_TIMER_CTRL_ENABLE | ARCH_TIMER_CTRL_IT_STAT)) > > It's a bit hard to read. Perhaps > > #define FOO_MASK (_ENABLE | _STAT) > #define _FIRING ... (FOO_MASK | _MASK == FOO_MASK) The expression above is a direct translation of the architecture reference manual, and I'd rather not have that hidden behind a bunch of obscure macros. [...] > > + irqc->hw_domain = irq_domain_create_linear(of_node_to_fwnode(node), > > + irqc->nr_hw + AIC_NR_FIQ, > > + &aic_irq_domain_ops, irqc); > > If you are sure it will be always OF-only, why not to use > irq_domain_add_linear()? The OF-only API is deprecated, and there is no point in using it for *new* code, specially when things like IPI allocation require the use of the modern API. For arm64 root controllers, that's the way to go. Thanks, M. -- Without deviation from the norm, progress is not possible.