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

[Andre Przywara]

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@xxxxxxx>
> > ---
> >   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