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 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:
- 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.
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@denx.de