Re: [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: support binary pm_cfg_obj

From: Luca Ceresoli via buildroot <buildroot@buildroot.org>
To: "Frager, Neal" <neal.frager@amd.com>
Cc: Brandon Maier <brandon.maier@collins.com>,
	"buildroot@buildroot.org" <buildroot@buildroot.org>
Subject: Re: [Buildroot] [PATCH 1/1] boot/uboot/uboot.mk: support binary pm_cfg_obj
Date: Tue, 13 Dec 2022 09:23:25 +0100	[thread overview]
Message-ID: <20221213092325.15176bbb@booty> (raw)
In-Reply-To: <CH2PR12MB500436FEFB9A1E70F891A504F0E29@CH2PR12MB5004.namprd12.prod.outlook.com>

Hi Neal,

On Mon, 12 Dec 2022 12:52:37 +0000
"Frager, Neal" <neal.frager@amd.com> wrote:

> Hi Luca,
> 
> > BR2_TARGET_UBOOT_ZYNQMP_PM_CFG only works with C files, as it always 
> > tries to process them through tools/zynqmp_pm_cfg_obj_convert.py. 
> > Rework the logic so if the pm_cfg_obj isn't a C file, it is provided 
> > directly to U-Boot.  
> 
> > Can you describe a use case where this would be useful? While I intuitively understand there _might_ be some, I cannot find any good example based on my experience.  
> 
> > Building the pm_cfg_obj from source code is very easily done by Buildroot, unlike building the PMUFW. Additionally one PMUFW binary can be used on multiple zynqmp hardware, while the cfg obj is very likely to change across different projects, and a somewhat readable text file is way more understandable than a binary blob.  
> 
> I believe Brandon should weigh in on this with his opinion as well.  As I thought about it this morning, I just wanted to share my two cents for what they are worth.
> 
> I agree with you that most users would want to build everything with buildroot, if they could.  Right now, we support using pmufw binaries only because we do not have a solution for building the pmufw itself with buildroot.
> If users could build the pmufw itself with buildroot, I am sure most of them would.
> 
> However, at the same time, does Brandon's patch really hurt anything?  By allowing users to provide a pre-built pm_cfg_obj binary blob, it does not prevent users from still providing a c file.
> Basically, it allows for the capability of building the pm_cfg_obj.c file with Xilinx tools and just providing the binary blob for being included in the spl/boot.bin by buildroot without removing any capability.
> 
> So from my view, this patch may not be critical, but it does not break anything either.  I am ok with whatever you and Brandon decide about committing it.

I think any line of code that is not very useful (or not useful at all)
does hurt. People have to maintain it, code is less readable, etc.

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot