[PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)

[PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)

[Laurent Pinchart]

default_720p and sh_mobile_lcdc_check_interface are used at device
initialization time only. Mark them as __devinitconst and __devinit
respectively.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/video/sh_mobile_lcdcfb.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
index 8b18360..a6bf4fb 100644
--- a/drivers/video/sh_mobile_lcdcfb.c
+++ b/drivers/video/sh_mobile_lcdcfb.c
@@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct notifier_block *nb,
  * Probe/remove and driver init/exit
  */
 
-static const struct fb_videomode default_720p = {
+static const struct fb_videomode default_720p __devinitconst = {
 	.name = "HDMI 720p",
 	.xres = 1280,
 	.yres = 720,
@@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
+static int __devinit
+sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
 {
 	int interface_type = ch->cfg.interface_type;
 
-- 
1.7.3.4

Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with

[Guennadi Liakhovetski]

Hi Laurent

On Tue, 13 Dec 2011, Laurent Pinchart wrote:

> default_720p and sh_mobile_lcdc_check_interface are used at device
> initialization time only. Mark them as __devinitconst and __devinit
> respectively.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/video/sh_mobile_lcdcfb.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/sh_mobile_lcdcfb.c b/drivers/video/sh_mobile_lcdcfb.c
> index 8b18360..a6bf4fb 100644
> --- a/drivers/video/sh_mobile_lcdcfb.c
> +++ b/drivers/video/sh_mobile_lcdcfb.c
> @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct notifier_block *nb,
>   * Probe/remove and driver init/exit
>   */
>  
> -static const struct fb_videomode default_720p = {
> +static const struct fb_videomode default_720p __devinitconst = {
>  	.name = "HDMI 720p",
>  	.xres = 1280,
>  	.yres = 720,
> @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> +static int __devinit
> +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)

Personally, I don't like this type of line splitting very much, I prefer 
the grep output to show the function type and all attributes, but this, 
certainly, would not be the reason to change this specific hunk:-) But 
this might be: the whole file so far has all function definitions at least 
up to and including the first parameter on one line, so, I find, this 
change would introduce an inconsistency in the file style. The line would 
__only__ be 83 characters long if you put it all on one line. I think, it 
would look better then:-)

>  {
>  	int interface_type = ch->cfg.interface_type;
>  
> -- 
> 1.7.3.4

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)

[Laurent Pinchart]

Hi Guennadi,

On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > default_720p and sh_mobile_lcdc_check_interface are used at device
> > initialization time only. Mark them as __devinitconst and __devinit
> > respectively.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > 
> >  drivers/video/sh_mobile_lcdcfb.c |    5 +++--
> >  1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > --- a/drivers/video/sh_mobile_lcdcfb.c
> > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > notifier_block *nb,
> > 
> >   * Probe/remove and driver init/exit
> >   */
> > 
> > -static const struct fb_videomode default_720p = {
> > +static const struct fb_videomode default_720p __devinitconst = {
> > 
> >  	.name = "HDMI 720p",
> >  	.xres = 1280,
> >  	.yres = 720,
> > 
> > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > platform_device *pdev)
> > 
> >  	return 0;
> >  
> >  }
> > 
> > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > *ch) +static int __devinit
> > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> 
> Personally, I don't like this type of line splitting very much, I prefer
> the grep output to show the function type and all attributes, but this,
> certainly, would not be the reason to change this specific hunk:-) But
> this might be: the whole file so far has all function definitions at least
> up to and including the first parameter on one line, so, I find, this
> change would introduce an inconsistency in the file style. The line would
> __only__ be 83 characters long if you put it all on one line. I think, it
> would look better then:-)

It's a matter of limits, as usual... I find 80 to be a nice limit, although in 
some cases I'm fine with exceeding it (one example is a long list of #defines 
with a small comment describing each macro, as in the list of FOURCCs in 
linux/videodev2.h for instance). For code I try to respect the 80 characters 
limit. Another reason is that it produces checkpatch.pl warnings, which force 
me to manually look at all the warnings to check which ones are acceptable.

Regarding consistency, other patches in this series use the same style, so 
that function won't feel alone anymore ;-)

I can change this (and other instances of the same coding style) if you 
insist. BTW, a possible improvement would be to shorten the sh_mobile_lcdc_ 
prefix. I find it too long, and I was thinking about reducing it to shm_lcdc_ 
(although shm reffers to shared memory, so that might not be the best idea).

> >  {
> >  
> >  	int interface_type = ch->cfg.interface_type;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with

[Guennadi Liakhovetski]

On Wed, 14 Dec 2011, Laurent Pinchart wrote:

> Hi Guennadi,
> 
> On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > default_720p and sh_mobile_lcdc_check_interface are used at device
> > > initialization time only. Mark them as __devinitconst and __devinit
> > > respectively.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > 
> > >  drivers/video/sh_mobile_lcdcfb.c |    5 +++--
> > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > > notifier_block *nb,
> > > 
> > >   * Probe/remove and driver init/exit
> > >   */
> > > 
> > > -static const struct fb_videomode default_720p = {
> > > +static const struct fb_videomode default_720p __devinitconst = {
> > > 
> > >  	.name = "HDMI 720p",
> > >  	.xres = 1280,
> > >  	.yres = 720,
> > > 
> > > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > > platform_device *pdev)
> > > 
> > >  	return 0;
> > >  
> > >  }
> > > 
> > > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > > *ch) +static int __devinit
> > > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> > 
> > Personally, I don't like this type of line splitting very much, I prefer
> > the grep output to show the function type and all attributes, but this,
> > certainly, would not be the reason to change this specific hunk:-) But
> > this might be: the whole file so far has all function definitions at least
> > up to and including the first parameter on one line, so, I find, this
> > change would introduce an inconsistency in the file style. The line would
> > __only__ be 83 characters long if you put it all on one line. I think, it
> > would look better then:-)
> 
> It's a matter of limits, as usual... I find 80 to be a nice limit, although in 
> some cases I'm fine with exceeding it (one example is a long list of #defines 
> with a small comment describing each macro, as in the list of FOURCCs in 
> linux/videodev2.h for instance). For code I try to respect the 80 characters 
> limit. Another reason is that it produces checkpatch.pl warnings, which force 
> me to manually look at all the warnings to check which ones are acceptable.
> 
> Regarding consistency, other patches in this series use the same style, so 
> that function won't feel alone anymore ;-)
> 
> I can change this (and other instances of the same coding style) if you 
> insist. BTW, a possible improvement would be to shorten the sh_mobile_lcdc_ 
> prefix. I find it too long, and I was thinking about reducing it to shm_lcdc_ 
> (although shm reffers to shared memory, so that might not be the best idea).

How about sh_lcdc?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 02/57] fbdev: sh_mobile_lcdc: Mark init-only symbols with __devinit(const)

[Laurent Pinchart]

Hi Guennadi,

On Wednesday 14 December 2011 12:07:26 Guennadi Liakhovetski wrote:
> On Wed, 14 Dec 2011, Laurent Pinchart wrote:
> > On Tuesday 13 December 2011 23:23:32 Guennadi Liakhovetski wrote:
> > > On Tue, 13 Dec 2011, Laurent Pinchart wrote:
> > > > default_720p and sh_mobile_lcdc_check_interface are used at device
> > > > initialization time only. Mark them as __devinitconst and __devinit
> > > > respectively.
> > > > 
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > > 
> > > >  drivers/video/sh_mobile_lcdcfb.c |    5 +++--
> > > >  1 files changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/video/sh_mobile_lcdcfb.c
> > > > b/drivers/video/sh_mobile_lcdcfb.c index 8b18360..a6bf4fb 100644
> > > > --- a/drivers/video/sh_mobile_lcdcfb.c
> > > > +++ b/drivers/video/sh_mobile_lcdcfb.c
> > > > @@ -1459,7 +1459,7 @@ static int sh_mobile_lcdc_notify(struct
> > > > notifier_block *nb,
> > > > 
> > > >   * Probe/remove and driver init/exit
> > > >   */
> > > > 
> > > > -static const struct fb_videomode default_720p = {
> > > > +static const struct fb_videomode default_720p __devinitconst = {
> > > > 
> > > >  	.name = "HDMI 720p",
> > > >  	.xres = 1280,
> > > >  	.yres = 720,
> > > > 
> > > > @@ -1528,7 +1528,8 @@ static int sh_mobile_lcdc_remove(struct
> > > > platform_device *pdev)
> > > > 
> > > >  	return 0;
> > > >  
> > > >  }
> > > > 
> > > > -static int sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan
> > > > *ch) +static int __devinit
> > > > +sh_mobile_lcdc_check_interface(struct sh_mobile_lcdc_chan *ch)
> > > 
> > > Personally, I don't like this type of line splitting very much, I
> > > prefer the grep output to show the function type and all attributes,
> > > but this, certainly, would not be the reason to change this specific
> > > hunk:-) But this might be: the whole file so far has all function
> > > definitions at least up to and including the first parameter on one
> > > line, so, I find, this change would introduce an inconsistency in the
> > > file style. The line would __only__ be 83 characters long if you put
> > > it all on one line. I think, it would look better then:-)
> > 
> > It's a matter of limits, as usual... I find 80 to be a nice limit,
> > although in some cases I'm fine with exceeding it (one example is a long
> > list of #defines with a small comment describing each macro, as in the
> > list of FOURCCs in linux/videodev2.h for instance). For code I try to
> > respect the 80 characters limit. Another reason is that it produces
> > checkpatch.pl warnings, which force me to manually look at all the
> > warnings to check which ones are acceptable.
> > 
> > Regarding consistency, other patches in this series use the same style,
> > so that function won't feel alone anymore ;-)
> > 
> > I can change this (and other instances of the same coding style) if you
> > insist. BTW, a possible improvement would be to shorten the
> > sh_mobile_lcdc_ prefix. I find it too long, and I was thinking about
> > reducing it to shm_lcdc_ (although shm reffers to shared memory, so that
> > might not be the best idea).
> 
> How about sh_lcdc?

Sounds good to me. Does anyone have an objection ?

-- 
Regards,

Laurent Pinchart