[SLOF] [PATCH slof 1/2] fdt: Factor out code to replace a phandle in place
thuth at redhat.com
Tue May 22 15:23:55 AEST 2018
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
>>> +: 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.
More information about the SLOF