| Message ID | 20230112172507.30579-8-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: imx-pxp: Miscellaneous enhancements | expand |
Am Donnerstag, dem 12.01.2023 um 19:25 +0200 schrieb Laurent Pinchart: > There's no need to use threaded IRQs with the PXP, as the interrupt > handler doesn't need to sleep and doesn't perform any time-consuming > operation. Switch to regular IRQ handler. > > This fixes lockups of the PXP noticed on i.MX7, where the PXP would stop > generating interrupts after a variable number of frames (from a few > dozens to a few hundreds). The root cause is however unknown. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> > --- > drivers/media/platform/nxp/imx-pxp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c > index aaafaf37439c..472907956145 100644 > --- a/drivers/media/platform/nxp/imx-pxp.c > +++ b/drivers/media/platform/nxp/imx-pxp.c > @@ -1812,8 +1812,8 @@ static int pxp_probe(struct platform_device *pdev) > > spin_lock_init(&dev->irqlock); > > - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, pxp_irq_handler, > - IRQF_ONESHOT, dev_name(&pdev->dev), dev); > + ret = devm_request_irq(&pdev->dev, irq, pxp_irq_handler, > + IRQF_TRIGGER_HIGH, dev_name(&pdev->dev), dev); Why does this explicitly configure the IRQ as active high? Those flags should normally be set via DT and only overridden by the driver if there is a very good reason to do so. Regards, Lucas > if (ret < 0) { > dev_err(&pdev->dev, "Failed to request irq: %d\n", ret); > return ret;
On Thu, Jan 12, 2023 at 06:34:36PM +0100, Lucas Stach wrote: > Am Donnerstag, dem 12.01.2023 um 19:25 +0200 schrieb Laurent Pinchart: > > There's no need to use threaded IRQs with the PXP, as the interrupt > > handler doesn't need to sleep and doesn't perform any time-consuming > > operation. Switch to regular IRQ handler. > > > > This fixes lockups of the PXP noticed on i.MX7, where the PXP would stop > > generating interrupts after a variable number of frames (from a few > > dozens to a few hundreds). The root cause is however unknown. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Michael Tretter <m.tretter@pengutronix.de> > > --- > > drivers/media/platform/nxp/imx-pxp.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c > > index aaafaf37439c..472907956145 100644 > > --- a/drivers/media/platform/nxp/imx-pxp.c > > +++ b/drivers/media/platform/nxp/imx-pxp.c > > @@ -1812,8 +1812,8 @@ static int pxp_probe(struct platform_device *pdev) > > > > spin_lock_init(&dev->irqlock); > > > > - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, pxp_irq_handler, > > - IRQF_ONESHOT, dev_name(&pdev->dev), dev); > > + ret = devm_request_irq(&pdev->dev, irq, pxp_irq_handler, > > + IRQF_TRIGGER_HIGH, dev_name(&pdev->dev), dev); > > Why does this explicitly configure the IRQ as active high? Those flags > should normally be set via DT and only overridden by the driver if > there is a very good reason to do so. Indeed, I'm not sure why I had this. I'll drop the flag. > > if (ret < 0) { > > dev_err(&pdev->dev, "Failed to request irq: %d\n", ret); > > return ret;
diff --git a/drivers/media/platform/nxp/imx-pxp.c b/drivers/media/platform/nxp/imx-pxp.c index aaafaf37439c..472907956145 100644 --- a/drivers/media/platform/nxp/imx-pxp.c +++ b/drivers/media/platform/nxp/imx-pxp.c @@ -1812,8 +1812,8 @@ static int pxp_probe(struct platform_device *pdev) spin_lock_init(&dev->irqlock); - ret = devm_request_threaded_irq(&pdev->dev, irq, NULL, pxp_irq_handler, - IRQF_ONESHOT, dev_name(&pdev->dev), dev); + ret = devm_request_irq(&pdev->dev, irq, pxp_irq_handler, + IRQF_TRIGGER_HIGH, dev_name(&pdev->dev), dev); if (ret < 0) { dev_err(&pdev->dev, "Failed to request irq: %d\n", ret); return ret;