Re: [PATCH 10/61] bootwrapper: flatdevtree fixes

Re: [PATCH 10/61] bootwrapper: flatdevtree fixes

[Milton Miller]

In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>

On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 1. ft_create_node was returning the internal pointer rather than a 
> phandle.
> 2. ft_find_device_rel was treating lookups relative to root as an 
> error.

No, it is treating lookups relative to NULL as an error.

Your patch changes it to treat lookups relative to the NULL phandle as 
relative to root.

I've no objections to the other part, can you split these?

>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
>  arch/powerpc/boot/flatdevtree.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/flatdevtree.c 
> b/arch/powerpc/boot/flatdevtree.c
> index b732644..5b69aeb 100644
> --- a/arch/powerpc/boot/flatdevtree.c
> +++ b/arch/powerpc/boot/flatdevtree.c
> @@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt, 
> const void *top,
>  {
>         char *node;
>
> -       node = ft_node_ph2node(cxt, top);
> -       if (node == NULL)
> -               return NULL;
> +       if (top) {
> +               node = ft_node_ph2node(cxt, top);
> +               if (node == NULL)
> +                       return NULL;
> +       } else {
> +               node = ft_root_node(cxt);
> +       }
>

milton

Re: [PATCH 10/61] bootwrapper: flatdevtree fixes

[Scott Wood]

Milton Miller wrote:
> In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>
> 
> On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 
>> 1. ft_create_node was returning the internal pointer rather than a 
>> phandle.
>> 2. ft_find_device_rel was treating lookups relative to root as an error.
> 
> 
> No, it is treating lookups relative to NULL as an error.
> 
> Your patch changes it to treat lookups relative to the NULL phandle as 
> relative to root.

Other parts of the code (such as ft_create_node) already do that. 
What's wrong with it?

> I've no objections to the other part, can you split these?

No, because change #1 won't work without change #2.

-Scott

Re: [PATCH 10/61] bootwrapper: flatdevtree fixes

[Milton Miller]

On Jul 18, 2007, at 1:56 PM, Scott Wood wrote:
> Milton Miller wrote:
>> In-Reply-To: <20070718013137.GA15217@ld0162-tx32.am.freescale.net>
>> On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
>>> 1. ft_create_node was returning the internal pointer rather than a 
>>> phandle.
>>> 2. ft_find_device_rel was treating lookups relative to root as an 
>>> error.
>> No, it is treating lookups relative to NULL as an error.
>> Your patch changes it to treat lookups relative to the NULL phandle 
>> as relative to root.
>
> Other parts of the code (such as ft_create_node) already do that. 
> What's wrong with it?

Well, that behavior may be ok, but the change log should say that is 
the behavior you are changing.

Ie I would record 2 as:

When ft_find_device_rel is asked to find relative to NULL, find 
realative to root instead of returning an error.

or:

ft_find_device_rel now treats lookups relative to NULL as relative to 
the root instead of erroring out.


My reading would say that lookups relative to the phandle of root would 
have workied before, no?

>
>> I've no objections to the other part, can you split these?
>
> No, because change #1 won't work without change #2.

Huh?

I can see how changing a routine to return an external not internal 
pointer is necessary before changing a caller creates a dependancy for 
2 on 1, but why does it require lookups relative to the phandle NULL be 
treated as lookups relative to the root?

milton

[PATCH 01/61] Use strcasecmp() rather than strncasecmp() when determining device node compatibility.

[Scott Wood]

The current code assumes "foo-bar" must always be compatible with a node
compatible with "foo", which breaks device trees where this is not so.

The "case" part is also wrong according to Open Firmware, but it's more
likely to have drivers and/or device trees depending on it, and thus
needs to be handled more carefully.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/kernel/prom.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 37ff99b..0b136a5 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -218,7 +218,7 @@ int __init of_flat_dt_is_compatible(unsigned long node, const char *compat)
 	if (cp == NULL)
 		return 0;
 	while (cplen > 0) {
-		if (strncasecmp(cp, compat, strlen(compat)) == 0)
+		if (strcasecmp(cp, compat) == 0)
 			return 1;
 		l = strlen(cp) + 1;
 		cp += l;
@@ -1099,7 +1099,7 @@ int of_device_is_compatible(const struct device_node *device,
 	if (cp == NULL)
 		return 0;
 	while (cplen > 0) {
-		if (strncasecmp(cp, compat, strlen(compat)) == 0)
+		if (strcasecmp(cp, compat) == 0)
 			return 1;
 		l = strlen(cp) + 1;
 		cp += l;
-- 
1.5.0.3

[PATCH 10/61] bootwrapper: flatdevtree fixes

[Scott Wood]

1. ft_create_node was returning the internal pointer rather than a phandle.
2. ft_find_device_rel was treating lookups relative to root as an error.

Signed-off-by: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/boot/flatdevtree.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/boot/flatdevtree.c b/arch/powerpc/boot/flatdevtree.c
index b732644..5b69aeb 100644
--- a/arch/powerpc/boot/flatdevtree.c
+++ b/arch/powerpc/boot/flatdevtree.c
@@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt, const void *top,
 {
 	char *node;
 
-	node = ft_node_ph2node(cxt, top);
-	if (node == NULL)
-		return NULL;
+	if (top) {
+		node = ft_node_ph2node(cxt, top);
+		if (node == NULL)
+			return NULL;
+	} else {
+		node = ft_root_node(cxt);
+	}
 
 	node = ft_find_descendent(cxt, node, srch_path);
 	return ft_get_phandle(cxt, node);
@@ -990,7 +994,7 @@ void *ft_create_node(struct ft_cxt *cxt, const void *parent, const char *name)
 			cxt->p = p;
 			ft_begin_node(cxt, name);
 			ft_end_node(cxt);
-			return p;
+			return ft_find_device_rel(cxt, parent, name);
 		}
 		p = next;
 	}
-- 
1.5.0.3

Re: [PATCH 10/61] bootwrapper: flatdevtree fixes

[Milton Miller]

On Wed Jul 18 11:33:08 EST 2007, Scott Wood wrote:
> 1. ft_create_node was returning the internal pointer rather than a 
> phandle.
> 2. ft_find_device_rel was treating lookups relative to root as an 
> error.

No, it is treating lookups relative to NULL as an error.

Your patch changes it to treat lookups relative to the NULL phandle as 
relative to root.

I've no objections to the other part, can you split these?

>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
>  arch/powerpc/boot/flatdevtree.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/flatdevtree.c 
> b/arch/powerpc/boot/flatdevtree.c
> index b732644..5b69aeb 100644
> --- a/arch/powerpc/boot/flatdevtree.c
> +++ b/arch/powerpc/boot/flatdevtree.c
> @@ -659,9 +659,13 @@ void *ft_find_device_rel(struct ft_cxt *cxt, 
> const void *top,
>  {
>         char *node;
>
> -       node = ft_node_ph2node(cxt, top);
> -       if (node == NULL)
> -               return NULL;
> +       if (top) {
> +               node = ft_node_ph2node(cxt, top);
> +               if (node == NULL)
> +                       return NULL;
> +       } else {
> +               node = ft_root_node(cxt);
> +       }
>

milton