[PATCH v2 1/3] firmware: zynqmp: Add config object support macro

[PATCH v2 1/3] firmware: zynqmp: Add config object support macro

[Stefan Herbrechtsmeier]

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

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

[Stefan Herbrechtsmeier]

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

[PATCH v2 3/3] firmware: zynqmp: Remove extraordinary return value

[Stefan Herbrechtsmeier]

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

Re: [PATCH v2 1/3] firmware: zynqmp: Add config object support macro

[Michal Simek]

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

Re: [PATCH v2 2/3] firmware: zynqmp: Mask expected and show unexpected warning

[Michal Simek]

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