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)
