[SLOF] [PATCH slof 1/2] fdt: Factor out code to replace a phandle in place
aik at ozlabs.ru
Mon May 21 17:17:13 AEST 2018
On 19/5/18 2:10 am, Thomas Huth wrote:
> 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
>> +: 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?
Not just dictionary, fdt-fl.fs uses this naming approach when adding to
flatten DT. Segher suggested, looks nicer.
>> +: (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.
"l," kind of allocates some bytes in some heap.
>> + 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 )
>> - 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 ...
>> \ 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?
Unlike already present and potentially dangerous "interrupt-xxx" hacking,
the new properties I am adding are storing only phandles (one or two). It
is worth commenting though, will add.
More information about the SLOF