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

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 1994 bytes --]

On 2012-11-20 17:54, Steffen Trumtrar wrote:
> Add helper to get fb_videomode from devicetree.
> 
> Signed-off-by: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Reviewed-by: Thierry Reding <thierry.reding@avionic-design.de>
> Acked-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Thierry Reding <thierry.reding@avionic-design.de>
> Tested-by: Philipp Zabel <p.zabel@pengutronix.de>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  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.

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

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 897 bytes --]