[PATCH 0/5] Enable power domain driver in ZynqMP and Versal

[PATCH 0/5] Enable power domain driver in ZynqMP and Versal

[Ashok Reddy Soma]

This patch series enables power domain driver in ZynqMP and Versal
platforms and its dependencies.
 - Add a preliminary check by loading overlay for APU0 and see if the
   pmufw accepts the config objects, based on that continue to load
   further objects or not.
 - This is needed to dynamically request for node of the IP's based on
   DT.
 - Load pmufw config object dynamically based on DT.



Ashok Reddy Soma (5):
  firmware: zynqmp: Change prototype of
    zynqmp_pmufw_load_config_object()
  firmware: zynqmp: Load config overlay for core0 to pmufw
  arm64: zynqmp: Enable power domain driver
  mailbox: zynqmp: Move struct zynqmp_ipi_msg from sys_proto.h
  arm64: versal: Enable power domain driver and its dependencies

 arch/arm/mach-zynqmp/include/mach/sys_proto.h |  5 -----
 configs/xilinx_versal_virt_defconfig          |  4 ++++
 configs/xilinx_zynqmp_virt_defconfig          |  2 ++
 drivers/firmware/firmware-zynqmp.c            | 22 ++++++++++++++++---
 drivers/mailbox/Kconfig                       |  2 +-
 drivers/mailbox/zynqmp-ipi.c                  |  2 +-
 include/zynqmp_firmware.h                     |  7 +++++-
 7 files changed, 33 insertions(+), 11 deletions(-)

-- 
2.17.1

[PATCH 1/5] firmware: zynqmp: Change prototype of zynqmp_pmufw_load_config_object()

[Ashok Reddy Soma]

zynqmp_pmufw_load_config_object() has some error cases and it is better
to return those errors. Change prototype of this function to return
errors.

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

 drivers/firmware/firmware-zynqmp.c | 8 +++++---
 include/zynqmp_firmware.h          | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c
index 0f0d2b07c0..34d9b47003 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -210,7 +210,7 @@ int zynqmp_pm_is_function_supported(const u32 api_id, const u32 id)
  * @cfg_obj: Pointer to the configuration object
  * @size:    Size of @cfg_obj in bytes
  */
-void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
+int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
 {
 	int err;
 	u32 ret_payload[PAYLOAD_ARG_CNT];
@@ -224,12 +224,12 @@ void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
 				0, ret_payload);
 	if (err == XST_PM_NO_ACCESS) {
 		printf("PMUFW no permission to change config object\n");
-		return;
+		return -EACCES;
 	}
 
 	if (err == XST_PM_ALREADY_CONFIGURED) {
 		debug("PMUFW Node is already configured\n");
-		return;
+		return -ENODEV;
 	}
 
 	if (err)
@@ -240,6 +240,8 @@ void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size)
 
 	if ((err || ret_payload[0]) && IS_ENABLED(CONFIG_SPL_BUILD))
 		panic("PMUFW config object loading failed in EL3\n");
+
+	return 0;
 }
 
 static int zynqmp_power_probe(struct udevice *dev)
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
index 6c4fd9a6c5..1c22a62207 100644
--- a/include/zynqmp_firmware.h
+++ b/include/zynqmp_firmware.h
@@ -449,7 +449,7 @@ enum pm_gem_config_type {
 unsigned int zynqmp_firmware_version(void);
 int zynqmp_pmufw_node(u32 id);
 int zynqmp_pmufw_config_close(void);
-void zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
+int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size);
 int xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2,
 		      u32 arg3, u32 *ret_payload);
 int zynqmp_pm_set_sd_config(u32 node, enum pm_sd_config_type config, u32 value);
-- 
2.17.1

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

[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.soma@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;
+
 int zynqmp_pmufw_node(u32 id)
 {
+	if (!config_enabled)
+		return 0;
+
 	/* Record power domain id */
 	xpm_configobject[NODE_ID_LOCATION] = id;
 
@@ -267,6 +272,15 @@ 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;
+	}
+
 	return 0;
 };
 
-- 
2.17.1

[PATCH 3/5] arm64: zynqmp: Enable power domain driver

[Ashok Reddy Soma]

Enable power domain driver to configure pmufw config object and request
node for all the IP's that are enabled in DT.

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

 configs/xilinx_zynqmp_virt_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
index 35894076c5..85cd5d6d9e 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -162,6 +162,8 @@ CONFIG_PHY_XILINX_GMII2RGMII=y
 CONFIG_PHY_FIXED=y
 CONFIG_XILINX_AXIEMAC=y
 CONFIG_ZYNQ_GEM=y
+CONFIG_POWER_DOMAIN=y
+CONFIG_ZYNQMP_POWER_DOMAIN=y
 CONFIG_DM_REGULATOR=y
 CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_PWM=y
-- 
2.17.1

[PATCH 4/5] mailbox: zynqmp: Move struct zynqmp_ipi_msg from sys_proto.h

[Ashok Reddy Soma]

Mailbox driver might be need for Versal and other future platforms.
To remove the dependency, move struct zynqmp_ipi_msg to
zynqmp_firmware.h so that mailbox driver compiles for other platforms
easily.

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

 arch/arm/mach-zynqmp/include/mach/sys_proto.h | 5 -----
 drivers/mailbox/zynqmp-ipi.c                  | 2 +-
 include/zynqmp_firmware.h                     | 5 +++++
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-zynqmp/include/mach/sys_proto.h b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
index 1c12eac715..9fffb4e541 100644
--- a/arch/arm/mach-zynqmp/include/mach/sys_proto.h
+++ b/arch/arm/mach-zynqmp/include/mach/sys_proto.h
@@ -46,11 +46,6 @@ enum {
 	TCM_SPLIT,
 };
 
-struct zynqmp_ipi_msg {
-	size_t len;
-	u32 *buf;
-};
-
 int zynq_board_read_rom_ethaddr(unsigned char *ethaddr);
 unsigned int zynqmp_get_silicon_version(void);
 
diff --git a/drivers/mailbox/zynqmp-ipi.c b/drivers/mailbox/zynqmp-ipi.c
index 959cce923c..3e4ec47389 100644
--- a/drivers/mailbox/zynqmp-ipi.c
+++ b/drivers/mailbox/zynqmp-ipi.c
@@ -11,10 +11,10 @@
 #include <dm.h>
 #include <mailbox-uclass.h>
 #include <dm/device_compat.h>
-#include <mach/sys_proto.h>
 #include <linux/ioport.h>
 #include <linux/io.h>
 #include <wait_bit.h>
+#include <zynqmp_firmware.h>
 
 /* IPI bitmasks, register base */
 /* TODO: move reg base to DT */
diff --git a/include/zynqmp_firmware.h b/include/zynqmp_firmware.h
index 1c22a62207..fa969bf336 100644
--- a/include/zynqmp_firmware.h
+++ b/include/zynqmp_firmware.h
@@ -492,4 +492,9 @@ enum zynqmp_pm_request_ack {
 /* PM API versions */
 #define PM_API_VERSION_2		2
 
+struct zynqmp_ipi_msg {
+	size_t len;
+	u32 *buf;
+};
+
 #endif /* _ZYNQMP_FIRMWARE_H_ */
-- 
2.17.1

[PATCH 5/5] arm64: versal: Enable power domain driver and its dependencies

[Ashok Reddy Soma]

Enable power domain driver to configure pmufw config object and request
node for all the IP's that are enabled in DT.

This driver depends on mailbox and IPI driver, hence enable them as well.
Add ARCH_VERSAL in the depends on of mailbox Kconfig to compile for
Versal platforms.

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

---

 configs/xilinx_versal_virt_defconfig | 4 ++++
 drivers/mailbox/Kconfig              | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/configs/xilinx_versal_virt_defconfig b/configs/xilinx_versal_virt_defconfig
index 0419992be4..a2d4debbf5 100644
--- a/configs/xilinx_versal_virt_defconfig
+++ b/configs/xilinx_versal_virt_defconfig
@@ -66,6 +66,8 @@ CONFIG_FPGA_XILINX=y
 CONFIG_FPGA_VERSALPL=y
 CONFIG_DM_I2C=y
 CONFIG_SYS_I2C_CADENCE=y
+CONFIG_DM_MAILBOX=y
+CONFIG_ZYNQMP_IPI=y
 CONFIG_MISC=y
 CONFIG_I2C_EEPROM=y
 CONFIG_SUPPORT_EMMC_BOOT=y
@@ -96,6 +98,8 @@ CONFIG_PHY_GIGE=y
 CONFIG_XILINX_AXIEMAC=y
 CONFIG_XILINX_AXIMRMAC=y
 CONFIG_ZYNQ_GEM=y
+CONFIG_POWER_DOMAIN=y
+CONFIG_ZYNQMP_POWER_DOMAIN=y
 CONFIG_ARM_DCC=y
 CONFIG_PL01X_SERIAL=y
 CONFIG_XILINX_UARTLITE=y
diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 73db2af0b8..acbdce11b7 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -54,7 +54,7 @@ config K3_SEC_PROXY
 
 config ZYNQMP_IPI
 	bool "Xilinx ZynqMP IPI controller support"
-	depends on DM_MAILBOX && ARCH_ZYNQMP
+	depends on DM_MAILBOX && (ARCH_ZYNQMP || ARCH_VERSAL)
 	help
 	  This enables support for the Xilinx ZynqMP Inter Processor Interrupt
 	  communication controller.
-- 
2.17.1

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

[Stefan Herbrechtsmeier]

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.soma@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.

>   int zynqmp_pmufw_node(u32 id)
>   {
> +	if (!config_enabled)
> +		return 0;
> +
>   	/* Record power domain id */
>   	xpm_configobject[NODE_ID_LOCATION] = id;
>   
> @@ -267,6 +272,15 @@ 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;
> +	}
> +
>   	return 0;
>   };
>   

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

[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.soma@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.

M

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

[Stefan Herbrechtsmeier]

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.soma@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.

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-oss@weidmueller.com> 
>Sent: Saturday, July 16, 2022 4:48 PM
>To: Simek, Michal <michal.simek@amd.com>; Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>; u-boot@lists.denx.de
>Cc: adrian.fiergolski@fastree3d.com; jh80.chung@samsung.com; sven@svenpeter.dev; kettenis@openbsd.org; sjg@chromium.org; git@xilinx.com; git (AMD-Xilinx) <git@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.soma@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

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

[Stefan Herbrechtsmeier]

Hi,

Am 19.07.22 um 06:44 schrieb Soma, Ashok Reddy:
> Hi Stefan,
> 
>> -----Original Message-----
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>
>> Sent: Saturday, July 16, 2022 4:48 PM
>> To: Simek, Michal <michal.simek@amd.com>; Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>; u-boot@lists.denx.de
>> Cc: adrian.fiergolski@fastree3d.com; jh80.chung@samsung.com; sven@svenpeter.dev; kettenis@openbsd.org; sjg@chromium.org; git@xilinx.com; git (AMD-Xilinx) <git@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.soma@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

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

[Soma, Ashok Reddy]

Hi Stefan,

I will send V2 with the changes.

Thanks,
Ashok

-----Original Message-----
From: Stefan Herbrechtsmeier <stefan@herbrechtsmeier.net> 
Sent: Wednesday, July 20, 2022 9:32 PM
To: Soma, Ashok Reddy <ashok.reddy.soma@amd.com>; Stefan Herbrechtsmeier <stefan.herbrechtsmeier-oss@weidmueller.com>; Simek, Michal <michal.simek@amd.com>; Ashok Reddy Soma <ashok.reddy.soma@xilinx.com>; u-boot@lists.denx.de
Cc: adrian.fiergolski@fastree3d.com; jh80.chung@samsung.com; sven@svenpeter.dev; kettenis@openbsd.org; sjg@chromium.org; git@xilinx.com; git (AMD-Xilinx) <git@amd.com>
Subject: Re: [PATCH 2/5] firmware: zynqmp: Load config overlay for core0 to pmufw

Hi,

Am 19.07.22 um 06:44 schrieb Soma, Ashok Reddy:
> Hi Stefan,
> 
>> -----Original Message-----
>> From: Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier-oss@weidmueller.com>
>> Sent: Saturday, July 16, 2022 4:48 PM
>> To: Simek, Michal <michal.simek@amd.com>; Ashok Reddy Soma 
>> <ashok.reddy.soma@xilinx.com>; u-boot@lists.denx.de
>> Cc: adrian.fiergolski@fastree3d.com; jh80.chung@samsung.com; 
>> sven@svenpeter.dev; kettenis@openbsd.org; sjg@chromium.org; 
>> git@xilinx.com; git (AMD-Xilinx) <git@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.soma@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