Hi Tomi,
Thank you for the patch.
On Mon, Apr 12, 2021 at 02:34:43PM +0300, Tomi Valkeinen wrote:
> CAL driver currently ignores VC related errors. To help catch error
> conditions, enable all the VC error interrupts and handle them in the
> interrupt handler by printing an error.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/platform/ti-vpe/cal-camerarx.c | 23 ++++++++++++++++----
> drivers/media/platform/ti-vpe/cal.c | 9 ++++++++
> 2 files changed, 28 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/platform/ti-vpe/cal-camerarx.c b/drivers/media/platform/ti-vpe/cal-camerarx.c
> index 974fcbb19547..0354f311c5d2 100644
> --- a/drivers/media/platform/ti-vpe/cal-camerarx.c
> +++ b/drivers/media/platform/ti-vpe/cal-camerarx.c
> @@ -226,24 +226,39 @@ static void cal_camerarx_enable_irqs(struct cal_camerarx *phy)
> CAL_CSI2_COMPLEXIO_IRQ_FIFO_OVR_MASK |
> CAL_CSI2_COMPLEXIO_IRQ_SHORT_PACKET_MASK |
> CAL_CSI2_COMPLEXIO_IRQ_ECC_NO_CORRECTION_MASK;
> -
> - /* Enable CIO error IRQs. */
> + const u32 vc_err_mask =
> + CAL_CSI2_VC_IRQ_CS_IRQ_MASK(0) |
> + CAL_CSI2_VC_IRQ_CS_IRQ_MASK(1) |
> + CAL_CSI2_VC_IRQ_CS_IRQ_MASK(2) |
> + CAL_CSI2_VC_IRQ_CS_IRQ_MASK(3) |
> + CAL_CSI2_VC_IRQ_ECC_CORRECTION_IRQ_MASK(0) |
> + CAL_CSI2_VC_IRQ_ECC_CORRECTION_IRQ_MASK(1) |
> + CAL_CSI2_VC_IRQ_ECC_CORRECTION_IRQ_MASK(2) |
> + CAL_CSI2_VC_IRQ_ECC_CORRECTION_IRQ_MASK(3);
> +
> + /* Enable CIO & VC error IRQs. */
> cal_write(phy->cal, CAL_HL_IRQENABLE_SET(0),
> - CAL_HL_IRQ_CIO_MASK(phy->instance));
> + CAL_HL_IRQ_CIO_MASK(phy->instance) | CAL_HL_IRQ_VC_MASK(phy->instance));
Line wrap ? Same in multiple places below. I know there's no strict 80
columns limit anymore, but I don't think longer lines help with
readability in this patch (not to mention the coding style inconsistency
with the rest of the driver).
Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> cal_write(phy->cal, CAL_CSI2_COMPLEXIO_IRQENABLE(phy->instance),
> cio_err_mask);
> + cal_write(phy->cal, CAL_CSI2_VC_IRQENABLE(phy->instance),
> + vc_err_mask);
> }
>
> static void cal_camerarx_disable_irqs(struct cal_camerarx *phy)
> {
> /* Disable CIO error irqs */
> cal_write(phy->cal, CAL_HL_IRQENABLE_CLR(0),
> - CAL_HL_IRQ_CIO_MASK(phy->instance));
> + CAL_HL_IRQ_CIO_MASK(phy->instance) | CAL_HL_IRQ_VC_MASK(phy->instance));
> cal_write(phy->cal, CAL_CSI2_COMPLEXIO_IRQENABLE(phy->instance), 0);
> + cal_write(phy->cal, CAL_CSI2_VC_IRQENABLE(phy->instance), 0);
> }
>
> static void cal_camerarx_ppi_enable(struct cal_camerarx *phy)
> {
> + cal_write_field(phy->cal, CAL_CSI2_PPI_CTRL(phy->instance),
> + 1, CAL_CSI2_PPI_CTRL_ECC_EN_MASK);
> +
> cal_write_field(phy->cal, CAL_CSI2_PPI_CTRL(phy->instance),
> 1, CAL_CSI2_PPI_CTRL_IF_EN_MASK);
> }
> diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
> index 0abcc83841c6..092041ddbcfb 100644
> --- a/drivers/media/platform/ti-vpe/cal.c
> +++ b/drivers/media/platform/ti-vpe/cal.c
> @@ -577,6 +577,15 @@ static irqreturn_t cal_irq(int irq_cal, void *data)
> cal_write(cal, CAL_CSI2_COMPLEXIO_IRQSTATUS(i),
> cio_stat);
> }
> +
> + if (status & CAL_HL_IRQ_VC_MASK(i)) {
> + u32 vc_stat = cal_read(cal, CAL_CSI2_VC_IRQSTATUS(i));
> +
> + dev_err_ratelimited(cal->dev,
> + "CIO%u VC error: %#08x\n", i, vc_stat);
> +
> + cal_write(cal, CAL_CSI2_VC_IRQSTATUS(i), vc_stat);
> + }
> }
> }
>
--
Regards,
Laurent Pinchart
