Re: [PATCH v1 07/16] OMAP3: hwmod DSS: Create platform_driver for each DSS HW IP

2010-10-08 Thread Thomas Petazzoni
Hello,

On Fri, 8 Oct 2010 12:41:35 +0530
"Guruswamy, Senthilvadivu"  wrote:

> [Senthil] I put it above as part of all the static methods and
> structures. Will look at the other omap drivers before I change.

Yes, but the platform_driver structure, ->probe() and ->remove()
methods are not usually at a random place with all the other static
methods and structure definitions. They are located *just* above the
module init/cleanup routines.

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


RE: [PATCH v1 07/16] OMAP3: hwmod DSS: Create platform_driver for each DSS HW IP

2010-10-08 Thread Guruswamy, Senthilvadivu


> -Original Message-
> From: Thomas Petazzoni [mailto:thomas.petazz...@free-electrons.com]
> Sent: Friday, October 08, 2010 1:20 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 07/16] OMAP3: hwmod DSS: Create platform_driver for
> each DSS HW IP
> 
> Hello Senthil,
> 
> I forgot one comment: in most Linux drivers, it is customary to put the
> platform_driver definition just above the driver initialization
> function, both of them at the end of the file. Of course, it's nothing
> mandatory, but most of the drivers are organized this way, making it
> easy for developers to find their way in all drivers.
> 
> So, in other words, this:
> 
[Senthil] I put it above as part of all the static methods and structures.
Will look at the other omap drivers before I change.
> > +/* DISPC HW IP initialisation */
> > +static int omap_dispchw_probe(struct platform_device *pdev)
> > +{
> > +   return 0;
> > +}
> > +
> > +static int omap_dispchw_remove(struct platform_device *pdev)
> > +{
> > +   return 0;
> > +}
> > +
> > +static struct platform_driver omap_dispchw_driver = {
> > +   .probe  = omap_dispchw_probe,
> > +   .remove = omap_dispchw_remove,
> > +   .driver = {
> > +   .name   = "dss_dispc",
> > +   .owner  = THIS_MODULE,
> > +   },
> > +};
> 
> Should be just above this:
> 
> > +static int __init omap_dispc_init(void)
> > +{
> > +   return platform_driver_register(&omap_dispchw_driver);
> > +}
> > +
> > +device_initcall(omap_dispc_init);
> 
> 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


Re: [PATCH v1 07/16] OMAP3: hwmod DSS: Create platform_driver for each DSS HW IP

2010-10-07 Thread Thomas Petazzoni
Hello Senthil,

I forgot one comment: in most Linux drivers, it is customary to put the
platform_driver definition just above the driver initialization
function, both of them at the end of the file. Of course, it's nothing
mandatory, but most of the drivers are organized this way, making it
easy for developers to find their way in all drivers.

So, in other words, this:

> +/* DISPC HW IP initialisation */
> +static int omap_dispchw_probe(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +
> +static int omap_dispchw_remove(struct platform_device *pdev)
> +{
> + return 0;
> +}
> +
> +static struct platform_driver omap_dispchw_driver = {
> + .probe  = omap_dispchw_probe,
> + .remove = omap_dispchw_remove,
> + .driver = {
> + .name   = "dss_dispc",
> + .owner  = THIS_MODULE,
> + },
> +};

Should be just above this:

> +static int __init omap_dispc_init(void)
> +{
> + return platform_driver_register(&omap_dispchw_driver);
> +}
> +
> +device_initcall(omap_dispc_init);

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


Re: [PATCH v1 07/16] OMAP3: hwmod DSS: Create platform_driver for each DSS HW IP

2010-10-07 Thread Thomas Petazzoni
Hello Senthil,

On Wed,  6 Oct 2010 16:44:50 +0530
Guruswamy Senthilvadivu  wrote:

> +static struct platform_driver omap_dsshw_driver = {
> + .probe  = omap_dsshw_probe,
> + .remove = omap_dsshw_remove,
> + .shutdown   = NULL,
> + .suspend= NULL,
> + .resume = NULL,

Those are unneeded, the compiler will initialize those fields to zero
automatically if you don't specify any value.

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