| Message ID | 20230516140531.20722-2-stefan.herbrechtsmeier-oss@weidmueller.com |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
| Series | [v3,1/2] firmware: zynqmp: Remove extraordinary return value | expand |
On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: > From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > Move the permission to change a config object message from > zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function > to simplify the code and check the permission only if required. > > Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> > > --- > > Changes in v4: > - Reword > - Move the check back to zynqmp_pmufw_node because the check need to be > run after the config object load. > - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node > > Changes in v3: > - Added > > drivers/firmware/firmware-zynqmp.c | 36 ++++++++++++++---------------- > 1 file changed, 17 insertions(+), 19 deletions(-) > > diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c > index 2b1ad5d2c3..6dc745bd14 100644 > --- a/drivers/firmware/firmware-zynqmp.c > +++ b/drivers/firmware/firmware-zynqmp.c > @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { > > int zynqmp_pmufw_config_close(void) > { > - zynqmp_pmufw_load_config_object(xpm_configobject_close, > - sizeof(xpm_configobject_close)); > - return 0; > + return zynqmp_pmufw_load_config_object(xpm_configobject_close, > + sizeof(xpm_configobject_close)); > } > > int zynqmp_pmufw_node(u32 id) > { > - static bool skip_config; > - int ret; > + static bool checked; > + static bool skip; I see interesting behavior in connection to these variables. I did this change and keep test variable to see behavior. diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 6dc745bd1424..becbea7b64ea 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) sizeof(xpm_configobject_close)); } +static bool checked; +static bool skip; + int zynqmp_pmufw_node(u32 id) { - static bool checked; - static bool skip; + static bool test; + + printf("----------------%s, id %d, ch %d, skp %d - test %d\n", __func__, id, checked, skip, test); if (!checked) { checked = true; @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) int ret; struct udevice *child; + checked = 0; + skip = 0; + if ((IS_ENABLED(CONFIG_SPL_BUILD) && IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || <debug_uart> zynqmp_power_domain zynqmp_power_domain: Request for id: 34 zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa -----------zynqmp_pmufw_node ACCESS OK --------------------zynqmp_pmufw_load_config_object --------------------zynqmp_pmufw_load_config_object44 -----------zynqmp_pmufw_node ACCESS OK --------------------zynqmp_pmufw_load_config_object --------------------zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 zynq_serial_setbrg: CLK 99999999 U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 +0200) CPU: ZynqMP Silicon: v3 Chip: xck26 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa -----------zynqmp_pmufw_node ACCESS OK --------------------zynqmp_pmufw_load_config_object --------------------zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB Model: ZynqMP KV260 revB Board: Xilinx ZynqMP DRAM: 2 GiB (effective 4 GiB) zynqmp_power_domain zynqmp_power_domain: Request for id: 46 zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa -----------zynqmp_pmufw_node ACCESS OK --------------------zynqmp_pmufw_load_config_object --------------------zynqmp_pmufw_load_config_object44 -----------zynqmp_pmufw_node ACCESS OK --------------------zynqmp_pmufw_load_config_object --------------------zynqmp_pmufw_load_config_object44 zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 PMUFW: v1.1 zynqmp_power_domain zynqmp_power_domain: Request for id: 38 zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa -----------zynqmp_pmufw_node ACCESS OK As you see test variable is in BSS section but it is not initialized at this stage. If you look at arch/arm/lib/crt0_64.S debug uart is called before calling board_init_f and bss is cleared before board_init_r is called. It means variables should be placed to different section or initialized them directly from the code. > > - if (skip_config) > - return 0; > + if (!checked) { > + checked = true; > > - /* Record power domain id */ > - xpm_configobject[NODE_ID_LOCATION] = id; > + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { > + printf("PMUFW: No permission to change config object\n"); > + skip = true; > + } > + } > > - ret = zynqmp_pmufw_load_config_object(xpm_configobject, > - sizeof(xpm_configobject)); > + if (skip) > + return -EACCES; > > - if (ret == -EACCES && id == NODE_OCM_BANK_0) > - skip_config = true; > + /* Record power domain id */ > + xpm_configobject[NODE_ID_LOCATION] = id; > > - return 0; > + return zynqmp_pmufw_load_config_object(xpm_configobject, > + sizeof(xpm_configobject)); > } With this change there is also need to change drivers/power/domain/zynqmp-power-domain.c to handle return value for case that node is already configured. I would prefer to have separate error code for it just in case. static int zynqmp_power_domain_request(struct power_domain *power_domain) { + int ret = 0; + dev_dbg(power_domain->dev, "Request for id: %ld\n", power_domain->id); - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) - return zynqmp_pmufw_node(power_domain->id); + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) { + ret = zynqmp_pmufw_node(power_domain->id); + if (ret == -ENODEV) + ret = 0; + } - return 0; + return ret; } > > static int do_pm_probe(void) > @@ -250,8 +253,6 @@ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) > err = xilinx_pm_request(PM_SET_CONFIGURATION, (u32)(u64)cfg_obj, 0, 0, > 0, ret_payload); > if (err == XST_PM_NO_ACCESS) { > - if (((u32 *)cfg_obj)[NODE_ID_LOCATION] == NODE_OCM_BANK_0) > - printf("PMUFW: No permission to change config object\n"); > return -EACCES; > } > > @@ -295,9 +296,6 @@ static int zynqmp_power_probe(struct udevice *dev) > ret >> ZYNQMP_PM_VERSION_MAJOR_SHIFT, > ret & ZYNQMP_PM_VERSION_MINOR_MASK); > > - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) > - zynqmp_pmufw_node(NODE_OCM_BANK_0); > - > return 0; > }; > Thanks, Michal
Am 17.05.2023 um 14:12 schrieb Michal Simek: > On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >> >> Move the permission to change a config object message from >> zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function >> to simplify the code and check the permission only if required. >> >> Signed-off-by: Stefan Herbrechtsmeier >> <stefan.herbrechtsmeier@weidmueller.com> >> >> --- >> >> Changes in v4: >> - Reword >> - Move the check back to zynqmp_pmufw_node because the check need to be >> run after the config object load. >> - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node >> >> Changes in v3: >> - Added >> >> drivers/firmware/firmware-zynqmp.c | 36 ++++++++++++++---------------- >> 1 file changed, 17 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/firmware/firmware-zynqmp.c >> b/drivers/firmware/firmware-zynqmp.c >> index 2b1ad5d2c3..6dc745bd14 100644 >> --- a/drivers/firmware/firmware-zynqmp.c >> +++ b/drivers/firmware/firmware-zynqmp.c >> @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { >> int zynqmp_pmufw_config_close(void) >> { >> - zynqmp_pmufw_load_config_object(xpm_configobject_close, >> - sizeof(xpm_configobject_close)); >> - return 0; >> + return zynqmp_pmufw_load_config_object(xpm_configobject_close, >> + sizeof(xpm_configobject_close)); >> } >> int zynqmp_pmufw_node(u32 id) >> { >> - static bool skip_config; >> - int ret; >> + static bool checked; >> + static bool skip; > > I see interesting behavior in connection to these variables. > I did this change and keep test variable to see behavior. > > > diff --git a/drivers/firmware/firmware-zynqmp.c > b/drivers/firmware/firmware-zynqmp.c > index 6dc745bd1424..becbea7b64ea 100644 > --- a/drivers/firmware/firmware-zynqmp.c > +++ b/drivers/firmware/firmware-zynqmp.c > @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) > sizeof(xpm_configobject_close)); > } > > +static bool checked; > +static bool skip; > + > int zynqmp_pmufw_node(u32 id) > { > - static bool checked; > - static bool skip; > + static bool test; > + > + printf("----------------%s, id %d, ch %d, skp %d - test %d\n", > __func__, id, checked, skip, test); > > if (!checked) { > checked = true; > @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) > int ret; > struct udevice *child; > > + checked = 0; > + skip = 0; > + > if ((IS_ENABLED(CONFIG_SPL_BUILD) && > IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && > IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || > > > <debug_uart> > zynqmp_power_domain zynqmp_power_domain: Request for id: 34 > zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa > zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa > -----------zynqmp_pmufw_node ACCESS OK > --------------------zynqmp_pmufw_load_config_object > --------------------zynqmp_pmufw_load_config_object44 > -----------zynqmp_pmufw_node ACCESS OK > --------------------zynqmp_pmufw_load_config_object > --------------------zynqmp_pmufw_load_config_object44 > zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 > zynq_serial_setbrg: CLK 99999999 > > > U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 > +0200) > > CPU: ZynqMP > Silicon: v3 > Chip: xck26 > zynqmp_power_domain zynqmp_power_domain: Request for id: 38 > zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa > -----------zynqmp_pmufw_node ACCESS OK > --------------------zynqmp_pmufw_load_config_object > --------------------zynqmp_pmufw_load_config_object44 > zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 > Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB > Model: ZynqMP KV260 revB > Board: Xilinx ZynqMP > DRAM: 2 GiB (effective 4 GiB) > zynqmp_power_domain zynqmp_power_domain: Request for id: 46 > zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa > zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa > -----------zynqmp_pmufw_node ACCESS OK > --------------------zynqmp_pmufw_load_config_object > --------------------zynqmp_pmufw_load_config_object44 > -----------zynqmp_pmufw_node ACCESS OK > --------------------zynqmp_pmufw_load_config_object > --------------------zynqmp_pmufw_load_config_object44 > zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 > PMUFW: v1.1 > zynqmp_power_domain zynqmp_power_domain: Request for id: 38 > zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa > -----------zynqmp_pmufw_node ACCESS OK > > > As you see test variable is in BSS section but it is not initialized > at this stage. If you look at arch/arm/lib/crt0_64.S debug uart is > called before calling board_init_f and bss is cleared before > board_init_r is called. What does "but BSS and initialized non-const data are still not available" mean? Could we use variables from the data section like "static bool check = true"? > It means variables should be placed to different section or > initialized them directly from the code. I think the zynqmp_power variable could have the same problem. The initialization from the code doesn't work because the class is dynamic probed. zynqmp_pmufw_node --> zynqmp_pmufw_load_config_object --> xilinx_pm_request -(SPL)-> ipi_req --> do_pm_probe Maybe we need to rework the driver to use private driver data and probe the driver early in the chain. >> - if (skip_config) >> - return 0; >> + if (!checked) { >> + checked = true; >> - /* Record power domain id */ >> - xpm_configobject[NODE_ID_LOCATION] = id; >> + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { >> + printf("PMUFW: No permission to change config object\n"); >> + skip = true; >> + } >> + } >> - ret = zynqmp_pmufw_load_config_object(xpm_configobject, >> - sizeof(xpm_configobject)); >> + if (skip) >> + return -EACCES; >> - if (ret == -EACCES && id == NODE_OCM_BANK_0) >> - skip_config = true; >> + /* Record power domain id */ >> + xpm_configobject[NODE_ID_LOCATION] = id; >> - return 0; >> + return zynqmp_pmufw_load_config_object(xpm_configobject, >> + sizeof(xpm_configobject)); >> } > > With this change there is also need to change > drivers/power/domain/zynqmp-power-domain.c > > to handle return value for case that node is already configured. > I would prefer to have separate error code for it just in case. > > static int zynqmp_power_domain_request(struct power_domain > *power_domain) > { > + int ret = 0; > + > dev_dbg(power_domain->dev, "Request for id: %ld\n", > power_domain->id); > > - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) > - return zynqmp_pmufw_node(power_domain->id); > + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) { > + ret = zynqmp_pmufw_node(power_domain->id); > + if (ret == -ENODEV) > + ret = 0; > + } > > - return 0; > + return ret; > } > Should I add a patch to the series before this patch? > >> static int do_pm_probe(void) >> @@ -250,8 +253,6 @@ int zynqmp_pmufw_load_config_object(const void >> *cfg_obj, size_t size) >> err = xilinx_pm_request(PM_SET_CONFIGURATION, >> (u32)(u64)cfg_obj, 0, 0, >> 0, ret_payload); >> if (err == XST_PM_NO_ACCESS) { >> - if (((u32 *)cfg_obj)[NODE_ID_LOCATION] == NODE_OCM_BANK_0) >> - printf("PMUFW: No permission to change config object\n"); >> return -EACCES; >> } >> @@ -295,9 +296,6 @@ static int zynqmp_power_probe(struct udevice *dev) >> ret >> ZYNQMP_PM_VERSION_MAJOR_SHIFT, >> ret & ZYNQMP_PM_VERSION_MINOR_MASK); >> - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) >> - zynqmp_pmufw_node(NODE_OCM_BANK_0); >> - >> return 0; >> };
On 5/17/23 16:11, Stefan Herbrechtsmeier wrote: > Am 17.05.2023 um 14:12 schrieb Michal Simek: >> On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: >>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>> >>> Move the permission to change a config object message from >>> zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function >>> to simplify the code and check the permission only if required. >>> >>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>> >>> --- >>> >>> Changes in v4: >>> - Reword >>> - Move the check back to zynqmp_pmufw_node because the check need to be >>> run after the config object load. >>> - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node >>> >>> Changes in v3: >>> - Added >>> >>> drivers/firmware/firmware-zynqmp.c | 36 ++++++++++++++---------------- >>> 1 file changed, 17 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/firmware/firmware-zynqmp.c >>> b/drivers/firmware/firmware-zynqmp.c >>> index 2b1ad5d2c3..6dc745bd14 100644 >>> --- a/drivers/firmware/firmware-zynqmp.c >>> +++ b/drivers/firmware/firmware-zynqmp.c >>> @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { >>> int zynqmp_pmufw_config_close(void) >>> { >>> - zynqmp_pmufw_load_config_object(xpm_configobject_close, >>> - sizeof(xpm_configobject_close)); >>> - return 0; >>> + return zynqmp_pmufw_load_config_object(xpm_configobject_close, >>> + sizeof(xpm_configobject_close)); >>> } >>> int zynqmp_pmufw_node(u32 id) >>> { >>> - static bool skip_config; >>> - int ret; >>> + static bool checked; >>> + static bool skip; >> >> I see interesting behavior in connection to these variables. >> I did this change and keep test variable to see behavior. >> >> >> diff --git a/drivers/firmware/firmware-zynqmp.c >> b/drivers/firmware/firmware-zynqmp.c >> index 6dc745bd1424..becbea7b64ea 100644 >> --- a/drivers/firmware/firmware-zynqmp.c >> +++ b/drivers/firmware/firmware-zynqmp.c >> @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) >> sizeof(xpm_configobject_close)); >> } >> >> +static bool checked; >> +static bool skip; >> + >> int zynqmp_pmufw_node(u32 id) >> { >> - static bool checked; >> - static bool skip; >> + static bool test; >> + >> + printf("----------------%s, id %d, ch %d, skp %d - test %d\n", >> __func__, id, checked, skip, test); >> >> if (!checked) { >> checked = true; >> @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) >> int ret; >> struct udevice *child; >> >> + checked = 0; >> + skip = 0; >> + >> if ((IS_ENABLED(CONFIG_SPL_BUILD) && >> IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && >> IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || >> >> >> <debug_uart> >> zynqmp_power_domain zynqmp_power_domain: Request for id: 34 >> zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa >> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa >> -----------zynqmp_pmufw_node ACCESS OK >> --------------------zynqmp_pmufw_load_config_object >> --------------------zynqmp_pmufw_load_config_object44 >> -----------zynqmp_pmufw_node ACCESS OK >> --------------------zynqmp_pmufw_load_config_object >> --------------------zynqmp_pmufw_load_config_object44 >> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 >> zynq_serial_setbrg: CLK 99999999 >> >> >> U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 +0200) >> >> CPU: ZynqMP >> Silicon: v3 >> Chip: xck26 >> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa >> -----------zynqmp_pmufw_node ACCESS OK >> --------------------zynqmp_pmufw_load_config_object >> --------------------zynqmp_pmufw_load_config_object44 >> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 >> Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB >> Model: ZynqMP KV260 revB >> Board: Xilinx ZynqMP >> DRAM: 2 GiB (effective 4 GiB) >> zynqmp_power_domain zynqmp_power_domain: Request for id: 46 >> zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa >> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa >> -----------zynqmp_pmufw_node ACCESS OK >> --------------------zynqmp_pmufw_load_config_object >> --------------------zynqmp_pmufw_load_config_object44 >> -----------zynqmp_pmufw_node ACCESS OK >> --------------------zynqmp_pmufw_load_config_object >> --------------------zynqmp_pmufw_load_config_object44 >> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 >> PMUFW: v1.1 >> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa >> -----------zynqmp_pmufw_node ACCESS OK >> >> >> As you see test variable is in BSS section but it is not initialized at this >> stage. If you look at arch/arm/lib/crt0_64.S debug uart is called before >> calling board_init_f and bss is cleared before board_init_r is called. > > What does "but BSS and initialized non-const data are still not available" > mean? Could we use variables from the data section like "static bool check = true"? Yes - when you move that variable to data section then it should be fine. Or just move it like this struct fru_table fru_data __section(".data"); > >> It means variables should be placed to different section or initialized them >> directly from the code. > > I think the zynqmp_power variable could have the same problem. If it is called in early phase then yes. > > The initialization from the code doesn't work because the class is dynamic probed. > zynqmp_pmufw_node --> zynqmp_pmufw_load_config_object --> xilinx_pm_request > -(SPL)-> ipi_req --> do_pm_probe > > Maybe we need to rework the driver to use private driver data and probe the > driver early in the chain. But in SPL flow bss is initialized before board_init_r() which is done before sending request to PMUFW. <debug_uart> --------------board_init_f >>SPL: board_init_r() zynq_serial_setbrg: CLK 99999999 U-Boot SPL 2023.07-rc2-00051-g08bab040a7d7-dirty (May 18 2023 - 13:27:14 +0200) --------------------zynqmp_pmufw_load_config_object Loading new PMUFW cfg obj (2032 bytes) ipi_req, tx/rx - 0/0 ipi_req, tx/rx - 536871440/ > >>> - if (skip_config) >>> - return 0; >>> + if (!checked) { >>> + checked = true; >>> - /* Record power domain id */ >>> - xpm_configobject[NODE_ID_LOCATION] = id; >>> + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { >>> + printf("PMUFW: No permission to change config object\n"); >>> + skip = true; >>> + } >>> + } >>> - ret = zynqmp_pmufw_load_config_object(xpm_configobject, >>> - sizeof(xpm_configobject)); >>> + if (skip) >>> + return -EACCES; >>> - if (ret == -EACCES && id == NODE_OCM_BANK_0) >>> - skip_config = true; >>> + /* Record power domain id */ >>> + xpm_configobject[NODE_ID_LOCATION] = id; >>> - return 0; >>> + return zynqmp_pmufw_load_config_object(xpm_configobject, >>> + sizeof(xpm_configobject)); >>> } >> >> With this change there is also need to change >> drivers/power/domain/zynqmp-power-domain.c >> >> to handle return value for case that node is already configured. >> I would prefer to have separate error code for it just in case. >> >> static int zynqmp_power_domain_request(struct power_domain *power_domain) >> { >> + int ret = 0; >> + >> dev_dbg(power_domain->dev, "Request for id: %ld\n", power_domain->id); >> >> - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) >> - return zynqmp_pmufw_node(power_domain->id); >> + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) { >> + ret = zynqmp_pmufw_node(power_domain->id); >> + if (ret == -ENODEV) >> + ret = 0; >> + } >> >> - return 0; >> + return ret; >> } >> > Should I add a patch to the series before this patch? That would be the best solution. Thanks, Michal
Hi Michal, Am 18.05.2023 um 13:29 schrieb Michal Simek: > On 5/17/23 16:11, Stefan Herbrechtsmeier wrote: >> Am 17.05.2023 um 14:12 schrieb Michal Simek: >>> On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: >>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>>> >>>> Move the permission to change a config object message from >>>> zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function >>>> to simplify the code and check the permission only if required. >>>> >>>> Signed-off-by: Stefan Herbrechtsmeier >>>> <stefan.herbrechtsmeier@weidmueller.com> >>>> >>>> --- >>>> >>>> Changes in v4: >>>> - Reword >>>> - Move the check back to zynqmp_pmufw_node because the check need >>>> to be >>>> run after the config object load. >>>> - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node >>>> >>>> Changes in v3: >>>> - Added >>>> >>>> drivers/firmware/firmware-zynqmp.c | 36 >>>> ++++++++++++++---------------- >>>> 1 file changed, 17 insertions(+), 19 deletions(-) >>>> >>>> diff --git a/drivers/firmware/firmware-zynqmp.c >>>> b/drivers/firmware/firmware-zynqmp.c >>>> index 2b1ad5d2c3..6dc745bd14 100644 >>>> --- a/drivers/firmware/firmware-zynqmp.c >>>> +++ b/drivers/firmware/firmware-zynqmp.c >>>> @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { >>>> int zynqmp_pmufw_config_close(void) >>>> { >>>> - zynqmp_pmufw_load_config_object(xpm_configobject_close, >>>> - sizeof(xpm_configobject_close)); >>>> - return 0; >>>> + return zynqmp_pmufw_load_config_object(xpm_configobject_close, >>>> + sizeof(xpm_configobject_close)); >>>> } >>>> int zynqmp_pmufw_node(u32 id) >>>> { >>>> - static bool skip_config; >>>> - int ret; >>>> + static bool checked; >>>> + static bool skip; >>> >>> I see interesting behavior in connection to these variables. >>> I did this change and keep test variable to see behavior. >>> >>> >>> diff --git a/drivers/firmware/firmware-zynqmp.c >>> b/drivers/firmware/firmware-zynqmp.c >>> index 6dc745bd1424..becbea7b64ea 100644 >>> --- a/drivers/firmware/firmware-zynqmp.c >>> +++ b/drivers/firmware/firmware-zynqmp.c >>> @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) >>> sizeof(xpm_configobject_close)); >>> } >>> >>> +static bool checked; >>> +static bool skip; >>> + >>> int zynqmp_pmufw_node(u32 id) >>> { >>> - static bool checked; >>> - static bool skip; >>> + static bool test; >>> + >>> + printf("----------------%s, id %d, ch %d, skp %d - test >>> %d\n", __func__, id, checked, skip, test); >>> >>> if (!checked) { >>> checked = true; >>> @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice >>> *dev) >>> int ret; >>> struct udevice *child; >>> >>> + checked = 0; >>> + skip = 0; >>> + >>> if ((IS_ENABLED(CONFIG_SPL_BUILD) && >>> IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && >>> IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || >>> >>> >>> <debug_uart> >>> zynqmp_power_domain zynqmp_power_domain: Request for id: 34 >>> zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa >>> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa >>> -----------zynqmp_pmufw_node ACCESS OK >>> --------------------zynqmp_pmufw_load_config_object >>> --------------------zynqmp_pmufw_load_config_object44 >>> -----------zynqmp_pmufw_node ACCESS OK >>> --------------------zynqmp_pmufw_load_config_object >>> --------------------zynqmp_pmufw_load_config_object44 >>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 >>> zynq_serial_setbrg: CLK 99999999 >>> >>> >>> U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 >>> +0200) >>> >>> CPU: ZynqMP >>> Silicon: v3 >>> Chip: xck26 >>> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >>> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa >>> -----------zynqmp_pmufw_node ACCESS OK >>> --------------------zynqmp_pmufw_load_config_object >>> --------------------zynqmp_pmufw_load_config_object44 >>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 >>> Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB >>> Model: ZynqMP KV260 revB >>> Board: Xilinx ZynqMP >>> DRAM: 2 GiB (effective 4 GiB) >>> zynqmp_power_domain zynqmp_power_domain: Request for id: 46 >>> zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa >>> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa >>> -----------zynqmp_pmufw_node ACCESS OK >>> --------------------zynqmp_pmufw_load_config_object >>> --------------------zynqmp_pmufw_load_config_object44 >>> -----------zynqmp_pmufw_node ACCESS OK >>> --------------------zynqmp_pmufw_load_config_object >>> --------------------zynqmp_pmufw_load_config_object44 >>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 >>> PMUFW: v1.1 >>> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >>> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa >>> -----------zynqmp_pmufw_node ACCESS OK >>> >>> >>> As you see test variable is in BSS section but it is not initialized >>> at this stage. If you look at arch/arm/lib/crt0_64.S debug uart is >>> called before calling board_init_f and bss is cleared before >>> board_init_r is called. >> >> What does "but BSS and initialized non-const data are still not >> available" mean? Could we use variables from the data section like >> "static bool check = true"? > > Yes - when you move that variable to data section then it should be fine. > Or just move it like this > struct fru_table fru_data __section(".data"); Okay, I will use global data and move it to the data section. >>> It means variables should be placed to different section or >>> initialized them directly from the code. >> >> I think the zynqmp_power variable could have the same problem. > > If it is called in early phase then yes. It is used by the ipi_req function to know if the driver need to be probed. But this is only a problem if the U-Boot is used in EL3. >> The initialization from the code doesn't work because the class is >> dynamic probed. >> zynqmp_pmufw_node --> zynqmp_pmufw_load_config_object --> >> xilinx_pm_request >> -(SPL)-> ipi_req --> do_pm_probe >> >> Maybe we need to rework the driver to use private driver data and >> probe the driver early in the chain. > > But in SPL flow bss is initialized before board_init_r() which is done > before sending request to PMUFW. > > <debug_uart> > --------------board_init_f > >>SPL: board_init_r() > zynq_serial_setbrg: CLK 99999999 > > U-Boot SPL 2023.07-rc2-00051-g08bab040a7d7-dirty (May 18 2023 - > 13:27:14 +0200) > --------------------zynqmp_pmufw_load_config_object > Loading new PMUFW cfg obj (2032 bytes) > ipi_req, tx/rx - 0/0 > ipi_req, tx/rx - 536871440/ The problem is the zynqmp_power_domain driver. This driver is used before relocation and even before PMU Firmware load in SPL. The first could be fixed by the data section but after the error forwarding change below the driver could break the SPL and the firmware need do be loaded in the probe function. >>>> - if (skip_config) >>>> - return 0; >>>> + if (!checked) { >>>> + checked = true; >>>> - /* Record power domain id */ >>>> - xpm_configobject[NODE_ID_LOCATION] = id; >>>> + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { >>>> + printf("PMUFW: No permission to change config >>>> object\n"); >>>> + skip = true; >>>> + } >>>> + } >>>> - ret = zynqmp_pmufw_load_config_object(xpm_configobject, >>>> - sizeof(xpm_configobject)); >>>> + if (skip) >>>> + return -EACCES; >>>> - if (ret == -EACCES && id == NODE_OCM_BANK_0) >>>> - skip_config = true; >>>> + /* Record power domain id */ >>>> + xpm_configobject[NODE_ID_LOCATION] = id; >>>> - return 0; >>>> + return zynqmp_pmufw_load_config_object(xpm_configobject, >>>> + sizeof(xpm_configobject)); >>>> } >>> >>> With this change there is also need to change >>> drivers/power/domain/zynqmp-power-domain.c >>> >>> to handle return value for case that node is already configured. >>> I would prefer to have separate error code for it just in case. >>> >>> static int zynqmp_power_domain_request(struct power_domain >>> *power_domain) >>> { >>> + int ret = 0; >>> + >>> dev_dbg(power_domain->dev, "Request for id: %ld\n", >>> power_domain->id); >>> >>> - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) >>> - return zynqmp_pmufw_node(power_domain->id); >>> + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) { >>> + ret = zynqmp_pmufw_node(power_domain->id); >>> + if (ret == -ENODEV) >>> + ret = 0; >>> + } >>> >>> - return 0; >>> + return ret; >>> } >>> >> Should I add a patch to the series before this patch? > > That would be the best solution. I assume this breaks the SPL if the CONFIG_SPL_POWER_DOMAIN is enabled because the dev_power_domain_on will fail. Regards Stefan
Hi, On 5/22/23 12:55, Stefan Herbrechtsmeier wrote: > Hi Michal, > > Am 18.05.2023 um 13:29 schrieb Michal Simek: > >> On 5/17/23 16:11, Stefan Herbrechtsmeier wrote: >>> Am 17.05.2023 um 14:12 schrieb Michal Simek: >>>> On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: >>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>>>> >>>>> Move the permission to change a config object message from >>>>> zynqmp_pmufw_load_config_object function to zynqmp_pmufw_node function >>>>> to simplify the code and check the permission only if required. >>>>> >>>>> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com> >>>>> >>>>> --- >>>>> >>>>> Changes in v4: >>>>> - Reword >>>>> - Move the check back to zynqmp_pmufw_node because the check need to be >>>>> run after the config object load. >>>>> - Return error in zynqmp_pmufw_config_close and zynqmp_pmufw_node >>>>> >>>>> Changes in v3: >>>>> - Added >>>>> >>>>> drivers/firmware/firmware-zynqmp.c | 36 ++++++++++++++---------------- >>>>> 1 file changed, 17 insertions(+), 19 deletions(-) >>>>> >>>>> diff --git a/drivers/firmware/firmware-zynqmp.c >>>>> b/drivers/firmware/firmware-zynqmp.c >>>>> index 2b1ad5d2c3..6dc745bd14 100644 >>>>> --- a/drivers/firmware/firmware-zynqmp.c >>>>> +++ b/drivers/firmware/firmware-zynqmp.c >>>>> @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { >>>>> int zynqmp_pmufw_config_close(void) >>>>> { >>>>> - zynqmp_pmufw_load_config_object(xpm_configobject_close, >>>>> - sizeof(xpm_configobject_close)); >>>>> - return 0; >>>>> + return zynqmp_pmufw_load_config_object(xpm_configobject_close, >>>>> + sizeof(xpm_configobject_close)); >>>>> } >>>>> int zynqmp_pmufw_node(u32 id) >>>>> { >>>>> - static bool skip_config; >>>>> - int ret; >>>>> + static bool checked; >>>>> + static bool skip; >>>> >>>> I see interesting behavior in connection to these variables. >>>> I did this change and keep test variable to see behavior. >>>> >>>> >>>> diff --git a/drivers/firmware/firmware-zynqmp.c >>>> b/drivers/firmware/firmware-zynqmp.c >>>> index 6dc745bd1424..becbea7b64ea 100644 >>>> --- a/drivers/firmware/firmware-zynqmp.c >>>> +++ b/drivers/firmware/firmware-zynqmp.c >>>> @@ -67,10 +67,14 @@ int zynqmp_pmufw_config_close(void) >>>> sizeof(xpm_configobject_close)); >>>> } >>>> >>>> +static bool checked; >>>> +static bool skip; >>>> + >>>> int zynqmp_pmufw_node(u32 id) >>>> { >>>> - static bool checked; >>>> - static bool skip; >>>> + static bool test; >>>> + >>>> + printf("----------------%s, id %d, ch %d, skp %d - test %d\n", >>>> __func__, id, checked, skip, test); >>>> >>>> if (!checked) { >>>> checked = true; >>>> @@ -379,6 +391,9 @@ static int zynqmp_firmware_bind(struct udevice *dev) >>>> int ret; >>>> struct udevice *child; >>>> >>>> + checked = 0; >>>> + skip = 0; >>>> + >>>> if ((IS_ENABLED(CONFIG_SPL_BUILD) && >>>> IS_ENABLED(CONFIG_SPL_POWER_DOMAIN) && >>>> IS_ENABLED(CONFIG_ZYNQMP_POWER_DOMAIN)) || >>>> >>>> >>>> <debug_uart> >>>> zynqmp_power_domain zynqmp_power_domain: Request for id: 34 >>>> zynqmp_pmufw_node, id 34, ch 0, skp 0 - test 255/815a2fa >>>> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 255/815a2fa >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> --------------------zynqmp_pmufw_load_config_object >>>> --------------------zynqmp_pmufw_load_config_object44 >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> --------------------zynqmp_pmufw_load_config_object >>>> --------------------zynqmp_pmufw_load_config_object44 >>>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 34 >>>> zynq_serial_setbrg: CLK 99999999 >>>> >>>> >>>> U-Boot 2023.07-rc2-00053-gaf7817988644-dirty (May 17 2023 - 14:03:37 +0200) >>>> >>>> CPU: ZynqMP >>>> Silicon: v3 >>>> Chip: xck26 >>>> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >>>> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/815a2fa >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> --------------------zynqmp_pmufw_load_config_object >>>> --------------------zynqmp_pmufw_load_config_object44 >>>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 38 >>>> Detected name: zynqmp-smk-k26-xcl2g-revA-sck-kv-g-revB >>>> Model: ZynqMP KV260 revB >>>> Board: Xilinx ZynqMP >>>> DRAM: 2 GiB (effective 4 GiB) >>>> zynqmp_power_domain zynqmp_power_domain: Request for id: 46 >>>> zynqmp_pmufw_node, id 46, ch 0, skp 0 - test 0/7ffd42fa >>>> zynqmp_pmufw_node, id 11, ch 1, skp 0 - test 0/7ffd42fa >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> --------------------zynqmp_pmufw_load_config_object >>>> --------------------zynqmp_pmufw_load_config_object44 >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> --------------------zynqmp_pmufw_load_config_object >>>> --------------------zynqmp_pmufw_load_config_object44 >>>> zynqmp_power_domain zynqmp_power_domain: Domain ON for id: 46 >>>> PMUFW: v1.1 >>>> zynqmp_power_domain zynqmp_power_domain: Request for id: 38 >>>> zynqmp_pmufw_node, id 38, ch 1, skp 0 - test 1/7ffd42fa >>>> -----------zynqmp_pmufw_node ACCESS OK >>>> >>>> >>>> As you see test variable is in BSS section but it is not initialized at this >>>> stage. If you look at arch/arm/lib/crt0_64.S debug uart is called before >>>> calling board_init_f and bss is cleared before board_init_r is called. >>> >>> What does "but BSS and initialized non-const data are still not available" >>> mean? Could we use variables from the data section like "static bool check = >>> true"? >> >> Yes - when you move that variable to data section then it should be fine. >> Or just move it like this >> struct fru_table fru_data __section(".data"); > > Okay, I will use global data and move it to the data section. > >>>> It means variables should be placed to different section or initialized them >>>> directly from the code. >>> >>> I think the zynqmp_power variable could have the same problem. >> >> If it is called in early phase then yes. > It is used by the ipi_req function to know if the driver need to be probed. But > this is only a problem if the U-Boot is used in EL3. > >>> The initialization from the code doesn't work because the class is dynamic >>> probed. >>> zynqmp_pmufw_node --> zynqmp_pmufw_load_config_object --> xilinx_pm_request >>> -(SPL)-> ipi_req --> do_pm_probe >>> >>> Maybe we need to rework the driver to use private driver data and probe the >>> driver early in the chain. >> >> But in SPL flow bss is initialized before board_init_r() which is done before >> sending request to PMUFW. >> >> <debug_uart> >> --------------board_init_f >> >>SPL: board_init_r() >> zynq_serial_setbrg: CLK 99999999 >> >> U-Boot SPL 2023.07-rc2-00051-g08bab040a7d7-dirty (May 18 2023 - 13:27:14 +0200) >> --------------------zynqmp_pmufw_load_config_object >> Loading new PMUFW cfg obj (2032 bytes) >> ipi_req, tx/rx - 0/0 >> ipi_req, tx/rx - 536871440/ > > The problem is the zynqmp_power_domain driver. This driver is used before > relocation and even before PMU Firmware load in SPL. The first could be fixed by > the data section but after the error forwarding change below the driver could > break the SPL and the firmware need do be loaded in the probe function. > >>>>> - if (skip_config) >>>>> - return 0; >>>>> + if (!checked) { >>>>> + checked = true; >>>>> - /* Record power domain id */ >>>>> - xpm_configobject[NODE_ID_LOCATION] = id; >>>>> + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { >>>>> + printf("PMUFW: No permission to change config object\n"); >>>>> + skip = true; >>>>> + } >>>>> + } >>>>> - ret = zynqmp_pmufw_load_config_object(xpm_configobject, >>>>> - sizeof(xpm_configobject)); >>>>> + if (skip) >>>>> + return -EACCES; >>>>> - if (ret == -EACCES && id == NODE_OCM_BANK_0) >>>>> - skip_config = true; >>>>> + /* Record power domain id */ >>>>> + xpm_configobject[NODE_ID_LOCATION] = id; >>>>> - return 0; >>>>> + return zynqmp_pmufw_load_config_object(xpm_configobject, >>>>> + sizeof(xpm_configobject)); >>>>> } >>>> >>>> With this change there is also need to change >>>> drivers/power/domain/zynqmp-power-domain.c >>>> >>>> to handle return value for case that node is already configured. >>>> I would prefer to have separate error code for it just in case. >>>> >>>> static int zynqmp_power_domain_request(struct power_domain *power_domain) >>>> { >>>> + int ret = 0; >>>> + >>>> dev_dbg(power_domain->dev, "Request for id: %ld\n", power_domain->id); >>>> >>>> - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) >>>> - return zynqmp_pmufw_node(power_domain->id); >>>> + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) { >>>> + ret = zynqmp_pmufw_node(power_domain->id); >>>> + if (ret == -ENODEV) >>>> + ret = 0; >>>> + } >>>> >>>> - return 0; >>>> + return ret; >>>> } >>>> >>> Should I add a patch to the series before this patch? >> >> That would be the best solution. > > I assume this breaks the SPL if the CONFIG_SPL_POWER_DOMAIN is enabled because > the dev_power_domain_on will fail. I likely never tried it myself. I don't think pmufw keeps track of resources and all power domains are enabled after power up anyway. M
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 2b1ad5d2c3..6dc745bd14 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -63,29 +63,32 @@ static unsigned int xpm_configobject_close[] = { int zynqmp_pmufw_config_close(void) { - zynqmp_pmufw_load_config_object(xpm_configobject_close, - sizeof(xpm_configobject_close)); - return 0; + return zynqmp_pmufw_load_config_object(xpm_configobject_close, + sizeof(xpm_configobject_close)); } int zynqmp_pmufw_node(u32 id) { - static bool skip_config; - int ret; + static bool checked; + static bool skip; - if (skip_config) - return 0; + if (!checked) { + checked = true; - /* Record power domain id */ - xpm_configobject[NODE_ID_LOCATION] = id; + if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) { + printf("PMUFW: No permission to change config object\n"); + skip = true; + } + } - ret = zynqmp_pmufw_load_config_object(xpm_configobject, - sizeof(xpm_configobject)); + if (skip) + return -EACCES; - if (ret == -EACCES && id == NODE_OCM_BANK_0) - skip_config = true; + /* Record power domain id */ + xpm_configobject[NODE_ID_LOCATION] = id; - return 0; + return zynqmp_pmufw_load_config_object(xpm_configobject, + sizeof(xpm_configobject)); } static int do_pm_probe(void) @@ -250,8 +253,6 @@ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) err = xilinx_pm_request(PM_SET_CONFIGURATION, (u32)(u64)cfg_obj, 0, 0, 0, ret_payload); if (err == XST_PM_NO_ACCESS) { - if (((u32 *)cfg_obj)[NODE_ID_LOCATION] == NODE_OCM_BANK_0) - printf("PMUFW: No permission to change config object\n"); return -EACCES; } @@ -295,9 +296,6 @@ static int zynqmp_power_probe(struct udevice *dev) ret >> ZYNQMP_PM_VERSION_MAJOR_SHIFT, ret & ZYNQMP_PM_VERSION_MINOR_MASK); - if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) - zynqmp_pmufw_node(NODE_OCM_BANK_0); - return 0; };