[U-Boot] [PATCH] configs: Remove CONFIG_SPL_FS_EXT4 for all MX6 platforms
Tom Rini
trini at konsulko.com
Sun May 26 12:30:53 UTC 2019
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.
> > >
> > > 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.
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.
> > 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.
> > 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.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190526/e4b8a173/attachment.sig>
More information about the U-Boot
mailing list