Hi Michal,
>>
>> +static int __init handle_shared_mem_bank(struct domain *d, paddr_t gbase,
>> + bool owner_dom_io,
>> + const char *role_str,
>> + const struct membank *shm_bank)
>> +{
>> + paddr_t pbase, psize;
>> + int ret;
>> +
>> + BUG_ON(!shm_bank);
> not needed
>
>> +
>> + pbase = shm_bank->start;
>> + psize = shm_bank->size;
> please add empty line here
Will do
>>
>> int __init process_shm(struct domain *d, struct kernel_info *kinfo,
>> const struct dt_device_node *node)
>> {
>> @@ -249,32 +290,10 @@ int __init process_shm(struct domain *d, struct
>> kernel_info *kinfo,
>> if ( dt_property_read_string(shm_node, "role", &role_str) == 0 )
>> owner_dom_io = false;
> Looking at owner_dom_io, why don't you move parsing role and setting
> owner_dom_io accordingly to handle_shared_mem_bank()?
I think I wanted to keep all dt_* functions on the same level inside
process_shm, otherwise yes, I could
pass down shm_node and do the reading of role_str in handle_shared_mem_bank, or
I could derive
owner_dom_io from role_str being passed or not, something like:
role_str = NULL;
dt_property_read_string(shm_node, "role", &role_str)
[inside handle_shared_mem_bank]:
If ( role_str )
owner_dom_io = false;
And pass only role_str to handle_shared_mem_bank.
Is this comment to reduce the number of parameters passed? I guess it’s not for
where we call
dt_property_read_string isn’t it?
Cheers,
Luca