From: Tom Rini <trini@konsulko.com>
To: Simon Glass <sjg@chromium.org>
Cc: "Heinrich Schuchardt" <xypron.glpk@gmx.de>,
"U-Boot Mailing List" <u-boot@lists.denx.de>,
"Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
"Pali Rohár" <pali@kernel.org>,
"Mark Kettenis" <mark.kettenis@xs4all.nl>
Subject: Re: [PATCH v2 7/9] Make EFI_LOADER depend on DM and OF_CONTROL
Date: Thu, 29 Jul 2021 09:52:47 -0400 [thread overview]
Message-ID: <20210729135247.GO9379@bill-the-cat> (raw)
In-Reply-To: <CAPnjgZ0EpL3mcetdt2r=eMVbATRAZpYkENn14qwb18UMo1_6GQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4866 bytes --]
On Wed, Jul 28, 2021 at 07:44:37PM -0600, Simon Glass wrote:
> Hi,
>
> On Wed, 28 Jul 2021 at 17:55, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Jul 29, 2021 at 01:45:49AM +0200, Heinrich Schuchardt wrote:
> > >
> > >
> > > On 7/27/21 12:07 AM, Tom Rini wrote:
> > > > On Fri, Jul 02, 2021 at 12:36:18PM -0600, Simon Glass wrote:
> > > >
> > > > > This feature should never have been made available when driver model
> > > > > or devicetree are disabled. Add these as conditions, so that we don't
> > > > > create even more barriers to migration.
> > > > >
> > > > > Add a note about the substantial size increment associated with this
> > > > > option.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > > ---
> > > > >
> > > > > Changes in v2:
> > > > > - Split out new patch to make EFI_LOADER depend on DM and OF_CONTROL
> > > > > - Note the approximate size of this feature in the help
> > > > >
> > > > > lib/efi_loader/Kconfig | 4 +++-
> > > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > index 6242caceb7f..466abfed300 100644
> > > > > --- a/lib/efi_loader/Kconfig
> > > > > +++ b/lib/efi_loader/Kconfig
> > > > > @@ -1,6 +1,6 @@
> > > > > config EFI_LOADER
> > > > > bool "Support running UEFI applications"
> > > > > - depends on OF_LIBFDT && ( \
> > > > > + depends on OF_LIBFDT && DM && OF_CONTROL && ( \
> > >
> > > Didn't Tom eliminate all boards without CONFIG_DM? Shouldn't we get rid
> > > of this symbol?
> >
> > No, but I did send out a message about that today as we're very close.
> > Much closer than I had expected us to be.
>
> Note we will still have SPL_DM, I think.
Right.
> > > Are there boards using DM and not OF_CONTROL or OF_CONTROL and not DM?
> >
> > Yes, a few. It's vexpress_aemv8a_semi, warp (fixed by
> > https://patchwork.ozlabs.org/project/uboot/patch/20210402180552.1075997-2-pbrobinson@gmail.com/
> > so false positive), cm_t335, devkit8000, armadillo-800eva, kzm9g and sniper.
> >
> > > Why are these separate symbols? Isn't this symbol to be eliminated, too?
> >
> > Simon?
>
> If we do eliminate DM it will be in a separate series. Like Tom I am
> pleasantly surprised at how close we are.
>
> But please let's consider patches on their merits. It is fine to reply
> with a wishlist but even better to reply with a follow-up patch.
OK. So, build-wise, EFI_LOADER does not require OF_CONTROL.
> Somewhat related, I think we need to create a separate symbol which
> means (OF_CONTROL && !OF_PLATDATA) so we can just check one thing.
>
> Also I think we should push of-platdata, since otherwise we're going
> to hit the same problem of migrating SPL boards to DM one day.
Note that we don't have CONFIG_OF_PLATDATA just
CONFIG_(SPL|TPL)_OF_PLATDATA.
> > > lib/efi_loader/efi_disk.c is the only place where we maintain duplicate
> > > code for DM and non-DM. A dependency on CONFIG_BLK (which itself depends
> > > on CONFIG_DM) would make more sense to me. But only in a patch
> > > eliminating the non-BLK code.
> >
> > I just know that off-hand, partition + disk + block has some corner
> > case, but maybe that corner case is unintentional in terms of usage
> > today.
> >
> > > > > ARM && (SYS_CPU = arm1136 || \
> > > > > SYS_CPU = arm1176 || \
> > > > > SYS_CPU = armv7 || \
> > > > > @@ -25,6 +25,8 @@ config EFI_LOADER
> > > > > will expose the UEFI API to a loaded application, enabling it to
> > > > > reuse U-Boot's device drivers.
> > > > >
> > > > > + For ARM 32-bit, this adds about 90KB to the size of U-Boot.
> > > > > +
> > >
> > > There is no unit ISO prefix K. Do you mean KiB?
> > >
> > > 90 KiB may be the value today. Will you update it regularly? Otherwise
> > > don't put a number here.
> > >
> > > I can't see that the effect on size is truly architecture specific. Why
> > > do you refer to 32bit ARM?
> > >
> > > Such a comment would better fit into a documentation chapter on
> > > downsizing U-Boot.
> >
> > Yes, we should probably drop that specific note.
>
> From my POV I really like these notes in Kconfig. They appear in a few
> places and provide people with rough guidance. I'd like to see more of
> them. I don't know how we can keep them up-to-date, although I'd argue
> that they should stay constant, if we are holding to our no-bloat
> ideal.
I feel like EFI gets a bit of an undeserved reputation here. It's not
growing any worse than the rest of the world is over fixes and error
correction (which is to say, 16/32/40 bytes here and there). And
there's not "big" new default features coming in.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
next prev parent reply other threads:[~2021-07-29 13:53 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-02 18:36 [PATCH v2 0/9] efi: Various tidy-ups and drop the default Simon Glass
2021-07-02 18:36 ` [PATCH v2 1/9] configs: Resync with savedefconfig Simon Glass
2021-07-02 18:36 ` [PATCH v2 2/9] Makefile: Drop include/asm directory as well as symlink Simon Glass
2021-07-28 22:56 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 3/9] disk: Tidy up #ifdefs in part_efi Simon Glass
2021-07-28 22:56 ` Tom Rini
2021-07-28 23:21 ` Heinrich Schuchardt
2021-07-02 18:36 ` [PATCH v2 4/9] Use LIB_UUID with ACPIGEN and FS_BTRFS Simon Glass
2021-07-28 22:56 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 5/9] Allow efi_loader header to be included always Simon Glass
2021-07-28 22:56 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 6/9] lib: Create a new Kconfig option for charset conversion Simon Glass
2021-07-28 22:57 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 7/9] Make EFI_LOADER depend on DM and OF_CONTROL Simon Glass
2021-07-26 22:07 ` Tom Rini
2021-07-28 23:45 ` Heinrich Schuchardt
2021-07-28 23:55 ` Tom Rini
2021-07-29 1:44 ` Simon Glass
2021-07-29 13:52 ` Tom Rini [this message]
2021-07-30 19:02 ` Simon Glass
2021-07-30 21:33 ` Tom Rini
2021-07-30 21:48 ` Simon Glass
2021-07-30 22:08 ` Tom Rini
2022-03-28 6:35 ` Simon Glass
2022-03-28 14:15 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 8/9] Add an option for EBBR Simon Glass
2021-07-29 0:12 ` Heinrich Schuchardt
2021-07-29 0:40 ` Tom Rini
2021-07-02 18:36 ` [PATCH v2 9/9] efi: Make EBBR optional Simon Glass
2021-07-02 20:11 ` Tom Rini
2021-07-02 20:38 ` Simon Glass
2021-07-29 0:35 ` Heinrich Schuchardt
2021-07-02 19:50 ` [PATCH v2 0/9] efi: Various tidy-ups and drop the default Mark Kettenis
2021-07-02 20:04 ` Simon Glass
2021-07-02 20:19 ` Tom Rini
2021-07-02 20:50 ` Simon Glass
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210729135247.GO9379@bill-the-cat \
--to=trini@konsulko.com \
--cc=ilias.apalodimas@linaro.org \
--cc=mark.kettenis@xs4all.nl \
--cc=pali@kernel.org \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
--cc=xypron.glpk@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.