[U-Boot] [PATCH 1/2 v3] fdt: Add a do_fixup_by_path_string() function

The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Signed-off-by: Chunhe Lan Chunhe.Lan@freescale.com --- include/fdt_support.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 863024f..1de4a1d 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path, const char *prop, const void *val, int len, int create); void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop, u32 val, int create); + +static inline void do_fixup_by_path_string(void *fdt, const char *path, + const char *prop, const char *status) +{ + do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1); +} + void do_fixup_by_prop(void *fdt, const char *pname, const void *pval, int plen, const char *prop, const void *val, int len,

Hi Chunhe Lan,
On 08/17/2011 02:24 AM, Chunhe Lan wrote:
[snip]
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
const char *prop, const char *status)
+{
- do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling do_fixup_by_path_string(fdt, path, prop, status); vs. simply calling do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
The do_fixup_by_path_string() saves two parameters "strlen(status) + 1, 1" at the cost of Yet Another Function. Is it worth it?
Thanks, gvb

On Thu, 18 Aug 2011 08:29:51 +0800, Jerry Van Baren gvb.uboot@gmail.com wrote:
Hi Chunhe Lan,
On 08/17/2011 02:24 AM, Chunhe Lan wrote:
[snip]
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
const char *prop, const char *status)
+{
- do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling do_fixup_by_path_string(fdt, path, prop, status); vs. simply calling do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
The do_fixup_by_path_string() saves two parameters "strlen(status) + 1, 1" at the cost of Yet Another Function. Is it worth it?
Yes, I think that it is worth. The encapsulation of function is used for that purpose.
Please refer to do_fixup_by_path_u32(), and it only has two lines code.
Thanks.
-Jack Lan

On 08/17/2011 07:29 PM, Jerry Van Baren wrote:
Hi Chunhe Lan,
On 08/17/2011 02:24 AM, Chunhe Lan wrote:
[snip]
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
const char *prop, const char *status)
+{
- do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
After all the good advice from Scott et al., the patch turns into a pretty trivial one-liner. I am questioning the advantage of calling do_fixup_by_path_string(fdt, path, prop, status); vs. simply calling do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
The do_fixup_by_path_string() saves two parameters "strlen(status) + 1, 1" at the cost of Yet Another Function. Is it worth it?
I think it's a nice convenience, with no runtime cost. It avoids any chance of a mismatch between the string passed to do_fixup_by_path and the string passed to strlen.
More functions versus open-coding is not generally a bad thing.
-Scott

On Aug 17, 2011, at 1:24 AM, Chunhe Lan wrote:
The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Fix where you break the line, should like more like (use upto 75 chars per line):
The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Signed-off-by: Chunhe Lan Chunhe.Lan@freescale.com
include/fdt_support.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 863024f..1de4a1d 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path, const char *prop, const void *val, int len, int create); void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop, u32 val, int create);
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
const char *prop, const char *status)
+{
- do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
void do_fixup_by_prop(void *fdt, const char *pname, const void *pval, int plen, const char *prop, const void *val, int len, -- 1.5.6.5

On Mon, 29 Aug 2011 02:15:15 +0800, Kumar Gala kumar.gala@freescale.com wrote:
On Aug 17, 2011, at 1:24 AM, Chunhe Lan wrote:
The do_fixup_by_path_string() will set the specified node's property to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Fix where you break the line, should like more like (use upto 75 chars per line):
OK.
Thanks.
-Jack Lan
The do_fixup_by_path_string() will set the specified node's property
to the value contained in "status". It would just be a wrapper for do_fixup_by_path() that calls strlen on the argument.
Signed-off-by: Chunhe Lan Chunhe.Lan@freescale.com
include/fdt_support.h | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/fdt_support.h b/include/fdt_support.h index 863024f..1de4a1d 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -36,6 +36,13 @@ void do_fixup_by_path(void *fdt, const char *path, const char *prop, const void *val, int len, int create); void do_fixup_by_path_u32(void *fdt, const char *path, const char *prop, u32 val, int create);
+static inline void do_fixup_by_path_string(void *fdt, const char *path,
const char *prop, const char *status)
+{
- do_fixup_by_path(fdt, path, prop, status, strlen(status) + 1, 1);
+}
void do_fixup_by_prop(void *fdt, const char *pname, const void *pval, int plen, const char *prop, const void *val, int len, -- 1.5.6.5
participants (6)
-
Chunhe Lan
-
Chunhe Lan
-
Jerry Van Baren
-
Kumar Gala
-
Lan Chunhe
-
Scott Wood