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 5C30CC433F5 for ; Mon, 29 Nov 2021 22:02:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232806AbhK2WFs (ORCPT ); Mon, 29 Nov 2021 17:05:48 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53392 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229987AbhK2WDs (ORCPT ); Mon, 29 Nov 2021 17:03:48 -0500 Received: from sin.source.kernel.org (sin.source.kernel.org [IPv6:2604:1380:40e1:4800::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D4169C061D6D; Mon, 29 Nov 2021 11:32:01 -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 sin.source.kernel.org (Postfix) with ESMTPS id 2A1D7CE13DE; Mon, 29 Nov 2021 19:32:00 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 53271C53FAD; Mon, 29 Nov 2021 19:31:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1638214318; bh=2Hmk66f0LCHA6CH3FyMG0wildTaLAEfV129JiS/S6Zk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=DqQ/ZgEHckFLB88XiBiN7ZXK4EyWX+kfpUlnJQlK9Ufz/Y0lWixhFINugsZpoxjbF D3aDQgk88vF3WB41FiSqgJLxR9rvFyGmDgM4AKa2+ika7dnx1OPDxv9Cyvu4RgZ4vz BNhXP+33ad3z2oq4BFATug6uAlG33hAGo6+H03a4NYkwmQ9VIlC3eiQe0tffAFaFKe bZO51VEFQZc6Zb03sUb/62jdyM0RrkPDFCm+8H0K/wKDW4sRhZ93rVbQ6lBiCd+Ejd HUmHuQdxiabQv799/SuTFtdVwSdyVKj4bD8fzXmgewxn3YfLlKizaCKts0ETBss3kT NOi6W0qkQq4sg== 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 1mrmNY-008gZo-9T; Mon, 29 Nov 2021 19:31:56 +0000 Date: Mon, 29 Nov 2021 19:31:55 +0000 Message-ID: <87mtlmn4gk.wl-maz@kernel.org> From: Marc Zyngier To: Rob Herring Cc: "linux-kernel@vger.kernel.org" , devicetree@vger.kernel.org, Android Kernel Team , John Crispin , Biwen Li , Chris Brandt , Geert Uytterhoeven Subject: Re: [PATCH] of/irq: Add a quirk for controllers with their own definition of interrupt-map In-Reply-To: References: <20211122103032.517923-1-maz@kernel.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: robh@kernel.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, kernel-team@android.com, john@phrozen.org, biwen.li@nxp.com, chris.brandt@renesas.com, geert+renesas@glider.be 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, 29 Nov 2021 19:15:27 +0000, Rob Herring wrote: > > On Mon, Nov 22, 2021 at 4:30 AM Marc Zyngier wrote: > > > > Since 041284181226 ("of/irq: Allow matching of an interrupt-map local > > to an interrupt controller"), a handful of interrupt controllers have > > stopped working correctly. This is due to the DT exposing a non-sensical > > interrupt-map property, and their drivers relying on the kernel ignoring > > this property. > > > > Since we cannot realistically fix this terrible behaviour, add a quirk > > for the limited set of devices that have implemented this monster, > > and document that this is a pretty bad practice. > > > > Cc: Rob Herring > > Cc: John Crispin > > Cc: Biwen Li > > Cc: Chris Brandt > > Cc: Geert Uytterhoeven > > Signed-off-by: Marc Zyngier > > --- > > drivers/of/irq.c | 37 +++++++++++++++++++++++++++++++++++-- > > 1 file changed, 35 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > > index b10f015b2e37..27a5173c813c 100644 > > --- a/drivers/of/irq.c > > +++ b/drivers/of/irq.c > > @@ -76,6 +76,36 @@ struct device_node *of_irq_find_parent(struct device_node *child) > > } > > EXPORT_SYMBOL_GPL(of_irq_find_parent); > > > > +/* > > + * These interrupt controllers abuse interrupt-map for unspeakable > > + * reasons and rely on the core code to *ignore* it (the drivers do > > + * their own parsing of the property). > > + * > > + * If you think of adding to the list for something *new*, think > > + * again. There is a high chance that you will be sent back to the > > + * drawing board. > > + */ > > +static const char * const of_irq_imap_abusers[] = { > > + "CBEA,platform-spider-pic", > > + "sti,platform-spider-pic", > > + "realtek,rtl-intc", > > + "fsl,ls1021a-extirq", > > + "fsl,ls1043a-extirq", > > + "fsl,ls1088a-extirq", > > + "renesas,rza1-irqc", > > +}; > > I guess this list was obtained by with a: git grep '"interrupt-map"' Yes. Anyone having its own interrupt-map parser is likely to have the same problem. > I suppose that should be sufficient to find all the cases. I'd like to > be able to identify this case just from a DT file, but it's not really > clear Indeed. Not to mention that the PPC stuff doesn't has its DT hidden in some firmware. > Perhaps a simpler solution to all this is only handle interrupt-map > with interrupt-controller if it points to its own node. That works for > Apple and I don't see a need beyond that case. The problem is that interrupt-map can point to more than a single controller. What if the map points to a both a local interrupt and a a remote one? It feels weird to standardise on a behaviour that seems to contradict the spec and to single out the one that (IMO) matches the expected behaviour. At the end of the day, I'll implement whichever solution you prefer. > > +static bool of_irq_abuses_interrupt_map(struct device_node *np) > > +{ > > + int i; > > + > > + for (i = 0; i < ARRAY_SIZE(of_irq_imap_abusers); i++) > > + if (of_device_is_compatible(np, of_irq_imap_abusers[i])) > > + return true; > > + > > + return false; > > With a NULL terminated list, you can use of_device_compatible_match() instead . Ah, neat. Thanks, M. -- Without deviation from the norm, progress is not possible.