On Sun, 26 May 2019 at 14:30, Marek Vasut <ma...@denx.de> wrote: > > On 5/26/19 6:18 PM, Ezequiel Garcia wrote: > > On Sun, 26 May 2019 at 12:05, Lukasz Majewski <lu...@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 <ota...@ossystems.com.br> > >>>>>>>>>>>>>>>> Cc: Fabio Estevam <fabio.este...@nxp.com> > >>>>>>>>>>>>>>>> Cc: Peng Fan <peng....@nxp.com> > >>>>>>>>>>>>>>>> Cc: Marek Vasut <ma...@denx.de> > >>>>>>>>>>>>>>>> Cc: Stefano Babic <sba...@denx.de> > >>>>>>>>>>>>>>>> Cc: Stefan Roese <s...@denx.de> > >>>>>>>>>>>>>>>> Cc: "Eric BĂ©nard" <e...@eukrea.com> > >>>>>>>>>>>>>>>> Cc: Breno Lima <breno.l...@nxp.com> > >>>>>>>>>>>>>>>> Cc: Francesco Montefoschi > >>>>>>>>>>>>>>>> <francesco.montefos...@udoo.org> Signed-off-by: > >>>>>>>>>>>>>>>> Ezequiel Garcia <ezequ...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot