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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 630AAC5475B for ; Fri, 8 Mar 2024 10:16:34 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type:Cc: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=yfwF0KX+4nxV7Op0i63stpVVk1+YW8nhEsYG/wYUDiM=; b=0melNUzeG7utIhYmTSrI3/VzKq hLY4IZp6ngwLJfwT2iF41DcRyTd/LuueGIAWXueiS9i6k571J14WACiE+iqkiTVQBnJd48ag+TnZa dUI2Io9JkykKAY40+V0ccpaw8icWqi+JBR21dXP2SgznYQLML8at8yFqCl0dySjTVHrMr5mKWvEIA HWUPL0URLGV9PXsKb3/aHFWO3DsBZsx+1yVDREIkeNoALx8kSWULtaudzyBBTIoh9Yv/z7jhbYqVy cTr7jMlQq7Tzf4LRZfmotDaAx4Q07iQ0Uj4W7n9gYQMbX0JjXCtOLoS2tT8Hzk1oAFhTnQaqHYSlj zvONNleA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1riXGy-00000008iqk-0fJv; Fri, 08 Mar 2024 10:16:16 +0000 Received: from esa.microchip.iphmx.com ([68.232.153.233]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1riXGv-00000008iq1-1Twk for linux-arm-kernel@lists.infradead.org; Fri, 08 Mar 2024 10:16:15 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1709892974; x=1741428974; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=Q8cQjBrAEx7ffkcBZwFyj+7ctJtXOoVmf2a9mz1sm6U=; b=Vo24ck2Fdp3pfpkBEuqisiOXOO1OiHC+VNt/5PQL3nQ7BbxbYfa9Ps/E 8ILwwbRYGSM1CmWwn4RbpM+S32WK8sJu1uR7e0XzxmJ9U1eUW3Ks0v6yx S3XnuJ7Ma5SVvxDnsVQF1R5r3GfTjfvQsGslkMOOPh2W/WZeXB9G9quAz 5+gHjDqf8di+WefBZCvWBjfqFcAhIR7Z+xlHUypAk4HX4wNUPTl1QdN3W iF6gBv5l0uf848yLYc90x5w/1BBmwU1a9A7fUqgRynm3yTyAD1RzZtn8u cVtWSVRji0qcMpvqG5MtLBp20UlwFR7bAOFD5z0dig82L9GFQ7ots2eBo Q==; X-CSE-ConnectionGUID: GYN+Phv8SneCqTAifiO+dQ== X-CSE-MsgGUID: A8+JuFB9QEC81NKv8EN3gA== X-IronPort-AV: E=Sophos;i="6.07,109,1708412400"; d="asc'?scan'208";a="248162359" X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN Received: from unknown (HELO email.microchip.com) ([170.129.1.10]) by esa5.microchip.iphmx.com with ESMTP/TLS/ECDHE-RSA-AES128-GCM-SHA256; 08 Mar 2024 03:16:10 -0700 Received: from chn-vm-ex01.mchp-main.com (10.10.85.143) by chn-vm-ex04.mchp-main.com (10.10.85.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Fri, 8 Mar 2024 03:16:05 -0700 Received: from wendy (10.10.85.11) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35 via Frontend Transport; Fri, 8 Mar 2024 03:16:02 -0700 Date: Fri, 8 Mar 2024 10:15:18 +0000 From: Conor Dooley To: Subject: Re: [PATCH v4 29/39] irqchip/atmel-aic5: Add support to get nirqs from DT for sam9x60 & sam9x7 Message-ID: <20240308-reissue-badass-9f8883b4e2e6@wendy> References: <20240223171342.669133-1-varshini.rajendran@microchip.com> <20240223172905.673053-1-varshini.rajendran@microchip.com> MIME-Version: 1.0 In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240308_021613_428195_907833B6 X-CRM114-Status: GOOD ( 38.30 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, alexandre.belloni@bootlin.com, mani@kernel.org, andre.przywara@arm.com, claudiu.beznea@tuxon.dev, robh+dt@kernel.org, Durai.ManickamKR@microchip.com, linux-arm-kernel@lists.infradead.org, krzysztof.kozlowski+dt@linaro.org, tglx@linutronix.de, shawnguo@kernel.org, linux-kernel@vger.kernel.org Content-Type: multipart/mixed; boundary="===============6163000142617590804==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============6163000142617590804== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="/QFzMk5rsZSajjp0" Content-Disposition: inline --/QFzMk5rsZSajjp0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 08, 2024 at 08:50:43AM +0000, Varshini.Rajendran@microchip.com = wrote: > On 03/03/24 5:51 pm, claudiu beznea wrote: > > EXTERNAL EMAIL: Do not click links or open attachments unless you know = the content is safe > >=20 > > On 23.02.2024 19:29, Varshini Rajendran wrote: > >> Add support to get number of IRQs from the respective DT node for sam9= x60 > >> and sam9x7 devices. Since only this factor differs between the two SoC= s, > >> this patch adds support for the same. Adapt the sam9x60 dtsi > >> accordingly. > >> > >> Signed-off-by: Varshini Rajendran > >> --- > >> Changes in v4: > >> - Changed the implementation to fetch the NIRQs from DT as per the > >> comment to avoid introducing a new compatible when this is the only > >> difference between the SoCs related to this IP. > >> --- > >> arch/arm/boot/dts/microchip/sam9x60.dtsi | 1 + > >> drivers/irqchip/irq-atmel-aic5.c | 11 ++++++++--- Driver and binding changes should be in different patches. Having them in the same patch is usually a red flag for ABI breakage. > >> 2 files changed, 9 insertions(+), 3 deletions(-) > >> > >> diff --git a/arch/arm/boot/dts/microchip/sam9x60.dtsi b/arch/arm/boot/= dts/microchip/sam9x60.dtsi > >> index 73d570a17269..e405f68c9f54 100644 > >> --- a/arch/arm/boot/dts/microchip/sam9x60.dtsi > >> +++ b/arch/arm/boot/dts/microchip/sam9x60.dtsi > >> @@ -1201,6 +1201,7 @@ aic: interrupt-controller@fffff100 { > >> interrupt-controller; > >> reg =3D <0xfffff100 0x100>; > >> atmel,external-irqs =3D <31>; > >> + microchip,nr-irqs =3D <50>; > >> }; > >> > >> dbgu: serial@fffff200 { > >> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-at= mel-aic5.c > >> index 145535bd7560..5d96ad8860d3 100644 > >> --- a/drivers/irqchip/irq-atmel-aic5.c > >> +++ b/drivers/irqchip/irq-atmel-aic5.c > >> @@ -398,11 +398,16 @@ static int __init sama5d4_aic5_of_init(struct de= vice_node *node, > >> } > >> IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", sama5d4_aic5_of_i= nit); > >> > >> -#define NR_SAM9X60_IRQS 50 > >> - > >> static int __init sam9x60_aic5_of_init(struct device_node *node, > >> struct device_node *parent) > >> { > >> - return aic5_of_init(node, parent, NR_SAM9X60_IRQS); > >> + int ret, nr_irqs; > >> + > >> + ret =3D of_property_read_u32(node, "microchip,nr-irqs", &nr_irqs= ); > >> + if (ret) { > >> + pr_err("Not found microchip,nr-irqs property\n"); > >=20 > > This breaks the ABI. You should ensure old device trees are still worki= ng > > with this patch. >=20 > The only older device that uses this API is sam9x60 and the newly added= =20 > sam9x7. This change has been tested to be working fine in both the=20 > devices. Does it still work for a sam9x60 that does not have "microchip,nr-irqs"? I can't see how it would, because you remove the define and return an error. That's a pretty clear ABI breakage to me and I don't think it is justified. > If you still want me to use the macros as a fallback in the=20 > failure case I can do it. But this change was proposed to avoid adding=20 > macros in the first place. I can remove the error check just like they=20 > do while getting other device tree properties. Or if this is just a=20 > concern of the old devices working with the new change, then sam9x60=20 > works. Please let me know how to proceed. I just noticed that this property is not documented in a binding. The first thing you would will be asked when trying to add that is "why can this not be determined based on the compatible", which means back to having a define in the driver. That said, having specific $soc_aic5_of_init() functions for each SoC seems silly when usually only the number of interrupts changes. The number of IRQs could be in the match data and you could use aic5_of_init in your IRQCHIP_DECLARE directly. > >> + return ret; > >> + } > >> + return aic5_of_init(node, parent, nr_irqs); > >> } > >> IRQCHIP_DECLARE(sam9x60_aic5, "microchip,sam9x60-aic", sam9x60_aic5_= of_init); >=20 > --=20 > Thanks and Regards, > Varshini Rajendran. >=20 --/QFzMk5rsZSajjp0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZerlKQAKCRB4tDGHoIJi 0pT9AQD3Y7yDIjab/erLnSk3oz54G8s2/Z1B0YCajxnyEsOP3wEApgIrOU1U1kkV 5w39xuPTAXOMj04cpNxFVJM+dJ9rowc= =stem -----END PGP SIGNATURE----- --/QFzMk5rsZSajjp0-- --===============6163000142617590804== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============6163000142617590804==--