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@xxxxxxxxxxxx>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
> ---
> 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
