Re: [PATCH] davinci: vpbe: pass different platform names to handledifferent ip's

[Sekhar Nori]

On 11/19/2012 6:48 PM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@xxxxxx>
> 
> The vpbe driver can handle different platforms DM644X, DM36X and
> DM355. To differentiate between this platforms venc_type/vpbe_type
> was passed as part of platform data which was incorrect. The correct
> way to differentiate to handle this case is by passing different
> platform names.
> 
> This patch creates platform_device_id[] array supporting different
> platforms and assigns id_table to the platform driver, and finally
> in the probe gets the actual device by using platform_get_device_id()
> and gets the appropriate driver data for that platform.
> 
> Taking this approach will also make the DT transition easier.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@xxxxxx>
> Signed-off-by: Manjunath Hadli <manjunath.hadli@xxxxxx>

Looks good to me except some comments below. After addressing those,
please feel free to add my:

Acked-by: Sekhar Nori <nsekhar@xxxxxx>

I assume you want to merge this from media tree to manage dependencies?

> ---
>  arch/arm/mach-davinci/board-dm644x-evm.c      |    8 ++--
>  arch/arm/mach-davinci/dm644x.c                |    7 +--
>  drivers/media/platform/davinci/vpbe.c         |    9 +++-
>  drivers/media/platform/davinci/vpbe_display.c |    4 +-
>  drivers/media/platform/davinci/vpbe_osd.c     |   27 +++++++++-
>  drivers/media/platform/davinci/vpbe_venc.c    |   67 +++++++++++++++++--------
>  include/media/davinci/vpbe_osd.h              |    5 +-
>  include/media/davinci/vpbe_venc.h             |    5 +-
>  8 files changed, 94 insertions(+), 38 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/board-dm644x-evm.c b/arch/arm/mach-davinci/board-dm644x-evm.c
> index f22572ce..b00ade4 100644
> --- a/arch/arm/mach-davinci/board-dm644x-evm.c
> +++ b/arch/arm/mach-davinci/board-dm644x-evm.c
> @@ -689,7 +689,7 @@ static struct vpbe_output dm644xevm_vpbe_outputs[] = {
>  			.std		= VENC_STD_ALL,
>  			.capabilities	= V4L2_OUT_CAP_STD,
>  		},
> -		.subdev_name	= VPBE_VENC_SUBDEV_NAME,
> +		.subdev_name	= DM644X_VPBE_VENC_SUBDEV_NAME,
>  		.default_mode	= "ntsc",
>  		.num_modes	= ARRAY_SIZE(dm644xevm_enc_std_timing),
>  		.modes		= dm644xevm_enc_std_timing,
> @@ -701,7 +701,7 @@ static struct vpbe_output dm644xevm_vpbe_outputs[] = {
>  			.type		= V4L2_OUTPUT_TYPE_ANALOG,
>  			.capabilities	= V4L2_OUT_CAP_DV_TIMINGS,
>  		},
> -		.subdev_name	= VPBE_VENC_SUBDEV_NAME,
> +		.subdev_name	= DM644X_VPBE_VENC_SUBDEV_NAME,
>  		.default_mode	= "480p59_94",
>  		.num_modes	= ARRAY_SIZE(dm644xevm_enc_preset_timing),
>  		.modes		= dm644xevm_enc_preset_timing,
> @@ -712,10 +712,10 @@ static struct vpbe_config dm644xevm_display_cfg = {
>  	.module_name	= "dm644x-vpbe-display",
>  	.i2c_adapter_id	= 1,
>  	.osd		= {
> -		.module_name	= VPBE_OSD_SUBDEV_NAME,
> +		.module_name	= DM644X_VPBE_OSD_SUBDEV_NAME,
>  	},
>  	.venc		= {
> -		.module_name	= VPBE_VENC_SUBDEV_NAME,
> +		.module_name	= DM644X_VPBE_VENC_SUBDEV_NAME,
>  	},
>  	.num_outputs	= ARRAY_SIZE(dm644xevm_vpbe_outputs),
>  	.outputs	= dm644xevm_vpbe_outputs,
> diff --git a/arch/arm/mach-davinci/dm644x.c b/arch/arm/mach-davinci/dm644x.c
> index cd0c8b1..7b785ec 100644
> --- a/arch/arm/mach-davinci/dm644x.c
> +++ b/arch/arm/mach-davinci/dm644x.c
> @@ -670,11 +670,11 @@ static struct resource dm644x_osd_resources[] = {
>  };
>  
>  static struct osd_platform_data dm644x_osd_data = {
> -	.vpbe_type     = VPBE_VERSION_1,
> +	.field_inv_wa_enable = 0,

Stray change in the patch? You anyway do not need to zero initialize.

>  };
>  
>  static struct platform_device dm644x_osd_dev = {
> -	.name		= VPBE_OSD_SUBDEV_NAME,
> +	.name		= DM644X_VPBE_OSD_SUBDEV_NAME,
>  	.id		= -1,
>  	.num_resources	= ARRAY_SIZE(dm644x_osd_resources),
>  	.resource	= dm644x_osd_resources,
> @@ -752,12 +752,11 @@ static struct platform_device dm644x_vpbe_display = {
>  };
>  
>  static struct venc_platform_data dm644x_venc_pdata = {
> -	.venc_type	= VPBE_VERSION_1,
>  	.setup_clock	= dm644x_venc_setup_clock,
>  };
>  
>  static struct platform_device dm644x_venc_dev = {
> -	.name		= VPBE_VENC_SUBDEV_NAME,
> +	.name		= DM644X_VPBE_VENC_SUBDEV_NAME,
>  	.id		= -1,
>  	.num_resources	= ARRAY_SIZE(dm644x_venc_resources),
>  	.resource	= dm644x_venc_resources,
> diff --git a/drivers/media/platform/davinci/vpbe.c b/drivers/media/platform/davinci/vpbe.c
> index 7f5cf9b..0dd3c62 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -558,9 +558,14 @@ static int platform_device_get(struct device *dev, void *data)
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct vpbe_device *vpbe_dev = data;
>  
> -	if (strcmp("vpbe-osd", pdev->name) == 0)
> +	if (!strcmp(DM644X_VPBE_OSD_SUBDEV_NAME, pdev->name) ||
> +	    !strcmp(DM365_VPBE_OSD_SUBDEV_NAME, pdev->name) ||
> +	    !strcmp(DM355_VPBE_OSD_SUBDEV_NAME, pdev->name))

How about using strstr("vpbe-osd", pdev->name) != NULL instead? Here and
in multiple other places in the patch.

> @@ -341,8 +356,8 @@ static int venc_set_576p50(struct v4l2_subdev *sd)
>  
>  	v4l2_dbg(debug, 2, sd, "venc_set_576p50\n");
>  
> -	if ((pdata->venc_type != VPBE_VERSION_1) &&
> -	  (pdata->venc_type != VPBE_VERSION_2))
> +	if ((venc->venc_type != VPBE_VERSION_1) &&
> +	  (venc->venc_type != VPBE_VERSION_2))

The broken line should be aligned correctly.

Thanks,
Sekhar

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/