Re: [PATCH 03/16] media: ti-vpe: cal: catch error irqs and print errors

[Tomi Valkeinen <tomi.valkeinen@xxxxxx>]

On 16/03/2020 14:22, Laurent Pinchart wrote:
> Hi Tomi,
>
> Thank you for the patch.
>
> On Fri, Mar 13, 2020 at 01:41:08PM +0200, Tomi Valkeinen wrote:
> > CAL reports various errors via IRQs, which are not handled at all by the
> > current driver. Add code to enable and catch those IRQs and print
> > errors. This will make it much easier to notice and debug issues with
> > sensors.
> >
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxx>
> > ---
> >   drivers/media/platform/ti-vpe/cal.c      | 46 +++++++++++++++++++++++-
> >   drivers/media/platform/ti-vpe/cal_regs.h |  3 ++
> >   2 files changed, 48 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> > index b4a9f4d16ce4..f6ce0558752a 100644
> > --- a/drivers/media/platform/ti-vpe/cal.c
> > +++ b/drivers/media/platform/ti-vpe/cal.c
> > @@ -692,6 +692,21 @@ static void cal_quickdump_regs(struct cal_dev *dev)
> >    */
> >   static void enable_irqs(struct cal_ctx *ctx)
> >   {
> > +	const u32 cio_err_mask =
> > +		((1 << 20) - 1) |	/* lane errors */
> > +		BIT(27) |		/* FIFO_OVR */
> > +		BIT(28) |		/* SHORT_PACKET */
> > +		BIT(30);		/* ECC_NO_CORRECTION */
>
> Could you create macros for those bits ?
>
> > +
> > +	/* Enable CIO error irqs */
> > +	reg_write(ctx->dev, CAL_HL_IRQENABLE_SET(1),
> > +		  CAL_HL_IRQ_CIO_MASK(ctx->csi2_port));
> > +	reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_IRQENABLE(ctx->csi2_port),
> > +		  cio_err_mask);
> > +
> > +	/* Always enable OCP error */
> > +	reg_write(ctx->dev, CAL_HL_IRQENABLE_SET(1), BIT(6));
>
> And for this bit too.

Yep, will do.

> > +
> >   	/* Enable IRQ_WDMA_END 0/1 */
> >   	reg_write(ctx->dev, CAL_HL_IRQENABLE_SET(2), 1 << (ctx->csi2_port - 1));
> >   	/* Enable IRQ_WDMA_START 0/1 */
> > @@ -702,6 +717,12 @@ static void enable_irqs(struct cal_ctx *ctx)
> >   
> >   static void disable_irqs(struct cal_ctx *ctx)
> >   {
> > +	/* Disable CIO error irqs */
> > +	reg_write(ctx->dev, CAL_HL_IRQENABLE_CLR(1),
> > +		  CAL_HL_IRQ_CIO_MASK(ctx->csi2_port));
> > +	reg_write(ctx->dev, CAL_CSI2_COMPLEXIO_IRQENABLE(ctx->csi2_port),
> > +		  0);
> > +
> >   	/* Disable IRQ_WDMA_END 0/1 */
> >   	reg_write(ctx->dev, CAL_HL_IRQENABLE_CLR(2), 1 << (ctx->csi2_port - 1));
> >   	/* Disable IRQ_WDMA_START 0/1 */
> > @@ -1169,7 +1190,30 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
> >   	struct cal_dev *dev = (struct cal_dev *)data;
> >   	struct cal_ctx *ctx;
> >   	struct cal_dmaqueue *dma_q;
> > -	u32 irqst2, irqst3;
> > +	u32 irqst1, irqst2, irqst3;
> > +
> > +	irqst1 = reg_read(dev, CAL_HL_IRQSTATUS(1));
> > +	if (irqst1) {
> > +		int i;
> > +
> > +		reg_write(dev, CAL_HL_IRQSTATUS(1), irqst1);
> > +
> > +		if (irqst1 & BIT(6))
> > +			dev_err_ratelimited(&dev->pdev->dev, "OCP ERROR\n");
> > +
> > +		for (i = 1; i <= 2; ++i) {
> > +			if (irqst1 & CAL_HL_IRQ_CIO_MASK(i)) {
> > +				u32 cio_stat = reg_read(dev,
> > +							CAL_CSI2_COMPLEXIO_IRQSTATUS(i));
> > +
> > +				dev_err_ratelimited(&dev->pdev->dev,
> > +						    "CIO%d error: %#08x\n", i, cio_stat);
> > +
> > +				reg_write(dev, CAL_CSI2_COMPLEXIO_IRQSTATUS(i),
> > +					  cio_stat);
> > +			}
> > +		}
> > +	}
> >   
> >   	/* Check which DMA just finished */
> >   	irqst2 = reg_read(dev, CAL_HL_IRQSTATUS(2));
> > diff --git a/drivers/media/platform/ti-vpe/cal_regs.h b/drivers/media/platform/ti-vpe/cal_regs.h
> > index 0b76d1186074..a29198cc3efe 100644
> > --- a/drivers/media/platform/ti-vpe/cal_regs.h
> > +++ b/drivers/media/platform/ti-vpe/cal_regs.h
> > @@ -158,6 +158,9 @@
> >   #define CAL_HL_IRQ_ENABLED				0x1
> >   #define CAL_HL_IRQ_PENDING				0x1
> >   
> > +#define CAL_HL_IRQ_CIO_MASK(i)			BIT(16 + (i-1) * 8)
>
> This should be
>
> 	BIT(16 + ((i)-1) * 8)
>
> > +#define CAL_HL_IRQ_VC_MASK(i)			BIT(17 + (i-1) * 8)
>
> Same here.

Right. Thanks!

 Tomi

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki