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 <[email protected]> > > > --- > > > 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 > > > >
