diff mbox series

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

Message ID 20190218100313.14529-3-niklas.soderlund+renesas@ragnatech.se (mailing list archive)
State Superseded
Delegated to: Kieran Bingham
Headers show
Series rcar-csi2: Update start procedures to latest revision of datasheet | expand

Commit Message

Niklas Söderlund Feb. 18, 2019, 10:03 a.m. UTC
Latest information from hardware engineers reveals that H3 ES2 and ES3
of behaves 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>
---
 drivers/media/platform/rcar-vin/rcar-csi2.c | 39 ++++++++++++++++++---
 1 file changed, 35 insertions(+), 4 deletions(-)

Comments

Kieran Bingham Feb. 18, 2019, 11:01 a.m. UTC | #1
Hi Niklas,

On 18/02/2019 10:03, Niklas Söderlund wrote:
> Latest information from hardware engineers reveals that H3 ES2 and ES3
> of behaves differently when working with link speeds bellow 250 Mpbs.

of? "of the rcar-csi2?"

> Add a SoC match for H3 ES2.* and use the correct startup sequence.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Assuming the step1 table is accurate, which I have not yet been able to
validate:

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 fbbe86a7a0fe14ab..50486301c21b4bae 100644
> --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> @@ -932,6 +932,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 */ },
> +	};
> +

This looks reasonable - but I can't identify a table to verify these values.

Is this generated from the flow charts in section 25.3.9?


> +	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);
> @@ -994,6 +1013,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,
> @@ -1059,11 +1086,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 */ },
>  };
>  
> @@ -1081,10 +1112,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) behaves differently but

s/behaves/behave/

> +	 * shares the same compatible string.

s/shares/share/

>  	 */
> -	attr = soc_device_match(r8a7795es1);
> +	attr = soc_device_match(r8a7795);
>  	if (attr)
>  		priv->info = attr->data;
>  
>
Ulrich Hecht Feb. 18, 2019, 11:12 a.m. UTC | #2
> On February 18, 2019 at 11:03 AM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote:
> 
> 
> Latest information from hardware engineers reveals that H3 ES2 and ES3
> of behaves differently when working with link speeds bellow 250 Mpbs.
> Add a SoC match for H3 ES2.* and use the correct startup sequence.

It would be helpful to explain how they behave differently. My guess is that the extra steps "Set the PHTW to H′0139 0105." and "Set the PHTW to the appropriate values for the HS reception frequency." from the flowchart can/must be omitted on ES2+, but I think it would be better if that were stated explicitly somewhere.

With that fixed:

Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>

CU
Uli
Niklas Söderlund March 7, 2019, 12:27 a.m. UTC | #3
Hi Ulrich,

Thanks for your feedback.

On 2019-02-18 12:12:19 +0100, Ulrich Hecht wrote:
> 
> > On February 18, 2019 at 11:03 AM Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote:
> > 
> > 
> > Latest information from hardware engineers reveals that H3 ES2 and ES3
> > of behaves differently when working with link speeds bellow 250 Mpbs.
> > Add a SoC match for H3 ES2.* and use the correct startup sequence.
> 
> It would be helpful to explain how they behave differently. My guess is that the extra steps "Set the PHTW to H′0139 0105." and "Set the PHTW to the appropriate values for the HS reception frequency." from the flowchart can/must be omitted on ES2+, but I think it would be better if that were stated explicitly somewhere.

I wish I could add a more descriptive message on how they changed and 
why. All I have are the register values in a flow chart. As you point 
out one can describe how the raw values are different, but that is all 
in the code. What I really would like to add is why :-)

> 
> With that fixed:
> 
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
> 
> CU
> Uli
Niklas Söderlund March 7, 2019, 12:30 a.m. UTC | #4
Hi Kieran,

Thanks for your feedback.

On 2019-02-18 11:01:51 +0000, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 18/02/2019 10:03, Niklas Söderlund wrote:
> > Latest information from hardware engineers reveals that H3 ES2 and ES3
> > of behaves differently when working with link speeds bellow 250 Mpbs.
> 
> of? "of the rcar-csi2?"

s/of// :-)

> 
> > Add a SoC match for H3 ES2.* and use the correct startup sequence.
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Assuming the step1 table is accurate, which I have not yet been able to
> validate:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

I have collected your tag with the spelling fixes bellow, thanks!

> 
> > ---
> >  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 fbbe86a7a0fe14ab..50486301c21b4bae 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-csi2.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
> > @@ -932,6 +932,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 */ },
> > +	};
> > +
> 
> This looks reasonable - but I can't identify a table to verify these values.
> 
> Is this generated from the flow charts in section 25.3.9?

Yes.

> 
> 
> > +	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);
> > @@ -994,6 +1013,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,
> > @@ -1059,11 +1086,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 */ },
> >  };
> >  
> > @@ -1081,10 +1112,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) behaves differently but
> 
> s/behaves/behave/
> 
> > +	 * shares the same compatible string.
> 
> s/shares/share/
> 
> >  	 */
> > -	attr = soc_device_match(r8a7795es1);
> > +	attr = soc_device_match(r8a7795);
> >  	if (attr)
> >  		priv->info = attr->data;
> >  
> > 
> 
> 
> -- 
> Regards
> --
> Kieran
diff mbox series

Patch

diff --git a/drivers/media/platform/rcar-vin/rcar-csi2.c b/drivers/media/platform/rcar-vin/rcar-csi2.c
index fbbe86a7a0fe14ab..50486301c21b4bae 100644
--- a/drivers/media/platform/rcar-vin/rcar-csi2.c
+++ b/drivers/media/platform/rcar-vin/rcar-csi2.c
@@ -932,6 +932,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);
@@ -994,6 +1013,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,
@@ -1059,11 +1086,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 */ },
 };
 
@@ -1081,10 +1112,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) behaves differently but
+	 * shares the same compatible string.
 	 */
-	attr = soc_device_match(r8a7795es1);
+	attr = soc_device_match(r8a7795);
 	if (attr)
 		priv->info = attr->data;