[PATCH v2 0/3] rcar-csi2: Update start procedures to latest revision of datasheet

[PATCH v2 0/3] rcar-csi2: Update start procedures to latest revision of datasheet

[Niklas Söderlund]

Hi,

This series update the driver to match changes in the latest datasheet
(rev 1.50). All changes are related to register setup when starting the
stream.

This series depends on [PATCH v2] rcar-csi2: Propagate the FLD signal 
for NTSC and PAL.

Niklas Söderlund (3):
  rcar-csi2: Update V3M and E3 start procedure
  rcar-csi2: Update start procedure for H3 ES2
  rcar-csi2: Move setting of Field Detection Control Register

 drivers/media/platform/rcar-vin/rcar-csi2.c | 51 +++++++++++++++++----
 1 file changed, 41 insertions(+), 10 deletions(-)

-- 
2.21.0

[PATCH v2 1/3] rcar-csi2: Update V3M and E3 start procedure

[Niklas Söderlund]

The latest datasheet (rev 1.50) updates the start procedure for V3M and
E3. Update the driver to match these changes.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index d9b29dbbcc2949de..6be81d4839f35a0e 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -922,11 +922,11 @@ static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
 static int rcsi2_confirm_start_v3m_e3(struct rcar_csi2 *priv)
 {
 	static const struct phtw_value step1[] = {
-		{ .data = 0xed, .code = 0x34 },
-		{ .data = 0xed, .code = 0x44 },
-		{ .data = 0xed, .code = 0x54 },
-		{ .data = 0xed, .code = 0x84 },
-		{ .data = 0xed, .code = 0x94 },
+		{ .data = 0xee, .code = 0x34 },
+		{ .data = 0xee, .code = 0x44 },
+		{ .data = 0xee, .code = 0x54 },
+		{ .data = 0xee, .code = 0x84 },
+		{ .data = 0xee, .code = 0x94 },
 		{ /* sentinel */ },
 	};
 
-- 
2.21.0

Re: [PATCH v2 1/3] rcar-csi2: Update V3M and E3 start procedure

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sat, Mar 09, 2019 at 12:57:00AM +0100, Niklas Söderlund wrote:
> The latest datasheet (rev 1.50) updates the start procedure for V3M and
> E3. Update the driver to match these changes.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>

This matches the datasheet, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index d9b29dbbcc2949de..6be81d4839f35a0e 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -922,11 +922,11 @@ static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
>  static int rcsi2_confirm_start_v3m_e3(struct rcar_csi2 *priv)
>  {
>  	static const struct phtw_value step1[] = {
> -		{ .data = 0xed, .code = 0x34 },
> -		{ .data = 0xed, .code = 0x44 },
> -		{ .data = 0xed, .code = 0x54 },
> -		{ .data = 0xed, .code = 0x84 },
> -		{ .data = 0xed, .code = 0x94 },
> +		{ .data = 0xee, .code = 0x34 },
> +		{ .data = 0xee, .code = 0x44 },
> +		{ .data = 0xee, .code = 0x54 },
> +		{ .data = 0xee, .code = 0x84 },
> +		{ .data = 0xee, .code = 0x94 },
>  		{ /* sentinel */ },
>  	};
>  

-- 
Regards,

Laurent Pinchart

[PATCH v2 2/3] rcar-csi2: Update start procedure for H3 ES2

[Niklas Söderlund]

Latest information from hardware engineers reveals that H3 ES2 and ES3
behave differently when working with link speeds bellow 250 Mpbs.
Add a SoC match for H3 ES2.* and use the correct startup sequence.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 39 ++++++++++++++++++---
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 6be81d4839f35a0e..07d5c8c66b7cd382 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -914,6 +914,25 @@ static int rcsi2_init_phtw_h3_v3h_m3n(struct rcar_csi2 *priv, unsigned int mbps)
 	return rcsi2_phtw_write_array(priv, step2);
 }
 
+static int rcsi2_init_phtw_h3es2(struct rcar_csi2 *priv, unsigned int mbps)
+{
+	static const struct phtw_value step1[] = {
+		{ .data = 0xcc, .code = 0xe2 },
+		{ .data = 0x01, .code = 0xe3 },
+		{ .data = 0x11, .code = 0xe4 },
+		{ .data = 0x01, .code = 0xe5 },
+		{ .data = 0x10, .code = 0x04 },
+		{ .data = 0x38, .code = 0x08 },
+		{ .data = 0x01, .code = 0x00 },
+		{ .data = 0x4b, .code = 0xac },
+		{ .data = 0x03, .code = 0x00 },
+		{ .data = 0x80, .code = 0x07 },
+		{ /* sentinel */ },
+	};
+
+	return rcsi2_phtw_write_array(priv, step1);
+}
+
 static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
 {
 	return rcsi2_phtw_write_mbps(priv, mbps, phtw_mbps_v3m_e3, 0x44);
@@ -976,6 +995,14 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
 	.num_channels = 4,
 };
 
+static const struct rcar_csi2_info rcar_csi2_info_r8a7795es2 = {
+	.init_phtw = rcsi2_init_phtw_h3es2,
+	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
+	.csi0clkfreqrange = 0x20,
+	.num_channels = 4,
+	.clear_ulps = true,
+};
+
 static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
 	.hsfreqrange = hsfreqrange_m3w_h3es1,
 	.num_channels = 4,
@@ -1041,11 +1068,15 @@ static const struct of_device_id rcar_csi2_of_table[] = {
 };
 MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
 
-static const struct soc_device_attribute r8a7795es1[] = {
+static const struct soc_device_attribute r8a7795[] = {
 	{
 		.soc_id = "r8a7795", .revision = "ES1.*",
 		.data = &rcar_csi2_info_r8a7795es1,
 	},
+	{
+		.soc_id = "r8a7795", .revision = "ES2.*",
+		.data = &rcar_csi2_info_r8a7795es2,
+	},
 	{ /* sentinel */ },
 };
 
@@ -1063,10 +1094,10 @@ static int rcsi2_probe(struct platform_device *pdev)
 	priv->info = of_device_get_match_data(&pdev->dev);
 
 	/*
-	 * r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
-	 * have it's own compatible string.
+	 * The different ES versions of r8a7795 (H3) behave differently but
+	 * share the same compatible string.
 	 */
-	attr = soc_device_match(r8a7795es1);
+	attr = soc_device_match(r8a7795);
 	if (attr)
 		priv->info = attr->data;
 
-- 
2.21.0

Re: [PATCH v2 2/3] rcar-csi2: Update start procedure for H3 ES2

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sat, Mar 09, 2019 at 12:57:01AM +0100, Niklas Söderlund wrote:
> Latest information from hardware engineers reveals that H3 ES2 and ES3
> behave differently when working with link speeds bellow 250 Mpbs.
> Add a SoC match for H3 ES2.* and use the correct startup sequence.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 39 ++++++++++++++++++---
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 6be81d4839f35a0e..07d5c8c66b7cd382 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -914,6 +914,25 @@ static int rcsi2_init_phtw_h3_v3h_m3n(struct rcar_csi2 *priv, unsigned int mbps)
>  	return rcsi2_phtw_write_array(priv, step2);
>  }
>  
> +static int rcsi2_init_phtw_h3es2(struct rcar_csi2 *priv, unsigned int mbps)
> +{
> +	static const struct phtw_value step1[] = {
> +		{ .data = 0xcc, .code = 0xe2 },
> +		{ .data = 0x01, .code = 0xe3 },
> +		{ .data = 0x11, .code = 0xe4 },
> +		{ .data = 0x01, .code = 0xe5 },
> +		{ .data = 0x10, .code = 0x04 },
> +		{ .data = 0x38, .code = 0x08 },
> +		{ .data = 0x01, .code = 0x00 },
> +		{ .data = 0x4b, .code = 0xac },
> +		{ .data = 0x03, .code = 0x00 },
> +		{ .data = 0x80, .code = 0x07 },
> +		{ /* sentinel */ },
> +	};
> +
> +	return rcsi2_phtw_write_array(priv, step1);

Another option would have been to condition the mbps check in
rcsi2_init_phtw_h3_v3h_m3n() to the ES version, which would save a bit
of memory as we could remove the above table, but we would need to add a
field to the rcar_csi2_info structure so we may not save much in the
end.

I wonder, however, if you could move the step1 and step2 tables out of
rcsi2_init_phtw_h3_v3h_m3n() and reuse them here, or possibly create a
__rcsi2_init_phtw_h3_v3h_m3n() with two rcsi2_init_phtw_h3_v3h_m3n() and
wrappers rcsi2_init_phtw_h3es2() wrappers.

> +}
> +
>  static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
>  {
>  	return rcsi2_phtw_write_mbps(priv, mbps, phtw_mbps_v3m_e3, 0x44);
> @@ -976,6 +995,14 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
>  	.num_channels = 4,
>  };
>  
> +static const struct rcar_csi2_info rcar_csi2_info_r8a7795es2 = {
> +	.init_phtw = rcsi2_init_phtw_h3es2,
> +	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
> +	.csi0clkfreqrange = 0x20,
> +	.num_channels = 4,
> +	.clear_ulps = true,
> +};
> +
>  static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
>  	.hsfreqrange = hsfreqrange_m3w_h3es1,
>  	.num_channels = 4,
> @@ -1041,11 +1068,15 @@ static const struct of_device_id rcar_csi2_of_table[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
>  
> -static const struct soc_device_attribute r8a7795es1[] = {
> +static const struct soc_device_attribute r8a7795[] = {
>  	{
>  		.soc_id = "r8a7795", .revision = "ES1.*",
>  		.data = &rcar_csi2_info_r8a7795es1,
>  	},
> +	{
> +		.soc_id = "r8a7795", .revision = "ES2.*",
> +		.data = &rcar_csi2_info_r8a7795es2,
> +	},
>  	{ /* sentinel */ },
>  };
>  
> @@ -1063,10 +1094,10 @@ static int rcsi2_probe(struct platform_device *pdev)
>  	priv->info = of_device_get_match_data(&pdev->dev);
>  
>  	/*
> -	 * r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
> -	 * have it's own compatible string.
> +	 * The different ES versions of r8a7795 (H3) behave differently but
> +	 * share the same compatible string.
>  	 */
> -	attr = soc_device_match(r8a7795es1);
> +	attr = soc_device_match(r8a7795);
>  	if (attr)
>  		priv->info = attr->data;
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 2/3] rcar-csi2: Update start procedure for H3 ES2

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2019-03-11 11:27:25 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sat, Mar 09, 2019 at 12:57:01AM +0100, Niklas Söderlund wrote:
> > Latest information from hardware engineers reveals that H3 ES2 and ES3
> > behave differently when working with link speeds bellow 250 Mpbs.
> > Add a SoC match for H3 ES2.* and use the correct startup sequence.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 39 ++++++++++++++++++---
> >  1 file changed, 35 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 6be81d4839f35a0e..07d5c8c66b7cd382 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -914,6 +914,25 @@ static int rcsi2_init_phtw_h3_v3h_m3n(struct rcar_csi2 *priv, unsigned int mbps)
> >  	return rcsi2_phtw_write_array(priv, step2);
> >  }
> >  
> > +static int rcsi2_init_phtw_h3es2(struct rcar_csi2 *priv, unsigned int mbps)
> > +{
> > +	static const struct phtw_value step1[] = {
> > +		{ .data = 0xcc, .code = 0xe2 },
> > +		{ .data = 0x01, .code = 0xe3 },
> > +		{ .data = 0x11, .code = 0xe4 },
> > +		{ .data = 0x01, .code = 0xe5 },
> > +		{ .data = 0x10, .code = 0x04 },
> > +		{ .data = 0x38, .code = 0x08 },
> > +		{ .data = 0x01, .code = 0x00 },
> > +		{ .data = 0x4b, .code = 0xac },
> > +		{ .data = 0x03, .code = 0x00 },
> > +		{ .data = 0x80, .code = 0x07 },
> > +		{ /* sentinel */ },
> > +	};
> > +
> > +	return rcsi2_phtw_write_array(priv, step1);
> 
> Another option would have been to condition the mbps check in
> rcsi2_init_phtw_h3_v3h_m3n() to the ES version, which would save a bit
> of memory as we could remove the above table, but we would need to add a
> field to the rcar_csi2_info structure so we may not save much in the
> end.

I agree this would not be my preferred solution.

> 
> I wonder, however, if you could move the step1 and step2 tables out of
> rcsi2_init_phtw_h3_v3h_m3n() and reuse them here, or possibly create a
> __rcsi2_init_phtw_h3_v3h_m3n() with two rcsi2_init_phtw_h3_v3h_m3n() and
> wrappers rcsi2_init_phtw_h3es2() wrappers.

This feels like a nicer solution, will use it in next version. Thanks 
for the good suggestion.

> 
> > +}
> > +
> >  static int rcsi2_init_phtw_v3m_e3(struct rcar_csi2 *priv, unsigned int mbps)
> >  {
> >  	return rcsi2_phtw_write_mbps(priv, mbps, phtw_mbps_v3m_e3, 0x44);
> > @@ -976,6 +995,14 @@ static const struct rcar_csi2_info rcar_csi2_info_r8a7795es1 = {
> >  	.num_channels = 4,
> >  };
> >  
> > +static const struct rcar_csi2_info rcar_csi2_info_r8a7795es2 = {
> > +	.init_phtw = rcsi2_init_phtw_h3es2,
> > +	.hsfreqrange = hsfreqrange_h3_v3h_m3n,
> > +	.csi0clkfreqrange = 0x20,
> > +	.num_channels = 4,
> > +	.clear_ulps = true,
> > +};
> > +
> >  static const struct rcar_csi2_info rcar_csi2_info_r8a7796 = {
> >  	.hsfreqrange = hsfreqrange_m3w_h3es1,
> >  	.num_channels = 4,
> > @@ -1041,11 +1068,15 @@ static const struct of_device_id rcar_csi2_of_table[] = {
> >  };
> >  MODULE_DEVICE_TABLE(of, rcar_csi2_of_table);
> >  
> > -static const struct soc_device_attribute r8a7795es1[] = {
> > +static const struct soc_device_attribute r8a7795[] = {
> >  	{
> >  		.soc_id = "r8a7795", .revision = "ES1.*",
> >  		.data = &rcar_csi2_info_r8a7795es1,
> >  	},
> > +	{
> > +		.soc_id = "r8a7795", .revision = "ES2.*",
> > +		.data = &rcar_csi2_info_r8a7795es2,
> > +	},
> >  	{ /* sentinel */ },
> >  };
> >  
> > @@ -1063,10 +1094,10 @@ static int rcsi2_probe(struct platform_device *pdev)
> >  	priv->info = of_device_get_match_data(&pdev->dev);
> >  
> >  	/*
> > -	 * r8a7795 ES1.x behaves differently than the ES2.0+ but doesn't
> > -	 * have it's own compatible string.
> > +	 * The different ES versions of r8a7795 (H3) behave differently but
> > +	 * share the same compatible string.
> >  	 */
> > -	attr = soc_device_match(r8a7795es1);
> > +	attr = soc_device_match(r8a7795);
> >  	if (attr)
> >  		priv->info = attr->data;
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund

[PATCH v2 3/3] rcar-csi2: Move setting of Field Detection Control Register

[Niklas Söderlund]

Latest datasheet (rev 1.50) clarifies that the FLD register should be
set after LINKCNT.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index 07d5c8c66b7cd382..077e0d344b395b54 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -529,7 +529,6 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
 	rcsi2_write(priv, PHTC_REG, 0);
 
 	/* Configure */
-	rcsi2_write(priv, FLD_REG, fld);
 	rcsi2_write(priv, VCDT_REG, vcdt);
 	if (vcdt2)
 		rcsi2_write(priv, VCDT2_REG, vcdt2);
@@ -560,6 +559,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
 	rcsi2_write(priv, PHYCNT_REG, phycnt);
 	rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
 		    LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
+	rcsi2_write(priv, FLD_REG, fld);
 	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
 	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
 
-- 
2.21.0

Re: [PATCH v2 3/3] rcar-csi2: Move setting of Field Detection Control Register

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Sat, Mar 09, 2019 at 12:57:02AM +0100, Niklas Söderlund wrote:
> Latest datasheet (rev 1.50) clarifies that the FLD register should be
> set after LINKCNT.

Wasn't this already the case in rev 1.00 ?

> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 07d5c8c66b7cd382..077e0d344b395b54 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -529,7 +529,6 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	rcsi2_write(priv, PHTC_REG, 0);
>  
>  	/* Configure */
> -	rcsi2_write(priv, FLD_REG, fld);
>  	rcsi2_write(priv, VCDT_REG, vcdt);
>  	if (vcdt2)
>  		rcsi2_write(priv, VCDT2_REG, vcdt2);
> @@ -560,6 +559,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	rcsi2_write(priv, PHYCNT_REG, phycnt);
>  	rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
>  		    LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> +	rcsi2_write(priv, FLD_REG, fld);
>  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
>  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
>  

The change looks reasonable, so after updating the commit message,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 3/3] rcar-csi2: Move setting of Field Detection Control Register

[Niklas Söderlund]

Hi Laurent,

Thanks for your feedback.

On 2019-03-11 11:39:10 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Sat, Mar 09, 2019 at 12:57:02AM +0100, Niklas Söderlund wrote:
> > Latest datasheet (rev 1.50) clarifies that the FLD register should be
> > set after LINKCNT.
> 
> Wasn't this already the case in rev 1.00 ?

Yes it is, thanks for noticing, will fix for next version.

> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> > ---
> >  drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > index 07d5c8c66b7cd382..077e0d344b395b54 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -529,7 +529,6 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> >  	rcsi2_write(priv, PHTC_REG, 0);
> >  
> >  	/* Configure */
> > -	rcsi2_write(priv, FLD_REG, fld);
> >  	rcsi2_write(priv, VCDT_REG, vcdt);
> >  	if (vcdt2)
> >  		rcsi2_write(priv, VCDT2_REG, vcdt2);
> > @@ -560,6 +559,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
> >  	rcsi2_write(priv, PHYCNT_REG, phycnt);
> >  	rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
> >  		    LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> > +	rcsi2_write(priv, FLD_REG, fld);
> >  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
> >  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
> >  
> 
> The change looks reasonable, so after updating the commit message,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Niklas Söderlund

Re: [PATCH v2 3/3] rcar-csi2: Move setting of Field Detection Control Register

[Hans Verkuil]

On 3/9/19 12:57 AM, Niklas Söderlund wrote:
> Latest datasheet (rev 1.50) clarifies that the FLD register should be
> set after LINKCNT.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> ---
>  drivers/media/platform/rcar-vin/rcar-csi2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
> index 07d5c8c66b7cd382..077e0d344b395b54 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -529,7 +529,6 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	rcsi2_write(priv, PHTC_REG, 0);
>  
>  	/* Configure */
> -	rcsi2_write(priv, FLD_REG, fld);
>  	rcsi2_write(priv, VCDT_REG, vcdt);
>  	if (vcdt2)
>  		rcsi2_write(priv, VCDT2_REG, vcdt2);
> @@ -560,6 +559,7 @@ static int rcsi2_start_receiver(struct rcar_csi2 *priv)
>  	rcsi2_write(priv, PHYCNT_REG, phycnt);
>  	rcsi2_write(priv, LINKCNT_REG, LINKCNT_MONITOR_EN |
>  		    LINKCNT_REG_MONI_PACT_EN | LINKCNT_ICLK_NONSTOP);
> +	rcsi2_write(priv, FLD_REG, fld);
>  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ);
>  	rcsi2_write(priv, PHYCNT_REG, phycnt | PHYCNT_SHUTDOWNZ | PHYCNT_RSTZ);
>  
> 

This patch doesn't apply because the fld variable no longer exists.

I think the fix is easy, but I prefer it if you rebase and repost this patch.
I've merged the others, so it's just this patch that's the problem.

Regards,

	Hans