On 5/16/23 13:23, Stefan Herbrechtsmeier wrote:
Hi,
Am 16.05.2023 um 10:26 schrieb Michal Simek:
Hi,
first of all sorry for delay.
On 4/27/23 12:31, Stefan Herbrechtsmeier wrote:
From: Stefan Herbrechtsmeier <stefan.herbrechtsme...@weidmueller.com>
Move the check of the permission to change a config object from
zynqmp_pmufw_node function to zynqmp_pmufw_load_config_object function
to simplify the code and check the permission only if required.
Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsme...@weidmueller.com>
---
Changes in v3:
- Added
drivers/firmware/firmware-zynqmp.c | 32 +++++++++++++++---------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/firmware/firmware-zynqmp.c
b/drivers/firmware/firmware-zynqmp.c
index 2b1ad5d2c3..d12159fa78 100644
--- a/drivers/firmware/firmware-zynqmp.c
+++ b/drivers/firmware/firmware-zynqmp.c
@@ -70,20 +70,11 @@ int zynqmp_pmufw_config_close(void)
int zynqmp_pmufw_node(u32 id)
{
- static bool skip_config;
- int ret;
-
- if (skip_config)
- return 0;
-
/* Record power domain id */
xpm_configobject[NODE_ID_LOCATION] = id;
- ret = zynqmp_pmufw_load_config_object(xpm_configobject,
- sizeof(xpm_configobject));
-
- if (ret == -EACCES && id == NODE_OCM_BANK_0)
- skip_config = true;
+ zynqmp_pmufw_load_config_object(xpm_configobject,
+ sizeof(xpm_configobject));
This is not right.
It should be
return zynqmp_pmufw_load... for error propagation.
At the moment the zynqmp_pmufw_node and zynqmp_pmufw_config_close doesn't return
an error. Should the zynqmp_pmufw_load_config_object return 0 or -EACCES if it
is skipped?
In context zynqmp_pmufw_node and it's dependency on returning EACESS for failure
case which your code depends on here
+ if (zynqmp_pmufw_node(NODE_OCM_BANK_0) == -EACCES) {
+ printf("PMUFW: No permission to change config
object\n");
+ skip = true;
+ }
And for second part around skip and return code. I would say what you have is
fine. It means returning -EACCES is appropriate here.
And as I see do_zynqmp_pmufw should also return that value. That command will
simply fail if there is no permission.
And for close part I would say the same. Error should be propagated.
I expect current command behavior when you call "pmufw node close" on regular
system will just pass but it should just fail because command wasn't successful.
Thanks,
Michal
