> -----Original Message-----
> From: Thomas Petazzoni [mailto:thomas.petazz...@free-electrons.com]
> Sent: Friday, October 08, 2010 12:36 AM
> To: Guruswamy, Senthilvadivu
> Cc: khil...@deeprootsystems.com; tomi.valkei...@nokia.com; p...@pwsan.com;
> Hiremath, Vaibhav; linux-omap@vger.kernel.org
> Subject: Re: [PATCH v1 05/16] OMAP3 DSS Driver register moved to
> mach_omap2
>
> Hello,
>
> The patch title is a bit misleading, maybe it should rather be
> something like "Move OMAP3 DSS driver registration to
> mach-omap2/devices.c"/
>
> On Wed, 6 Oct 2010 16:44:48 +0530
> Guruswamy Senthilvadivu <svad...@ti.com> wrote:
>
> > /*---------------------------------------------------------------------
> ------*/
> > +#ifdef CONFIG_OMAP2_DSS
> > +
> > +static struct platform_device omap_display_device = {
> > + .name = "omapdisplay",
> > + .id = -1,
> > + .dev = {
> > + .platform_data = NULL,
> > + },
>
> This .dev = {} part is useless. The compiler will automatically
> initialize unset fields to zero.
>
[Senthil] Thanks. When I do code movement I don't change it.
I can submit additional patches to improvise the code.
> > +};
> > +
> > +void __init omap_display_init(struct omap_dss_board_info
> > + *board_data)
>
> *board_data should probably be on the same line as the argument type.
>
[Senthil] Taken.
> > +{
> > +
>
> The general kernel coding style seems to be that there shouldn't be
> such empty newlines at the beginning of functions.
>
> > + omap_display_device.dev.platform_data = board_data;
> > +
> > + if (platform_device_register(&omap_display_device) < 0)
> > + printk(KERN_ERR "Unable to register OMAP-Display device\n");
> > +
> > +
>
> Unneeded newlines.
>
[Senthil] Taken.
> > + return ;
>
> This return is not needed, we are at the end of a void function.
>
> > @@ -712,7 +712,7 @@ static struct platform_driver omap_dss_driver = {
> > .suspend = omap_dss_suspend,
> > .resume = omap_dss_resume,
> > .driver = {
> > - .name = "omapdss",
> > + .name = "omapdisplay",
> > .owner = THIS_MODULE,
> > },
> > };
>
> There are other boards instantiating a platform_device with the omapdss
> name, so I think this change is going to break those boards. In my
> not-so-old linux-omap tree :
>
> $ grep "\.name.*omapdss" *
> board-3430sdp.c: .name = "omapdss",
> board-am3517evm.c: .name = "omapdss",
> board-cm-t35.c: .name = "omapdss",
> board-devkit8000.c: .name = "omapdss",
> board-igep0020.c: .name = "omapdss",
> board-omap3beagle.c: .name = "omapdss",
> board-omap3evm.c: .name = "omapdss",
> board-omap3pandora.c: .name = "omapdss",
> board-omap3stalker.c: .name = "omapdss",
> board-rx51-video.c: .name = "omapdss",
>
> Shouldn't these board files also be updated to use the new
> omap_display_init() function ?
>
[Senthil] Yes, Taken. I would request for testing these additional changes.
> Regards,
>
> Thomas
> --
> Thomas Petazzoni, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
