Re: [PATCH v2] media: imx-jpeg: Disable some unused interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 06.06.2022 05:33, Ming Qian wrote:
From: Mirela Rabulea (OSS) <mirela.rabulea@xxxxxxxxxxx>
Sent: 2022年6月6日 0:43
To: Ming Qian <ming.qian@xxxxxxx>; mchehab@xxxxxxxxxx;
hverkuil-cisco@xxxxxxxxx
Cc: shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
festevam@xxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>;
linux-media@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH v2] media: imx-jpeg: Disable some unused interrupt

Hi Ming,

On 30.05.2022 10:47, Ming Qian wrote:
The interrupt STMBUF_HALF may be triggered after frame done.
It may led to system hang if driver try to access the register after
power off.
And interrupt STMBUF_HALF and STMBUF_RTND have no other effect.
So disable them and the unused interrupts.

Fixes: 2db16c6ed72ce ("media: imx-jpeg: Add V4L2 driver for i.MX8 JPEG
Encoder/Decoder")
Signed-off-by: Ming Qian <ming.qian@xxxxxxx>
---
v2
- add Fixes tag
   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.c
b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.c
index c482228262a3..258fbee7ab66 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.c
@@ -76,7 +76,7 @@ void print_wrapper_info(struct device *dev, void
__iomem *reg)

   void mxc_jpeg_enable_irq(void __iomem *reg, int slot)
   {
-	writel(0xFFFFFFFF, reg + MXC_SLOT_OFFSET(slot, SLOT_IRQ_EN));
+	writel(0xF0C, reg + MXC_SLOT_OFFSET(slot, SLOT_IRQ_EN));

There is another way, less aggressive, to go around this, disable all the
interrupts once FRMDONE is received (or some other error condition),
interrupts will get re-enabled at the next device_run. I checked this works, in
mxc_jpeg_dec_irq:
buffers_done:
	writel(0x0, reg + MXC_SLOT_OFFSET(slot, SLOT_IRQ_EN));


Hi Mirela,

Yes, I think it should work if we disable slot interrupt when frame done.
And which solution do you prefer?

Hi Ming, I don't have a strong preference, but just in case we decide to later use some of these interrupts, I would suggest to go with the disabling after FRMDONE.

Please do not forget to clean the other code related to "Instance released before the end of transaction".

Thanks,
Mirela


Ming

Either way, I would also replace this:
	if (!ctx) {
		dev_err(dev,
			"Instance released before the end of transaction.\n");
		/* soft reset only resets internal state, not registers */
		mxc_jpeg_sw_reset(reg);
		/* clear all interrupts */
		writel(0xFFFFFFFF, reg + MXC_SLOT_OFFSET(slot, SLOT_STATUS));
		goto job_unlock;
	}

With something like:
BUG_ON(!ctx)

The initial intent of this code was to cope with the same problem,
STMBUF_HALF interrupt received after FRMDONE, which could not be cleared,
but it was not done right, I can see the hang in some rare cases. We should not
run into it anymore, with interrupts disabled, either the way you proposed, or
mine.

Regards,
Mirela

   }

   void mxc_jpeg_sw_reset(void __iomem *reg)



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux