Re: [PATCH v4 6/8] firmware: smccc: lfa: Register DT interrupt

[Andre Przywara <andre.przywara@arm.com>]

Hi,

(sorry, forgot to send the reply earlier, that goes together with the 
ACPI reply)

On 9/21/26 18:09, Sudeep Holla wrote:
> On Fri, Sep 18, 2026 at 04:11:09PM +0200, Andre Przywara wrote:
>> The Arm Live Firmware Activation spec describes an asynchronous
>> notification mechanism, where the platform can notify the host OS about
>> newly pending image updates.
>> In the absence of the ACPI notification mechanism also a simple
>> devicetree node can describe an interrupt.
>>
>> Add code to find the respective DT node and register the specified
>> interrupt, to trigger the activation if needed.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> ---
>>   drivers/firmware/smccc/lfa_fw.c | 39 +++++++++++++++++++++++++++++++++
>>   1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/firmware/smccc/lfa_fw.c b/drivers/firmware/smccc/lfa_fw.c
>> index 7a55b4a6785d8..a23df331ffde9 100644
>> --- a/drivers/firmware/smccc/lfa_fw.c
>> +++ b/drivers/firmware/smccc/lfa_fw.c
>> @@ -16,6 +16,8 @@
>>   #include <linux/list.h>
>>   #include <linux/module.h>
>>   #include <linux/nmi.h>
>> +#include <linux/of.h>
>> +#include <linux/of_irq.h>
>>   #include <linux/psci.h>
>>   #include <linux/stop_machine.h>
>>   #include <linux/string.h>
>> @@ -871,6 +873,37 @@ static void lfa_remove_acpi(struct device *dev)
>>   }
>>   #endif
>>   
>> +static irqreturn_t lfa_irq_handler_thread(int irq, void *dev_id)
>> +{
>> +	int ret;
>> +
>> +	while (!(ret = activate_pending_image()))
>> +		;
> 
> Yet again trusting firmware too much!

Yes, same problem as with the ACPI patch, and will be fixed by the same 
new code.
>> +
>> +	if (ret != -ENOENT)
>> +		pr_warn("notified image activation failed: %d\n", ret);
>> +
>> +	return IRQ_HANDLED;
>> +}
>> +
>> +static int lfa_register_dt(struct device *dev)
>> +{
>> +	struct device_node *np;
>> +	unsigned int irq;
>> +
>> +	np = of_find_compatible_node(NULL, NULL, "arm,lfa");
>> +	if (!np)
>> +		return -ENODEV;
>> +
>> +	irq = irq_of_parse_and_map(np, 0);
>> +	of_node_put(np);
>> +	if (!irq)
>> +		return -ENODEV;
>> +
>> +	return devm_request_threaded_irq(dev, irq, NULL, lfa_irq_handler_thread,
>> +					 IRQF_COND_ONESHOT, NULL, NULL);
> 
> And where can I find free_irq() ? Yes devm_() can do that for you but how
> does that work with races around removal of device for example. I prefer
> to have free_irq() at the very beginning of lfa_remove().

OK, added that.

> 
>> +}
>> +
>>   static int lfa_smccc_probe(struct arm_smccc_device *sdev)
>>   {
>>   	struct arm_smccc_1_2_regs reg = { 0 };
>> @@ -913,6 +946,12 @@ static int lfa_smccc_probe(struct arm_smccc_device *sdev)
>>   			pr_info("registered LFA ACPI notification\n");
>>   		if (err != -ENODEV)
>>   			return err;
>> +	} else {
>> +		err = lfa_register_dt(&sdev->dev);
>> +		if (!err)
>> +			pr_info("registered LFA DT notification interrupt\n");
>> +		if (err != -ENODEV)
>> +			return err;
> 
> More code addition with absolutely no error handling 🙁.

Right, but fixed now by the same code that fixed the ACPI part.

Cheers,
Andre

> 
> 
>>   	}
>>   
>>   	return 0;
>> -- 
>> 2.43.0
>>
>