HI Christian,
On 15 September 2014 07:06, Christian Gmeiner christian.gmeiner@gmail.com wrote:
This new function is used to set all display-timings properties based on fb_videomode.
display-timings { timing0 { clock-frequency = <25000000>; 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.gmeiner@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 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); --
Regards, Simon