* [PATCH] remoteproc: keystone: Request IRQs in probe()
@ 2026-03-02 20:17 Andrew Davis
2026-03-06 17:19 ` Mathieu Poirier
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Davis @ 2026-03-02 20:17 UTC (permalink / raw)
To: Bjorn Andersson, Mathieu Poirier
Cc: linux-remoteproc, linux-kernel, Andrew Davis
IRQs can be registered in probe and only need to be enabled/disabled
during remoteproc start/stop. This lets us catch IRQ issues early
and simplify remoteproc start/stop.
Signed-off-by: Andrew Davis <afd@ti.com>
---
drivers/remoteproc/keystone_remoteproc.c | 41 +++++++++---------------
1 file changed, 15 insertions(+), 26 deletions(-)
diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
index 4d6550b485675..e7fde55097866 100644
--- a/drivers/remoteproc/keystone_remoteproc.c
+++ b/drivers/remoteproc/keystone_remoteproc.c
@@ -173,35 +173,16 @@ static int keystone_rproc_start(struct rproc *rproc)
INIT_WORK(&ksproc->workqueue, handle_event);
- ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
- dev_name(ksproc->dev), ksproc);
- if (ret) {
- dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
- ret);
- goto out;
- }
+ enable_irq(ksproc->irq_ring);
+ enable_irq(ksproc->irq_fault);
- ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
- 0, dev_name(ksproc->dev), ksproc);
+ ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
if (ret) {
- dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
- ret);
- goto free_vring_irq;
+ flush_work(&ksproc->workqueue);
+ return ret;
}
- ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
- if (ret)
- goto free_exc_irq;
-
return 0;
-
-free_exc_irq:
- free_irq(ksproc->irq_fault, ksproc);
-free_vring_irq:
- free_irq(ksproc->irq_ring, ksproc);
- flush_work(&ksproc->workqueue);
-out:
- return ret;
}
/*
@@ -215,8 +196,8 @@ static int keystone_rproc_stop(struct rproc *rproc)
struct keystone_rproc *ksproc = rproc->priv;
keystone_rproc_dsp_reset(ksproc);
- free_irq(ksproc->irq_fault, ksproc);
- free_irq(ksproc->irq_ring, ksproc);
+ disable_irq(ksproc->irq_fault);
+ disable_irq(ksproc->irq_ring);
flush_work(&ksproc->workqueue);
return 0;
@@ -427,10 +408,18 @@ static int keystone_rproc_probe(struct platform_device *pdev)
ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
if (ksproc->irq_ring < 0)
return ksproc->irq_ring;
+ ret = devm_request_irq(dev, ksproc->irq_ring, keystone_rproc_vring_interrupt,
+ IRQF_NO_AUTOEN, dev_name(dev), ksproc);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to request vring interrupt\n");
ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
if (ksproc->irq_fault < 0)
return ksproc->irq_fault;
+ ret = devm_request_irq(dev, ksproc->irq_fault, keystone_rproc_exception_interrupt,
+ IRQF_NO_AUTOEN, dev_name(dev), ksproc);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable exception interrupt\n");
ksproc->kick_gpio = devm_gpiod_get(dev, "kick", GPIOD_ASIS);
ret = PTR_ERR_OR_ZERO(ksproc->kick_gpio);
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: keystone: Request IRQs in probe()
2026-03-02 20:17 [PATCH] remoteproc: keystone: Request IRQs in probe() Andrew Davis
@ 2026-03-06 17:19 ` Mathieu Poirier
2026-03-06 17:29 ` Andrew Davis
0 siblings, 1 reply; 4+ messages in thread
From: Mathieu Poirier @ 2026-03-06 17:19 UTC (permalink / raw)
To: Andrew Davis; +Cc: Bjorn Andersson, linux-remoteproc, linux-kernel
Good day,
On Mon, Mar 02, 2026 at 02:17:34PM -0600, Andrew Davis wrote:
> IRQs can be registered in probe and only need to be enabled/disabled
> during remoteproc start/stop. This lets us catch IRQ issues early
> and simplify remoteproc start/stop.
>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
> drivers/remoteproc/keystone_remoteproc.c | 41 +++++++++---------------
> 1 file changed, 15 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
> index 4d6550b485675..e7fde55097866 100644
> --- a/drivers/remoteproc/keystone_remoteproc.c
> +++ b/drivers/remoteproc/keystone_remoteproc.c
> @@ -173,35 +173,16 @@ static int keystone_rproc_start(struct rproc *rproc)
>
> INIT_WORK(&ksproc->workqueue, handle_event);
>
> - ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
> - dev_name(ksproc->dev), ksproc);
> - if (ret) {
> - dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
> - ret);
> - goto out;
> - }
> + enable_irq(ksproc->irq_ring);
> + enable_irq(ksproc->irq_fault);
>
> - ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
> - 0, dev_name(ksproc->dev), ksproc);
> + ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
> if (ret) {
> - dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
> - ret);
> - goto free_vring_irq;
> + flush_work(&ksproc->workqueue);
> + return ret;
> }
>
> - ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
> - if (ret)
> - goto free_exc_irq;
> -
> return 0;
> -
> -free_exc_irq:
> - free_irq(ksproc->irq_fault, ksproc);
> -free_vring_irq:
> - free_irq(ksproc->irq_ring, ksproc);
> - flush_work(&ksproc->workqueue);
> -out:
> - return ret;
> }
>
> /*
> @@ -215,8 +196,8 @@ static int keystone_rproc_stop(struct rproc *rproc)
> struct keystone_rproc *ksproc = rproc->priv;
>
> keystone_rproc_dsp_reset(ksproc);
> - free_irq(ksproc->irq_fault, ksproc);
> - free_irq(ksproc->irq_ring, ksproc);
> + disable_irq(ksproc->irq_fault);
> + disable_irq(ksproc->irq_ring);
> flush_work(&ksproc->workqueue);
>
> return 0;
> @@ -427,10 +408,18 @@ static int keystone_rproc_probe(struct platform_device *pdev)
> ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
> if (ksproc->irq_ring < 0)
> return ksproc->irq_ring;
> + ret = devm_request_irq(dev, ksproc->irq_ring, keystone_rproc_vring_interrupt,
> + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to request vring interrupt\n");
>
> ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
> if (ksproc->irq_fault < 0)
> return ksproc->irq_fault;
> + ret = devm_request_irq(dev, ksproc->irq_fault, keystone_rproc_exception_interrupt,
> + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
request_irq() sets irqflags IRQF_COND_ONESHOT, something that is not done here.
Are you sure this is what you want?
Thanks,
Mathieu
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to enable exception interrupt\n");
>
> ksproc->kick_gpio = devm_gpiod_get(dev, "kick", GPIOD_ASIS);
> ret = PTR_ERR_OR_ZERO(ksproc->kick_gpio);
> --
> 2.39.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: keystone: Request IRQs in probe()
2026-03-06 17:19 ` Mathieu Poirier
@ 2026-03-06 17:29 ` Andrew Davis
2026-03-09 14:26 ` Mathieu Poirier
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Davis @ 2026-03-06 17:29 UTC (permalink / raw)
To: Mathieu Poirier; +Cc: Bjorn Andersson, linux-remoteproc, linux-kernel
On 3/6/26 11:19 AM, Mathieu Poirier wrote:
> Good day,
>
> On Mon, Mar 02, 2026 at 02:17:34PM -0600, Andrew Davis wrote:
>> IRQs can be registered in probe and only need to be enabled/disabled
>> during remoteproc start/stop. This lets us catch IRQ issues early
>> and simplify remoteproc start/stop.
>>
>> Signed-off-by: Andrew Davis <afd@ti.com>
>> ---
>> drivers/remoteproc/keystone_remoteproc.c | 41 +++++++++---------------
>> 1 file changed, 15 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
>> index 4d6550b485675..e7fde55097866 100644
>> --- a/drivers/remoteproc/keystone_remoteproc.c
>> +++ b/drivers/remoteproc/keystone_remoteproc.c
>> @@ -173,35 +173,16 @@ static int keystone_rproc_start(struct rproc *rproc)
>>
>> INIT_WORK(&ksproc->workqueue, handle_event);
>>
>> - ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
>> - dev_name(ksproc->dev), ksproc);
>> - if (ret) {
>> - dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
>> - ret);
>> - goto out;
>> - }
>> + enable_irq(ksproc->irq_ring);
>> + enable_irq(ksproc->irq_fault);
>>
>> - ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
>> - 0, dev_name(ksproc->dev), ksproc);
>> + ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
>> if (ret) {
>> - dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
>> - ret);
>> - goto free_vring_irq;
>> + flush_work(&ksproc->workqueue);
>> + return ret;
>> }
>>
>> - ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
>> - if (ret)
>> - goto free_exc_irq;
>> -
>> return 0;
>> -
>> -free_exc_irq:
>> - free_irq(ksproc->irq_fault, ksproc);
>> -free_vring_irq:
>> - free_irq(ksproc->irq_ring, ksproc);
>> - flush_work(&ksproc->workqueue);
>> -out:
>> - return ret;
>> }
>>
>> /*
>> @@ -215,8 +196,8 @@ static int keystone_rproc_stop(struct rproc *rproc)
>> struct keystone_rproc *ksproc = rproc->priv;
>>
>> keystone_rproc_dsp_reset(ksproc);
>> - free_irq(ksproc->irq_fault, ksproc);
>> - free_irq(ksproc->irq_ring, ksproc);
>> + disable_irq(ksproc->irq_fault);
>> + disable_irq(ksproc->irq_ring);
>> flush_work(&ksproc->workqueue);
>>
>> return 0;
>> @@ -427,10 +408,18 @@ static int keystone_rproc_probe(struct platform_device *pdev)
>> ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
>> if (ksproc->irq_ring < 0)
>> return ksproc->irq_ring;
>> + ret = devm_request_irq(dev, ksproc->irq_ring, keystone_rproc_vring_interrupt,
>> + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to request vring interrupt\n");
>>
>> ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
>> if (ksproc->irq_fault < 0)
>> return ksproc->irq_fault;
>> + ret = devm_request_irq(dev, ksproc->irq_fault, keystone_rproc_exception_interrupt,
>> + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
>
> request_irq() sets irqflags IRQF_COND_ONESHOT, something that is not done here.
> Are you sure this is what you want?
>
devm_request_irq() looks to also set IRQF_COND_ONESHOT matching request_irq().
Not sure it would matter anyway as these IRQs are not shared, behavior would
be unchanged.
Andrew
> Thanks,
> Mathieu
>
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to enable exception interrupt\n");
>>
>> ksproc->kick_gpio = devm_gpiod_get(dev, "kick", GPIOD_ASIS);
>> ret = PTR_ERR_OR_ZERO(ksproc->kick_gpio);
>> --
>> 2.39.2
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] remoteproc: keystone: Request IRQs in probe()
2026-03-06 17:29 ` Andrew Davis
@ 2026-03-09 14:26 ` Mathieu Poirier
0 siblings, 0 replies; 4+ messages in thread
From: Mathieu Poirier @ 2026-03-09 14:26 UTC (permalink / raw)
To: Andrew Davis; +Cc: Bjorn Andersson, linux-remoteproc, linux-kernel
On Fri, Mar 06, 2026 at 11:29:09AM -0600, Andrew Davis wrote:
> On 3/6/26 11:19 AM, Mathieu Poirier wrote:
> > Good day,
> >
> > On Mon, Mar 02, 2026 at 02:17:34PM -0600, Andrew Davis wrote:
> > > IRQs can be registered in probe and only need to be enabled/disabled
> > > during remoteproc start/stop. This lets us catch IRQ issues early
> > > and simplify remoteproc start/stop.
> > >
> > > Signed-off-by: Andrew Davis <afd@ti.com>
> > > ---
> > > drivers/remoteproc/keystone_remoteproc.c | 41 +++++++++---------------
> > > 1 file changed, 15 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/remoteproc/keystone_remoteproc.c b/drivers/remoteproc/keystone_remoteproc.c
> > > index 4d6550b485675..e7fde55097866 100644
> > > --- a/drivers/remoteproc/keystone_remoteproc.c
> > > +++ b/drivers/remoteproc/keystone_remoteproc.c
> > > @@ -173,35 +173,16 @@ static int keystone_rproc_start(struct rproc *rproc)
> > > INIT_WORK(&ksproc->workqueue, handle_event);
> > > - ret = request_irq(ksproc->irq_ring, keystone_rproc_vring_interrupt, 0,
> > > - dev_name(ksproc->dev), ksproc);
> > > - if (ret) {
> > > - dev_err(ksproc->dev, "failed to enable vring interrupt, ret = %d\n",
> > > - ret);
> > > - goto out;
> > > - }
> > > + enable_irq(ksproc->irq_ring);
> > > + enable_irq(ksproc->irq_fault);
> > > - ret = request_irq(ksproc->irq_fault, keystone_rproc_exception_interrupt,
> > > - 0, dev_name(ksproc->dev), ksproc);
> > > + ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
> > > if (ret) {
> > > - dev_err(ksproc->dev, "failed to enable exception interrupt, ret = %d\n",
> > > - ret);
> > > - goto free_vring_irq;
> > > + flush_work(&ksproc->workqueue);
> > > + return ret;
> > > }
> > > - ret = keystone_rproc_dsp_boot(ksproc, rproc->bootaddr);
> > > - if (ret)
> > > - goto free_exc_irq;
> > > -
> > > return 0;
> > > -
> > > -free_exc_irq:
> > > - free_irq(ksproc->irq_fault, ksproc);
> > > -free_vring_irq:
> > > - free_irq(ksproc->irq_ring, ksproc);
> > > - flush_work(&ksproc->workqueue);
> > > -out:
> > > - return ret;
> > > }
> > > /*
> > > @@ -215,8 +196,8 @@ static int keystone_rproc_stop(struct rproc *rproc)
> > > struct keystone_rproc *ksproc = rproc->priv;
> > > keystone_rproc_dsp_reset(ksproc);
> > > - free_irq(ksproc->irq_fault, ksproc);
> > > - free_irq(ksproc->irq_ring, ksproc);
> > > + disable_irq(ksproc->irq_fault);
> > > + disable_irq(ksproc->irq_ring);
> > > flush_work(&ksproc->workqueue);
> > > return 0;
> > > @@ -427,10 +408,18 @@ static int keystone_rproc_probe(struct platform_device *pdev)
> > > ksproc->irq_ring = platform_get_irq_byname(pdev, "vring");
> > > if (ksproc->irq_ring < 0)
> > > return ksproc->irq_ring;
> > > + ret = devm_request_irq(dev, ksproc->irq_ring, keystone_rproc_vring_interrupt,
> > > + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "failed to request vring interrupt\n");
> > > ksproc->irq_fault = platform_get_irq_byname(pdev, "exception");
> > > if (ksproc->irq_fault < 0)
> > > return ksproc->irq_fault;
> > > + ret = devm_request_irq(dev, ksproc->irq_fault, keystone_rproc_exception_interrupt,
> > > + IRQF_NO_AUTOEN, dev_name(dev), ksproc);
> >
> > request_irq() sets irqflags IRQF_COND_ONESHOT, something that is not done here.
> > Are you sure this is what you want?
> >
>
> devm_request_irq() looks to also set IRQF_COND_ONESHOT matching request_irq().
>
You are correct - I have applied this patch.
> Not sure it would matter anyway as these IRQs are not shared, behavior would
> be unchanged.
>
> Andrew
>
> > Thanks,
> > Mathieu
> >
> > > + if (ret)
> > > + return dev_err_probe(dev, ret, "failed to enable exception interrupt\n");
> > > ksproc->kick_gpio = devm_gpiod_get(dev, "kick", GPIOD_ASIS);
> > > ret = PTR_ERR_OR_ZERO(ksproc->kick_gpio);
> > > --
> > > 2.39.2
> > >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-09 14:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 20:17 [PATCH] remoteproc: keystone: Request IRQs in probe() Andrew Davis
2026-03-06 17:19 ` Mathieu Poirier
2026-03-06 17:29 ` Andrew Davis
2026-03-09 14:26 ` Mathieu Poirier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox