On Sun, Aug 13, 2023 at 07:36:45AM -0600, Simon Glass wrote: > Hi Tom, > > On Sun, 13 Aug 2023 at 06:40, Tom Rini <tr...@konsulko.com> wrote: > > > > 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 <tr...@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 <tr...@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 <tr...@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 <tr...@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.g...@linaro.org> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini > > > > > > > > > > > > <tr...@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 > > > > > > > > > > > > > > <tr...@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 > > > > > > > > > > > > > > > > <tr...@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.g...@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-tr...@konsulko.com/ > > > > > > > > > > > > > > > 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. > > So is it only possible to add one fragment file to a build?
The example above is two, and yes, N config fragments works (they are merged in listed order). > I see what you are saying, but from my side I am trying to enumerate > the boards, since generally I (like) build things without explicitly > specifying each board defconfig. Yes, but that's not possible in this case I think. And I'm really just trying to figure out how we can make CI a little easier. But maybe we can't / don't bother in this case and keep fixing up the sandbox defconfig files as needed. -- Tom
signature.asc
Description: PGP signature
