On 11/19/2012 6:48 PM, Prabhakar Lad wrote: > From: Lad, Prabhakar <[email protected]> > > 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 <[email protected]> > Signed-off-by: Manjunath Hadli <[email protected]>
Looks good to me except some comments below. After addressing those, please feel free to add my: Acked-by: Sekhar Nori <[email protected]> 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 _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
