From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier Subject: Re: [PATCH v3 1/8] irqchip/mips-gic: Only register IPI domain when SMP is enabled Date: Thu, 07 Jul 2022 09:22:26 +0100 Message-ID: <87czehmiwt.wl-maz@kernel.org> References: <20220701200056.46555-1-samuel@sholland.org> <20220701200056.46555-2-samuel@sholland.org> <20220705135243.ydbwfo4kois64elr@mobilestation> Mime-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 9C53540C93 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 3D81240C8F DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 5EA9D80B7C DKIM-Filter: OpenDKIM Filter v2.11.0 smtp1.osuosl.org 75AA780A98 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1657182181; bh=77PtLpTS/qpbww/8J/LGrQtKTBirOJTX8MN8Ui9XQsg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=C4XXSYfqBm7jhADxfzqk/6WEI/haJhe/1MyTEVUiYwb5P0Ar3A4NKXEAQujuPspPd s3qSefHxSM6yAycSBOEk1x5rsX41Lz5CXWMfbKx+MFmlkdtWT2//rWeC4JZmK+kuzf ECU/jn91qz2SjvVIYKTSlVgpyAEteSUY53CHGeOdfan/ZYOpV2RhOmIOCcQwf6MG7i sgmXwysrqKOA4mKnZ3tQfGEWbFUHusfDO84IAYjAhD58ooo5Ed5J2OwoavWvff5Afm EAomLwlKGskSsbFGf0ELGfPJoWMt9Y95OsdWqoGMlW85HJC5m/Hz4sRuAeBe6tWwp9 HHg5eNbfeNzNA== In-Reply-To: <20220705135243.ydbwfo4kois64elr@mobilestation> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Sender: "iommu" To: Serge Semin Cc: Mark Rutland , Krzysztof =?UTF-8?B?V2lsY3p5xYRza2k=?= , linux-hyperv-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rich Felker , linux-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-sh-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linus Walleij , Dave Hansen , linux-mips-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "James E.J. Bottomley" , Max Filippov , Jan Beulich , "H. Peter Anvin" , "K. Y. Srinivasan" , Sven Schnelle , Boris Ostrovsky , Rob Herring , Wei Liu , Florian Fainelli , Stephen Hemminger , kernel test robot , Yoshinori Sato , Samuel Holland On Tue, 05 Jul 2022 14:52:43 +0100, Serge Semin wrote: > > Hi Samuel > > On Fri, Jul 01, 2022 at 03:00:49PM -0500, Samuel Holland wrote: > > The MIPS GIC irqchip driver may be selected in a uniprocessor > > configuration, but it unconditionally registers an IPI domain. > > > > Limit the part of the driver dealing with IPIs to only be compiled when > > GENERIC_IRQ_IPI is enabled, which corresponds to an SMP configuration. > > Thanks for the patch. Some comment is below. > > > > > Reported-by: kernel test robot > > Signed-off-by: Samuel Holland > > --- > > > > Changes in v3: > > - New patch to fix build errors in uniprocessor MIPS configs > > > > drivers/irqchip/Kconfig | 3 +- > > drivers/irqchip/irq-mips-gic.c | 80 +++++++++++++++++++++++----------- > > 2 files changed, 56 insertions(+), 27 deletions(-) > > > > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > > index 1f23a6be7d88..d26a4ff7c99f 100644 > > --- a/drivers/irqchip/Kconfig > > +++ b/drivers/irqchip/Kconfig > > @@ -322,7 +322,8 @@ config KEYSTONE_IRQ > > > > config MIPS_GIC > > bool > > - select GENERIC_IRQ_IPI > > + select GENERIC_IRQ_IPI if SMP > > > + select IRQ_DOMAIN_HIERARCHY > > It seems to me that the IRQ domains hierarchy is supposed to be > created only if IPI is required. At least that's what the MIPS GIC > driver implies. Thus we can go further and CONFIG_IRQ_DOMAIN_HIERARCHY > ifdef-out the gic_irq_domain_alloc() and gic_irq_domain_free() > methods definition together with the initialization: > > static const struct irq_domain_ops gic_irq_domain_ops = { > .xlate = gic_irq_domain_xlate, > +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY > .alloc = gic_irq_domain_alloc, > .free = gic_irq_domain_free, > +#endif > .map = gic_irq_domain_map, > }; > > If the GENERIC_IRQ_IPI config is enabled, CONFIG_IRQ_DOMAIN_HIERARCHY > will be automatically selected (see the config definition in > kernel/irq/Kconfig). If the IRQs hierarchy is needed for some another > functionality like GENERIC_MSI_IRQ_DOMAIN or GPIOs then they will > explicitly enable the IRQ_DOMAIN_HIERARCHY config thus activating the > denoted .alloc and .free methods definitions. > > To sum up you can get rid of the IRQ_DOMAIN_HIERARCHY config > force-select from this patch and make the MIPS GIC driver code a bit > more coherent. > > @Marc, please correct me if were wrong. Either way probably works correctly, but Samuel's approach is more readable IMO. It is far easier to reason about a high-level feature (GENERIC_IRQ_IPI) than an implementation detail (IRQ_DOMAIN_HIERARCHY). If you really want to save a handful of bytes, you can make the callbacks conditional on GENERIC_IRQ_IPI, and be done with it. But this can come as an additional patch. Thanks, M. -- Without deviation from the norm, progress is not possible.