Hi,

Am 19.07.22 um 06:44 schrieb Soma, Ashok Reddy:
Hi Stefan,

-----Original Message-----
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-...@weidmueller.com>
Sent: Saturday, July 16, 2022 4:48 PM
To: Simek, Michal <michal.si...@amd.com>; Ashok Reddy Soma 
<ashok.reddy.s...@xilinx.com>; u-boot@lists.denx.de
Cc: adrian.fiergol...@fastree3d.com; jh80.ch...@samsung.com; s...@svenpeter.dev; 
kette...@openbsd.org; s...@chromium.org; g...@xilinx.com; git (AMD-Xilinx) 
<g...@amd.com>
Subject: Re: [PATCH 2/5] firmware: zynqmp: Load config overlay for core0 to 
pmufw

CAUTION: This message has originated from an External Source. Please use proper 
judgment and caution when opening attachments, clicking links, or responding to 
this email.


Am 15.07.2022 um 18:34 schrieb Michal Simek:


On 7/15/22 18:13, Stefan Herbrechtsmeier wrote:
Am 15.07.2022 um 11:39 schrieb Ashok Reddy Soma:
Try loading pmufw config overlay for core0, if it doesn't return any
error it means pmufw is accepting nodes for other IP's. Otherwise
dont try to load config object for any other IP, just return from
zynqmp_pmufw_node function.

Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@xilinx.com>
---

   drivers/firmware/firmware-zynqmp.c | 14 ++++++++++++++
   1 file changed, 14 insertions(+)

diff --git a/drivers/firmware/firmware-zynqmp.c
b/drivers/firmware/firmware-zynqmp.c
index 34d9b47003..288151533e 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -68,8 +68,13 @@ int zynqmp_pmufw_config_close(void)
       return 0;
   }
+static bool config_enabled;
+

Please move the variable inside the function.

How can this work? When you move it to zynqmp_pmufw_node() then won't
be visible in zynqmp_power_probe() and vice-versa.

If you reuse the zynqmp_pmufw_node function in zynqmp_power_probe function you 
can check the id parameter to update the config_enabled variable in 
zynqmp_pmufw_node.

are you suggesting to change like this ?  this works fine for me. Shall i send V2 
with this >
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -29,6 +29,7 @@ struct zynqmp_power {
  } zynqmp_power;
#define NODE_ID_LOCATION 5
+#define APU0_ID                        NODE_APU_0

Why don't you use the NODE_APU_0 define direct?

  static unsigned int xpm_configobject[] = {
         
/**********************************************************************/
@@ -68,18 +69,22 @@ int zynqmp_pmufw_config_close(void)
         return 0;
  }
-static bool config_enabled; int zynqmp_pmufw_node(u32 id)
  {
-       if (!config_enabled)
+       static bool config_enabled;

I would invert the meaning from enabled to skip ...

+       int ret;
+
+       if (!config_enabled && id != APU0_ID)

to simplify the check.

        if (skip)

                 return 0;
/* Record power domain id */
         xpm_configobject[NODE_ID_LOCATION] = id;
- zynqmp_pmufw_load_config_object(xpm_configobject,
-                                       sizeof(xpm_configobject));
+       ret = zynqmp_pmufw_load_config_object(xpm_configobject,
+                                             sizeof(xpm_configobject));
+       if(!ret && id == APU0_ID)
+               config_enabled = true;

        if(ret && id == APU0_ID)
                skip = true;

return 0;
  }
@@ -272,14 +277,8 @@ 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)) {
-               xpm_configobject[NODE_ID_LOCATION] = NODE_APU_0;
-
-               ret = zynqmp_pmufw_load_config_object(xpm_configobject,
-                                                     sizeof(xpm_configobject));
-               if (!ret)
-                       config_enabled = true;
-       }
+       if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
+               zynqmp_pmufw_node(APU0_ID);
return 0;

Regards
  Stefan

Reply via email to