On 2012-11-20 17:54, Steffen Trumtrar wrote:
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar <redacted>
Reviewed-by: Thierry Reding <redacted>
Acked-by: Thierry Reding <redacted>
Tested-by: Thierry Reding <redacted>
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(-)
quoted hunk
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.
quoted hunk
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
Hi Tomi,
On Wednesday 21 November 2012 14:49:30 Tomi Valkeinen wrote:
On 2012-11-20 17:54, Steffen Trumtrar wrote:
quoted
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar <redacted>
Reviewed-by: Thierry Reding <redacted>
Acked-by: Thierry Reding <redacted>
Tested-by: Thierry Reding <redacted>
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.
I agree, I favor structure declaration as well when possible.
quoted
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
On Wed, Nov 21, 2012 at 02:49:30PM +0200, Tomi Valkeinen wrote:
On 2012-11-20 17:54, Steffen Trumtrar wrote:
quoted
Add helper to get fb_videomode from devicetree.
Signed-off-by: Steffen Trumtrar <redacted>
Reviewed-by: Thierry Reding <redacted>
Acked-by: Thierry Reding <redacted>
Tested-by: Thierry Reding <redacted>
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(-)
quoted
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.
Okay. Seems to fit with the rest of the file;
quoted
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.
Well, I don't remember at the moment who requested this, but it was not my
idea to put them there. So, this is a matter of style I guess.
But maybe I understood that wrong.
Regards,
Steffen
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 2012-11-21 18:24, Steffen Trumtrar wrote:
On Wed, Nov 21, 2012 at 02:49:30PM +0200, Tomi Valkeinen wrote:
quoted
quoted
@@ -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.
Well, I don't remember at the moment who requested this, but it was not my
idea to put them there. So, this is a matter of style I guess.
But maybe I understood that wrong.
Right, one reviewer says this way, and another says that way =).
With the header files I've made I only use #ifs with #else, when I want
to give a static inline empty/no-op implementation for the function in
case the feature is not compiled into the kernel.
As you said, matter of taste. Up to you.
Tomi