From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4267FC433F5 for ; Mon, 14 Feb 2022 10:53:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1349519AbiBNKx0 (ORCPT ); Mon, 14 Feb 2022 05:53:26 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:46076 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1349297AbiBNKxO (ORCPT ); Mon, 14 Feb 2022 05:53:14 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0BBF46D94D; Mon, 14 Feb 2022 02:17:49 -0800 (PST) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9DA2761175; Mon, 14 Feb 2022 10:17:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 0FF33C340E9; Mon, 14 Feb 2022 10:17:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1644833868; bh=Bd6563qakfMdmj15vig4Q1sz4MW1UKmd2W6/fruhJec=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=teMl9bcL/WSPL1BKbQA9S71chpnkvdJ7NUfCpI3MjZG8e9rQ+4gsERQulgCVyAf9n 30Tc5hYfL2pPUUD9hMCUZs6Wv2qxcPsCYA80LVdlIZ8WN8ykmeVFzxPNyarQsmOVr5 5hrlsWUenbX0yH0BliKYNFyoJe9v/+ZZSM6Lxasbrh8o7hsqU0dmHqg2MaZqXpXQgg v1yOMt6k05EZJSC64DSCDUMpTduS7SpCE9Qzf61edRrrliSr/e+tWtoutxiWNMMHT9 dBMRM+KNTI3Etw+nZTllwm412S5bVgoP2K8R2ONhhpm5YpLovE+pcLtK+VMM5UHAUP AZgFLrI8V1OcQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1nJYQT-007iID-HE; Mon, 14 Feb 2022 10:17:45 +0000 Date: Mon, 14 Feb 2022 10:17:45 +0000 Message-ID: <87o8394ura.wl-maz@kernel.org> From: Marc Zyngier To: Samuel Holland Cc: Thomas Gleixner , Rob Herring , Frank Rowand , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] of/irq: Use interrupts-extended to find parent In-Reply-To: <20220214051318.2273-1-samuel@sholland.org> References: <20220214051318.2273-1-samuel@sholland.org> 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 (x86_64-pc-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 X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: samuel@sholland.org, tglx@linutronix.de, robh+dt@kernel.org, frowand.list@gmail.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On Mon, 14 Feb 2022 05:13:17 +0000, Samuel Holland wrote: > > Some OF irqchips, such as the RISC-V PLIC, use interrupts-extended to > specify their parent domain(s). That binding does not allow using the > interrupt-parent property in the irqchip node, which prevents > of_irq_init from properly detecting the irqchip hierarchy. Is this because there is more than a single interrupt parent? > > If no interrupt-parent property is present in the enclosing bus or root > node, then desc->interrupt_parent will be NULL for both the per-CPU > RISC-V INTCs (the actual root domains) and the RISC-V PLIC. Similarly, > if the bus or root node specifies `interrupt-parent = <&plic>`, then > of_irq_init will hit the `desc->interrupt_parent == np` check, and again > all parents will be NULL. So things happen to work today for some boards > due to Makefile ordering. > > However, things break when another irqchip ("foo") is stacked on top of > the PLIC. The bus/root node will have `interrupt-parent = <&foo>`, > since that is what all of the other peripherals need. When of_irq_init > runs, it will try to find the PLIC's parent domain. But because > of_irq_find_parent ignores interrupts-extended, it will fall back to > using the interrupt-parent property of the PLIC's parent node (i.e. the > bus or root node), and see "foo" as the PLIC's parent domain. But this > is wrong, because "foo" is actually the PLIC's child domain! Let me see if I parsed this correctly. You have: int-parent int-extended foo -----------> PLIC --------------> root-irqchip Is that correct? > > So of_irq_init wrongly attempts to init the stacked irqchip before the > PLIC. This fails and breaks boot. > > Fix this by having of_irq_find_parent return the first node referenced > by interrupts-extended when that property is present. Even if the > property references multiple different IRQ domains, this will still work > reliably in of_irq_init as long as all referenced domains are the same > distance away from some root domain (e.g. the RISC-V INTCs referenced by > the PLIC's interrupts-extended are always all root domains). I'm a bit worried that the distance assumption may not always hold. Maybe it isn't something we need to deal with right now, but a comment wouldn't hurt to make this assumption clear. > > Signed-off-by: Samuel Holland > --- > > drivers/of/irq.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > index 2b07677a386b..0c20e22b91f5 100644 > --- a/drivers/of/irq.c > +++ b/drivers/of/irq.c > @@ -60,7 +60,8 @@ struct device_node *of_irq_find_parent(struct device_node *child) > return NULL; > > do { > - if (of_property_read_u32(child, "interrupt-parent", &parent)) { > + if (of_property_read_u32(child, "interrupt-parent", &parent) && > + of_property_read_u32(child, "interrupts-extended", &parent)) { > p = of_get_parent(child); > } else { > if (of_irq_workarounds & OF_IMAP_NO_PHANDLE) With the comment added: Acked-by: Marc Zyngier M. -- Without deviation from the norm, progress is not possible.