[U-Boot] [RFC] fdt expert advice needed

[U-Boot] [RFC] fdt expert advice needed

[Matthias Fuchs]

Hi FDT experts,

some time ago I posted a patch to fix U-Boot's approach to
set all ns16550 compatible nodes' clock-frequency property
in ppc4xx device tree's:

do_fixup_by_compat_u32(blob, "ns16550", "clock-frequency", gd->uart_clk, 1);

Typically this works fine. It is intended to configure all CPU internal UART's clock.
We have some additional UARTs attached to an external bus. From the device
tree's point of view they have a different path, but are also ns16550 compatible.

So the above line will also overwrite thier clock-frequency. This must not be done,
because they have a separate external clock and the value in the device tree is correct!

Now I want to replace the above code to only touch the ns16550 compatible nodes
that are direct childs of /plb/opb. Not those under /plb/ebc and even not those under 
/plb/opb/ebc. This is not easy to do with the fdt API. Below you find my first and dirty
hack.

Isn't there a more simple way to do so? Don't bother me about the printf, variable names etc.
It's just for discussion.

Please tell me if I have to explain my code :-)

Matthias

[U-Boot] [RFC] fdt expert advice needed

[Scott Wood]

Matthias Fuchs wrote:
> Now I want to replace the above code to only touch the ns16550 compatible nodes
> that are direct childs of /plb/opb. Not those under /plb/ebc and even not those under 
> /plb/opb/ebc. This is not easy to do with the fdt API. Below you find my first and dirty
> hack.
> 
> Isn't there a more simple way to do so? Don't bother me about the printf, variable names etc.
> It's just for discussion.

Take the node that you want to search under (/plb/opb), and pass it to 
fdt_next_node(), with an initial depth of zero.  Continue until depth 
returns to zero (or less).

-Scott

[U-Boot] [RFC] fdt expert advice needed

[Scott Wood]

Scott Wood wrote:
> Matthias Fuchs wrote:
>> Now I want to replace the above code to only touch the ns16550 compatible nodes
>> that are direct childs of /plb/opb. Not those under /plb/ebc and even not those under 
>> /plb/opb/ebc. This is not easy to do with the fdt API. Below you find my first and dirty
>> hack.
>>
>> Isn't there a more simple way to do so? Don't bother me about the printf, variable names etc.
>> It's just for discussion.
> 
> Take the node that you want to search under (/plb/opb), and pass it to 
> fdt_next_node(), with an initial depth of zero.  Continue until depth 
> returns to zero (or less).

I just saw that you want to restrict it to direct children -- for that, 
ignore any nodes where depth != 1.

-Scott

[U-Boot] [RFC] fdt expert advice needed

[Matthias Fuchs]

Hi Scott,

thanks for your hint. Please see my updated code below.
Do you think it's ok this way?

Matthias

---
 cpu/ppc4xx/fdt.c |   24 ++++++++++++++++++++++--
 1 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/cpu/ppc4xx/fdt.c b/cpu/ppc4xx/fdt.c
index c55e1cf..f9ff560 100644
--- a/cpu/ppc4xx/fdt.c
+++ b/cpu/ppc4xx/fdt.c
@@ -113,6 +113,7 @@ void fdt_pcie_setup(void *blob)
 void ft_cpu_setup(void *blob, bd_t *bd)
 {
 	sys_info_t sys_info;
+	int off, ndepth = 0;
 
 	get_sys_info(&sys_info);
 
@@ -133,9 +134,28 @@ void ft_cpu_setup(void *blob, bd_t *bd)
 	fdt_fixup_memory(blob, (u64)bd->bi_memstart, (u64)bd->bi_memsize);
 
 	/*
-	 * Setup all baudrates for the UARTs
+	 * Setup all UART clocks for CPU internal UARTs
+	 * (only these UARTs are definetely clocked by gd->uart_clk)
+	 *
+	 * These UARTs are direct childs of /plb/opb. This code
+	 * does not touch any UARTs that are connected to the ebc.
 	 */
-	do_fixup_by_compat_u32(blob, "ns16550", "clock-frequency", gd->uart_clk, 1);
+	off = fdt_path_offset(blob, "/plb/opb");
+	while ((off = fdt_next_node(blob, off, &ndepth)) >= 0) {
+		/*
+		 * process all sub nodes and stop when we are back
+		 * at the starting depth
+		 */
+		if (ndepth == 0)
+			break;
+
+		/* only update direct childs */
+		if ((ndepth == 1) &&
+		    (fdt_node_check_compatible(blob, off, "ns16550") == 0))
+			fdt_setprop(blob, off,
+				    "clock-frequency",
+				    (void*)&(gd->uart_clk), 4);
+	}
 
 	/*
 	 * Fixup all ethernet nodes
-- 
1.5.6.3


> Scott Wood wrote:
> > Matthias Fuchs wrote:
> >> Now I want to replace the above code to only touch the ns16550 compatible nodes
> >> that are direct childs of /plb/opb. Not those under /plb/ebc and even not those under 
> >> /plb/opb/ebc. This is not easy to do with the fdt API. Below you find my first and dirty
> >> hack.
> >>
> >> Isn't there a more simple way to do so? Don't bother me about the printf, variable names etc.
> >> It's just for discussion.
> > 
> > Take the node that you want to search under (/plb/opb), and pass it to 
> > fdt_next_node(), with an initial depth of zero.  Continue until depth 
> > returns to zero (or less).
> 
> I just saw that you want to restrict it to direct children -- for that, 
> ignore any nodes where depth != 1.
> 
> -Scott

[U-Boot] [RFC] fdt expert advice needed

[Scott Wood]

On Sun, Feb 01, 2009 at 10:30:20PM +0100, Matthias Fuchs wrote:
> -	do_fixup_by_compat_u32(blob, "ns16550", "clock-frequency", gd->uart_clk, 1);
> +	off = fdt_path_offset(blob, "/plb/opb");
> +	while ((off = fdt_next_node(blob, off, &ndepth)) >= 0) {
> +		/*
> +		 * process all sub nodes and stop when we are back
> +		 * at the starting depth
> +		 */
> +		if (ndepth == 0)
> +			break;

Should be ndepth <= 0; it can be negative if opb is the last node under
plb to be visited.

Looks OK otherwise.

-Scott