On 2/18/19 11:15 AM, Niklas Söderlund wrote:
> Restart the CSI-2 link if the CSI-2 receiver detects an error during
> reception. The driver did nothing when a link error happened and the
> data flow simply stopped without the user knowing why.
>
> Change the driver to try and recover from errors by restarting the link
> and informing the user that something is not right. For obvious reasons
> it's not possible to recover from all errors (video source disconnected
> for example) but in such cases the user is at least informed of the
> error and the same behavior of the stopped data flow is retained.
What you really would like to have is that when a CSI error is detected,
the CSI driver can ask upstream whether or not a disconnect has taken place.
If that was the case, then there is no point in restarting the CSI.
While a disconnect is very uncommon for a sensor, it is of course perfectly
normal if an HDMI-to-CSI bridge was connected to the CSI port.
Unfortunately, we don't have such functionality, but perhaps this is something
to think about?
This does mean, however, that I don't like the dev_err since it doesn't have
to be an error. I would suggest replacing the first dev_err by dev_info and
the second by dev_warn.
Regards,
Hans
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
> drivers/media/platform/rcar-vin/rcar-csi2.c | 52 ++++++++++++++++++++-
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index f90b380478775015..0506fe4106d5c012 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -84,6 +84,9 @@ struct rcar_csi2;
>
> /* Interrupt Enable */
> #define INTEN_REG 0x30
> +#define INTEN_INT_AFIFO_OF BIT(27)
> +#define INTEN_INT_ERRSOTHS BIT(4)
> +#define INTEN_INT_ERRSOTSYNCHS BIT(3)
>
> /* Interrupt Source Mask */
> #define INTCLOSE_REG 0x34
> @@ -540,6 +543,10 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> if (mbps < 0)
> return mbps;
>
> + /* Enable interrupts. */
> + rcsi2_write(priv, INTEN_REG, INTEN_INT_AFIFO_OF | INTEN_INT_ERRSOTHS
> + | INTEN_INT_ERRSOTSYNCHS);
> +
> /* Init */
> rcsi2_write(priv, TREF_REG, TREF_TREF);
> rcsi2_write(priv, PHTC_REG, 0);
> @@ -702,6 +709,43 @@ static const struct v4l2_subdev_ops rcar_csi2_subdev_ops = {
> .pad = &rcar_csi2_pad_ops,
> };
>
> +static irqreturn_t rcsi2_irq(int irq, void *data)
> +{
> + struct rcar_csi2 *priv = data;
> + u32 status, err_status;
> +
> + status = rcsi2_read(priv, INTSTATE_REG);
> + err_status = rcsi2_read(priv, INTERRSTATE_REG);
> +
> + if (!status)
> + return IRQ_HANDLED;
> +
> + rcsi2_write(priv, INTSTATE_REG, status);
> +
> + if (!err_status)
> + return IRQ_HANDLED;
> +
> + rcsi2_write(priv, INTERRSTATE_REG, err_status);
> +
> + dev_err(priv->dev, "Transfer error, restarting CSI-2 receiver\n");
> +
> + return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t rcsi2_irq_thread(int irq, void *data)
> +{
> + struct rcar_csi2 *priv = data;
> +
> + mutex_lock(&priv->lock);
> + rcsi2_stop(priv);
> + usleep_range(1000, 2000);
> + if (rcsi2_start(priv))
> + dev_err(priv->dev, "Failed to restart CSI-2 receiver\n");
> + mutex_unlock(&priv->lock);
> +
> + return IRQ_HANDLED;
> +}
> +
> /* -----------------------------------------------------------------------------
> * Async handling and registration of subdevices and links.
> */
> @@ -982,7 +1026,7 @@ static int rcsi2_probe_resources(struct rcar_csi2 *priv,
> struct platform_device *pdev)
> {
> struct resource *res;
> - int irq;
> + int irq, ret;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> priv->base = devm_ioremap_resource(&pdev->dev, res);
> @@ -993,6 +1037,12 @@ static int rcsi2_probe_resources(struct rcar_csi2 *priv,
> if (irq < 0)
> return irq;
>
> + ret = devm_request_threaded_irq(&pdev->dev, irq, rcsi2_irq,
> + rcsi2_irq_thread, IRQF_SHARED,
> + KBUILD_MODNAME, priv);
> + if (ret)
> + return ret;
> +
> priv->rstc = devm_reset_control_get(&pdev->dev, NULL);
> if (IS_ERR(priv->rstc))
> return PTR_ERR(priv->rstc);
>
