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