Re: [PATCH v12 4/6] fbmon: add of_videomode helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Tomi,

On Wednesday 21 November 2012 14:49:30 Tomi Valkeinen wrote:
> On 2012-11-20 17:54, Steffen Trumtrar wrote:
> > Add helper to get fb_videomode from devicetree.
> > 
> > Signed-off-by: Steffen Trumtrar <s.trumtrar@xxxxxxxxxxxxxx>
> > Reviewed-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>
> > Acked-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>
> > Tested-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx>
> > Tested-by: Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > 
> >  drivers/video/fbmon.c |   42 +++++++++++++++++++++++++++++++++++++++++-
> >  include/linux/fb.h    |    7 +++++++
> >  2 files changed, 48 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/fb.h b/include/linux/fb.h
> > index 920cbe3..41b5e49 100644
> > --- a/include/linux/fb.h
> > +++ b/include/linux/fb.h
> > @@ -15,6 +15,8 @@
> > 
> >  #include <linux/slab.h>
> >  #include <asm/io.h>
> >  #include <linux/videomode.h>
> > 
> > +#include <linux/of.h>
> > +#include <linux/of_videomode.h>
> 
> Guess what? =)
> 
> To be honest, I don't know what the general opinion is about including
> header files from header files. But I always leave them out if they are
> not strictly needed.

I agree, I favor structure declaration as well when possible.

> >  struct vm_area_struct;
> >  struct fb_info;
> > 
> > @@ -715,6 +717,11 @@ extern void fb_destroy_modedb(struct fb_videomode
> > *modedb);> 
> >  extern int fb_find_mode_cvt(struct fb_videomode *mode, int margins, int
> >  rb); extern unsigned char *fb_ddc_read(struct i2c_adapter *adapter);
> > 
> > +#if IS_ENABLED(CONFIG_OF_VIDEOMODE)
> > +extern int of_get_fb_videomode(const struct device_node *np,
> > +			       struct fb_videomode *fb,
> > +			       unsigned int index);
> > +#endif
> > 
> >  #if IS_ENABLED(CONFIG_VIDEOMODE)
> >  extern int fb_videomode_from_videomode(const struct videomode *vm,
> >  
> >  				       struct fb_videomode *fbmode);
> 
> Do you really need these #ifs in the header files? They do make it look
> a bit messy. If somebody uses the functions and CONFIG_VIDEOMODE is not
> enabled, he'll get a linker error anyway.

-- 
Regards,

Laurent Pinchart

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux