Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
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 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 --- 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)) || 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 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
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
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 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 --- 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)) || 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 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
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
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 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 --- 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)) || 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 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
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
Am 17.05.2023 um 14:12 schrieb Michal Simek: On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: From: Stefan Herbrechtsmeier 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 --- 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)) || 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 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 -->
Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
On 5/16/23 16:05, Stefan Herbrechtsmeier wrote: From: Stefan Herbrechtsmeier 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 --- 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)) || 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 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) { +
[PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message
From: Stefan Herbrechtsmeier 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 --- 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; - 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; }; -- 2.30.2