| Message ID | 1313119113-26302-1-git-send-email-Chunhe.Lan@freescale.com |
|---|---|
| State | Superseded |
| Headers | show |
On 08/11/2011 10:18 PM, Chunhe Lan wrote: > Add a fdt_set_node_status function that will set the specified > node's status to the value contained in "status". If the node > doesn't have "status" property that will be created. > > Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com> > --- > common/fdt_support.c | 17 ++++++++++++++++- > include/fdt_support.h | 3 +++ > 2 files changed, 19 insertions(+), 1 deletions(-) > > diff --git a/common/fdt_support.c b/common/fdt_support.c > index 19b2ef6..384523b 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -2,7 +2,7 @@ > * (C) Copyright 2007 > * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com > * > - * Copyright 2010 Freescale Semiconductor, Inc. > + * Copyright 2010-2011 Freescale Semiconductor, Inc. > * > * See file CREDITS for list of people who contributed to this > * project. > @@ -904,6 +904,21 @@ void fdt_del_node_and_alias(void *blob, const char *alias) > fdt_delprop(blob, off, alias); > } > > +/* > + * Sets the specified node's status to the value contained in "status". > + * If the first character of the specified path is "/" then we use > + * alias as a path. Otherwise, we look for an alias of that name. > + */ > +void fdt_set_node_status(void *fdt, const char *alias, const char *status) > +{ > + const char *path = fdt_get_alias(fdt, alias); > + > + if (!path) > + path = alias; > + > + do_fixup_by_path(fdt, path, "status", status, strlen(status) + 1, 1); > +} fdt_get_alias() is not needed -- do_fixup_by_path() calls fdt_path_offset() which handles aliases. Instead of something specific to status, how about adding do_fixup_by_path_string()? > /* Helper to read a big number; size is in cells (not bytes) */ > static inline u64 of_read_number(const __be32 *cell, int size) > { > diff --git a/include/fdt_support.h b/include/fdt_support.h > index 863024f..74d7c83 100644 > --- a/include/fdt_support.h > +++ b/include/fdt_support.h > @@ -2,6 +2,8 @@ > * (C) Copyright 2007 > * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com > * > + * Copyright (C) 2011 Freescale Semiconductor, Inc. > + * > * See file CREDITS for list of people who contributed to this > * project. > * > @@ -85,6 +87,7 @@ int fdt_fixup_nor_flash_size(void *blob); > > void fdt_fixup_mtdparts(void *fdt, void *node_info, int node_info_size); > void fdt_del_node_and_alias(void *blob, const char *alias); > +void fdt_set_node_status(void *fdt, const char *alias, const char *status); > u64 fdt_translate_address(void *blob, int node_offset, const u32 *in_addr); > int fdt_node_offset_by_compat_reg(void *blob, const char *compat, > phys_addr_t compat_off); A copyright line for a single function prototype? -Scott
On Thu, Aug 11, 2011 at 11:18 PM, Chunhe Lan <Chunhe.Lan@freescale.com> wrote: > +/* > + * Sets the specified node's status to the value contained in "status". > + * If the first character of the specified path is "/" then we use > + * alias as a path. Otherwise, we look for an alias of that name. > + */ > +void fdt_set_node_status(void *fdt, const char *alias, const char *status) > +{ > + const char *path = fdt_get_alias(fdt, alias); > + > + if (!path) > + path = alias; > + > + do_fixup_by_path(fdt, path, "status", status, strlen(status) + 1, 1); > +} If you're going to take some internal code and post it upstream, you should at least use the latest version of that code. The latest version of this function has this comment: /* * Given an alias or a path for a node, set the status of that node. * * If 'alias' is not a valid alias, then it is treated as a full path to the * node. No error checking is performed. * * This function is normally called to set the status for a virtual MDIO node. */ However, since you're moving this function to fdt_support, the last sentence should be dropped. > +++ b/include/fdt_support.h > @@ -2,6 +2,8 @@ > * (C) Copyright 2007 > * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com > * > + * Copyright (C) 2011 Freescale Semiconductor, Inc. > + * Like Scott said, adding a single function prototype is not sufficient for a new copyright claim. You have to add a significant amount of original code in order to add a new copyright line.
On Mon, 15 Aug 2011 08:08:04 +0800, Tabi Timur-B04825 <B04825@freescale.com> wrote: > On Thu, Aug 11, 2011 at 11:18 PM, Chunhe Lan <Chunhe.Lan@freescale.com> > wrote: > >> +/* >> + * Sets the specified node's status to the value contained in "status". >> + * If the first character of the specified path is "/" then we use >> + * alias as a path. Otherwise, we look for an alias of that name. >> + */ >> +void fdt_set_node_status(void *fdt, const char *alias, const char >> *status) >> +{ >> + const char *path = fdt_get_alias(fdt, alias); >> + >> + if (!path) >> + path = alias; >> + >> + do_fixup_by_path(fdt, path, "status", status, strlen(status) + >> 1, 1); >> +} > > If you're going to take some internal code and post it upstream, you > should at least use the latest version of that code. The latest > version of this function has this comment: > > /* > * Given an alias or a path for a node, set the status of that node. > * > * If 'alias' is not a valid alias, then it is treated as a full path to > the > * node. No error checking is performed. > * > * This function is normally called to set the status for a virtual MDIO > node. > */ > > However, since you're moving this function to fdt_support, the last > sentence should be dropped. OK. Thanks. >> +++ b/include/fdt_support.h >> @@ -2,6 +2,8 @@ >> * (C) Copyright 2007 >> * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com >> * >> + * Copyright (C) 2011 Freescale Semiconductor, Inc. >> + * > > Like Scott said, adding a single function prototype is not sufficient > for a new copyright claim. You have to add a significant amount of > original code in order to add a new copyright line. Thanks for your comments. -Jack Lan
On Sat, 13 Aug 2011 00:24:33 +0800, Scott Wood <scottwood@freescale.com> wrote: > On 08/11/2011 10:18 PM, Chunhe Lan wrote: >> Add a fdt_set_node_status function that will set the specified >> node's status to the value contained in "status". If the node >> doesn't have "status" property that will be created. >> >> Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com> >> --- >> common/fdt_support.c | 17 ++++++++++++++++- >> include/fdt_support.h | 3 +++ >> 2 files changed, 19 insertions(+), 1 deletions(-) >> >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index 19b2ef6..384523b 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -2,7 +2,7 @@ >> * (C) Copyright 2007 >> * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com >> * >> - * Copyright 2010 Freescale Semiconductor, Inc. >> + * Copyright 2010-2011 Freescale Semiconductor, Inc. >> * >> * See file CREDITS for list of people who contributed to this >> * project. >> @@ -904,6 +904,21 @@ void fdt_del_node_and_alias(void *blob, const char >> *alias) >> fdt_delprop(blob, off, alias); >> } >> >> +/* >> + * Sets the specified node's status to the value contained in "status". >> + * If the first character of the specified path is "/" then we use >> + * alias as a path. Otherwise, we look for an alias of that name. >> + */ >> +void fdt_set_node_status(void *fdt, const char *alias, const char >> *status) >> +{ >> + const char *path = fdt_get_alias(fdt, alias); >> + >> + if (!path) >> + path = alias; >> + >> + do_fixup_by_path(fdt, path, "status", status, strlen(status) + 1, 1); >> +} > > fdt_get_alias() is not needed -- do_fixup_by_path() calls > fdt_path_offset() which handles aliases. > > Instead of something specific to status, how about adding > do_fixup_by_path_string()? I think that function of do_fixup_by_path should have completed that function. Here, we only want to set the status property of node. At the same time, Tabi Timur also is using the function of fdt_set_node_status. >> /* Helper to read a big number; size is in cells (not bytes) */ >> static inline u64 of_read_number(const __be32 *cell, int size) >> { >> diff --git a/include/fdt_support.h b/include/fdt_support.h >> index 863024f..74d7c83 100644 >> --- a/include/fdt_support.h >> +++ b/include/fdt_support.h >> @@ -2,6 +2,8 @@ >> * (C) Copyright 2007 >> * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com >> * >> + * Copyright (C) 2011 Freescale Semiconductor, Inc. >> + * >> * See file CREDITS for list of people who contributed to this >> * project. >> * >> @@ -85,6 +87,7 @@ int fdt_fixup_nor_flash_size(void *blob); >> >> void fdt_fixup_mtdparts(void *fdt, void *node_info, int >> node_info_size); >> void fdt_del_node_and_alias(void *blob, const char *alias); >> +void fdt_set_node_status(void *fdt, const char *alias, const char >> *status); >> u64 fdt_translate_address(void *blob, int node_offset, const u32 >> *in_addr); >> int fdt_node_offset_by_compat_reg(void *blob, const char *compat, >> phys_addr_t compat_off); > > A copyright line for a single function prototype? Thanks. I will remove this line. -Jack Lan
On 08/14/2011 10:40 PM, Lan Chunhe wrote: > On Sat, 13 Aug 2011 00:24:33 +0800, Scott Wood <scottwood@freescale.com> > wrote: > >> fdt_get_alias() is not needed -- do_fixup_by_path() calls >> fdt_path_offset() which handles aliases. >> >> Instead of something specific to status, how about adding >> do_fixup_by_path_string()? > > I think that function of do_fixup_by_path should have > completed that function. do_fixup_by_path_string() would just be an inline wrapper for do_fixup_by_path() that calls strlen on the argument. And then you could just do: do_fixup_by_path_string(fdt, "nand_flash", "status", "okay"); I don't see a need for a function dedicated to the status property. -Scott
On Tue, 16 Aug 2011 02:12:17 +0800, Scott Wood <scottwood@freescale.com> wrote: > On 08/14/2011 10:40 PM, Lan Chunhe wrote: >> On Sat, 13 Aug 2011 00:24:33 +0800, Scott Wood <scottwood@freescale.com> >> wrote: >> >>> fdt_get_alias() is not needed -- do_fixup_by_path() calls >>> fdt_path_offset() which handles aliases. >>> >>> Instead of something specific to status, how about adding >>> do_fixup_by_path_string()? >> >> I think that function of do_fixup_by_path should have >> completed that function. > > do_fixup_by_path_string() would just be an inline wrapper for > do_fixup_by_path() that calls strlen on the argument. > > And then you could just do: > > do_fixup_by_path_string(fdt, "nand_flash", "status", "okay"); > > I don't see a need for a function dedicated to the status property. OK. Thanks. -Jack Lan
diff --git a/common/fdt_support.c b/common/fdt_support.c index 19b2ef6..384523b 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -2,7 +2,7 @@ * (C) Copyright 2007 * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com * - * Copyright 2010 Freescale Semiconductor, Inc. + * Copyright 2010-2011 Freescale Semiconductor, Inc. * * See file CREDITS for list of people who contributed to this * project. @@ -904,6 +904,21 @@ void fdt_del_node_and_alias(void *blob, const char *alias) fdt_delprop(blob, off, alias); } +/* + * Sets the specified node's status to the value contained in "status". + * If the first character of the specified path is "/" then we use + * alias as a path. Otherwise, we look for an alias of that name. + */ +void fdt_set_node_status(void *fdt, const char *alias, const char *status) +{ + const char *path = fdt_get_alias(fdt, alias); + + if (!path) + path = alias; + + do_fixup_by_path(fdt, path, "status", status, strlen(status) + 1, 1); +} + /* Helper to read a big number; size is in cells (not bytes) */ static inline u64 of_read_number(const __be32 *cell, int size) { diff --git a/include/fdt_support.h b/include/fdt_support.h index 863024f..74d7c83 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -2,6 +2,8 @@ * (C) Copyright 2007 * Gerald Van Baren, Custom IDEAS, vanbaren@cideas.com * + * Copyright (C) 2011 Freescale Semiconductor, Inc. + * * See file CREDITS for list of people who contributed to this * project. * @@ -85,6 +87,7 @@ int fdt_fixup_nor_flash_size(void *blob); void fdt_fixup_mtdparts(void *fdt, void *node_info, int node_info_size); void fdt_del_node_and_alias(void *blob, const char *alias); +void fdt_set_node_status(void *fdt, const char *alias, const char *status); u64 fdt_translate_address(void *blob, int node_offset, const u32 *in_addr); int fdt_node_offset_by_compat_reg(void *blob, const char *compat, phys_addr_t compat_off);
Add a fdt_set_node_status function that will set the specified node's status to the value contained in "status". If the node doesn't have "status" property that will be created. Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com> --- common/fdt_support.c | 17 ++++++++++++++++- include/fdt_support.h | 3 +++ 2 files changed, 19 insertions(+), 1 deletions(-)