| Message ID | 20220308113657.221101-1-takahiro.akashi@linaro.org |
|---|---|
| Headers | show |
| Series | efi_loader: more tightly integrate UEFI disks to driver model | expand |
On 3/8/22 12:36, AKASHI Takahiro wrote: > With this patch set[1] applied, UEFI subsystem maintains a list of its > disk objects dynamically at runtime based on block device's probing. > (See "issues" below.) > > [1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk On sandbox_defconfig with CONFIG_EFI_SELFTEST=y I see this error which does not occur without your patches: Executing 'load file protocol' lib/efi_selftest/efi_selftest_load_file.c(220): ERROR: Wrong remaining device path lib/efi_selftest/efi_selftest_load_file.c(396): ERROR: Failed to load image lib/efi_selftest/efi_selftest.c(114): ERROR: Executing 'load file protocol' failed Could you, please, check. Best regards Heinrich
On Tue, Mar 08, 2022 at 01:59:08PM +0100, Heinrich Schuchardt wrote: > On 3/8/22 12:36, AKASHI Takahiro wrote: > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > disk objects dynamically at runtime based on block device's probing. > > (See "issues" below.) > > > > [1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > On sandbox_defconfig with CONFIG_EFI_SELFTEST=y I see this error which > does not occur without your patches: > > Executing 'load file protocol' > lib/efi_selftest/efi_selftest_load_file.c(220): > ERROR: Wrong remaining device path > lib/efi_selftest/efi_selftest_load_file.c(396): > ERROR: Failed to load image > lib/efi_selftest/efi_selftest.c(114): > ERROR: Executing 'load file protocol' failed > > Could you, please, check. I will, but I also request you to review *the code* itself. -Takahiro Akashi > Best regards > > Heinrich
On 3/8/22 14:04, AKASHI Takahiro wrote: > On Tue, Mar 08, 2022 at 01:59:08PM +0100, Heinrich Schuchardt wrote: >> On 3/8/22 12:36, AKASHI Takahiro wrote: >>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>> disk objects dynamically at runtime based on block device's probing. >>> (See "issues" below.) >>> >>> [1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk >> >> On sandbox_defconfig with CONFIG_EFI_SELFTEST=y I see this error which >> does not occur without your patches: >> >> Executing 'load file protocol' >> lib/efi_selftest/efi_selftest_load_file.c(220): >> ERROR: Wrong remaining device path >> lib/efi_selftest/efi_selftest_load_file.c(396): >> ERROR: Failed to load image >> lib/efi_selftest/efi_selftest.c(114): >> ERROR: Executing 'load file protocol' failed I missed to put Simon's event series first. With both series the error vanishes. Best regards Heinrich >> >> Could you, please, check. > > I will, but I also request you to review *the code* itself. > > -Takahiro Akashi > >> Best regards >> >> Heinrich
Hi, On Tue, 8 Mar 2022 at 06:29, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/8/22 14:04, AKASHI Takahiro wrote: > > On Tue, Mar 08, 2022 at 01:59:08PM +0100, Heinrich Schuchardt wrote: > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > >>> disk objects dynamically at runtime based on block device's probing. > >>> (See "issues" below.) > >>> > >>> [1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk > >> > >> On sandbox_defconfig with CONFIG_EFI_SELFTEST=y I see this error which > >> does not occur without your patches: > >> > >> Executing 'load file protocol' > >> lib/efi_selftest/efi_selftest_load_file.c(220): > >> ERROR: Wrong remaining device path > >> lib/efi_selftest/efi_selftest_load_file.c(396): > >> ERROR: Failed to load image > >> lib/efi_selftest/efi_selftest.c(114): > >> ERROR: Executing 'load file protocol' failed > > I missed to put Simon's event series first. With both series the error > vanishes. +Tom Rini I'm really looking forward to getting this all in! It is another one that has been hanging around for too long. - Simon
On 3/8/22 12:36, AKASHI Takahiro wrote: > With this patch set[1] applied, UEFI subsystem maintains a list of its > disk objects dynamically at runtime based on block device's probing. > (See "issues" below.) > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk This series together with Simon's series breaks multiple boards due to size constraints: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 Please, investigate how to work around this issue. Best regards Heinrich
Hi, On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > disk objects dynamically at runtime based on block device's probing. > > (See "issues" below.) > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > This series together with Simon's series breaks multiple boards due to > size constraints: > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > Please, investigate how to work around this issue. tbs2910 - perhaps we should just drop this board? It doesn't use DM_SERIAL and still uses OF_EMBED rcar3_salvator-x - can we put the partition changes behind a config? phycore-rk3288 - something going on in SPL, perhaps needs an additional config to disable it? Regards, Simon
On 3/8/22 17:56, Simon Glass wrote: > Hi, > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 3/8/22 12:36, AKASHI Takahiro wrote: >>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>> disk objects dynamically at runtime based on block device's probing. >>> (See "issues" below.) >>> >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >> >> This series together with Simon's series breaks multiple boards due to >> size constraints: >> >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >> >> Please, investigate how to work around this issue. > > tbs2910 - perhaps we should just drop this board? It doesn't use > DM_SERIAL and still uses OF_EMBED > rcar3_salvator-x - can we put the partition changes behind a config? This board came up before. 34f2577e926da ("ARM: renesas: reduce rcar3_salvator-x image size"). It is generally at the size limit. > phycore-rk3288 - something going on in SPL, perhaps needs an > additional config to disable it? Do we need any of your events on SPL? Best regards Heinrich > > Regards, > Simon
Hi Heinrich, On Tue, 8 Mar 2022 at 10:26, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/8/22 17:56, Simon Glass wrote: > > Hi, > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > >>> disk objects dynamically at runtime based on block device's probing. > >>> (See "issues" below.) > >>> > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > >> > >> This series together with Simon's series breaks multiple boards due to > >> size constraints: > >> > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > >> > >> Please, investigate how to work around this issue. > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > DM_SERIAL and still uses OF_EMBED > > rcar3_salvator-x - can we put the partition changes behind a config? > > This board came up before. 34f2577e926da ("ARM: renesas: reduce > rcar3_salvator-x image size"). It is generally at the size limit. > > > phycore-rk3288 - something going on in SPL, perhaps needs an > > additional config to disable it? > > Do we need any of your events on SPL? Not so far. Regards, Simon
On 08.03.22 17:56, Simon Glass wrote: > Hi, > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 3/8/22 12:36, AKASHI Takahiro wrote: >>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>> disk objects dynamically at runtime based on block device's probing. >>> (See "issues" below.) >>> >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >> >> This series together with Simon's series breaks multiple boards due to >> size constraints: >> >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >> >> Please, investigate how to work around this issue. > > tbs2910 - perhaps we should just drop this board? It doesn't use > DM_SERIAL and still uses OF_EMBED Are we again at the same point? You are breaking working boards with (for these boards) useless additions, and all you come up with is "remove this board". Of course without adding the board maintainer. What a shame! Soeren > rcar3_salvator-x - can we put the partition changes behind a config? > phycore-rk3288 - something going on in SPL, perhaps needs an > additional config to disable it? > > Regards, > Simon
Hi Soeren, On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > On 08.03.22 17:56, Simon Glass wrote: > > Hi, > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > >>> disk objects dynamically at runtime based on block device's probing. > >>> (See "issues" below.) > >>> > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > >> > >> This series together with Simon's series breaks multiple boards due to > >> size constraints: > >> > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > >> > >> Please, investigate how to work around this issue. > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > DM_SERIAL and still uses OF_EMBED > > Are we again at the same point? You are breaking working boards with > (for these boards) useless additions, and all you come up with is > "remove this board". Of course without adding the board maintainer. I'm just expressing reasonable frustration that this board uses OF_EMBED and does not use DM_SERIAL, after all of this time. Why should the rest of the U-Boot developers care more about this board than the maintainer? Regards, Simon
On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > Hi Soeren, > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > Hi, > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > >> > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > >>> disk objects dynamically at runtime based on block device's probing. > > >>> (See "issues" below.) > > >>> > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > >> > > >> This series together with Simon's series breaks multiple boards due to > > >> size constraints: > > >> > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > >> > > >> Please, investigate how to work around this issue. > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > DM_SERIAL and still uses OF_EMBED > > > > Are we again at the same point? You are breaking working boards with > > (for these boards) useless additions, and all you come up with is > > "remove this board". Of course without adding the board maintainer. > > I'm just expressing reasonable frustration that this board uses > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > should the rest of the U-Boot developers care more about this board > than the maintainer? Please keep in mind Simon that we've had zero releases with the DM_SERIAL migration warning being posted, v2022.04 will be the first one.
Heinrich, Simon, On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: > On 3/8/22 12:36, AKASHI Takahiro wrote: > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > disk objects dynamically at runtime based on block device's probing. > > (See "issues" below.) > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > This series together with Simon's series breaks multiple boards due to > size constraints: > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > Please, investigate how to work around this issue. I have already mentioned this size issue in my cover-letter in order to let reviewers aware of it and discuss a possible solution: ===8<=== Issues: ======= * The image size of U-Boot may increase. CI build test complains, for instance, rcar3_salvator-x: "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" phycore-rk3288: "SPL image is too large (size 0x8800 than 0x8000)" See [2]. [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results ===>8=== I have dug into rcar3_salvator-x case; I removed *all* the commits in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC and CONFIG_DM_EVENT, which are all required for enabling my patch, with Simon's patch applied on top of v2022.04-rc3. Then I still see this size problem: ===8<=== ... MKIMAGE u-boot.img u-boot.img exceeds file size limit: limit: 0x100000 bytes actual: 0x100036 bytes excess: 0x36 bytes ===>8=== So I have no way to deal with it. FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will increase the binary size by up to 0x1b2 for rcar3_salvator-x and it seems the binary has almost already reached its maximum size even now. -Takahiro Akashi > Best regards > > Heinrich
Hi Tom, On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > Hi Soeren, > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > Hi, > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > >> > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > >>> disk objects dynamically at runtime based on block device's probing. > > > >>> (See "issues" below.) > > > >>> > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > >> > > > >> This series together with Simon's series breaks multiple boards due to > > > >> size constraints: > > > >> > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > >> > > > >> Please, investigate how to work around this issue. > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > DM_SERIAL and still uses OF_EMBED > > > > > > Are we again at the same point? You are breaking working boards with > > > (for these boards) useless additions, and all you come up with is > > > "remove this board". Of course without adding the board maintainer. > > > > I'm just expressing reasonable frustration that this board uses > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > should the rest of the U-Boot developers care more about this board > > than the maintainer? > > Please keep in mind Simon that we've had zero releases with the > DM_SERIAL migration warning being posted, v2022.04 will be the first > one. Yes, understood :-) For OF_EMBED though...? It was actually quite hard to add a migration message until we added the CONFIG_SERIAL base thing and that was a pain to do. For those of us who take on larger refactors etc., we end up spending a lot of our time on these few platforms. I'm not picking on tbs2910in in particular. Regards, Simon
Hi Takahiro, On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Heinrich, Simon, > > On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > > disk objects dynamically at runtime based on block device's probing. > > > (See "issues" below.) > > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > This series together with Simon's series breaks multiple boards due to > > size constraints: > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > Please, investigate how to work around this issue. > > I have already mentioned this size issue in my cover-letter > in order to let reviewers aware of it and discuss a possible solution: > > ===8<=== > Issues: > ======= > * The image size of U-Boot may increase. CI build test complains, > for instance, > rcar3_salvator-x: > "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" > phycore-rk3288: > "SPL image is too large (size 0x8800 than 0x8000)" > > See [2]. > > [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results > ===>8=== > > I have dug into rcar3_salvator-x case; I removed *all* the commits > in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC > and CONFIG_DM_EVENT, which are all required for enabling my patch, > with Simon's patch applied on top of v2022.04-rc3. > > Then I still see this size problem: > ===8<=== > ... > MKIMAGE u-boot.img > u-boot.img exceeds file size limit: > limit: 0x100000 bytes > actual: 0x100036 bytes > excess: 0x36 bytes > ===>8=== > > So I have no way to deal with it. > > FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will > increase the binary size by up to 0x1b2 for rcar3_salvator-x and > it seems the binary has almost already reached its maximum size > even now. So you do need EVENT_DYNAMIC? Does it make sense to make enabling the partition support an option, instead of mandatory? Regards, Simon
Hi Simon, On Tue, Mar 08, 2022 at 07:34:15PM -0700, Simon Glass wrote: > Hi Takahiro, > > On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > Heinrich, Simon, > > > > On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: > > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > disk objects dynamically at runtime based on block device's probing. > > > > (See "issues" below.) > > > > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > This series together with Simon's series breaks multiple boards due to > > > size constraints: > > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > Please, investigate how to work around this issue. > > > > I have already mentioned this size issue in my cover-letter > > in order to let reviewers aware of it and discuss a possible solution: > > > > ===8<=== > > Issues: > > ======= > > * The image size of U-Boot may increase. CI build test complains, > > for instance, > > rcar3_salvator-x: > > "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" > > phycore-rk3288: > > "SPL image is too large (size 0x8800 than 0x8000)" > > > > See [2]. > > > > [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results > > ===>8=== > > > > I have dug into rcar3_salvator-x case; I removed *all* the commits > > in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC > > and CONFIG_DM_EVENT, which are all required for enabling my patch, > > with Simon's patch applied on top of v2022.04-rc3. > > > > Then I still see this size problem: > > ===8<=== > > ... > > MKIMAGE u-boot.img > > u-boot.img exceeds file size limit: > > limit: 0x100000 bytes > > actual: 0x100036 bytes > > excess: 0x36 bytes > > ===>8=== > > > > So I have no way to deal with it. > > > > FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will > > increase the binary size by up to 0x1b2 for rcar3_salvator-x and > > it seems the binary has almost already reached its maximum size > > even now. > > So you do need EVENT_DYNAMIC? Unfortunately, yes. When I rebased my patch set to your v2, I tried to use *static* bindings, but some of ut tests, including dm_test_blk_base and dm_test_blk_usb, failed. This can happen because, with static bindings, efi's callback function (efi_disk_probe) is unconditionally called even when UEFI subsystem has not been initialized yet. -Takahiro Akashi > Does it make sense to make enabling the partition support an option, > instead of mandatory? > > Regards, > Simon
On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > Hi Tom, > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > Hi Soeren, > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > Hi, > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > >> > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > >>> (See "issues" below.) > > > > >>> > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > >> > > > > >> This series together with Simon's series breaks multiple boards due to > > > > >> size constraints: > > > > >> > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > >> > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > (for these boards) useless additions, and all you come up with is > > > > "remove this board". Of course without adding the board maintainer. > > > > > > I'm just expressing reasonable frustration that this board uses > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > should the rest of the U-Boot developers care more about this board > > > than the maintainer? > > > > Please keep in mind Simon that we've had zero releases with the > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > one. > > Yes, understood :-) For OF_EMBED though...? No deadline and 50 boards. > It was actually quite hard to add a migration message until we added > the CONFIG_SERIAL base thing and that was a pain to do. > > For those of us who take on larger refactors etc., we end up spending > a lot of our time on these few platforms. I'm not picking on tbs2910in > in particular. Well, the flip side of the problem here is that there's a number of platforms with real constraints to them and it keeps being "can we drop this yet?" without CC'ing the board maintainer on the series that once again pushes a given platform to the limit. I would expect no size growth to tbs2910 for the topic of this series since it disables EFI_LOADER entirely, so why is it a problem?
Hi Takahiro, On Tue, 8 Mar 2022 at 19:48, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > Hi Simon, > > On Tue, Mar 08, 2022 at 07:34:15PM -0700, Simon Glass wrote: > > Hi Takahiro, > > > > On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > Heinrich, Simon, > > > > > > On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: > > > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > disk objects dynamically at runtime based on block device's probing. > > > > > (See "issues" below.) > > > > > > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > > > This series together with Simon's series breaks multiple boards due to > > > > size constraints: > > > > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > > > Please, investigate how to work around this issue. > > > > > > I have already mentioned this size issue in my cover-letter > > > in order to let reviewers aware of it and discuss a possible solution: > > > > > > ===8<=== > > > Issues: > > > ======= > > > * The image size of U-Boot may increase. CI build test complains, > > > for instance, > > > rcar3_salvator-x: > > > "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" > > > phycore-rk3288: > > > "SPL image is too large (size 0x8800 than 0x8000)" > > > > > > See [2]. > > > > > > [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results > > > ===>8=== > > > > > > I have dug into rcar3_salvator-x case; I removed *all* the commits > > > in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC > > > and CONFIG_DM_EVENT, which are all required for enabling my patch, > > > with Simon's patch applied on top of v2022.04-rc3. > > > > > > Then I still see this size problem: > > > ===8<=== > > > ... > > > MKIMAGE u-boot.img > > > u-boot.img exceeds file size limit: > > > limit: 0x100000 bytes > > > actual: 0x100036 bytes > > > excess: 0x36 bytes > > > ===>8=== > > > > > > So I have no way to deal with it. > > > > > > FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will > > > increase the binary size by up to 0x1b2 for rcar3_salvator-x and > > > it seems the binary has almost already reached its maximum size > > > even now. > > > > So you do need EVENT_DYNAMIC? > > Unfortunately, yes. > When I rebased my patch set to your v2, I tried to use *static* > bindings, but some of ut tests, including dm_test_blk_base and > dm_test_blk_usb, failed. OK. Well maybe there is a filesystem in there that is not needed? 1MB is a huge size! Can we disable EFI_LOADER on that board? > > This can happen because, with static bindings, efi's callback function > (efi_disk_probe) is unconditionally called even when UEFI subsystem has > not been initialized yet. Yes, I have seen things like that too. > > -Takahiro Akashi > > > Does it make sense to make enabling the partition support an option, > > instead of mandatory? What about this one? ^^ Regards, Simon
Hi Tom, On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > Hi Soeren, > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > >> > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > >>> (See "issues" below.) > > > > > >>> > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > >> > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > >> size constraints: > > > > > >> > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > >> > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > (for these boards) useless additions, and all you come up with is > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > should the rest of the U-Boot developers care more about this board > > > > than the maintainer? > > > > > > Please keep in mind Simon that we've had zero releases with the > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > one. > > > > Yes, understood :-) For OF_EMBED though...? > > No deadline and 50 boards. Er, there has been a build message about that since the beginning, so people ignored it. Do we really need to make the build fail for these sorts of things? Perhaps so, but it is a sad situation. > > > It was actually quite hard to add a migration message until we added > > the CONFIG_SERIAL base thing and that was a pain to do. > > > > For those of us who take on larger refactors etc., we end up spending > > a lot of our time on these few platforms. I'm not picking on tbs2910in > > in particular. > > Well, the flip side of the problem here is that there's a number of > platforms with real constraints to them and it keeps being "can we drop > this yet?" without CC'ing the board maintainer on the series that once > again pushes a given platform to the limit. I would expect no size > growth to tbs2910 for the topic of this series since it disables > EFI_LOADER entirely, so why is it a problem? The partition changes are going to add some size anyway, I expect. I have not actually analysed it though. Perhaps we can just disable a filesystem? Regards, Simon
Hi Tom, On Tue, 8 Mar 2022 at 20:10, Simon Glass <sjg@chromium.org> wrote: > > Hi Tom, > > On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > > Hi Soeren, > > > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > >> > > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > > >>> (See "issues" below.) > > > > > > >>> > > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > >> > > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > > >> size constraints: > > > > > > >> > > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > >> > > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > > (for these boards) useless additions, and all you come up with is > > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > > should the rest of the U-Boot developers care more about this board > > > > > than the maintainer? > > > > > > > > Please keep in mind Simon that we've had zero releases with the > > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > > one. > > > > > > Yes, understood :-) For OF_EMBED though...? > > > > No deadline and 50 boards. > > Er, there has been a build message about that since the beginning, so > people ignored it. Do we really need to make the build fail for these > sorts of things? Perhaps so, but it is a sad situation. > > > > > > It was actually quite hard to add a migration message until we added > > > the CONFIG_SERIAL base thing and that was a pain to do. > > > > > > For those of us who take on larger refactors etc., we end up spending > > > a lot of our time on these few platforms. I'm not picking on tbs2910in > > > in particular. > > > > Well, the flip side of the problem here is that there's a number of > > platforms with real constraints to them and it keeps being "can we drop > > this yet?" without CC'ing the board maintainer on the series that once > > again pushes a given platform to the limit. I would expect no size > > growth to tbs2910 for the topic of this series since it disables > > EFI_LOADER entirely, so why is it a problem? > > The partition changes are going to add some size anyway, I expect. I > have not actually analysed it though. Perhaps we can just disable a > filesystem? One more thing, if we get around to supporting file access through a device that is also going to add some size, although perhaps not a huge amount. Regards, SImon > Simon
On Tue, Mar 08, 2022 at 08:10:01PM -0700, Simon Glass wrote: > Hi Takahiro, > > On Tue, 8 Mar 2022 at 19:48, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > Hi Simon, > > > > On Tue, Mar 08, 2022 at 07:34:15PM -0700, Simon Glass wrote: > > > Hi Takahiro, > > > > > > On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: > > > > > > > > Heinrich, Simon, > > > > > > > > On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: > > > > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > disk objects dynamically at runtime based on block device's probing. > > > > > > (See "issues" below.) > > > > > > > > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > > > > > This series together with Simon's series breaks multiple boards due to > > > > > size constraints: > > > > > > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > > > > > Please, investigate how to work around this issue. > > > > > > > > I have already mentioned this size issue in my cover-letter > > > > in order to let reviewers aware of it and discuss a possible solution: > > > > > > > > ===8<=== > > > > Issues: > > > > ======= > > > > * The image size of U-Boot may increase. CI build test complains, > > > > for instance, > > > > rcar3_salvator-x: > > > > "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" > > > > phycore-rk3288: > > > > "SPL image is too large (size 0x8800 than 0x8000)" > > > > > > > > See [2]. > > > > > > > > [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results > > > > ===>8=== > > > > > > > > I have dug into rcar3_salvator-x case; I removed *all* the commits > > > > in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC > > > > and CONFIG_DM_EVENT, which are all required for enabling my patch, > > > > with Simon's patch applied on top of v2022.04-rc3. > > > > > > > > Then I still see this size problem: > > > > ===8<=== > > > > ... > > > > MKIMAGE u-boot.img > > > > u-boot.img exceeds file size limit: > > > > limit: 0x100000 bytes > > > > actual: 0x100036 bytes > > > > excess: 0x36 bytes > > > > ===>8=== > > > > > > > > So I have no way to deal with it. > > > > > > > > FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will > > > > increase the binary size by up to 0x1b2 for rcar3_salvator-x and > > > > it seems the binary has almost already reached its maximum size > > > > even now. > > > > > > So you do need EVENT_DYNAMIC? > > > > Unfortunately, yes. > > When I rebased my patch set to your v2, I tried to use *static* > > bindings, but some of ut tests, including dm_test_blk_base and > > dm_test_blk_usb, failed. > > OK. Well maybe there is a filesystem in there that is not needed? 1MB > is a huge size! Can we disable EFI_LOADER on that board? Well, EFI_LOADER is by default 'y' for arm64. Basically, I doubt that this default is reasonable. > > > > This can happen because, with static bindings, efi's callback function > > (efi_disk_probe) is unconditionally called even when UEFI subsystem has > > not been initialized yet. > > Yes, I have seen things like that too. > > > > > -Takahiro Akashi > > > > > Does it make sense to make enabling the partition support an option, > > > instead of mandatory? > > What about this one? ^^ First of all, according to my rough attempt, the patches for adding efi_disk callback functions may increase the binary size by 0x31c, while the patches for adding UCLASS_PARTITION adds another 0x3ba. This means that "enabling the partition support an option" can help a bit but doesn't help well enough overall. FYI, adding dev_read/write(udev) interfaces costs another 0x1df. -Takahiro Akashi > Regards, > Simon
> Date: Tue, 8 Mar 2022 22:00:35 -0500 > From: Tom Rini <trini@konsulko.com> > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > Hi Soeren, > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > Hi, > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > >> > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > >>> (See "issues" below.) > > > > > >>> > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > >> > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > >> size constraints: > > > > > >> > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > >> > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > (for these boards) useless additions, and all you come up with is > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > should the rest of the U-Boot developers care more about this board > > > > than the maintainer? > > > > > > Please keep in mind Simon that we've had zero releases with the > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > one. > > > > Yes, understood :-) For OF_EMBED though...? > > No deadline and 50 boards. And neither should there be. OF_EMBED is the obvious choice when chainloading U-Boot from a proprietary initial loader that loads ELF binaries.
Hi Akashi-san, > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > Reviewed-by: Simon Glass <sjg@chromium.org> > --- > drivers/core/Makefile | 2 +- > drivers/core/root.c | 2 + > drivers/core/tag.c | 139 ++++++++++++++++++++++++++++++ > include/asm-generic/global_data.h | 4 + > include/dm/tag.h | 110 +++++++++++++++++++++++ > 5 files changed, 256 insertions(+), 1 deletion(-) > create mode 100644 drivers/core/tag.c > create mode 100644 include/dm/tag.h > > diff --git a/drivers/core/Makefile b/drivers/core/Makefile > index 5edd4e413576..3742e7574525 100644 > --- a/drivers/core/Makefile > +++ b/drivers/core/Makefile > @@ -2,7 +2,7 @@ > # > # Copyright (c) 2013 Google, Inc > > -obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o > +obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o > obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o > obj-$(CONFIG_DEVRES) += devres.o > obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o > diff --git a/drivers/core/root.c b/drivers/core/root.c > index 8efb4256b27e..86b3884fc674 100644 > --- a/drivers/core/root.c > +++ b/drivers/core/root.c > @@ -199,6 +199,8 @@ int dm_init(bool of_live) > return ret; > } > > + INIT_LIST_HEAD((struct list_head *)&gd->dmtag_list); > + > return 0; > } > > diff --git a/drivers/core/tag.c b/drivers/core/tag.c > new file mode 100644 > index 000000000000..6829bcd8806c > --- /dev/null > +++ b/drivers/core/tag.c > @@ -0,0 +1,139 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright (c) 2021 Linaro Limited > + * Author: AKASHI Takahiro > + */ > + > +#include <malloc.h> > +#include <asm/global_data.h> > +#include <dm/tag.h> > +#include <linux/err.h> > +#include <linux/list.h> > +#include <linux/types.h> > + > +struct udevice; > + > +DECLARE_GLOBAL_DATA_PTR; > + > +int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr) > +{ > + struct dmtag_node *node; > + > + if (!dev || tag >= DM_TAG_COUNT) > + return -EINVAL; > + > + list_for_each_entry(node, &gd->dmtag_list, sibling) { > + if (node->dev == dev && node->tag == tag) > + return -EEXIST; > + } > + > + node = calloc(sizeof(*node), 1); > + if (!node) > + return -ENOSPC; Isn't -ENOMEM better here? > + > + node->dev = dev; > + node->tag = tag; > + node->ptr = ptr; > + list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list); > + > + return 0; > +} > + > +int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val) Is this used anywhere else apart from selftests? > +{ > + struct dmtag_node *node; Thanks /Ilias
On 3/9/22 06:07, AKASHI Takahiro wrote: > On Tue, Mar 08, 2022 at 08:10:01PM -0700, Simon Glass wrote: >> Hi Takahiro, >> >> On Tue, 8 Mar 2022 at 19:48, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: >>> >>> Hi Simon, >>> >>> On Tue, Mar 08, 2022 at 07:34:15PM -0700, Simon Glass wrote: >>>> Hi Takahiro, >>>> >>>> On Tue, 8 Mar 2022 at 19:10, AKASHI Takahiro <takahiro.akashi@linaro.org> wrote: >>>>> >>>>> Heinrich, Simon, >>>>> >>>>> On Tue, Mar 08, 2022 at 05:49:13PM +0100, Heinrich Schuchardt wrote: >>>>>> On 3/8/22 12:36, AKASHI Takahiro wrote: >>>>>>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>>>>>> disk objects dynamically at runtime based on block device's probing. >>>>>>> (See "issues" below.) >>>>>>> >>>>>>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >>>>>> >>>>>> This series together with Simon's series breaks multiple boards due to >>>>>> size constraints: >>>>>> >>>>>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >>>>>> >>>>>> Please, investigate how to work around this issue. >>>>> >>>>> I have already mentioned this size issue in my cover-letter >>>>> in order to let reviewers aware of it and discuss a possible solution: >>>>> >>>>> ===8<=== >>>>> Issues: >>>>> ======= >>>>> * The image size of U-Boot may increase. CI build test complains, >>>>> for instance, >>>>> rcar3_salvator-x: >>>>> "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" >>>>> phycore-rk3288: >>>>> "SPL image is too large (size 0x8800 than 0x8000)" >>>>> >>>>> See [2]. >>>>> >>>>> [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results >>>>> ===>8=== >>>>> >>>>> I have dug into rcar3_salvator-x case; I removed *all* the commits >>>>> in this series and yet enabled CONFIG_EVENT, CONFIG_EVENT_DYNAMIC >>>>> and CONFIG_DM_EVENT, which are all required for enabling my patch, >>>>> with Simon's patch applied on top of v2022.04-rc3. >>>>> >>>>> Then I still see this size problem: >>>>> ===8<=== >>>>> ... >>>>> MKIMAGE u-boot.img >>>>> u-boot.img exceeds file size limit: >>>>> limit: 0x100000 bytes >>>>> actual: 0x100036 bytes >>>>> excess: 0x36 bytes >>>>> ===>8=== >>>>> >>>>> So I have no way to deal with it. >>>>> >>>>> FYI, the combination of EVENT, EVENT_DYNAMIC and DM_EVENT will >>>>> increase the binary size by up to 0x1b2 for rcar3_salvator-x and >>>>> it seems the binary has almost already reached its maximum size >>>>> even now. >>>> >>>> So you do need EVENT_DYNAMIC? >>> >>> Unfortunately, yes. >>> When I rebased my patch set to your v2, I tried to use *static* >>> bindings, but some of ut tests, including dm_test_blk_base and >>> dm_test_blk_usb, failed. >> >> OK. Well maybe there is a filesystem in there that is not needed? 1MB >> is a huge size! Can we disable EFI_LOADER on that board? > > Well, EFI_LOADER is by default 'y' for arm64. > Basically, I doubt that this default is reasonable. All major distros support booting via UEFI. Fedora and Suse have specifically opted to make this the preferred way to boot on ARM. Same is true for BSD. So why do you have doubts? Best regards Heinrich > >>> >>> This can happen because, with static bindings, efi's callback function >>> (efi_disk_probe) is unconditionally called even when UEFI subsystem has >>> not been initialized yet. >> >> Yes, I have seen things like that too. >> >>> >>> -Takahiro Akashi >>> >>>> Does it make sense to make enabling the partition support an option, >>>> instead of mandatory? >> >> What about this one? ^^ > > First of all, according to my rough attempt, > the patches for adding efi_disk callback functions may increase > the binary size by 0x31c, while the patches for adding UCLASS_PARTITION > adds another 0x3ba. > This means that "enabling the partition support an option" can help a bit > but doesn't help well enough overall. > > FYI, adding dev_read/write(udev) interfaces costs another 0x1df. > > -Takahiro Akashi > >> Regards, >> Simon
On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > Hi Tom, > > On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > > Hi Soeren, > > > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > >> > > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > > >>> (See "issues" below.) > > > > > > >>> > > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > >> > > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > > >> size constraints: > > > > > > >> > > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > >> > > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > > (for these boards) useless additions, and all you come up with is > > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > > should the rest of the U-Boot developers care more about this board > > > > > than the maintainer? > > > > > > > > Please keep in mind Simon that we've had zero releases with the > > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > > one. > > > > > > Yes, understood :-) For OF_EMBED though...? > > > > No deadline and 50 boards. > > Er, there has been a build message about that since the beginning, so > people ignored it. Do we really need to make the build fail for these > sorts of things? Perhaps so, but it is a sad situation. Yes, in hind-sight, "don't do that" wasn't the right path. It would be a good idea to start a different thread and see what / how the platforms can be migrated away. > > > It was actually quite hard to add a migration message until we added > > > the CONFIG_SERIAL base thing and that was a pain to do. > > > > > > For those of us who take on larger refactors etc., we end up spending > > > a lot of our time on these few platforms. I'm not picking on tbs2910in > > > in particular. > > > > Well, the flip side of the problem here is that there's a number of > > platforms with real constraints to them and it keeps being "can we drop > > this yet?" without CC'ing the board maintainer on the series that once > > again pushes a given platform to the limit. I would expect no size > > growth to tbs2910 for the topic of this series since it disables > > EFI_LOADER entirely, so why is it a problem? > > The partition changes are going to add some size anyway, I expect. I > have not actually analysed it though. Perhaps we can just disable a > filesystem? I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes here-and-there is probably a non issue. But it shouldn't be kilobytes. It really shouldn't push things over the line. And on the tbs2910 side, Soeren, can you look at enabling LTO for this platform? That would likely buy a good bit of space savings. That might well be needed to do further DM migrations/etc.
Hi Tom, On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > > > Hi Soeren, > > > > > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > >> > > > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > > > >>> (See "issues" below.) > > > > > > > >>> > > > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > > >> > > > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > > > >> size constraints: > > > > > > > >> > > > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > > >> > > > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > > > (for these boards) useless additions, and all you come up with is > > > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > > > should the rest of the U-Boot developers care more about this board > > > > > > than the maintainer? > > > > > > > > > > Please keep in mind Simon that we've had zero releases with the > > > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > > > one. > > > > > > > > Yes, understood :-) For OF_EMBED though...? > > > > > > No deadline and 50 boards. > > > > Er, there has been a build message about that since the beginning, so > > people ignored it. Do we really need to make the build fail for these > > sorts of things? Perhaps so, but it is a sad situation. > > Yes, in hind-sight, "don't do that" wasn't the right path. It would be > a good idea to start a different thread and see what / how the platforms > can be migrated away. I think there is a use case for it now - e.g. booting Apple M1 which uses a separate bootloader. IMO a .img or .fit file would be better in some cases but people seem to be allergic to implementing U-Boot things in their code bases. We have the same requirement for the EFI app since UEFI does not implement the U-Boot .img file. So if we are going to support this, perhaps we should create a new option for it. But honestly I am just too weary to consider yet another migration. We need to finish some, e.g. Kconfig. > > > > > It was actually quite hard to add a migration message until we added > > > > the CONFIG_SERIAL base thing and that was a pain to do. > > > > > > > > For those of us who take on larger refactors etc., we end up spending > > > > a lot of our time on these few platforms. I'm not picking on tbs2910in > > > > in particular. > > > > > > Well, the flip side of the problem here is that there's a number of > > > platforms with real constraints to them and it keeps being "can we drop > > > this yet?" without CC'ing the board maintainer on the series that once > > > again pushes a given platform to the limit. I would expect no size > > > growth to tbs2910 for the topic of this series since it disables > > > EFI_LOADER entirely, so why is it a problem? > > > > The partition changes are going to add some size anyway, I expect. I > > have not actually analysed it though. Perhaps we can just disable a > > filesystem? > > I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes > here-and-there is probably a non issue. But it shouldn't be kilobytes. > It really shouldn't push things over the line. > > And on the tbs2910 side, Soeren, can you look at enabling LTO for this > platform? That would likely buy a good bit of space savings. That > might well be needed to do further DM migrations/etc. Regards, Simon
On Wed, Mar 09, 2022 at 01:41:04PM +0200, Ilias Apalodimas wrote: > Hi Akashi-san, > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > Reviewed-by: Simon Glass <sjg@chromium.org> > > --- > > drivers/core/Makefile | 2 +- > > drivers/core/root.c | 2 + > > drivers/core/tag.c | 139 ++++++++++++++++++++++++++++++ > > include/asm-generic/global_data.h | 4 + > > include/dm/tag.h | 110 +++++++++++++++++++++++ > > 5 files changed, 256 insertions(+), 1 deletion(-) > > create mode 100644 drivers/core/tag.c > > create mode 100644 include/dm/tag.h > > > > diff --git a/drivers/core/Makefile b/drivers/core/Makefile > > index 5edd4e413576..3742e7574525 100644 > > --- a/drivers/core/Makefile > > +++ b/drivers/core/Makefile > > @@ -2,7 +2,7 @@ > > # > > # Copyright (c) 2013 Google, Inc > > > > -obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o > > +obj-y += device.o fdtaddr.o lists.o root.o uclass.o util.o tag.o > > obj-$(CONFIG_$(SPL_TPL_)ACPIGEN) += acpi.o > > obj-$(CONFIG_DEVRES) += devres.o > > obj-$(CONFIG_$(SPL_)DM_DEVICE_REMOVE) += device-remove.o > > diff --git a/drivers/core/root.c b/drivers/core/root.c > > index 8efb4256b27e..86b3884fc674 100644 > > --- a/drivers/core/root.c > > +++ b/drivers/core/root.c > > @@ -199,6 +199,8 @@ int dm_init(bool of_live) > > return ret; > > } > > > > + INIT_LIST_HEAD((struct list_head *)&gd->dmtag_list); > > + > > return 0; > > } > > > > diff --git a/drivers/core/tag.c b/drivers/core/tag.c > > new file mode 100644 > > index 000000000000..6829bcd8806c > > --- /dev/null > > +++ b/drivers/core/tag.c > > @@ -0,0 +1,139 @@ > > +// SPDX-License-Identifier: GPL-2.0+ > > +/* > > + * Copyright (c) 2021 Linaro Limited > > + * Author: AKASHI Takahiro > > + */ > > + > > +#include <malloc.h> > > +#include <asm/global_data.h> > > +#include <dm/tag.h> > > +#include <linux/err.h> > > +#include <linux/list.h> > > +#include <linux/types.h> > > + > > +struct udevice; > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr) > > +{ > > + struct dmtag_node *node; > > + > > + if (!dev || tag >= DM_TAG_COUNT) > > + return -EINVAL; > > + > > + list_for_each_entry(node, &gd->dmtag_list, sibling) { > > + if (node->dev == dev && node->tag == tag) > > + return -EEXIST; > > + } > > + > > + node = calloc(sizeof(*node), 1); > > + if (!node) > > + return -ENOSPC; > > Isn't -ENOMEM better here? Ah, yes. > > + > > + node->dev = dev; > > + node->tag = tag; > > + node->ptr = ptr; > > + list_add_tail(&node->sibling, (struct list_head *)&gd->dmtag_list); > > + > > + return 0; > > +} > > + > > +int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val) > > Is this used anywhere else apart from selftests? Currently, no. It exists just for future simpler use cases. -Takahiro Akashi > > +{ > > + struct dmtag_node *node; > > > Thanks > /Ilias
Hi Tom, Takahiro, On Wed, 9 Mar 2022 at 08:33, Simon Glass <sjg@chromium.org> wrote: > > Hi Tom, > > On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: > > > > On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > > > > Hi Soeren, > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > > > > > > > >> > > > > > > > > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > > > >>> With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > > > >>> disk objects dynamically at runtime based on block device's probing. > > > > > > > > >>> (See "issues" below.) > > > > > > > > >>> > > > > > > > > >>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > > > >> > > > > > > > > >> This series together with Simon's series breaks multiple boards due to > > > > > > > > >> size constraints: > > > > > > > > >> > > > > > > > > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > > > >> > > > > > > > > >> Please, investigate how to work around this issue. > > > > > > > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > > > > (for these boards) useless additions, and all you come up with is > > > > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > > > > should the rest of the U-Boot developers care more about this board > > > > > > > than the maintainer? > > > > > > > > > > > > Please keep in mind Simon that we've had zero releases with the > > > > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > > > > one. > > > > > > > > > > Yes, understood :-) For OF_EMBED though...? > > > > > > > > No deadline and 50 boards. > > > > > > Er, there has been a build message about that since the beginning, so > > > people ignored it. Do we really need to make the build fail for these > > > sorts of things? Perhaps so, but it is a sad situation. > > > > Yes, in hind-sight, "don't do that" wasn't the right path. It would be > > a good idea to start a different thread and see what / how the platforms > > can be migrated away. > > I think there is a use case for it now - e.g. booting Apple M1 which > uses a separate bootloader. IMO a .img or .fit file would be better in > some cases but people seem to be allergic to implementing U-Boot > things in their code bases. We have the same requirement for the EFI > app since UEFI does not implement the U-Boot .img file. > > So if we are going to support this, perhaps we should create a new > option for it. But honestly I am just too weary to consider yet > another migration. We need to finish some, e.g. Kconfig. Back to the original topic, I found that some partition code is present in SPL and cannot be dropped. This is a long-standing thing but matters a little more now that there is a driver and blk_post_probe() contains some code. This series is foundational in two ways: - adding driver model support for partitions - starting the clean-up of the EFI code to better use drive rmodel Heinrich, can we please hold off on any new EFI changes in -next until we can get this series landed? Any conflicts will set us back again. I will see if I can send a little series to clean that up and to reduce the board sizes for the non-SPL problems. With that, perhaps we can apply both series within a few days? [..] Regards, Simon
On 09.03.22 16:33, Simon Glass wrote: > Hi Tom, > > On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: >> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: >>> Hi Tom, >>> >>> On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: >>>> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: >>>>> Hi Tom, >>>>> >>>>> On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: >>>>>> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: >>>>>>> Hi Soeren, >>>>>>> >>>>>>> On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 08.03.22 17:56, Simon Glass wrote: >>>>>>>>> Hi, >>>>>>>>> >>>>>>>>> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >>>>>>>>>> On 3/8/22 12:36, AKASHI Takahiro wrote: >>>>>>>>>>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>>>>>>>>>> disk objects dynamically at runtime based on block device's probing. >>>>>>>>>>> (See "issues" below.) >>>>>>>>>>> >>>>>>>>>>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >>>>>>>>>> This series together with Simon's series breaks multiple boards due to >>>>>>>>>> size constraints: >>>>>>>>>> >>>>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >>>>>>>>>> >>>>>>>>>> Please, investigate how to work around this issue. >>>>>>>>> tbs2910 - perhaps we should just drop this board? It doesn't use >>>>>>>>> DM_SERIAL and still uses OF_EMBED >>>>>>>> Are we again at the same point? You are breaking working boards with >>>>>>>> (for these boards) useless additions, and all you come up with is >>>>>>>> "remove this board". Of course without adding the board maintainer. >>>>>>> I'm just expressing reasonable frustration that this board uses >>>>>>> OF_EMBED and does not use DM_SERIAL, after all of this time. Why >>>>>>> should the rest of the U-Boot developers care more about this board >>>>>>> than the maintainer? I cannot see what is is reasonable here. I care about this board, what you can simply see by the fact that I even picked this thread from the mailing list while you "forgot" to cc me. OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. I'm surprised that you can speak for "the rest of the U-Boot developers", and that you want to push your frustration onto tbs2910 developers and users. Why is it my fault that other people add code size without guarding config options? Why is it my fault that nobody informed me that there is again a size problem? >>>>>> Please keep in mind Simon that we've had zero releases with the >>>>>> DM_SERIAL migration warning being posted, v2022.04 will be the first >>>>>> one. >>>>> Yes, understood :-) For OF_EMBED though...? >>>> No deadline and 50 boards. >>> Er, there has been a build message about that since the beginning, so >>> people ignored it. Do we really need to make the build fail for these >>> sorts of things? Perhaps so, but it is a sad situation. >> Yes, in hind-sight, "don't do that" wasn't the right path. It would be >> a good idea to start a different thread and see what / how the platforms >> can be migrated away. For tbs2910 this is just a workaround for a strange property of the imx build system. OF_SEPARATE created a broken u-boot.imx when I tried last time. > I think there is a use case for it now - e.g. booting Apple M1 which > uses a separate bootloader. IMO a .img or .fit file would be better in > some cases but people seem to be allergic to implementing U-Boot > things in their code bases. We have the same requirement for the EFI > app since UEFI does not implement the U-Boot .img file. > > So if we are going to support this, perhaps we should create a new > option for it. But honestly I am just too weary to consider yet > another migration. We need to finish some, e.g. Kconfig. > >>>>> It was actually quite hard to add a migration message until we added >>>>> the CONFIG_SERIAL base thing and that was a pain to do. >>>>> >>>>> For those of us who take on larger refactors etc., we end up spending >>>>> a lot of our time on these few platforms. I'm not picking on tbs2910in >>>>> in particular. >>>> Well, the flip side of the problem here is that there's a number of >>>> platforms with real constraints to them and it keeps being "can we drop >>>> this yet?" without CC'ing the board maintainer on the series that once >>>> again pushes a given platform to the limit. I would expect no size >>>> growth to tbs2910 for the topic of this series since it disables >>>> EFI_LOADER entirely, so why is it a problem? >>> The partition changes are going to add some size anyway, I expect. I >>> have not actually analysed it though. Perhaps we can just disable a >>> filesystem? OK, you did not even analyse where the problem comes from. But disabling user visible functionality on my board is the natural solution to that? Strange. >> I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes >> here-and-there is probably a non issue. But it shouldn't be kilobytes. >> It really shouldn't push things over the line. >> >> And on the tbs2910 side, Soeren, can you look at enabling LTO for this >> platform? That would likely buy a good bit of space savings. That >> might well be needed to do further DM migrations/etc. I'm not familiar with LTO in U-Boot, but will have a look at the weekend. Regards, Soeren > Regards, > Simon
Hi Soeren, On Fri, 11 Mar 2022 at 15:43, Soeren Moch <smoch@web.de> wrote: > > > > On 09.03.22 16:33, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: > >> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > >>> Hi Tom, > >>> > >>> On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > >>>> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > >>>>> Hi Tom, > >>>>> > >>>>> On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > >>>>>> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > >>>>>>> Hi Soeren, > >>>>>>> > >>>>>>> On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > >>>>>>>> > >>>>>>>> > >>>>>>>> On 08.03.22 17:56, Simon Glass wrote: > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >>>>>>>>>> On 3/8/22 12:36, AKASHI Takahiro wrote: > >>>>>>>>>>> With this patch set[1] applied, UEFI subsystem maintains a list of its > >>>>>>>>>>> disk objects dynamically at runtime based on block device's probing. > >>>>>>>>>>> (See "issues" below.) > >>>>>>>>>>> > >>>>>>>>>>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > >>>>>>>>>> This series together with Simon's series breaks multiple boards due to > >>>>>>>>>> size constraints: > >>>>>>>>>> > >>>>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > >>>>>>>>>> > >>>>>>>>>> Please, investigate how to work around this issue. > >>>>>>>>> tbs2910 - perhaps we should just drop this board? It doesn't use > >>>>>>>>> DM_SERIAL and still uses OF_EMBED > >>>>>>>> Are we again at the same point? You are breaking working boards with > >>>>>>>> (for these boards) useless additions, and all you come up with is > >>>>>>>> "remove this board". Of course without adding the board maintainer. > >>>>>>> I'm just expressing reasonable frustration that this board uses > >>>>>>> OF_EMBED and does not use DM_SERIAL, after all of this time. Why > >>>>>>> should the rest of the U-Boot developers care more about this board > >>>>>>> than the maintainer? > I cannot see what is is reasonable here. > > I care about this board, what you can simply see by the fact that I even > picked this thread from the mailing list while you "forgot" to cc me. This is the patch I sent: [PATCH 2/5] tbs2910: Disable ext4 write It shows that you are on cc. What are you referring to? > > OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. > > I'm surprised that you can speak for "the rest of the U-Boot > developers", and that you want to push your frustration onto tbs2910 > developers and users. Why is it my fault that other people add code size > without guarding config options? Why is it my fault that nobody informed > me that there is again a size problem? Your board is up against the limit and this causes problems. Please take a look and see how you can add some margin. Takahiro's series does add size and this is unavoidable. See my series of today for some fixes for the SPL size, but for U-Boot proper we have to accept the growth. > >>>>>> Please keep in mind Simon that we've had zero releases with the > >>>>>> DM_SERIAL migration warning being posted, v2022.04 will be the first > >>>>>> one. > >>>>> Yes, understood :-) For OF_EMBED though...? > >>>> No deadline and 50 boards. > >>> Er, there has been a build message about that since the beginning, so > >>> people ignored it. Do we really need to make the build fail for these > >>> sorts of things? Perhaps so, but it is a sad situation. > >> Yes, in hind-sight, "don't do that" wasn't the right path. It would be > >> a good idea to start a different thread and see what / how the platforms > >> can be migrated away. > For tbs2910 this is just a workaround for a strange property of the imx > build system. OF_SEPARATE created a broken u-boot.imx when I tried last > time. OK, that is worth digging in to. > > I think there is a use case for it now - e.g. booting Apple M1 which > > uses a separate bootloader. IMO a .img or .fit file would be better in > > some cases but people seem to be allergic to implementing U-Boot > > things in their code bases. We have the same requirement for the EFI > > app since UEFI does not implement the U-Boot .img file. > > > > So if we are going to support this, perhaps we should create a new > > option for it. But honestly I am just too weary to consider yet > > another migration. We need to finish some, e.g. Kconfig. > > > >>>>> It was actually quite hard to add a migration message until we added > >>>>> the CONFIG_SERIAL base thing and that was a pain to do. > >>>>> > >>>>> For those of us who take on larger refactors etc., we end up spending > >>>>> a lot of our time on these few platforms. I'm not picking on tbs2910in > >>>>> in particular. > >>>> Well, the flip side of the problem here is that there's a number of > >>>> platforms with real constraints to them and it keeps being "can we drop > >>>> this yet?" without CC'ing the board maintainer on the series that once > >>>> again pushes a given platform to the limit. I would expect no size > >>>> growth to tbs2910 for the topic of this series since it disables > >>>> EFI_LOADER entirely, so why is it a problem? > >>> The partition changes are going to add some size anyway, I expect. I > >>> have not actually analysed it though. Perhaps we can just disable a > >>> filesystem? > OK, you did not even analyse where the problem comes from. But disabling > user visible functionality on my board is the natural solution to that? > Strange. As above, please create some space so people can continue to develop. There are refactors and features updates which require more code space. It is somewhat rare, but it happens perhaps every year. > >> I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes > >> here-and-there is probably a non issue. But it shouldn't be kilobytes. > >> It really shouldn't push things over the line. > >> > >> And on the tbs2910 side, Soeren, can you look at enabling LTO for this > >> platform? That would likely buy a good bit of space savings. That > >> might well be needed to do further DM migrations/etc. > I'm not familiar with LTO in U-Boot, but will have a look at the weekend. OK, I suggest getting it several KB under the limit if you can, or perhaps even drop the limit. Regards, Simon
Hi Simon, On 12.03.22 06:02, Simon Glass wrote: > Hi Soeren, > > On Fri, 11 Mar 2022 at 15:43, Soeren Moch<smoch@web.de> wrote: > > > > On 09.03.22 16:33, Simon Glass wrote: >>> Hi Tom, >>> >>> On Wed, 9 Mar 2022 at 07:25, Tom Rini<trini@konsulko.com> wrote: >>>> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: >>>>> Hi Tom, >>>>> >>>>> On Tue, 8 Mar 2022 at 20:00, Tom Rini<trini@konsulko.com> wrote: >>>>>> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: >>>>>>> Hi Tom, >>>>>>> >>>>>>> On Tue, 8 Mar 2022 at 17:13, Tom Rini<trini@konsulko.com> wrote: >>>>>>>> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: >>>>>>>>> Hi Soeren, >>>>>>>>> >>>>>>>>> On Tue, 8 Mar 2022 at 12:15, Soeren Moch<smoch@web.de> wrote: >>>>>>>>>> >>>>>>>>>> On 08.03.22 17:56, Simon Glass wrote: >>>>>>>>>>> Hi, >>>>>>>>>>> >>>>>>>>>>> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt<xypron.glpk@gmx.de> wrote: >>>>>>>>>>>> On 3/8/22 12:36, AKASHI Takahiro wrote: >>>>>>>>>>>>> With this patch set[1] applied, UEFI subsystem maintains a list of its >>>>>>>>>>>>> disk objects dynamically at runtime based on block device's probing. >>>>>>>>>>>>> (See "issues" below.) >>>>>>>>>>>>> >>>>>>>>>>>>> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >>>>>>>>>>>> This series together with Simon's series breaks multiple boards due to >>>>>>>>>>>> size constraints: >>>>>>>>>>>> >>>>>>>>>>>> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >>>>>>>>>>>> >>>>>>>>>>>> Please, investigate how to work around this issue. >>>>>>>>>>> tbs2910 - perhaps we should just drop this board? It doesn't use >>>>>>>>>>> DM_SERIAL and still uses OF_EMBED >>>>>>>>>> Are we again at the same point? You are breaking working boards with >>>>>>>>>> (for these boards) useless additions, and all you come up with is >>>>>>>>>> "remove this board". Of course without adding the board maintainer. >>>>>>>>> I'm just expressing reasonable frustration that this board uses >>>>>>>>> OF_EMBED and does not use DM_SERIAL, after all of this time. Why >>>>>>>>> should the rest of the U-Boot developers care more about this board >>>>>>>>> than the maintainer? >> I cannot see what is is reasonable here. >> >> I care about this board, what you can simply see by the fact that I even >> picked this thread from the mailing list while you "forgot" to cc me. > This is the patch I sent: > > [PATCH 2/5] tbs2910: Disable ext4 write > > It shows that you are on cc. What are you referring to? I'm referring to the exact same email thread that I answered in, which btw. is still this exact same thread for this answer. Why should I refer to the totally different email thread you cited here? >> OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. >> >> I'm surprised that you can speak for "the rest of the U-Boot >> developers", and that you want to push your frustration onto tbs2910 >> developers and users. Why is it my fault that other people add code size >> without guarding config options? Why is it my fault that nobody informed >> me that there is again a size problem? > Your board is up against the limit and this causes problems. Please > take a look and see how you can add some margin. Takahiro's series > does add size and this is unavoidable. See my series of today for some > fixes for the SPL size, but for U-Boot proper we have to accept the > growth. As it stands here this is just your opinion. Why exactly is this unavoidable? > >>>>>>>> Please keep in mind Simon that we've had zero releases with the >>>>>>>> DM_SERIAL migration warning being posted, v2022.04 will be the first >>>>>>>> one. >>>>>>> Yes, understood :-) For OF_EMBED though...? >>>>>> No deadline and 50 boards. >>>>> Er, there has been a build message about that since the beginning, so >>>>> people ignored it. Do we really need to make the build fail for these >>>>> sorts of things? Perhaps so, but it is a sad situation. >>>> Yes, in hind-sight, "don't do that" wasn't the right path. It would be >>>> a good idea to start a different thread and see what / how the platforms >>>> can be migrated away. >> For tbs2910 this is just a workaround for a strange property of the imx >> build system. OF_SEPARATE created a broken u-boot.imx when I tried last >> time. > OK, that is worth digging in to. Probably. I'm happy to test whatever someone comes up with. > > >>> I think there is a use case for it now - e.g. booting Apple M1 which >>> uses a separate bootloader. IMO a .img or .fit file would be better in >>> some cases but people seem to be allergic to implementing U-Boot >>> things in their code bases. We have the same requirement for the EFI >>> app since UEFI does not implement the U-Boot .img file. >>> >>> So if we are going to support this, perhaps we should create a new >>> option for it. But honestly I am just too weary to consider yet >>> another migration. We need to finish some, e.g. Kconfig. >>> >>>>>>> It was actually quite hard to add a migration message until we added >>>>>>> the CONFIG_SERIAL base thing and that was a pain to do. >>>>>>> >>>>>>> For those of us who take on larger refactors etc., we end up spending >>>>>>> a lot of our time on these few platforms. I'm not picking on tbs2910in >>>>>>> in particular. >>>>>> Well, the flip side of the problem here is that there's a number of >>>>>> platforms with real constraints to them and it keeps being "can we drop >>>>>> this yet?" without CC'ing the board maintainer on the series that once >>>>>> again pushes a given platform to the limit. I would expect no size >>>>>> growth to tbs2910 for the topic of this series since it disables >>>>>> EFI_LOADER entirely, so why is it a problem? >>>>> The partition changes are going to add some size anyway, I expect. I >>>>> have not actually analysed it though. Perhaps we can just disable a >>>>> filesystem? >> OK, you did not even analyse where the problem comes from. But disabling >> user visible functionality on my board is the natural solution to that? >> Strange. > As above, please create some space so people can continue to develop. > There are refactors and features updates which require more code > space. It is somewhat rare, but it happens perhaps every year. It has always been u-boot policy that additional new features should not break existing boards, usually by disabling these new features in defconfig. It is also not new that there are boards with size constraints. If someone causes regressions, then I at least expect that this is thoroughly analysed. >>>> I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes >>>> here-and-there is probably a non issue. But it shouldn't be kilobytes. >>>> It really shouldn't push things over the line. >>>> >>>> And on the tbs2910 side, Soeren, can you look at enabling LTO for this >>>> platform? That would likely buy a good bit of space savings. That >>>> might well be needed to do further DM migrations/etc. >> I'm not familiar with LTO in U-Boot, but will have a look at the weekend. > OK, I suggest getting it several KB under the limit if you can, or > perhaps even drop the limit. I already reduced tbs2910 image size several times by substantial amounts. And this is becoming more and more difficult. The size limit is real. Thanks Tom for the LTO suggestion, this will buy us another round/./ I sent a patch for that. But please, everyone, be careful with additional code size for existing boards. Additional code size is not unavoidable for disabled new features. You just did not try hard enough. Regards, Soeren > > Regards, > Simon
On Mon, Mar 14, 2022 at 09:27:40AM +0100, Soeren Moch wrote: > Hi Simon, > > On 12.03.22 06:02, Simon Glass wrote: > > Hi Soeren, > > > > On Fri, 11 Mar 2022 at 15:43, Soeren Moch<smoch@web.de> wrote: > > > > > > > > On 09.03.22 16:33, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Wed, 9 Mar 2022 at 07:25, Tom Rini<trini@konsulko.com> wrote: > > > > > On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Tue, 8 Mar 2022 at 20:00, Tom Rini<trini@konsulko.com> wrote: > > > > > > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 17:13, Tom Rini<trini@konsulko.com> wrote: > > > > > > > > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > > > > > > > > > Hi Soeren, > > > > > > > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch<smoch@web.de> wrote: > > > > > > > > > > > > > > > > > > > > > > On 08.03.22 17:56, Simon Glass wrote: > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > > > > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt<xypron.glpk@gmx.de> wrote: > > > > > > > > > > > > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > > > > > > > > > > > > > With this patch set[1] applied, UEFI subsystem maintains a list of its > > > > > > > > > > > > > > disk objects dynamically at runtime based on block device's probing. > > > > > > > > > > > > > > (See "issues" below.) > > > > > > > > > > > > > > > > > > > > > > > > > > > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > > > > > > > > > > > > This series together with Simon's series breaks multiple boards due to > > > > > > > > > > > > > size constraints: > > > > > > > > > > > > > > > > > > > > > > > > > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > > > > > > > > > > > > > > > > > > > > > > > > > Please, investigate how to work around this issue. > > > > > > > > > > > > tbs2910 - perhaps we should just drop this board? It doesn't use > > > > > > > > > > > > DM_SERIAL and still uses OF_EMBED > > > > > > > > > > > Are we again at the same point? You are breaking working boards with > > > > > > > > > > > (for these boards) useless additions, and all you come up with is > > > > > > > > > > > "remove this board". Of course without adding the board maintainer. > > > > > > > > > > I'm just expressing reasonable frustration that this board uses > > > > > > > > > > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > > > > > > > > > > should the rest of the U-Boot developers care more about this board > > > > > > > > > > than the maintainer? > > > I cannot see what is is reasonable here. > > > > > > I care about this board, what you can simply see by the fact that I even > > > picked this thread from the mailing list while you "forgot" to cc me. > > This is the patch I sent: > > > > [PATCH 2/5] tbs2910: Disable ext4 write > > > > It shows that you are on cc. What are you referring to? > I'm referring to the exact same email thread that I answered in, which > btw. is still this exact same thread for this answer. Why should I refer > to the totally different email thread you cited here? > > > OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. > > > > > > I'm surprised that you can speak for "the rest of the U-Boot > > > developers", and that you want to push your frustration onto tbs2910 > > > developers and users. Why is it my fault that other people add code size > > > without guarding config options? Why is it my fault that nobody informed > > > me that there is again a size problem? > > Your board is up against the limit and this causes problems. Please > > take a look and see how you can add some margin. Takahiro's series > > does add size and this is unavoidable. See my series of today for some > > fixes for the SPL size, but for U-Boot proper we have to accept the > > growth. > As it stands here this is just your opinion. Why exactly is this > unavoidable? > > > > > > > > > > > Please keep in mind Simon that we've had zero releases with the > > > > > > > > > DM_SERIAL migration warning being posted, v2022.04 will be the first > > > > > > > > > one. > > > > > > > > Yes, understood :-) For OF_EMBED though...? > > > > > > > No deadline and 50 boards. > > > > > > Er, there has been a build message about that since the beginning, so > > > > > > people ignored it. Do we really need to make the build fail for these > > > > > > sorts of things? Perhaps so, but it is a sad situation. > > > > > Yes, in hind-sight, "don't do that" wasn't the right path. It would be > > > > > a good idea to start a different thread and see what / how the platforms > > > > > can be migrated away. > > > For tbs2910 this is just a workaround for a strange property of the imx > > > build system. OF_SEPARATE created a broken u-boot.imx when I tried last > > > time. > > OK, that is worth digging in to. > Probably. I'm happy to test whatever someone comes up with. > > > > > > > > I think there is a use case for it now - e.g. booting Apple M1 which > > > > uses a separate bootloader. IMO a .img or .fit file would be better in > > > > some cases but people seem to be allergic to implementing U-Boot > > > > things in their code bases. We have the same requirement for the EFI > > > > app since UEFI does not implement the U-Boot .img file. > > > > > > > > So if we are going to support this, perhaps we should create a new > > > > option for it. But honestly I am just too weary to consider yet > > > > another migration. We need to finish some, e.g. Kconfig. > > > > > > > > > > > > It was actually quite hard to add a migration message until we added > > > > > > > > the CONFIG_SERIAL base thing and that was a pain to do. > > > > > > > > > > > > > > > > For those of us who take on larger refactors etc., we end up spending > > > > > > > > a lot of our time on these few platforms. I'm not picking on tbs2910in > > > > > > > > in particular. > > > > > > > Well, the flip side of the problem here is that there's a number of > > > > > > > platforms with real constraints to them and it keeps being "can we drop > > > > > > > this yet?" without CC'ing the board maintainer on the series that once > > > > > > > again pushes a given platform to the limit. I would expect no size > > > > > > > growth to tbs2910 for the topic of this series since it disables > > > > > > > EFI_LOADER entirely, so why is it a problem? > > > > > > The partition changes are going to add some size anyway, I expect. I > > > > > > have not actually analysed it though. Perhaps we can just disable a > > > > > > filesystem? > > > OK, you did not even analyse where the problem comes from. But disabling > > > user visible functionality on my board is the natural solution to that? > > > Strange. > > As above, please create some space so people can continue to develop. > > There are refactors and features updates which require more code > > space. It is somewhat rare, but it happens perhaps every year. > It has always been u-boot policy that additional new features should not > break existing boards, usually by disabling these new features in defconfig. > It is also not new that there are boards with size constraints. > > If someone causes regressions, then I at least expect that this is > thoroughly analysed. > > > > > I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes > > > > > here-and-there is probably a non issue. But it shouldn't be kilobytes. > > > > > It really shouldn't push things over the line. > > > > > > > > > > And on the tbs2910 side, Soeren, can you look at enabling LTO for this > > > > > platform? That would likely buy a good bit of space savings. That > > > > > might well be needed to do further DM migrations/etc. > > > I'm not familiar with LTO in U-Boot, but will have a look at the weekend. > > OK, I suggest getting it several KB under the limit if you can, or > > perhaps even drop the limit. > I already reduced tbs2910 image size several times by substantial > amounts. And this is becoming more and more difficult. The size limit is > real. > > Thanks Tom for the LTO suggestion, this will buy us another round/./ I > sent a patch for that. > > But please, everyone, be careful with additional code size for existing > boards. Additional code size is not unavoidable for disabled new > features. You just did not try hard enough. I just want to emphasize the last point here. We need to make sure we're making platforms better, not just bigger. And to preempt "EFI keeps growing", yes, in reasonably small amounts, implementing a spec (or fixing bugs against a spec) that the semi vendor is telling everyone to make sure their hardware is compliant with. And I check all of the growth for everyone, all of the time, for everything.
Hi Soeren, [I think you sent your email with html or something so it is a big mangled. I'll just add one comment] On Mon, 14 Mar 2022 at 02:27, Soeren Moch <smoch@web.de> wrote: > > Hi Simon, > > On 12.03.22 06:02, Simon Glass wrote: > > Hi Soeren, > > On Fri, 11 Mar 2022 at 15:43, Soeren Moch <smoch@web.de> wrote: > > > > On 09.03.22 16:33, Simon Glass wrote: > > Hi Tom, > > On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > > Hi Tom, > > On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > > Hi Tom, > > On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > > Hi Soeren, > > On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > > On 08.03.22 17:56, Simon Glass wrote: > > Hi, > > On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 3/8/22 12:36, AKASHI Takahiro wrote: > > With this patch set[1] applied, UEFI subsystem maintains a list of its > disk objects dynamically at runtime based on block device's probing. > (See "issues" below.) > > [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > > This series together with Simon's series breaks multiple boards due to > size constraints: > > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > > Please, investigate how to work around this issue. > > tbs2910 - perhaps we should just drop this board? It doesn't use > DM_SERIAL and still uses OF_EMBED > > Are we again at the same point? You are breaking working boards with > (for these boards) useless additions, and all you come up with is > "remove this board". Of course without adding the board maintainer. > > I'm just expressing reasonable frustration that this board uses > OF_EMBED and does not use DM_SERIAL, after all of this time. Why > should the rest of the U-Boot developers care more about this board > than the maintainer? > > I cannot see what is is reasonable here. > > I care about this board, what you can simply see by the fact that I even > picked this thread from the mailing list while you "forgot" to cc me. > > This is the patch I sent: > > [PATCH 2/5] tbs2910: Disable ext4 write > > It shows that you are on cc. What are you referring to? > > I'm referring to the exact same email thread that I answered in, which btw. is still this exact same thread for this answer. Why should I refer to the totally different email thread you cited here? > > OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. > > I'm surprised that you can speak for "the rest of the U-Boot > developers", and that you want to push your frustration onto tbs2910 > developers and users. Why is it my fault that other people add code size > without guarding config options? Why is it my fault that nobody informed > me that there is again a size problem? > > Your board is up against the limit and this causes problems. Please > take a look and see how you can add some margin. Takahiro's series > does add size and this is unavoidable. See my series of today for some > fixes for the SPL size, but for U-Boot proper we have to accept the > growth. > > As it stands here this is just your opinion. Why exactly is this unavoidable? > > Please keep in mind Simon that we've had zero releases with the > DM_SERIAL migration warning being posted, v2022.04 will be the first > one. > > Yes, understood :-) For OF_EMBED though...? > > No deadline and 50 boards. > > Er, there has been a build message about that since the beginning, so > people ignored it. Do we really need to make the build fail for these > sorts of things? Perhaps so, but it is a sad situation. > > Yes, in hind-sight, "don't do that" wasn't the right path. It would be > a good idea to start a different thread and see what / how the platforms > can be migrated away. > > For tbs2910 this is just a workaround for a strange property of the imx > build system. OF_SEPARATE created a broken u-boot.imx when I tried last > time. > > OK, that is worth digging in to. > > Probably. I'm happy to test whatever someone comes up with. > > > I think there is a use case for it now - e.g. booting Apple M1 which > uses a separate bootloader. IMO a .img or .fit file would be better in > some cases but people seem to be allergic to implementing U-Boot > things in their code bases. We have the same requirement for the EFI > app since UEFI does not implement the U-Boot .img file. > > So if we are going to support this, perhaps we should create a new > option for it. But honestly I am just too weary to consider yet > another migration. We need to finish some, e.g. Kconfig. > > It was actually quite hard to add a migration message until we added > the CONFIG_SERIAL base thing and that was a pain to do. > > For those of us who take on larger refactors etc., we end up spending > a lot of our time on these few platforms. I'm not picking on tbs2910in > in particular. > > Well, the flip side of the problem here is that there's a number of > platforms with real constraints to them and it keeps being "can we drop > this yet?" without CC'ing the board maintainer on the series that once > again pushes a given platform to the limit. I would expect no size > growth to tbs2910 for the topic of this series since it disables > EFI_LOADER entirely, so why is it a problem? > > The partition changes are going to add some size anyway, I expect. I > have not actually analysed it though. Perhaps we can just disable a > filesystem? > > OK, you did not even analyse where the problem comes from. But disabling > user visible functionality on my board is the natural solution to that? > Strange. > > As above, please create some space so people can continue to develop. > There are refactors and features updates which require more code > space. It is somewhat rare, but it happens perhaps every year. > > It has always been u-boot policy that additional new features should not break existing boards, usually by disabling these new features in defconfig. > It is also not new that there are boards with size constraints. > > If someone causes regressions, then I at least expect that this is thoroughly analysed. > > I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes > here-and-there is probably a non issue. But it shouldn't be kilobytes. > It really shouldn't push things over the line. > > And on the tbs2910 side, Soeren, can you look at enabling LTO for this > platform? That would likely buy a good bit of space savings. That > might well be needed to do further DM migrations/etc. > > I'm not familiar with LTO in U-Boot, but will have a look at the weekend. > > OK, I suggest getting it several KB under the limit if you can, or > perhaps even drop the limit. > > I already reduced tbs2910 image size several times by substantial amounts. And this is becoming more and more difficult. The size limit is real. > > Thanks Tom for the LTO suggestion, this will buy us another round. I sent a patch for that. > > But please, everyone, be careful with additional code size for existing boards. Additional code size is not unavoidable for disabled new features. You just did not try hard enough. Please take a look at Tahahiro's series and tell me how we can avoid adding a driver for partitions, when the whole point of the series is to add a driver for partitions :-) Regards, Simon
Hi Simon, On 14.03.22 18:08, Simon Glass wrote: > Hi Soeren, > > [I think you sent your email with html or something so it is a big > mangled. I'll just add one comment] > > On Mon, 14 Mar 2022 at 02:27, Soeren Moch <smoch@web.de> wrote: >> Hi Simon, >> >> On 12.03.22 06:02, Simon Glass wrote: >> >> Hi Soeren, >> >> On Fri, 11 Mar 2022 at 15:43, Soeren Moch <smoch@web.de> wrote: >> >> >> >> On 09.03.22 16:33, Simon Glass wrote: >> >> Hi Tom, >> >> On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: >> >> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: >> >> Hi Tom, >> >> On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: >> >> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: >> >> Hi Tom, >> >> On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: >> >> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: >> >> Hi Soeren, >> >> On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: >> >> On 08.03.22 17:56, Simon Glass wrote: >> >> Hi, >> >> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: >> >> On 3/8/22 12:36, AKASHI Takahiro wrote: >> >> With this patch set[1] applied, UEFI subsystem maintains a list of its >> disk objects dynamically at runtime based on block device's probing. >> (See "issues" below.) >> >> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk >> >> This series together with Simon's series breaks multiple boards due to >> size constraints: >> >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 >> >> Please, investigate how to work around this issue. >> >> tbs2910 - perhaps we should just drop this board? It doesn't use >> DM_SERIAL and still uses OF_EMBED >> >> Are we again at the same point? You are breaking working boards with >> (for these boards) useless additions, and all you come up with is >> "remove this board". Of course without adding the board maintainer. >> >> I'm just expressing reasonable frustration that this board uses >> OF_EMBED and does not use DM_SERIAL, after all of this time. Why >> should the rest of the U-Boot developers care more about this board >> than the maintainer? >> >> I cannot see what is is reasonable here. >> >> I care about this board, what you can simply see by the fact that I even >> picked this thread from the mailing list while you "forgot" to cc me. >> >> This is the patch I sent: >> >> [PATCH 2/5] tbs2910: Disable ext4 write >> >> It shows that you are on cc. What are you referring to? >> >> I'm referring to the exact same email thread that I answered in, which btw. is still this exact same thread for this answer. Why should I refer to the totally different email thread you cited here? >> >> OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. >> >> I'm surprised that you can speak for "the rest of the U-Boot >> developers", and that you want to push your frustration onto tbs2910 >> developers and users. Why is it my fault that other people add code size >> without guarding config options? Why is it my fault that nobody informed >> me that there is again a size problem? >> >> Your board is up against the limit and this causes problems. Please >> take a look and see how you can add some margin. Takahiro's series >> does add size and this is unavoidable. See my series of today for some >> fixes for the SPL size, but for U-Boot proper we have to accept the >> growth. >> >> As it stands here this is just your opinion. Why exactly is this unavoidable? >> >> Please keep in mind Simon that we've had zero releases with the >> DM_SERIAL migration warning being posted, v2022.04 will be the first >> one. >> >> Yes, understood :-) For OF_EMBED though...? >> >> No deadline and 50 boards. >> >> Er, there has been a build message about that since the beginning, so >> people ignored it. Do we really need to make the build fail for these >> sorts of things? Perhaps so, but it is a sad situation. >> >> Yes, in hind-sight, "don't do that" wasn't the right path. It would be >> a good idea to start a different thread and see what / how the platforms >> can be migrated away. >> >> For tbs2910 this is just a workaround for a strange property of the imx >> build system. OF_SEPARATE created a broken u-boot.imx when I tried last >> time. >> >> OK, that is worth digging in to. >> >> Probably. I'm happy to test whatever someone comes up with. >> >> >> I think there is a use case for it now - e.g. booting Apple M1 which >> uses a separate bootloader. IMO a .img or .fit file would be better in >> some cases but people seem to be allergic to implementing U-Boot >> things in their code bases. We have the same requirement for the EFI >> app since UEFI does not implement the U-Boot .img file. >> >> So if we are going to support this, perhaps we should create a new >> option for it. But honestly I am just too weary to consider yet >> another migration. We need to finish some, e.g. Kconfig. >> >> It was actually quite hard to add a migration message until we added >> the CONFIG_SERIAL base thing and that was a pain to do. >> >> For those of us who take on larger refactors etc., we end up spending >> a lot of our time on these few platforms. I'm not picking on tbs2910in >> in particular. >> >> Well, the flip side of the problem here is that there's a number of >> platforms with real constraints to them and it keeps being "can we drop >> this yet?" without CC'ing the board maintainer on the series that once >> again pushes a given platform to the limit. I would expect no size >> growth to tbs2910 for the topic of this series since it disables >> EFI_LOADER entirely, so why is it a problem? >> >> The partition changes are going to add some size anyway, I expect. I >> have not actually analysed it though. Perhaps we can just disable a >> filesystem? >> >> OK, you did not even analyse where the problem comes from. But disabling >> user visible functionality on my board is the natural solution to that? >> Strange. >> >> As above, please create some space so people can continue to develop. >> There are refactors and features updates which require more code >> space. It is somewhat rare, but it happens perhaps every year. >> >> It has always been u-boot policy that additional new features should not break existing boards, usually by disabling these new features in defconfig. >> It is also not new that there are boards with size constraints. >> >> If someone causes regressions, then I at least expect that this is thoroughly analysed. >> >> I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes >> here-and-there is probably a non issue. But it shouldn't be kilobytes. >> It really shouldn't push things over the line. >> >> And on the tbs2910 side, Soeren, can you look at enabling LTO for this >> platform? That would likely buy a good bit of space savings. That >> might well be needed to do further DM migrations/etc. >> >> I'm not familiar with LTO in U-Boot, but will have a look at the weekend. >> >> OK, I suggest getting it several KB under the limit if you can, or >> perhaps even drop the limit. >> >> I already reduced tbs2910 image size several times by substantial amounts. And this is becoming more and more difficult. The size limit is real. >> >> Thanks Tom for the LTO suggestion, this will buy us another round. I sent a patch for that. >> >> But please, everyone, be careful with additional code size for existing boards. Additional code size is not unavoidable for disabled new features. You just did not try hard enough. > Please take a look at Tahahiro's series and tell me how we can avoid > adding a driver for partitions, when the whole point of the series is > to add a driver for partitions :-) If this is just a new driver that I don't need (as before), why is it enabled for my board and causing regressions? Regards, Soeren > > Regards, > Simon
Hi Soeren, On Mon, 14 Mar 2022 at 13:17, Soeren Moch <smoch@web.de> wrote: > > Hi Simon, > > On 14.03.22 18:08, Simon Glass wrote: > > Hi Soeren, > > > > [I think you sent your email with html or something so it is a big > > mangled. I'll just add one comment] > > > > On Mon, 14 Mar 2022 at 02:27, Soeren Moch <smoch@web.de> wrote: > >> Hi Simon, > >> > >> On 12.03.22 06:02, Simon Glass wrote: > >> > >> Hi Soeren, > >> > >> On Fri, 11 Mar 2022 at 15:43, Soeren Moch <smoch@web.de> wrote: > >> > >> > >> > >> On 09.03.22 16:33, Simon Glass wrote: > >> > >> Hi Tom, > >> > >> On Wed, 9 Mar 2022 at 07:25, Tom Rini <trini@konsulko.com> wrote: > >> > >> On Tue, Mar 08, 2022 at 08:10:38PM -0700, Simon Glass wrote: > >> > >> Hi Tom, > >> > >> On Tue, 8 Mar 2022 at 20:00, Tom Rini <trini@konsulko.com> wrote: > >> > >> On Tue, Mar 08, 2022 at 07:32:59PM -0700, Simon Glass wrote: > >> > >> Hi Tom, > >> > >> On Tue, 8 Mar 2022 at 17:13, Tom Rini <trini@konsulko.com> wrote: > >> > >> On Tue, Mar 08, 2022 at 02:20:15PM -0700, Simon Glass wrote: > >> > >> Hi Soeren, > >> > >> On Tue, 8 Mar 2022 at 12:15, Soeren Moch <smoch@web.de> wrote: > >> > >> On 08.03.22 17:56, Simon Glass wrote: > >> > >> Hi, > >> > >> On Tue, 8 Mar 2022 at 09:49, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > >> > >> On 3/8/22 12:36, AKASHI Takahiro wrote: > >> > >> With this patch set[1] applied, UEFI subsystem maintains a list of its > >> disk objects dynamically at runtime based on block device's probing. > >> (See "issues" below.) > >> > >> [1]https://github.com/t-akashi/u-boot/tree/efi/dm_disk > >> > >> This series together with Simon's series breaks multiple boards due to > >> size constraints: > >> > >> https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/11197 > >> > >> Please, investigate how to work around this issue. > >> > >> tbs2910 - perhaps we should just drop this board? It doesn't use > >> DM_SERIAL and still uses OF_EMBED > >> > >> Are we again at the same point? You are breaking working boards with > >> (for these boards) useless additions, and all you come up with is > >> "remove this board". Of course without adding the board maintainer. > >> > >> I'm just expressing reasonable frustration that this board uses > >> OF_EMBED and does not use DM_SERIAL, after all of this time. Why > >> should the rest of the U-Boot developers care more about this board > >> than the maintainer? > >> > >> I cannot see what is is reasonable here. > >> > >> I care about this board, what you can simply see by the fact that I even > >> picked this thread from the mailing list while you "forgot" to cc me. > >> > >> This is the patch I sent: > >> > >> [PATCH 2/5] tbs2910: Disable ext4 write > >> > >> It shows that you are on cc. What are you referring to? > >> > >> I'm referring to the exact same email thread that I answered in, which btw. is still this exact same thread for this answer. Why should I refer to the totally different email thread you cited here? > >> > >> OF_EMBED and DM_SERIAL are not at all related to EFI or size constraints. > >> > >> I'm surprised that you can speak for "the rest of the U-Boot > >> developers", and that you want to push your frustration onto tbs2910 > >> developers and users. Why is it my fault that other people add code size > >> without guarding config options? Why is it my fault that nobody informed > >> me that there is again a size problem? > >> > >> Your board is up against the limit and this causes problems. Please > >> take a look and see how you can add some margin. Takahiro's series > >> does add size and this is unavoidable. See my series of today for some > >> fixes for the SPL size, but for U-Boot proper we have to accept the > >> growth. > >> > >> As it stands here this is just your opinion. Why exactly is this unavoidable? > >> > >> Please keep in mind Simon that we've had zero releases with the > >> DM_SERIAL migration warning being posted, v2022.04 will be the first > >> one. > >> > >> Yes, understood :-) For OF_EMBED though...? > >> > >> No deadline and 50 boards. > >> > >> Er, there has been a build message about that since the beginning, so > >> people ignored it. Do we really need to make the build fail for these > >> sorts of things? Perhaps so, but it is a sad situation. > >> > >> Yes, in hind-sight, "don't do that" wasn't the right path. It would be > >> a good idea to start a different thread and see what / how the platforms > >> can be migrated away. > >> > >> For tbs2910 this is just a workaround for a strange property of the imx > >> build system. OF_SEPARATE created a broken u-boot.imx when I tried last > >> time. > >> > >> OK, that is worth digging in to. > >> > >> Probably. I'm happy to test whatever someone comes up with. > >> > >> > >> I think there is a use case for it now - e.g. booting Apple M1 which > >> uses a separate bootloader. IMO a .img or .fit file would be better in > >> some cases but people seem to be allergic to implementing U-Boot > >> things in their code bases. We have the same requirement for the EFI > >> app since UEFI does not implement the U-Boot .img file. > >> > >> So if we are going to support this, perhaps we should create a new > >> option for it. But honestly I am just too weary to consider yet > >> another migration. We need to finish some, e.g. Kconfig. > >> > >> It was actually quite hard to add a migration message until we added > >> the CONFIG_SERIAL base thing and that was a pain to do. > >> > >> For those of us who take on larger refactors etc., we end up spending > >> a lot of our time on these few platforms. I'm not picking on tbs2910in > >> in particular. > >> > >> Well, the flip side of the problem here is that there's a number of > >> platforms with real constraints to them and it keeps being "can we drop > >> this yet?" without CC'ing the board maintainer on the series that once > >> again pushes a given platform to the limit. I would expect no size > >> growth to tbs2910 for the topic of this series since it disables > >> EFI_LOADER entirely, so why is it a problem? > >> > >> The partition changes are going to add some size anyway, I expect. I > >> have not actually analysed it though. Perhaps we can just disable a > >> filesystem? > >> > >> OK, you did not even analyse where the problem comes from. But disabling > >> user visible functionality on my board is the natural solution to that? > >> Strange. > >> > >> As above, please create some space so people can continue to develop. > >> There are refactors and features updates which require more code > >> space. It is somewhat rare, but it happens perhaps every year. > >> > >> It has always been u-boot policy that additional new features should not break existing boards, usually by disabling these new features in defconfig. > >> It is also not new that there are boards with size constraints. > >> > >> If someone causes regressions, then I at least expect that this is thoroughly analysed. > >> > >> I was a bit too absolutist there, sorry. Yes, a few hundreds of bytes > >> here-and-there is probably a non issue. But it shouldn't be kilobytes. > >> It really shouldn't push things over the line. > >> > >> And on the tbs2910 side, Soeren, can you look at enabling LTO for this > >> platform? That would likely buy a good bit of space savings. That > >> might well be needed to do further DM migrations/etc. > >> > >> I'm not familiar with LTO in U-Boot, but will have a look at the weekend. > >> > >> OK, I suggest getting it several KB under the limit if you can, or > >> perhaps even drop the limit. > >> > >> I already reduced tbs2910 image size several times by substantial amounts. And this is becoming more and more difficult. The size limit is real. > >> > >> Thanks Tom for the LTO suggestion, this will buy us another round. I sent a patch for that. > >> > >> But please, everyone, be careful with additional code size for existing boards. Additional code size is not unavoidable for disabled new features. You just did not try hard enough. > > Please take a look at Tahahiro's series and tell me how we can avoid > > adding a driver for partitions, when the whole point of the series is > > to add a driver for partitions :-) > If this is just a new driver that I don't need (as before), why is it > enabled for my board and causing regressions? If you disable partition support, you don't need it. Please just take a look at the series as I think it will make sense. Regards, SImon
With this patch set[1] applied, UEFI subsystem maintains a list of its disk objects dynamically at runtime based on block device's probing. (See "issues" below.) [1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk For instance, => dm tree Class Index Probed Driver Name ----------------------------------------------------------- root 0 [ + ] root_driver root_driver ... pci 0 [ + ] pci_generic_ecam |-- pcie@10000000 ... ahci 0 [ ] ahci_pci | |-- ahci_pci scsi 0 [ ] ahci_scsi | | `-- ahci_scsi usb 0 [ ] xhci_pci | `-- xhci_pci ... => efi devices Missing RNG device for EFI_RNG_PROTOCOL No EFI system partition Unable to find TPMv2 device Device Device Path ================ ==================== 000000013eee88d0 /VenHw(..) 000000013ffeb798 /VenHw(..)/Uart(0,0,D,D) 000000013eeeb810 /VenHw(..)/MAC(525252525252,1) => scsi rescan ... => efi devices Device Device Path ================ ==================== 000000013eee88d0 /VenHw(..) 000000013ffeb798 /VenHw(..)/Uart(0,0,D,D) 000000013eeeb810 /VenHw(..)/MAC(525252525252,1) 000000013eefb730 /VenHw(..)/Scsi(0,0) 000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a) 000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800) => usb start ... => efi devices Device Device Path ================ ==================== 000000013eee88d0 /VenHw(..) 000000013ffeb798 /VenHw(..)/Uart(0,0,D,D) 000000013eeeb810 /VenHw(..)/MAC(525252525252,1) 000000013eefb730 /VenHw(..)/Scsi(0,0) 000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a) 000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800) 000000013ef01330 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0) 000000013ef014b0 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a) 000000013ef018f0 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800) => dm tree ... pci 0 [ + ] pci_generic_ecam |-- pcie@10000000 ... ahci 0 [ + ] ahci_pci | |-- ahci_pci scsi 0 [ + ] ahci_scsi | | `-- ahci_scsi blk 2 [ + ] scsi_blk | | `-- ahci_scsi.id0lun0 partition 0 [ + ] blk_partition | | |-- ahci_scsi.id0lun0:1 partition 1 [ + ] blk_partition | | `-- ahci_scsi.id0lun0:2 usb 0 [ + ] xhci_pci | `-- xhci_pci usb_hub 0 [ + ] usb_hub | `-- usb_hub usb_mass_s 0 [ + ] usb_mass_storage | |-- usb_mass_storage blk 3 [ + ] usb_storage_blk | | `-- usb_mass_storage.lun0 partition 2 [ + ] blk_partition | | |-- usb_mass_storage.lun0:1 partition 3 [ + ] blk_partition | | `-- usb_mass_storage.lun0:2 ... => usb stop stopping USB.. => efi devices Device Device Path ================ ==================== 000000013eee88d0 /VenHw(..) 000000013ffeb798 /VenHw(..)/Uart(0,0,D,D) 000000013eeeb810 /VenHw(..)/MAC(525252525252,1) 000000013eefb730 /VenHw(..)/Scsi(0,0) 000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a) 000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800) Issues: ======= * The image size of U-Boot may increase. CI build test complains, for instance, rcar3_salvator-x: "u-boot.img exceeds file size limit: ... excess: 0x79c bytes" phycore-rk3288: "SPL image is too large (size 0x8800 than 0x8000)" See [2]. * For removal case, we may need more consideration since removing handles unconditionally may end up breaking integrity of handles (as some may still be held and referenced to by a UEFI app). [2] https://dev.azure.com/u-boot/u-boot/_build/results?buildId=3770&view=results Prerequisite: ============= * Simon's event[3] [3] https://lists.denx.de/pipermail/u-boot/2022-March/476844.html Patchs: ======= For easy understandings, patches may be categorized into separate groups of changes. Patch#1-#7: DM: add device_probe() for later use of events Patch#8-#10: DM: add a new feature (DM tag) Patch#11-#15: UEFI: dynamically create/remove efi_disk's for a raw disk and its partitions Patch#16-#17: UEFI: use udevice read/write interfaces Patch#18-#19: UEFI: fix-up efi_driver, aligning with changes in DM integration Change history: =============== v3 (Mar 8, 2022) * rebased on 2022.04-rc3 * rebased on v2 of Simon's event patch (hence removed his patch from this patch set) * fix a spl-build error in mmc_blk_probe() (patch#3) * fix a build warning in fsl_ata_probe()/sil_pci_probe() (patch#5) * remove a problematic log message in efi_disk_probe() (patch#14) v2 (Feb 10, 2022) * add/revise an error message if device_probe() fails (patch#3,#5) * fix a build error in sandbox_spl_defconfig (patch#8) * fix warnings in 'make htmldocs' (patch#8,#9,#18) * new commit: split efi_init_obj_list() (patch#14) * new commit: simplify efi_capsule pytest (patch#21) v1 (Feb 2, 2022) * rebased on 2022.04-rc1 * drop patches that have already been merged * modify a tag-range check with "tag >= DM_TAG_COUNT" (patch#9) * move dmtag_list to GD (global data) (patch#9) * add function descriptions and a document about DM tag feature (patch#9,10) * add tests for DM tag support (patch#11) * change 'depends on EVENT' to 'select EVENT' for EFI_LOADER (patch#14) * migrate IF_TYPE_EFI to IF_TYPE_EFI_LOADER (patch#18) RFCv2 (Dec 10, 2021) * rebased on 2022-rc3 * re-order and merge some related commits into ones * call device_probe() in MMC (not bind, but) probe hook (patch#5) * fix a wrong name of variable (patch#7) * add patch#9 * invoke device_probe() for virtio devices (patch#10) * add DM event notification (from Simon) (patch#11) * add DM tag support (patch#12) * move UCLASS_PARTITION driver under disk/ (patch#13) * create partition's dp using its parent's. This change is necessary in particular for 'efi_blk' efi_disk (patch#13) * modify the code so that we will use new features like tags and event notification (patch#13,15,16,20) * rename new functions from blk_read/write() to dev_read/write() (patch#17,18) * isolate changes in efi_driver from the rest (in efi_loader) (patch#19) * drop the previous patch#22 ("efi_selftest: block device: adjust dp for a test") due to the fix in patch#13 RFC (Nov 16, 2021) * initial RFC AKASHI Takahiro (19): scsi: call device_probe() after scanning usb: storage: call device_probe() after scanning mmc: call device_probe() after scanning nvme: call device_probe() after scanning sata: call device_probe() after scanning block: ide: call device_probe() after scanning virtio: call device_probe() in scanning dm: add tag support dm: tag: add some document test: dm: add tests for tag support dm: disk: add UCLASS_PARTITION dm: blk: add a device-probe hook for scanning disk partitions efi_loader: split efi_init_obj_list() into two stages efi_loader: disk: a helper function to create efi_disk objects from udevice efi_loader: disk: a helper function to delete efi_disk objects dm: disk: add read/write interfaces with udevice efi_loader: disk: use udevice instead of blk_desc efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER) devices efi_driver: align with efi_disk-dm integration cmd/virtio.c | 21 +- common/board_r.c | 2 +- common/main.c | 7 +- common/usb_storage.c | 4 + disk/Makefile | 3 + disk/disk-uclass.c | 247 +++++++++++++++++++++ doc/develop/driver-model/design.rst | 20 ++ drivers/ata/dwc_ahsata.c | 5 + drivers/ata/fsl_sata.c | 11 + drivers/ata/sata_mv.c | 5 + drivers/ata/sata_sil.c | 12 + drivers/block/blk-uclass.c | 4 + drivers/block/ide.c | 4 + drivers/core/Makefile | 2 +- drivers/core/root.c | 2 + drivers/core/tag.c | 139 ++++++++++++ drivers/mmc/mmc-uclass.c | 12 + drivers/nvme/nvme.c | 4 + drivers/scsi/scsi.c | 5 + include/asm-generic/global_data.h | 4 + include/dm/tag.h | 110 ++++++++++ include/dm/uclass-id.h | 1 + include/efi_loader.h | 6 +- include/part.h | 18 ++ lib/efi_driver/efi_block_device.c | 34 +-- lib/efi_loader/Kconfig | 3 + lib/efi_loader/efi_disk.c | 328 ++++++++++++++++++++-------- lib/efi_loader/efi_setup.c | 62 +++++- test/dm/Makefile | 1 + test/dm/tag.c | 84 +++++++ 30 files changed, 1029 insertions(+), 131 deletions(-) create mode 100644 disk/disk-uclass.c create mode 100644 drivers/core/tag.c create mode 100644 include/dm/tag.h create mode 100644 test/dm/tag.c