Re: [PATCH v3 2/2] firmware: zynqmp: Move config object permission check

[Michal Simek <michal.simek@amd.com>]

Hi,

first of all sorry for delay.

On 4/27/23 12:31, Stefan Herbrechtsmeier wrote:
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@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.herbrechtsmeier@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.

And tbh zynqmp_pmufw_config_close should also do the same thing.

The rest looks good to me.

Thanks,
Michal