[PATCH] rcar-csi2: restart CSI-2 link if error is detected

[PATCH] rcar-csi2: restart CSI-2 link if error is detected

[Niklas Söderlund]

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.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 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);
-- 
2.20.1

Re: [PATCH] rcar-csi2: restart CSI-2 link if error is detected

[Hans Verkuil]

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@ragnatech.se>
> ---
>  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);
> 

Re: [PATCH] rcar-csi2: restart CSI-2 link if error is detected

[Niklas Söderlund]

Hi Hans,

Thanks for your feedback.

On 2019-03-11 11:53:01 +0100, Hans Verkuil wrote:
> 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?

I think your idea sounds good and that such a functionality could be 
useful. I have a  feeling such a functionality could be related to 
notifications?

> 
> 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.

With the background you provides I agree that they should not be 
dev_err. I will update as you suggest for the next version, thanks.

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > ---
> >  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);
> > 
> 

-- 
Regards,
Niklas Söderlund

Re: [PATCH] rcar-csi2: restart CSI-2 link if error is detected

[Laurent Pinchart]

Hello Niklas,

On Tue, Mar 12, 2019 at 07:58:13PM +0100, Niklas Söderlund wrote:
> On 2019-03-11 11:53:01 +0100, Hans Verkuil wrote:
> > 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 error causes have you noticed in practice that would benefit from
this ?

> > 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.

Note that this may not always result in a CSI-2 error, the HDMI to CSI-2
bridge may continue sending valid timings with dummy (or random) data.

> > Unfortunately, we don't have such functionality, but perhaps this is something
> > to think about?
> 
> I think your idea sounds good and that such a functionality could be 
> useful. I have a  feeling such a functionality could be related to 
> notifications?
> 
> > 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.
> 
> With the background you provides I agree that they should not be 
> dev_err. I will update as you suggest for the next version, thanks.
> 
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> >> ---
> >>  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);

-- 
Regards,

Laurent Pinchart

Re: [PATCH] rcar-csi2: restart CSI-2 link if error is detected

[Niklas Söderlund]

Hi Laurent,

On 2019-03-12 22:22:01 +0200, Laurent Pinchart wrote:
> Hello Niklas,
> 
> On Tue, Mar 12, 2019 at 07:58:13PM +0100, Niklas Söderlund wrote:
> > On 2019-03-11 11:53:01 +0100, Hans Verkuil wrote:
> > > 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 error causes have you noticed in practice that would benefit from
> this ?

In practice I have not manage to produce this error using the ADV748x 
which would match a real world use-case. I have however demonstrated 
that it works by introducing a small delay between the bottom and top 
half of the irq handler and unplugging / replugging the HDMI cable to 
trigger the issue and see that it recovers.

> 
> > > 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.
> 
> Note that this may not always result in a CSI-2 error, the HDMI to CSI-2
> bridge may continue sending valid timings with dummy (or random) data.
> 
> > > Unfortunately, we don't have such functionality, but perhaps this is something
> > > to think about?
> > 
> > I think your idea sounds good and that such a functionality could be 
> > useful. I have a  feeling such a functionality could be related to 
> > notifications?
> > 
> > > 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.
> > 
> > With the background you provides I agree that they should not be 
> > dev_err. I will update as you suggest for the next version, thanks.
> > 
> > >> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > >> ---
> > >>  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);
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund