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 ? > 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 . -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot