[SLOF] [PATCH slof 1/2] fdt: Factor out code to replace a phandle in place

[Thomas Huth]

On 14.05.2018 12:03, Alexey Kardashevskiy wrote:
> We generate a fake XICS phandle in QEMU and SLOF replaces that phandle
> with the real one (i.e. SLOF's node address) in interrupt-parent and
> interrupt-map properties. These properties are handled differently -
> the interrupt-map is fixed in place while interrupt-parent is
> decoded+encoded+set as a property.
> 
> This changes interrupt-parent fixing code to do what the interrupt-map
> code does because soon we are going to have more phandles to fix and some
> contain an array of phandles (such as "ibm,npu").
> 
> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
> ---
>  board-qemu/slof/fdt.fs | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
> 
> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
> index 8e8921f..5284ae6 100644
> --- a/board-qemu/slof/fdt.fs
> +++ b/board-qemu/slof/fdt.fs
> @@ -279,6 +279,23 @@ fdt-claim-reserve
>     2drop
>  ;
>  
> +: fdt-replace-l, 4 - swap 4 + swap ;

Why "l," ? I thought the "comma" words would be used for compiling stuff
into the dictionary, but apparently you're not doing this here?

> +: (fdt-replace-phandles) ( old new propname propnamelen node )

Please use the ( before -- after ) notation here:

( old new propname propnamelen node -- )

> +    get-property IF 2drop EXIT THEN
> +    BEGIN
> +        dup
> +    WHILE                   ( old new prop-addr prop-len )
> +        over l@
> +        4 pick = IF
> +            2 pick 2 pick l! \ replace old with new in place
> +            TRUE TO (fdt-phandle-replaced)
> +        THEN
> +        fdt-replace-l,

Maybe you could simply use the "4 - swap 4 + swap" directly here? It's
only one line of code, so not sure whether it's worth to put this into a
separate Forth word.

> +    REPEAT
> +    2drop 2drop
> +;
> +
>  \ Replace one phandle "old" with a phandle "new" in "node" and recursively
>  \ in its child nodes:
>  : fdt-replace-all-phandles ( old new node -- )
> @@ -288,14 +305,8 @@ fdt-claim-reserve
>        ( old new prop-addr prop-len  R: node )
>        fdt-replace-interrupt-map
>     THEN
> -   s" interrupt-parent" r@ get-property 0= IF
> -      ( old new prop-addr prop-len  R: node )
> -      decode-int -rot 2drop                  ( old new val  R: node )
> -      2 pick = IF                            ( old new      R: node )
> -         dup encode-int s" interrupt-parent" r@ set-property
> -         TRUE TO (fdt-phandle-replaced)
> -      THEN
> -   THEN
> +
> +   2dup s" interrupt-parent" r@ (fdt-replace-phandles)
>     \ ... add more properties that have to be fixed here ...
>     r>
>     \ Now recurse over all child nodes:       ( old new node )

Apart from the nits, this looks ok to me for small properties. But how
do you make sure that you do not accidentally replace the wrong spot in
bigger properties later?

 Thomas