Hi Akashi, On Wed, Feb 10, 2021 at 09:50:33PM +0900, AKASHI Takahiro wrote: > On Wed, Feb 10, 2021 at 01:53:55PM +0200, Ilias Apalodimas wrote: > > Hi Wolfgang, > > > > Thanks for having a look, > > > > On Wed, Feb 10, 2021 at 12:23:47PM +0100, Wolfgang Denk wrote: > > > Dear Ilias, > > > > > > In message <20210210105425.356131-1-ilias.apalodi...@linaro.org> you > > > wrote: > > > > Up to now we've been adding all the efi related configuration to > > > > 'efidebug' command. The command name feels a bit weird to configure > > > > boot > > > > manager related commands. Since the bootmanager is growing and we > > > > intend > > I developed the command as a poorman's "efishell" as, at that time, > EDK2's shell didn't work well on U-Boot UEFI subsystem. > > As far as I remember, Alex (ex-maintainer) didn't like to take this > command, including "efidebug boot" subcommand, as a standard U-Boot command. >
UEFI has grown substantially since then though and we can now use it to install full distros. Having a command named 'efidebug' is the last place some would look to configure boot options. Does it only feel weird to me? > > > > to extend it with features like defining the initrd we want to expose to > > > > the kernel, it would make sense to split it on a command of it's own. > > > > > > > > So let's introduce a new command called bootmgr and move all of the > > > > existing Boot manager functionality there. > > > > > > As this is EFI specific, I would appreciate to have "efi" in the > > > command name, too. > > > > > > Maybe all EFi related commands should be collected as "efi <subcommand>" > > > like we did it with the "env" commands long ago. > > > > We could, I'll discuss this with Heinrich and see what he thinks. > > > > > > > > For backward compatibility e. g. 'efidebug' could be kept, but the > > > new name would be 'efi debug'; likewise, your new command would be > > > 'efi bootmgr' [or just 'efi boot' ?] > > As a matter of fact, "efi" is even now recognized as "efidebug" thanks to > command name completion as there is no other "efi*" command. > > Then, > efidebug boot ..., and > [efi]bootmgr boot ... > can be invoked literally as > efi boot ... > > So I don't see much advantage to Ilias' proposal. > Breaking the code in smaller, readable files, looked like a better option. I'd much prefer it over a single huge file implementing a variety of commands completely unrelated to each other, just because 'everything belongs to the efi spec'. It would also make our life and readability a lot easier in case we want to include specific commands on a built. The alternative you propose implies a file full of ifdefs, while we could just control them with Kconfig options and makefiles. > My suggestions are: > * alias "efi" to "efidebug" (if preferred), > * add new configuration options for efidebug's subcommands, > * only enable "boot"-related options by default (if needed) > > Personally, I don't like to move the portion of code from one file > to another since it will break git history. > How? The commit message clearly says "move the bootmgr functions to a diffrent file" and the git log on efidebug will clear that up. It will even show the file that was moved to. > In addition, I have proposed to make "bootefi bootmgr" a standalone > command/application, but Heinrich rejected it. That's a different discussion, but if you go down that road you'd have to move the 'bootefi selftest' as well? > Given Ilias' concern(?), I still believe that the change is logical > and makes more sense. My concern here is to help people wanting to use the bootmgr. I don't have a strong opinion on how that eventually can be implemented and that's why I am open to suggestions. For the record I do prefer seperate commands, maybe even prefixed by efi to make the distinction clear between u-boot native and EFI related commands. Cheers /Ilias > > -Takahiro Akashi > > > > > The efidebug for boot options wasn't introduced that long ago and I don't > > think anyone uses it in production. If someone would want to have it > > backwards > > compatible, please shout and we'll see what we can do, but I'd strongly > > prefer > > replacing it overall. If we truly want backwards compatibility though we > > must keep > > efidebug, changing the name to something like 'efi debug' just for the name > > similarity wouldn't help much as it would break things regardless. > > > > Heinrich feel free to ignore the followup patch fixing the documentation of > > efidebug. I'll change the name to something we all agree and fold in the > > doc > > changes in v2. > > > > Thanks > > /Ilias > > > > > > Thanks! > > > > > > Wolfgang Denk > > > > > > -- > > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de > > > In my experience the best way to get something done is to give it to > > > someone who is busy. - Terry Pratchett, _Going_Postal_