Re: [PATCH v3 2/2] firmware: zynqmp: Move permission to change config object message

2023-05-22 Thread Michal Simek

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

2023-05-22 Thread Stefan Herbrechtsmeier

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

2023-05-18 Thread 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");



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

2023-05-17 Thread Stefan Herbrechtsmeier

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

2023-05-17 Thread 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.


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

2023-05-16 Thread Stefan Herbrechtsmeier
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