Re: [PATCH v4 29/39] irqchip/atmel-aic5: Add support to get nirqs from DT for sam9x60 & sam9x7

From: claudiu beznea <claudiu.beznea@tuxon.dev>
To: Varshini.Rajendran@microchip.com, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	tglx@linutronix.de, Nicolas.Ferre@microchip.com,
	alexandre.belloni@bootlin.com, andre.przywara@arm.com,
	mani@kernel.org, shawnguo@kernel.org,
	Durai.ManickamKR@microchip.com, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v4 29/39] irqchip/atmel-aic5: Add support to get nirqs from DT for sam9x60 & sam9x7
Date: Sat, 9 Mar 2024 15:13:11 +0200	[thread overview]
Message-ID: <fe9a8936-3b7f-4460-8dba-85cc811d1253@tuxon.dev> (raw)
In-Reply-To: <ba7a59ae-d377-4514-89e7-86991420507c@microchip.com>

On 08.03.2024 10:50, 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
>>
>> On 23.02.2024 19:29, Varshini Rajendran wrote:
>>> Add support to get number of IRQs from the respective DT node for sam9x60
>>> and sam9x7 devices. Since only this factor differs between the two SoCs,
>>> this patch adds support for the same. Adapt the sam9x60 dtsi
>>> accordingly.
>>>
>>> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
>>> ---
>>> 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 ++++++++---
>>>   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 = <0xfffff100 0x100>;
>>>                                atmel,external-irqs = <31>;
>>> +                             microchip,nr-irqs = <50>;
>>>                        };
>>>
>>>                        dbgu: serial@fffff200 {
>>> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-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 device_node *node,
>>>   }
>>>   IRQCHIP_DECLARE(sama5d4_aic5, "atmel,sama5d4-aic", sama5d4_aic5_of_init);
>>>
>>> -#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 = of_property_read_u32(node, "microchip,nr-irqs", &nr_irqs);
>>> +     if (ret) {
>>> +             pr_err("Not found microchip,nr-irqs property\n");
>>
>> This breaks the ABI. You should ensure old device trees are still working
>> with this patch.
> 
> The only older device that uses this API is sam9x60 and the newly added 
> sam9x7. This change has been tested to be working fine in both the 
> devices.

As Conor explained, the code in this patch should work with device trees
from previous kernel releases (thus not having microchip,nr-irqs DT binding).

> If you still want me to use the macros as a fallback in the 
> failure case I can do it. But this change was proposed to avoid adding 
> macros in the first place. I can remove the error check just like they 
> do while getting other device tree properties. Or if this is just a 
> concern of the old devices working with the new change, then sam9x60 
> works. Please let me know how to proceed.
>>
>>> +             return ret;
>>> +     }
>>> +     return aic5_of_init(node, parent, nr_irqs);
>>>   }
>>>   IRQCHIP_DECLARE(sam9x60_aic5, "microchip,sam9x60-aic", sam9x60_aic5_of_init);
> 