Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 04 Mar 2022 08:04:48 +0000
Message-ID: <87ee3ita73.wl-maz@kernel.org>
From:   Marc Zyngier <maz@kernel.org>
To:     Anup Patel <anup@brainfault.org>
Cc:     Anup Patel <apatel@ventanamicro.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Daniel Lezcano <daniel.lezcano@linaro.org>,
        Atish Patra <atishp@atishpatra.org>,
        Alistair Francis <Alistair.Francis@wdc.com>,
        linux-riscv <linux-riscv@lists.infradead.org>,
        "linux-kernel@vger.kernel.org List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 3/6] RISC-V: Treat IPIs as normal Linux IRQs
In-Reply-To: <CAAhSdy3ACa-447MrLs=5GLjSSWKiKyR585RFWP_ycM3RZ+RTQg@mail.gmail.com>
References: <20220301042722.401113-1-apatel@ventanamicro.com>
        <20220301042722.401113-4-apatel@ventanamicro.com>
        <87czj53fjq.wl-maz@kernel.org>
        <CAAhSdy3ACa-447MrLs=5GLjSSWKiKyR585RFWP_ycM3RZ+RTQg@mail.gmail.com>
User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue)
 FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1
 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO)
MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue")
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

On Tue, 01 Mar 2022 17:40:55 +0000,
Anup Patel <anup@brainfault.org> wrote:
> 
> On Tue, Mar 1, 2022 at 8:07 PM Marc Zyngier <maz@kernel.org> wrote:
> > > +struct ipi_mux {
> > > +     struct irq_domain *domain;
> > > +     int parent_virq;
> > > +     void (*clear_ipi)(void);
> > > +     void (*send_ipi)(const struct cpumask *mask);
> > > +};
> >
> > Why do you need this in the arch code? It really looks like something
> > that is irqchip specific (single IPI signal on which actual IPIs are
> > overlayed). It is also something that other irqchips are already
> > implementing, so there is potential for consolidation.
> 
> I agree we can share the IPI muxing among irqchip drivers.
> 
> I was not sure where to place this IPI muxing so I made it
> RISC-V specific initially.
> 
> Can we place a simplified IPI muxing (with no RISC-V specific
> stuff) under drivers/irqchip or kernel/irq ??

We already have IPI-specific code in kernel/irq, so it should probably
live there.

> > > +struct irq_domain *riscv_ipi_mux_create(bool use_soft_irq,
> > > +                     void (*clear_ipi)(void),
> > > +                     void (*send_ipi)(const struct cpumask *mask))
> > > +{
> >
> > There really shouldn't be a need for such a registration interface
> > anyway (the current idiom is to allocate IPIs in the root irqchip, and
> > pass them to the arch code).
> >
> > Why can't you model it after the existing architectures?
> 
> I ended up with a lot of duplicate code between SBI IPI driver and
> SiFive CLINT driver so I factored out the IPI muxing as separate
> sources. We also have RISC-V AIA drivers using the same IPI muxing.
> 
> If we simplify the IPI muxing and move it out of arch/riscv then
> changes in this patch are straight forward to review.

It isn't only about making things easier to review. It is about having
consistent interfaces across architecture and reducing the amount of
glue between arch code and random drivers. If there is a lot of
similar code between your various irqchips, then it can be made common
between the irqchips. But the interface between arch code and those
should not be arch-specific.

	M.

-- 
Without deviation from the norm, progress is not possible.