Re: [PATCH Dovetail v3 5/6] dovetail: Decouple kernel/irq/pipeline.c from irqstate bit definitions

Florian Bezdeka <[email protected]> writes:

> This only affects architectures where CONFIG_GENERIC_ENTRY is not set.
>
> Moving the stall bit definitions into the architecture specific
> implementations allows re-use of those definition within the arch
> specifics.
>
> Moving the defines to include/linux/irq_pipeline.h turned out to
> trigger a lot of trouble as re-use will live in asm/ where including
> a "general" header is quite hard.
>
> The platform independent implementation will now need the following
> arch specific hooks, all operating on the platform specific
> struct pt_regs:
>   - arch_kentry_irqs_enabled()

I would go for arch_kentry_{test/is_}stalled() since this would clearly
hint at the stage stall bit.

>   - arch_kentry_lockdep_set()

Name could be confusing since you also have
arch_kentry_set_lockdep(). arch_kentry_test_hardirq() instead?

>   - arch_kentry_clear_irq_state()
>   - arch_kentry_set_irqs_stalled()

arch_kentry_{set_}stall{ed}()

>   - arch_kentry_set_lockdep()
>

I would pick arch_kentry_set_hardirq() to better match the general
naming scheme you have been using for these helpers, fixing the original
misnomer in the same move (the idea conveyed by the original name was
too vague/broadly scoped already).

> diff --git a/arch/arm/include/asm/irq_pipeline.h b/arch/arm/include/asm/irq_pipeline.h

> diff --git a/arch/arm64/include/asm/irq_pipeline.h b/arch/arm64/include/asm/irq_pipeline.h

> diff --git a/kernel/irq/pipeline.c b/kernel/irq/pipeline.c

Please split this into separate commits (arm: irq_pipeline, arm64:
irq_pipeline and genirq: irq_pipeline).

-- 
Philippe.