RE: [PATCH 2/5] firmware: zynqmp: Load config overlay for core0 to pmufw

[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
 
 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;
+       int ret;
+
+       if (!config_enabled && id != APU0_ID)
                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;
 
        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;

Thanks,
Ashok