[U-Boot] [PATCH] configs: Remove CONFIG_SPL_FS_EXT4 for all MX6 platforms
Ezequiel Garcia
ezequiel at vanguardiasur.com.ar
Mon May 27 02:59:13 UTC 2019
On Sun, 26 May 2019 at 14:30, Marek Vasut <marex at denx.de> wrote:
>
> On 5/26/19 6:18 PM, Ezequiel Garcia wrote:
> > 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.
>
> SPL size should not be reduced at the expense of useful functionality
> though.
>
> > 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.
>
> Maybe you can remove VFAT first, why ext4 in the first place ?
>
I guess we shouldn't remove VFAT either (or anything!) since it might
break someone setup as well.
> > Of course, anyone can still turn Ext4 -and many other features- on.
>
> I was under the impression that mainline should be as useful as
> possible. Maybe I'm mistaken and carrying random downstream patches is
> now the suggested approach ?
>
> > 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.
>
> That's fine, unless it breaks other peoples' existing setups .
>
I don't want to break anyone's setup (much less yours).
I never thought we considered defconfig as something people use,
for anything except starting point.
I'm happy to drop this patch (partially or fully).
Thanks,
Eze
More information about the U-Boot
mailing list