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

[Thomas Huth]

On 21.05.2018 09:17, Alexey Kardashevskiy wrote:
> 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
>>>     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?
> 
> Not just dictionary, fdt-fl.fs uses this naming approach when adding to
> flatten DT. Segher suggested, looks nicer.

Ok, sure, but you don't *add* anything here, e.g. you also don't call
fdt-allot like in the other "fdt-something," function. IMHO the comma is
rather confusing 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.
> 
> "l," kind of allocates some bytes in some heap.

But your "fdt-replace-l," does not allocate anything, it just increases
an address on the stack and a size on the stack at the same time.

 Thomas