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@xxxxxxxxxxxxxxxx>
> > > ---
> > >
> > > 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/
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
