[U-Boot] [PATCH] configs: Remove CONFIG_SPL_FS_EXT4 for all MX6 platforms
Ezequiel Garcia
ezequiel at vanguardiasur.com.ar
Sun May 26 16:18:15 UTC 2019
On Sun, 26 May 2019 at 12:05, Lukasz Majewski <lukma at denx.de> wrote:
>
> Hi Tom,
>
> > On Sun, May 26, 2019 at 01:46:53PM +0200, Lukasz Majewski wrote:
> > > Hi Tom,
> > >
> > > > On Sun, May 26, 2019 at 10:22:00AM +0200, Lukasz Majewski wrote:
> > > > > Dear Marek, Tom,
> > > > >
> > > > > > On 5/26/19 1:23 AM, Tom Rini wrote:
> > > > > > > On Sun, May 26, 2019 at 01:20:34AM +0200, Marek Vasut
> > > > > > > wrote:
> > > > > > >> On 5/26/19 1:08 AM, Tom Rini wrote:
> > > > > > >>> On Sun, May 26, 2019 at 12:57:08AM +0200, Marek Vasut
> > > > > > >>> wrote:
> > > > > > >>>> On 5/26/19 12:45 AM, Ezequiel Garcia wrote:
> > > > > > >>>>> On Sun, 2019-05-26 at 00:24 +0200, Marek Vasut
> > > > > > >>>>> wrote:
> > > > > > >>>>>> On 5/25/19 11:47 PM, Ezequiel Garcia wrote:
> > > > > > >>>>>>> On Sat, 2019-05-25 at 22:15 +0200, Marek Vasut
> > > > > > >>>>>>> wrote:
> > > > > > >>>>>>>> On 5/25/19 6:49 PM, Ezequiel Garcia wrote:
> > > > > > >>>>>>>>> i.MX6 platforms boot U-Boot second-stage from
> > > > > > >>>>>>>>> unformatted space, and should not need Ext
> > > > > > >>>>>>>>> filesystem support on SPL.
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> The commit was generated with:
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> git grep -l MX6 -- configs/ | xargs grep -l
> > > > > > >>>>>>>>> SPL_FS_EXT4 | xargs sed -i -e
> > > > > > >>>>>>>>> '/CONFIG_SPL_FS_EXT4=y/d'
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> This change has a dramatic impact on SPL size:
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> ./scripts/bloat-o-meter old new
> > > > > > >>>>>>>>> add/remove: 0/59 grow/shrink: 0/3 up/down: 0/-8674
> > > > > > >>>>>>>>> (-8674) [..]
> > > > > > >>>>>>>>> Total: Before=38320, After=29646, chg -22.64%
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> Cc: Otavio Salvador <otavio at ossystems.com.br>
> > > > > > >>>>>>>>> Cc: Fabio Estevam <fabio.estevam at nxp.com>
> > > > > > >>>>>>>>> Cc: Peng Fan <peng.fan at nxp.com>
> > > > > > >>>>>>>>> Cc: Marek Vasut <marex at denx.de>
> > > > > > >>>>>>>>> Cc: Stefano Babic <sbabic at denx.de>
> > > > > > >>>>>>>>> Cc: Stefan Roese <sr at denx.de>
> > > > > > >>>>>>>>> Cc: "Eric Bénard" <eric at eukrea.com>
> > > > > > >>>>>>>>> Cc: Breno Lima <breno.lima at nxp.com>
> > > > > > >>>>>>>>> Cc: Francesco Montefoschi
> > > > > > >>>>>>>>> <francesco.montefoschi at udoo.org> Signed-off-by:
> > > > > > >>>>>>>>> Ezequiel Garcia <ezequiel at collabora.com> ---
> > > > > > >>>>>>>>> Tested on Wandboard only. Maintainers, please ACK or
> > > > > > >>>>>>>>> NAK!
> > > > > > >>>>>>>>>
> > > > > > >>>>>>>>> configs/cgtqmx6eval_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6cuboxi_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6sabreauto_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6sabresd_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6slevk_spl_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6sxsabresd_spl_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6ul_14x14_evk_defconfig | 1 -
> > > > > > >>>>>>>>> configs/mx6ul_9x9_evk_defconfig | 1 -
> > > > > > >>>>>>>>> configs/novena_defconfig | 1 -
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> NAK, I boot my Novena from ext4 and this just broke
> > > > > > >>>>>>>> it.
> > > > > > >>>>>>>>
> > > > > > >>>>>>>> And also, NAK, this removes functionality from SPL
> > > > > > >>>>>>>> which worked fine before.
> > > > > > >>>>>>>
> > > > > > >>>>>>> I'll drop from Novena, but I think the patch still
> > > > > > >>>>>>> makes some sense, why do you want Ext4 on SPL?
> > > > > > >>>>>>
> > > > > > >>>>>> What other filesystem is available in SPL and doesn't
> > > > > > >>>>>> have patent problems ?
> > > > > > >>>>>
> > > > > > >>>>> Sorry for not being clear. I am asking why turn on a
> > > > > > >>>>> feature that is so heavy, on a system that won't need
> > > > > > >>>>> it (such as Sabre* boards, Wandboard and similar)?
> > > > > > >>>>
> > > > > > >>>> Two reasons:
> > > > > > >>>> 1) It was enabled, disabling it means removing
> > > > > > >>>> functionality for no good reason (oops, bloat, is not a
> > > > > > >>>> good reason), and that is not desired. 2) Booting from
> > > > > > >>>> block device implies booting from a filesystem,
> > > > > > >>>> otherwise you might overwrite various things on the
> > > > > > >>>> block device when updating the file (u-boot image).
> > > > > > >>>
> > > > > > >>> So, are you using SPL to load something from ext4 or
> > > > > > >>> not?
> > > > > > >>
> > > > > > >> Yes, I think that's what I said.
> > > > > > >>
> > > > > > >>> There are
> > > > > > >>> setups where people have configured the system such that
> > > > > > >>> SPL loads something from ext4 and that's why we have it
> > > > > > >>> available. Is anyone doing that on Novena? Or any iMX
> > > > > > >>> system?
> > > > > > >>
> > > > > > >> Quoting my first response in this thread:
> > > > > > >> "
> > > > > > >> NAK, I boot my Novena from ext4 and this just broke it.
> > > > > > >> "
> > > > > > >
> > > > > > > Actually, I wasn't sure from your first response if you're
> > > > > > > using SPL to load u-boot from EXT4 or not. So, Novena is a
> > > > > > > no and we should wait for more board maintainers to speak
> > > > > > > up to see if they use it or not, thanks!
> > > > > >
> > > > > > Novena is certainly a no. Since I use a couple of wandboards,
> > > > > > those are no as well.
> > > > > >
> > > > > > But I do not want to see useful functionality removed from SPL
> > > > > > only to make space for useless DM/DT bloat. Temporarily
> > > > > > band-aiding this real problem by removing functionality is a
> > > > > > no-go, no matter how you slice it. The real fix is to fix the
> > > > > > DM/DT and figure out a way to reduce it's size and _retain_
> > > > > > _all_ the functionality.
> > > > >
> > > > > I fully agree with Marek here - the DM/DT SPL support (in the
> > > > > form as it is now) is a bloat. The SPL size increases
> > > > > considerably.
> > > > >
> > > > > The _real_ issue is the lack of OF_PLATDATA support for DM/DTS
> > > > > drivers, which are used when we convert u-boot to DM/DTS.
> > > > >
> > > > > For boards, which have enough internal on-chip RAM (OCRAM in
> > > > > case of imx6q) it is doable to convert them to DM/DTS in SPL.
> > > > >
> > > > > However, nobody wants to say it loud, but the footprint
> > > > > considerably increases in SPL - for example:
> > > > >
> > > > > SPL (and I only use eMMC there - no SPI, no I2C):
> > > > > ------------------------------------------------
> > > > > Before conversion: 35 KiB
> > > > > DM conversion: 47 KiB
> > > > > (around ~25% increase)
> > > > >
> > > > >
> > > > > For u-boot proper:
> > > > > ----------------------------
> > > > > Footprint changes (u-boot.imx):
> > > > >
> > > > > Before conversion: 345 KiB
> > > > > DM conversion: 415 KiB
> > > > > (around + 17% increase)
> > > > >
> > > > >
> > > > > This has several implications:
> > > > >
> > > > > 1. We replace small, robust SPL (I speak only for i.MX) with
> > > > > something which is XX% larger and hence boot time increases. In
> > > > > short - customers are/will not be happy. We do introduce
> > > > > regression here.
> > > > > fdtdec_get_chosen_node
> > > > > 2. To make it usable (with the size comparable to what we do
> > > > > have now) we need to add OF_PLATDATA support to eMMC, SPI, I2C,
> > > > > etc. drivers. Test them and validate.
> > > > >
> > > > > That is why we now "just" convert only U-Boot proper to DM/DTS.
> > > > >
> > > > > As a follow up - it seems pragmatic to not touch SPL for now,
> > > > > and start the conversion ONLY when necessary drivers gain
> > > > > support for OF_PLATDATA (so we don't need DTS support in SPL).
> > > > >
> > > > > Unfortunately, it would take some time.
> > > >
> > > > Right, and again, DM support in SPL is not required as we haven't
> > > > sorted out some of the issues such as making sure it is still
> > > > small enough.
> > >
> > > Ok, so we do have agreement here. Good.
> > >
> > > >
> > > > Which is why I'm quite frustrated at the moment about "just now"
> > > > starting to convert these things for i.MX rather than several
> > > > years ago,
> > >
> > > Frankly speaking - I do prefer to use older, well tested and stable
> > > SW.
> > >
> > > And such situation we had with i.MX port. It was (and still is)
> > > working, being there before the whole DM/DTS stuff.
> > >
> > > I can understand that we advised new ports/archs to use DM/DTS. But
> > > i.MX has a lot of legacy code, which is already deployed. Changing
> > > it because "we move to DM/DTS" immediately is not practical as we
> > > loose the code base developed, tested and validated for years.
> >
> > DM/DT has been around for over 7 years now, so it too is "legacy
> > code". So it's not "immediately", it's been several years now of
> > trying to get people to migrate and introducing more firm deadlines
> > about it.
>
> I'm not talking about generic DM/DTS adoption - this goes pretty well.
>
> I'm specific about OF_PLATDATA in SPL, which is lagging behind in the
> whole U-Boot.
> To efficiently convert SPL on i.MX we do need it.
>
> >
> > And I'm _still_ not removing code that's close to a full year past the
> > published "We're going to remove this now" date.
>
> And this is a good approach. This code is a legacy one, but still
> working and sometimes used.
>
> >
> > > > when the first "hey, this might be too big for some platforms"
> > > > conversations started and we changed from "OF_PLATDATA is a
> > > > stop-gap" to "OF_PLATDATA is a required feature or we'll never be
> > > > small enough".
> > >
> > > Please consider that we even now have issues with some non-converted
> > > drivers. Some drivers were "under conversion" for years (spi
> > > framework, sf command). Please also note how many drivers actually
> > > support OF_PLATDATA?
> >
> > Yes, it's been hard to get some drivers converted because just not
> > doing it had no consequences. And yes, I'm sure drivers you need
> > will need OF_PLATDATA hooks added.
>
> Yes, certainly.
>
> >
> > > > And it's still unrelated to removing unused features from a build,
> > > > which this thread is about. If platforms aren't loading U-Boot
> > > > from EXT4, that's just unused bloat.
> > >
> > > I do agree with the above sentence. If the feature is not used - it
> > > shall be removed. However, if somebody deployed it already - then -
> > > well it must be supported or other, acceptable solution needs to be
> > > used (e.g. EXT4 -> VFAT conversion).
> >
> > Exactly. If people are using EXT4 on iMX6 then it should stay. We
> > dropped it from am335x_evm over a year ago because it wasn't used.
> >
>
> Yes. I also do agree.
>
I don't want to get into the DM discussion, the intention of this patch was
to reduce SPL size, not make room for a future increase.
That said, I am surprised it is so controversial to slim down a Defconfig,
which (at least in my mind) is just the "Maintainer Recommended Starting
Point". By removing Ext4 we stating that it's no needed for the board
"typical" use.
Of course, anyone can still turn Ext4 -and many other features- on.
Once again, this is only possible if we maintain SPL "starting point"
size as small as possible, which probably excludes DM or any other
form of bloat.
In fact, if I can keep removing features and still boot the board with
the typical setup, I'll be happy to keep suggesting such patches.
Thanks!
Eze
More information about the U-Boot
mailing list