Re: [ImageBuilder v2] Add support for 64-bit addresses/sizes

[Michal Orzel <michal.orzel@xxxxxxx>]

On 10/10/2022 12:48, Xenia Ragiadakou wrote:
> 
> 
> On 10/10/22 12:48, Michal Orzel wrote:
>> Hi Xenia,
>>
>> On 10/10/2022 10:52, Xenia Ragiadakou wrote:
>>>
>>>
>>> On 10/10/22 10:29, Michal Orzel wrote:
>>>
>>> Hi Michal
>>>
>>>> At the moment, ImageBuilder assumes that all addresses/sizes are
>>>> 32-bit max. It sets #{address,size}-cells to 0x2 and puts 0x0 as the
>>>> value for the first cell. Because of that, we cannot specify
>>>> MEMORY_START and MEMORY_END to be above 32-bits (e.g. to place the images
>>>> in the upper memory bank).
>>>>
>>>> Add support to properly handle 64-bit addresses/sizes:
>>>>    - add function split_into_halves to split the value passed as a first
>>>>      argument into upper and lower halves. These are then set as values for
>>>>      variables passed respetively as the second and third argument,
>>>
>>> s/respetively/respectively/
>> Ok.
>>
>>>
>>>>    - add function split_addr_size to split address and size and form a
>>>>      string to be passed to dt_set as data argument for reg property.
>>>>
>>>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>>>> ---
>>>> Changes in v2:
>>>> - redesign a patch based on master-next instead of NXP dynamic assignment 
>>>> patch
>>>> ---
>>>>    scripts/uboot-script-gen | 33 ++++++++++++++++++++++++++++-----
>>>>    1 file changed, 28 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
>>>> index b24dca2b7f7e..09d237d192c1 100755
>>>> --- a/scripts/uboot-script-gen
>>>> +++ b/scripts/uboot-script-gen
>>>> @@ -22,6 +22,29 @@ function dt_mknode()
>>>>        fi
>>>>    }
>>>>
>>>> +# Usage:
>>>> +# split_into_halves <value> <variable_to_store_upper> 
>>>> <variable_to_store_lower>
>>>> +function split_into_halves()
>>>> +{
>>>> +    local value=$1
>>>> +    local upper=$2
>>>> +    local lower=$3
>>>> +
>>>> +    eval "$upper=$(printf "0x%X\n" $(($value >> 32)))"
>>>> +    eval "$lower=$(printf "0x%X\n" $(($value & 0xFFFFFFFF)))"
>>>> +}
>>>> +
>>>> +function split_addr_size()
>>>> +{
>>>> +    local addr=$1
>>>> +    local size=$2
>>>> +
>>>> +    split_into_halves $addr addr_upper addr_lower
>>>> +    split_into_halves $size size_upper size_lower
>>>> +
>>>
>>> Just a minor observation, the variables addr_upper, addr_lower,
>>> size_upper and size_lower can be declared local.
>>>
>> This function is to be called to perform substitution and as such
>> is always executed within a subshell so no need for local.
> 
> So split_addr_size() is supposed to be executed only in a subshell ...
> Ok I did not think of that. So neither addr or size need to be declared
> local.
Exactly, but in ImageBuilder we don't seem to use $1, $2, ... directly so
that is why I added local only for the arguments passed to this function.

> 
> --
> Xenia

~Michal