Re: [PATCH] arm64: zynqmp: Support converting pm_cfg_obj.c

Hi Brandon,
On 23/06/20 19:08, Brandon Maier wrote:
On Tue, Jun 23, 2020 at 10:18 AM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi Brandon,
On 22/06/20 22:45, Brandon Maier wrote:
To use CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE, a developer must pull down the U-Boot source and run ./tools/zynqmp_pm_cfg_obj_convert.py to convert their pm_cfg_obj.c into U-Boot's PMU loader format, then feed that file back to U-Boot during build.
Instead, by doing the conversion in U-Boot during the build, we can simplify the developer's build system. And it ensures that if zynqmp_pm_cfg_obj_convert.py is updated, the pm_cfg_obj will stay in sync with U-Boot.
In the workflows I have used so far U-Boot tools are extracted in a different step w.r.t. U-Boot cross-compilation, so I never felt the need for this feature. But I understand this feature can be handy.
I'm trying to get it to work with Buildroot. Buildroot does have a package for uboot-tools, so I could use that to run it. I'd have to move the config and build commands into Buildroot's uboot package. Since the script belongs to U-Boot anyway, it seemed cleaner to integrate it directly into U-Boot so it could be used by other build systems too.
I understand. But I also understand Michal's concern: the U-Boot makefiles are quite complex already, and this feature is a relatively minor improvement.
Probably having the conversion in Buildroot could be even simpler as you can support _only_ the C format there and do the conversion unconditionally. Only one workflow, no ifs, simpler code. Don't forget to Cc: me if you send a patch for that, I'll be glad to review it.

Hi,
On 23/06/20 23:23, Luca Ceresoli wrote:
Hi Brandon,
On 23/06/20 19:08, Brandon Maier wrote:
On Tue, Jun 23, 2020 at 10:18 AM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi Brandon,
On 22/06/20 22:45, Brandon Maier wrote:
To use CONFIG_ZYNQMP_SPL_PM_CFG_OBJ_FILE, a developer must pull down the U-Boot source and run ./tools/zynqmp_pm_cfg_obj_convert.py to convert their pm_cfg_obj.c into U-Boot's PMU loader format, then feed that file back to U-Boot during build.
Instead, by doing the conversion in U-Boot during the build, we can simplify the developer's build system. And it ensures that if zynqmp_pm_cfg_obj_convert.py is updated, the pm_cfg_obj will stay in sync with U-Boot.
In the workflows I have used so far U-Boot tools are extracted in a different step w.r.t. U-Boot cross-compilation, so I never felt the need for this feature. But I understand this feature can be handy.
I'm trying to get it to work with Buildroot. Buildroot does have a package for uboot-tools, so I could use that to run it. I'd have to move the config and build commands into Buildroot's uboot package. Since the script belongs to U-Boot anyway, it seemed cleaner to integrate it directly into U-Boot so it could be used by other build systems too.
I understand. But I also understand Michal's concern: the U-Boot makefiles are quite complex already, and this feature is a relatively minor improvement.
Probably having the conversion in Buildroot could be even simpler as you can support _only_ the C format there and do the conversion unconditionally. Only one workflow, no ifs, simpler code. Don't forget to Cc: me if you send a patch for that, I'll be glad to review it.
I almost forgot: I wrote some notes on how this could be implemented in yocto a while back. The principles are pretty much the same so you might be interested in reading them:
https://www.yoctoproject.org/pipermail/meta-xilinx/2019-November/004578.html

On Tue, Jun 23, 2020 at 4:33 PM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi,
On 23/06/20 23:23, Luca Ceresoli wrote:
Hi Brandon,
On 23/06/20 19:08, Brandon Maier wrote:
On Tue, Jun 23, 2020 at 10:18 AM Luca Ceresoli luca@lucaceresoli.net wrote:
I'm trying to get it to work with Buildroot. Buildroot does have a package for uboot-tools, so I could use that to run it. I'd have to move the config and build commands into Buildroot's uboot package. Since the script belongs to U-Boot anyway, it seemed cleaner to integrate it directly into U-Boot so it could be used by other build systems too.
I understand. But I also understand Michal's concern: the U-Boot makefiles are quite complex already, and this feature is a relatively minor improvement.
Probably having the conversion in Buildroot could be even simpler as you can support _only_ the C format there and do the conversion unconditionally. Only one workflow, no ifs, simpler code. Don't forget to Cc: me if you send a patch for that, I'll be glad to review it.
Either U-Boot or Buildroot is fine by me. If Yocto alreads does it in their U-Boot package then I can see that being a precedent for patching Buildroot. I'll drop this patch to U-Boot then.
I almost forgot: I wrote some notes on how this could be implemented in yocto a while back. The principles are pretty much the same so you might be interested in reading them:
https://urldefense.proofpoint.com/v2/url?u=https-3A__www.yoctoproject.org_pi...
Thanks!
-- Luca

On 23. 06. 20 23:46, Brandon Maier wrote:
On Tue, Jun 23, 2020 at 4:33 PM Luca Ceresoli luca@lucaceresoli.net wrote:
Hi,
On 23/06/20 23:23, Luca Ceresoli wrote:
Hi Brandon,
On 23/06/20 19:08, Brandon Maier wrote:
On Tue, Jun 23, 2020 at 10:18 AM Luca Ceresoli luca@lucaceresoli.net wrote:
I'm trying to get it to work with Buildroot. Buildroot does have a package for uboot-tools, so I could use that to run it. I'd have to move the config and build commands into Buildroot's uboot package. Since the script belongs to U-Boot anyway, it seemed cleaner to integrate it directly into U-Boot so it could be used by other build systems too.
I understand. But I also understand Michal's concern: the U-Boot makefiles are quite complex already, and this feature is a relatively minor improvement.
Probably having the conversion in Buildroot could be even simpler as you can support _only_ the C format there and do the conversion unconditionally. Only one workflow, no ifs, simpler code. Don't forget to Cc: me if you send a patch for that, I'll be glad to review it.
Either U-Boot or Buildroot is fine by me. If Yocto alreads does it in their U-Boot package then I can see that being a precedent for patching Buildroot. I'll drop this patch to U-Boot then.
I have never started to use Luca's python script because I use different methods how to get this fragment. The first one was taking fsbl and simply extract pmu cfg fragment from it with using binutils. I have sent that script some time ago.
And the second method is simply take what Xilinx C file and link it with embeddedSW repository (just for headers) and build it by gcc.
A month ago I was experiment with this fragment and was trying to find out if we can have generic one which works on all platforms. As is visible this is fine for boot but not so good for live system.
The fragment itself by spec has feature for exchanging it at run time. I sent a patch to utilize it based on current code. https://lists.denx.de/pipermail/u-boot/2020-June/416310.html
It means I would more prefer to go in a way that pmufw will be able to accept board/platform/configuration specific fragments and have just one in u-boot which in general enable everything by default. But I don't have time to go to pmufw and fix it.
Thanks, Michal
participants (3)
-
Brandon Maier
-
Luca Ceresoli
-
Michal Simek