Re: [U-Boot] [PATCH 2/3] fdt: add fdt_add_display_timings(..) and friends
Hi Simon
On 15 September 2014 07:06, Christian Gmeiner christian.gmei...@gmail.com
wrote:
This new function is used to set all display-timings
properties based on fb_videomode.
display-timings {
timing0 {
clock-frequency = 2500;
hactive = 640;
vactive = 480;
hback-porch = 48;
hfront-porch = 16;
vback-porch = 31;
vfront-porch = 12;
hsync-len = 96;
vsync-len = 2;
};
};
Signed-off-by: Christian Gmeiner christian.gmei...@gmail.com
The ot1200 base patch has landed in u-boot-imx and I will work to get this EDID
stuff mainline too.
Thanks for the patch. There are a few style violations and I have a few
minor comments below.
$ ./tools/patman/patman -c1 -n
Cleaned 1 patches
0 errors, 4 warnings, 0 checks for
0001-fdt-add-fdt_add_display_timings-.-and-friends.patch:
warning: common/fdt_support.c,1400: line over 80 characters
warning: common/fdt_support.c,1406: braces {} are not necessary for single
statement blocks
warning: common/fdt_support.c,1412: braces {} are not necessary for single
statement blocks
warning: include/fdt_support.h,96: line over 80 characters
checkpatch.pl found 0 error(s), 4 warning(s), 0 checks(s)
Will fix them in the next patch series about this topic.
---
common/fdt_support.c | 56
+++
include/fdt_support.h | 5 +
2 files changed, 61 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 784a570..72004a3 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -11,6 +11,7 @@
#include stdio_dev.h
#include linux/ctype.h
#include linux/types.h
+#include linux/fb.h
#include asm/global_data.h
#include libfdt.h
#include fdt_support.h
@@ -1373,6 +1374,61 @@ err_size:
#endif
/*
+* fdt_find_display_timings: finde node containing display-timings
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
+* @parent: parent node containing display-timings
or -ve error code FDT_ERROR_...
ok
+*/
+int fdt_find_display_timings(void *fdt, const char *compat, const char
*parent)
+{
+ int coff = fdt_node_offset_by_compatible(fdt, -1, compat);
+ int poff = fdt_subnode_offset(fdt, coff, parent);
+ int noff = fdt_subnode_offset(fdt, poff, display-timings);
+
Can we return an error when we see one? Here it will return a somewhat
meaningless error if (say) the first call finds no node.
I can return the return code of the three functions. Something like:
int coff, poff, noff;
coff = fdt_node_offset_by_compatible(..);
if (coff 0)
return coff;
poff = fdt_subnode_offset(..);
if (poff 0)
Would that be better?
+ return noff;
+}
+
+/*
+* fdt_update_display_timings: update display-timings properties
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
compatible
ok
+* @parent: parent node containing display-timings
@parent: parent node containing display-timings subnode
yes... thats better.
+* @mode: ptr to fb_videomode
Well we know that from the code. Perhaps display timings to add to the
device tree
sounds good to me.
+*/
This function is exported so the comment should go in the header file.
okay.
+int fdt_update_display_timings(void *fdt, const char *compat, const char
*parent,
+ struct fb_videomode *mode)
+{
+ int noff = fdt_find_display_timings(fdt, compat, parent);
+
+ /* check if display-timings subnode does exist */
+ if (noff == -FDT_ERR_NOTFOUND) {
if (noff 0)
would be better
ok
+ return noff;
+ }
+
+ /* check if timing0 subnode exists */
+ noff = fdt_subnode_offset(fdt, noff, timing0);
+ if (noff == -FDT_ERR_NOTFOUND) {
same here
ok
+ return noff;
+ }
+
+ /* set all needed properties */
+ fdt_setprop_u32(fdt, noff, clock-frequency,
+ PICOS2KHZ(mode-pixclock) * 1000);
+ fdt_setprop_u32(fdt, noff, hactive, mode-xres);
+ fdt_setprop_u32(fdt, noff, vactive, mode-yres);
+ fdt_setprop_u32(fdt, noff, hback-porch, mode-left_margin);
+ fdt_setprop_u32(fdt, noff, hfront-porch, mode-right_margin);
+ fdt_setprop_u32(fdt, noff, vback-porch, mode-upper_margin);
+ fdt_setprop_u32(fdt, noff, vfront-porch, mode-lower_margin);
+ fdt_setprop_u32(fdt, noff, hsync-len, mode-hsync_len);
+ fdt_setprop_u32(fdt, noff, vsync-len, mode-vsync_len);
Should you have error checking here? We might run out of space.
Sounds like a good idea - will change the current code.
+
+ return 0;
+}
+
+/*
* Verify the physical address of device tree node for a given alias
*
* This function locates the device tree node of a given alias,
Re: [U-Boot] [PATCH 2/3] fdt: add fdt_add_display_timings(..) and friends
HI Christian,
On 15 September 2014 07:06, Christian Gmeiner christian.gmei...@gmail.com
wrote:
This new function is used to set all display-timings
properties based on fb_videomode.
display-timings {
timing0 {
clock-frequency = 2500;
hactive = 640;
vactive = 480;
hback-porch = 48;
hfront-porch = 16;
vback-porch = 31;
vfront-porch = 12;
hsync-len = 96;
vsync-len = 2;
};
};
Signed-off-by: Christian Gmeiner christian.gmei...@gmail.com
Thanks for the patch. There are a few style violations and I have a few
minor comments below.
$ ./tools/patman/patman -c1 -n
Cleaned 1 patches
0 errors, 4 warnings, 0 checks for
0001-fdt-add-fdt_add_display_timings-.-and-friends.patch:
warning: common/fdt_support.c,1400: line over 80 characters
warning: common/fdt_support.c,1406: braces {} are not necessary for single
statement blocks
warning: common/fdt_support.c,1412: braces {} are not necessary for single
statement blocks
warning: include/fdt_support.h,96: line over 80 characters
checkpatch.pl found 0 error(s), 4 warning(s), 0 checks(s)
---
common/fdt_support.c | 56
+++
include/fdt_support.h | 5 +
2 files changed, 61 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 784a570..72004a3 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -11,6 +11,7 @@
#include stdio_dev.h
#include linux/ctype.h
#include linux/types.h
+#include linux/fb.h
#include asm/global_data.h
#include libfdt.h
#include fdt_support.h
@@ -1373,6 +1374,61 @@ err_size:
#endif
/*
+* fdt_find_display_timings: finde node containing display-timings
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
+* @parent: parent node containing display-timings
or -ve error code FDT_ERROR_...
+*/
+int fdt_find_display_timings(void *fdt, const char *compat, const char
*parent)
+{
+ int coff = fdt_node_offset_by_compatible(fdt, -1, compat);
+ int poff = fdt_subnode_offset(fdt, coff, parent);
+ int noff = fdt_subnode_offset(fdt, poff, display-timings);
+
Can we return an error when we see one? Here it will return a somewhat
meaningless error if (say) the first call finds no node.
+ return noff;
+}
+
+/*
+* fdt_update_display_timings: update display-timings properties
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
compatible
+* @parent: parent node containing display-timings
@parent: parent node containing display-timings subnode
+* @mode: ptr to fb_videomode
Well we know that from the code. Perhaps display timings to add to the
device tree
+*/
This function is exported so the comment should go in the header file.
+int fdt_update_display_timings(void *fdt, const char *compat, const char
*parent,
+ struct fb_videomode *mode)
+{
+ int noff = fdt_find_display_timings(fdt, compat, parent);
+
+ /* check if display-timings subnode does exist */
+ if (noff == -FDT_ERR_NOTFOUND) {
if (noff 0)
would be better
+ return noff;
+ }
+
+ /* check if timing0 subnode exists */
+ noff = fdt_subnode_offset(fdt, noff, timing0);
+ if (noff == -FDT_ERR_NOTFOUND) {
same here
+ return noff;
+ }
+
+ /* set all needed properties */
+ fdt_setprop_u32(fdt, noff, clock-frequency,
+ PICOS2KHZ(mode-pixclock) * 1000);
+ fdt_setprop_u32(fdt, noff, hactive, mode-xres);
+ fdt_setprop_u32(fdt, noff, vactive, mode-yres);
+ fdt_setprop_u32(fdt, noff, hback-porch, mode-left_margin);
+ fdt_setprop_u32(fdt, noff, hfront-porch, mode-right_margin);
+ fdt_setprop_u32(fdt, noff, vback-porch, mode-upper_margin);
+ fdt_setprop_u32(fdt, noff, vfront-porch, mode-lower_margin);
+ fdt_setprop_u32(fdt, noff, hsync-len, mode-hsync_len);
+ fdt_setprop_u32(fdt, noff, vsync-len, mode-vsync_len);
Should you have error checking here? We might run out of space.
+
+ return 0;
+}
+
+/*
* Verify the physical address of device tree node for a given alias
*
* This function locates the device tree node of a given alias, and then
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 1bda686..4222ab4 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -91,6 +91,11 @@ int fdt_set_phandle(void *fdt, int nodeoffset, uint32_t
phandle);
unsigned int fdt_create_phandle(void *fdt, int nodeoffset);
int fdt_add_edid(void *blob, const char *compat, unsigned char *buf);
+struct fb_videomode;
+int fdt_find_display_timings(void *fdt, const char *compat, const char
*parent);
+int fdt_update_display_timings(void *fdt, const char *compat, const
[U-Boot] [PATCH 2/3] fdt: add fdt_add_display_timings(..) and friends
This new function is used to set all display-timings
properties based on fb_videomode.
display-timings {
timing0 {
clock-frequency = 2500;
hactive = 640;
vactive = 480;
hback-porch = 48;
hfront-porch = 16;
vback-porch = 31;
vfront-porch = 12;
hsync-len = 96;
vsync-len = 2;
};
};
Signed-off-by: Christian Gmeiner christian.gmei...@gmail.com
---
common/fdt_support.c | 56 +++
include/fdt_support.h | 5 +
2 files changed, 61 insertions(+)
diff --git a/common/fdt_support.c b/common/fdt_support.c
index 784a570..72004a3 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -11,6 +11,7 @@
#include stdio_dev.h
#include linux/ctype.h
#include linux/types.h
+#include linux/fb.h
#include asm/global_data.h
#include libfdt.h
#include fdt_support.h
@@ -1373,6 +1374,61 @@ err_size:
#endif
/*
+* fdt_find_display_timings: finde node containing display-timings
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
+* @parent: parent node containing display-timings
+*/
+int fdt_find_display_timings(void *fdt, const char *compat, const char *parent)
+{
+ int coff = fdt_node_offset_by_compatible(fdt, -1, compat);
+ int poff = fdt_subnode_offset(fdt, coff, parent);
+ int noff = fdt_subnode_offset(fdt, poff, display-timings);
+
+ return noff;
+}
+
+/*
+* fdt_update_display_timings: update display-timings properties
+*
+* @fdt: fdt to device tree
+* @compat: compatiable string to match
+* @parent: parent node containing display-timings
+* @mode: ptr to fb_videomode
+*/
+int fdt_update_display_timings(void *fdt, const char *compat, const char
*parent,
+ struct fb_videomode *mode)
+{
+ int noff = fdt_find_display_timings(fdt, compat, parent);
+
+ /* check if display-timings subnode does exist */
+ if (noff == -FDT_ERR_NOTFOUND) {
+ return noff;
+ }
+
+ /* check if timing0 subnode exists */
+ noff = fdt_subnode_offset(fdt, noff, timing0);
+ if (noff == -FDT_ERR_NOTFOUND) {
+ return noff;
+ }
+
+ /* set all needed properties */
+ fdt_setprop_u32(fdt, noff, clock-frequency,
+ PICOS2KHZ(mode-pixclock) * 1000);
+ fdt_setprop_u32(fdt, noff, hactive, mode-xres);
+ fdt_setprop_u32(fdt, noff, vactive, mode-yres);
+ fdt_setprop_u32(fdt, noff, hback-porch, mode-left_margin);
+ fdt_setprop_u32(fdt, noff, hfront-porch, mode-right_margin);
+ fdt_setprop_u32(fdt, noff, vback-porch, mode-upper_margin);
+ fdt_setprop_u32(fdt, noff, vfront-porch, mode-lower_margin);
+ fdt_setprop_u32(fdt, noff, hsync-len, mode-hsync_len);
+ fdt_setprop_u32(fdt, noff, vsync-len, mode-vsync_len);
+
+ return 0;
+}
+
+/*
* Verify the physical address of device tree node for a given alias
*
* This function locates the device tree node of a given alias, and then
diff --git a/include/fdt_support.h b/include/fdt_support.h
index 1bda686..4222ab4 100644
--- a/include/fdt_support.h
+++ b/include/fdt_support.h
@@ -91,6 +91,11 @@ int fdt_set_phandle(void *fdt, int nodeoffset, uint32_t
phandle);
unsigned int fdt_create_phandle(void *fdt, int nodeoffset);
int fdt_add_edid(void *blob, const char *compat, unsigned char *buf);
+struct fb_videomode;
+int fdt_find_display_timings(void *fdt, const char *compat, const char
*parent);
+int fdt_update_display_timings(void *fdt, const char *compat, const char
*parent,
+ struct fb_videomode *mode);
+
int fdt_verify_alias_address(void *fdt, int anode, const char *alias,
u64 addr);
u64 fdt_get_base_address(void *fdt, int node);
--
1.9.3
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
