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

[Lukasz Majewski <lukma@denx.de>]

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@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.

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.

> 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?

> 
> 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).

> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190526/1c83a528/attachment.sig>