This patch can be superseded now. It was replaced by another patch https://patchwork.linuxtv.org/project/linux-media/patch/20220607072315.23209-1-ming.qian@xxxxxxx/ > From: Ming Qian > Sent: 2022年6月7日 9:49 > To: Mirela Rabulea (OSS) <mirela.rabulea@xxxxxxxxxxx>; > 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 > > > From: Mirela Rabulea (OSS) <mirela.rabulea@xxxxxxxxxxx> > > Sent: 2022年6月6日 17:41 > > 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 > > > > > > > > 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 > > > > OK, I'll make another patch that disable slot interrupt when frame done, and > clean the code. > > > > > > > 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)
