[U-Boot] [PATCH] configs: Remove CONFIG_SPL_FS_EXT4 for all MX6 platforms

[Tom Rini <trini@konsulko.com>]

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@ossystems.com.br>
> > >>>>>>>>> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > >>>>>>>>> Cc: Peng Fan <peng.fan@nxp.com>
> > >>>>>>>>> Cc: Marek Vasut <marex@denx.de>
> > >>>>>>>>> Cc: Stefano Babic <sbabic@denx.de>
> > >>>>>>>>> Cc: Stefan Roese <sr@denx.de>
> > >>>>>>>>> Cc: "Eric Bénard" <eric@eukrea.com>
> > >>>>>>>>> Cc: Breno Lima <breno.lima@nxp.com>
> > >>>>>>>>> Cc: Francesco Montefoschi <francesco.montefoschi@udoo.org>
> > >>>>>>>>> Signed-off-by: Ezequiel Garcia <ezequiel@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.

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,
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".

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.

-- 
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/046fbeed/attachment.sig>