[PATCH 1/2] firmware: zynqmp: Mask expected and show unexpected warning

[Stefan Herbrechtsmeier]

Hi Michal,

Am 17.04.2023 um 12:16 schrieb Michal Simek:

> On 4/3/23 15:34, Stefan Herbrechtsmeier wrote:
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier at weidmueller.com>
>>
>> Mask the expected and show the unexpected warning "No permission to
>> change config object" for NODE_OCM_BANK_0 because this node is used to
>> detect if further zynqmp_pmufw_node function calls should be skipped.
>>
>> Signed-off-by: Stefan Herbrechtsmeier
>> <stefan.herbrechtsmeier at weidmueller.com>
>> ---
>>
>>   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 dc8e3ad2b9..8435b58ef9 100644
>> --- a/drivers/firmware/firmware-zynqmp.c
>> +++ b/drivers/firmware/firmware-zynqmp.c
>> @@ -251,7 +251,7 @@ 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) {
>> +               if (((u32 *)cfg_obj)[NODE_ID_LOCATION] !=
>> NODE_OCM_BANK_0) {
>>                          printf("PMUFW:  No permission to change
>> config object\n");
>>                          return err;
>>                  }
>
> First of all we should very likely create a macro for NODE_OCM_BANK_0
> to cover that dependency that it is used in 3 different locations
> which have to match.

Okay, I will add a PMUFW_CFG_OBJ_SUPPORT_NODE macro.

> The second is the change you have in 2/2 should be the part of this
> patch because when only 1/2 is applied you change behavior.

The patches should be independent, and the behavior change is intended.
The message should be printed if you don’t heave the permission for a
specific config object and not if the driver checks for support of
config objects. The NODE_OCM_BANK_0 call should never fail if load of
config objects is supported.

> And changes in 2/2 makes sense.
>
> I would be even fine to move skip_config out of zynqmp_pmufw_node()

The zynqmp_pmufw_node() function doesn't return an error and the
skip_config variable is static inside the function.

> and setting up skip_config value directly in zynqmp_power_probe() not
> to check in every call.

We still need to check the skip_config variable inside zynqmp_pmufw_node
to skip the load of the config object if the pmufw doesn't support it.

>
>  85         if (ret == XST_PM_NO_ACCESS && id == NODE_OCM_BANK_0)
>  86                 skip_config = true;

Regards
   Stefan

________________________________
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