On Sat, Aug 12, 2023 at 06:14:45PM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 16:38, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 11:03:36AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:28, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 08:24:59AM -0600, Simon Glass wrote:
Hi Tom,
On Sat, 12 Aug 2023 at 08:22, Tom Rini trini@konsulko.com wrote:
On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote: > Hi Tom, > > On Fri, 11 Aug 2023 at 09:56, Tom Rini trini@konsulko.com wrote: > > > > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote: > > > Hi Sughosh, > > > > > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu sughosh.ganu@linaro.org > wrote: > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote: > > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini trini@konsulko.com wrote: > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote: > > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini trini@konsulko.com > wrote: > > > > > > > > > > > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu > wrote: > > > > > > > > > > > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants. > This tool > > > > > > > > > > will be used subsequently for testing capsule generation > in binman. > > > > > > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu sughosh.ganu@linaro.org > > > > > > > > > > --- > > > > > > > > > > Changes since V7: None > > > > > > > > > > > > > > > > > > > > tools/Kconfig | 6 +++--- > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > > > > > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig > > > > > > > > > > index 6e23f44d55..353a855243 100644 > > > > > > > > > > --- a/tools/Kconfig > > > > > > > > > > +++ b/tools/Kconfig > > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512 > > > > > > > > > > Enable SHA512 support in the tools builds > > > > > > > > > > > > > > > > > > > > config TOOLS_MKEFICAPSULE > > > > > > > > > > - bool "Build efimkcapsule command" > > > > > > > > > > - default y if EFI_CAPSULE_ON_DISK > > > > > > > > > > + bool "Build mkeficapsule tool" > > > > > > > > > > + default y if EFI_CAPSULE_ON_DISK || SANDBOX > > > > > > > > > > help > > > > > > > > > > - This command allows users to create a UEFI > capsule file and, > > > > > > > > > > + This tool allows users to create a UEFI capsule > file and, > > > > > > > > > > optionally sign that file. If you want to enable > UEFI capsule > > > > > > > > > > update feature on your target, you certainly need > this. > > > > > > > > > > > > > > > > > > Sorry, what is this fixing exactly? > > > > > > > > > > > > > > > > The tool is required to be supported on the sandbox_spl > variant, since > > > > > > > > that is used for the binman tests in CI. Simon had then asked > me to > > > > > > > > add support for the tool on all sandbox variants. I missed > putting his > > > > > > > > R-b on this patch. > > > > > > > > > > > > > > OK, moving forward just depend on: > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-tri... > > > > > > > instead please, thanks. > > > > > > > > > > > > I will base my changes on top of your patch. However, we would > still > > > > > > need this patch as part of the series, since Simon wants the > capsules > > > > > > to be generated for all the sandbox variants. Thanks. > > > > > > > > > > No, this isn't needed. Any sandbox variant that needs capsules has > > > > > EFI_CAPSULE_ON_DISK enabled. > > > > > > > > Simon wants the capsules to be generated on all sandbox variants, > > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled. > > > > Which is why we need to have the tool enabled for all sandbox > > > > variants. > > > > > > I want to avoid #ifdefs in the sandbox .dts so far as possible. > > > > > > Tom, I'll let you make the final decision. > > > > > > In any case, the multiple-images thing needs to be fixed. > > > > Sughosh, please update the other sandbox defconfigs to just enable > > EFI_CAPSULE_ON_DISK. > > > > Simon, this I think is an example of where re-working > > configs/sandbox64_defconfig > > configs/sandbox_defconfig > > configs/sandbox_flattree_defconfig > > configs/sandbox_noinst_defconfig > > configs/sandbox_spl_defconfig > > configs/sandbox_vpl_defconfig > > > > To be configs/sandbox_defconfig + boards/sandbox/flattree.config, > > noinst.config, spl.config, vpl.config would be helpful. There's the > > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other > > variant just gets that, and we don't have to tweak N configs. > > You mean split configs? So far I am unable to build those...
I don't know what you mean by split configs. I mean that I think the only intentional difference between configs/sandbox_defconfig and configs/sandbox64_defconfig is: CONFIG_SANDBOX64=y CONFIG_DEFAULT_DEVICE_TREE="sandbox64"
And everything else is unintentional. And there's lots of other deltas like that between each of the other variants, and sandbox. And that this isn't the first, nor likely the last, time where we need to enable some option on other sandbox config files too, so that CI passes. This would all be avoided by using the config fragments mechanism so that we captured only the intentional delta of a fragment rather than maintaining N nearly identical, but not quite, files.
Well we do have other intentional differences, e.g. OF_LIVE. But OK if we can find a way to make fragments work with buildman (amd qconfig), then we could do this.
Yes, I was noting this in hopes of sparking your interest in figuring out how to handle fragments with buildman. It's similar to how we have the override option today.
We need a list of fragments somewhere, so that it is possible to enumerate the different board combinations. Does the main defconfig have a way to specify this, or could we add it?
No, I don't think that's the right way to go. I was thinking of something along the lines of how --adjust-cfg works, but instead it's a csv of additional targets to pass along with the defconfig name when invoking make.
So we would do them one at a time, with the 'name' of the board being some portion of the filename of the config-fragment file?
BTW CSV is not great for humans...perhaps a text file with columns like boards.cfg ?
I think you're still missing what I'm saying. There should not be a file that lists fragments. Outside of documentation, at least. I was saying csv above because it would make sense to do something like: ./tools/buildman/buildman --add-fragments=64bit,vpl sandbox And that would eventually do: make sandbox_config 64bit.config vpl.config Which has the standard Kconfig merging of configs/sandbox_defconfig boards/sandbox/64bit.config (replaces sandbox64_defconfig) and boards/sandbox/vpl.config And passing multiple files with a comma seems easiest.