[PATCH v2 1/3] firmware: zynqmp: Add config object support macro
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
The driver uses a specific node id to detect if the pmufw support config objects. Add a macro for the node id to ensure that it match at different locations.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
Changes in v2: - Added
drivers/firmware/firmware-zynqmp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index dc8e3ad2b9..baf5b0c253 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -19,6 +19,7 @@ #include <asm/arch/sys_proto.h>
#define PMUFW_PAYLOAD_ARG_CNT 8 +#define PMUFW_CFG_OBJ_SUPPORT_NODE NODE_OCM_BANK_0
#define XST_PM_NO_ACCESS 2002L #define XST_PM_ALREADY_CONFIGURED 2009L @@ -82,7 +83,7 @@ int zynqmp_pmufw_node(u32 id) ret = zynqmp_pmufw_load_config_object(xpm_configobject, sizeof(xpm_configobject));
- if (ret == XST_PM_NO_ACCESS && id == NODE_OCM_BANK_0) + if (ret == XST_PM_NO_ACCESS && id == PMUFW_CFG_OBJ_SUPPORT_NODE) skip_config = true;
return 0; @@ -251,7 +252,9 @@ 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) { + u32 id = ((u32 *)cfg_obj)[NODE_ID_LOCATION]; + + if (id == PMUFW_CFG_OBJ_SUPPORT_NODE) { printf("PMUFW: No permission to change config object\n"); return err; } @@ -299,7 +302,7 @@ static int zynqmp_power_probe(struct udevice *dev) ret & ZYNQMP_PM_VERSION_MINOR_MASK);
if (IS_ENABLED(CONFIG_ARCH_ZYNQMP)) - zynqmp_pmufw_node(NODE_OCM_BANK_0); + zynqmp_pmufw_node(PMUFW_CFG_OBJ_SUPPORT_NODE);
return 0; }; -- 2.30.2
________________________________ Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - Sitz: Detmold - Amtsgericht Lemgo HRB 3924; Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, André Sombecki; USt-ID-Nr. DE124599660
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Mask the expected and show the unexpected warning "No permission to change config object" for PMUFW_CFG_OBJ_SUPPORT_NODE because this node is used to detect if further zynqmp_pmufw_node function calls should be skipped.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
Changes in v2: - Use macro for node id
drivers/firmware/firmware-zynqmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index baf5b0c253..e763c639f7 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -254,7 +254,7 @@ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) if (err == XST_PM_NO_ACCESS) { u32 id = ((u32 *)cfg_obj)[NODE_ID_LOCATION];
- if (id == PMUFW_CFG_OBJ_SUPPORT_NODE) { + if (id != PMUFW_CFG_OBJ_SUPPORT_NODE) { printf("PMUFW: No permission to change config object\n"); return err; } -- 2.30.2
________________________________ Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - Sitz: Detmold - Amtsgericht Lemgo HRB 3924; Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, André Sombecki; USt-ID-Nr. DE124599660
On 4/19/23 11:04, Stefan Herbrechtsmeier wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Mask the expected and show the unexpected warning "No permission to change config object" for PMUFW_CFG_OBJ_SUPPORT_NODE because this node is used to detect if further zynqmp_pmufw_node function calls should be skipped.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Changes in v2:
Use macro for node id
drivers/firmware/firmware-zynqmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index baf5b0c253..e763c639f7 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -254,7 +254,7 @@ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) if (err == XST_PM_NO_ACCESS) { u32 id = ((u32 *)cfg_obj)[NODE_ID_LOCATION];
if (id == PMUFW_CFG_OBJ_SUPPORT_NODE) {
if (id != PMUFW_CFG_OBJ_SUPPORT_NODE) { printf("PMUFW: No permission to change config object\n"); return err; }
When only this patch is applied you return -EACCES but for setting up skip_config=true you need to return XST_PM_NO_ACCESS.
It means you are changing system behavior not just message that's why I still think it should be together with 3/3.
Anyway take a look at my snipset how I think it could be done in cleaner way.
Thanks, Michal
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Return a common -EACCES error value instead of a positive private error value XST_PM_NO_ACCESS (2002) in zynqmp_pmufw_load_config_object function if the config object is not loadable to simplify the error checking.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com ---
Changes in v2: - Use macro for node id
drivers/firmware/firmware-zynqmp.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index e763c639f7..7be4420a23 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -83,7 +83,7 @@ int zynqmp_pmufw_node(u32 id) ret = zynqmp_pmufw_load_config_object(xpm_configobject, sizeof(xpm_configobject));
- if (ret == XST_PM_NO_ACCESS && id == PMUFW_CFG_OBJ_SUPPORT_NODE) + if (ret == -EACCES && id == PMUFW_CFG_OBJ_SUPPORT_NODE) skip_config = true;
return 0; @@ -236,8 +236,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 - * Return: 0 on success otherwise negative errno. If the config object - * is not loadable returns positive errno XST_PM_NO_ACCESS(2002) + * Return: 0 on success otherwise negative errno. */ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) { @@ -254,10 +253,8 @@ int zynqmp_pmufw_load_config_object(const void *cfg_obj, size_t size) if (err == XST_PM_NO_ACCESS) { u32 id = ((u32 *)cfg_obj)[NODE_ID_LOCATION];
- if (id != PMUFW_CFG_OBJ_SUPPORT_NODE) { + if (id != PMUFW_CFG_OBJ_SUPPORT_NODE) printf("PMUFW: No permission to change config object\n"); - return err; - } return -EACCES; }
-- 2.30.2
________________________________ Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - Sitz: Detmold - Amtsgericht Lemgo HRB 3924; Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, André Sombecki; USt-ID-Nr. DE124599660
On 4/19/23 11:04, Stefan Herbrechtsmeier wrote:
From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
The driver uses a specific node id to detect if the pmufw support config objects. Add a macro for the node id to ensure that it match at different locations.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Changes in v2:
Added
drivers/firmware/firmware-zynqmp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index dc8e3ad2b9..baf5b0c253 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -19,6 +19,7 @@ #include <asm/arch/sys_proto.h>
#define PMUFW_PAYLOAD_ARG_CNT 8 +#define PMUFW_CFG_OBJ_SUPPORT_NODE NODE_OCM_BANK_0
#define XST_PM_NO_ACCESS 2002L #define XST_PM_ALREADY_CONFIGURED 2009L @@ -82,7 +83,7 @@ int zynqmp_pmufw_node(u32 id) ret = zynqmp_pmufw_load_config_object(xpm_configobject, sizeof(xpm_configobject));
if (ret == XST_PM_NO_ACCESS && id == NODE_OCM_BANK_0)
if (ret == XST_PM_NO_ACCESS && id == PMUFW_CFG_OBJ_SUPPORT_NODE) skip_config = true; return 0;@@ -251,7 +252,9 @@ 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) {
u32 id = ((u32 *)cfg_obj)[NODE_ID_LOCATION];if (id == PMUFW_CFG_OBJ_SUPPORT_NODE) { printf("PMUFW: No permission to change config object\n"); return err; }@@ -299,7 +302,7 @@ static int zynqmp_power_probe(struct udevice *dev) ret & ZYNQMP_PM_VERSION_MINOR_MASK);
if (IS_ENABLED(CONFIG_ARCH_ZYNQMP))
zynqmp_pmufw_node(NODE_OCM_BANK_0);
zynqmp_pmufw_node(PMUFW_CFG_OBJ_SUPPORT_NODE); return 0;};
-- 2.30.2
Kommanditgesellschaft - Sitz: Detmold - Amtsgericht Lemgo HRA 2790 - Komplementärin: Weidmüller Interface Führungsgesellschaft mbH - Sitz: Detmold - Amtsgericht Lemgo HRB 3924; Geschäftsführer: Dr. Timo Berger, Volker Bibelhausen, Dr. Sebastian Durst, André Sombecki; USt-ID-Nr. DE124599660
Reviewed-by: Michal Simek michal.simek@amd.com
Patch looks good but I think your footer is breaking the patch. Even b4 can't work with it and I see additional chars there because of encoding.
For example 224 - if (ret =3D=3D XST_PM_NO_ACCESS && id =3D=3D NODE_OCM_BANK_0) 225 + if (ret =3D=3D XST_PM_NO_ACCESS && id =3D=3D PMUFW_CFG_OBJ_SUPPORT_= 226 NODE)
Please fix it up and resend.
Thanks, Michal
participants (2)
-
Michal Simek -
Stefan Herbrechtsmeier