Re: [PATCH v1 05/16] OMAP3 DSS Driver register moved to mach_omap2

[Thomas Petazzoni <thomas.petazzoni@free-electrons.com>]

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 <svadivu@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.

> +};
> +
> +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.

> +{
> +

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.

> +	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 ?

Regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com