On Thu, Jun 17, 2021 at 10:18:07AM +0300, Ivaylo Dimitrov wrote: > > > On 17.06.21 г. 0:12 ч., Tom Rini wrote: > > On Thu, Jun 17, 2021 at 12:03:15AM +0300, Ivaylo Dimitrov wrote: > > > Hi Tom, > > > > > > On 16.06.21 г. 20:37 ч., Tom Rini wrote: > > > > On Wed, Jun 16, 2021 at 08:25:28PM +0300, Ivaylo Dimitrov wrote: > > > > > Hi, > > > > > > > > > > On 16.06.21 г. 15:13 ч., Tom Rini wrote: > > > > > > On Wed, Jun 16, 2021 at 08:10:08AM -0400, Tom Rini wrote: > > > > > > > On Wed, Jun 16, 2021 at 09:02:16AM +0300, Ivaylo Dimitrov wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > On 15.06.21 г. 15:34 ч., Tom Rini wrote: > > > > > > > > > On Tue, Jun 15, 2021 at 08:40:30AM +0300, Ivaylo Dimitrov > > > > > > > > > wrote: > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > On 22.05.21 г. 0:36 ч., Pali Rohár wrote: > > > > > > > > > > > On Friday 21 May 2021 10:44:18 Tom Rini wrote: > > > > > > > > > > > > On Wed, May 19, 2021 at 11:52:03AM -0400, Tom Rini > > > > > > > > > > > > wrote: > > > > > > > > > > > > > On Wed, May 19, 2021 at 03:27:48PM +0200, Pali Rohár > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tuesday 18 May 2021 21:26:40 Tom Rini wrote: > > > > > > > > > > > > > > > This board has not been converted to > > > > > > > > > > > > > > > CONFIG_DM_USB by the deadline. > > > > > > > > > > > > > > > Remove it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'm very disappointed that you want to remove Nokia > > > > > > > > > > > > > > N900 from U-Boot. > > > > > > > > > > > > > > > > > > > > > > > > > > > > I was waiting waiting half of year because other > > > > > > > > > > > > > > developers did not > > > > > > > > > > > > > > react to issues which were introduced and neither > > > > > > > > > > > > > > to patches which I > > > > > > > > > > > > > > sent (+ trying to remind open issues). And also I > > > > > > > > > > > > > > was waiting another > > > > > > > > > > > > > > half of year until other N900 related patches were > > > > > > > > > > > > > > merged. So the whole > > > > > > > > > > > > > > slowdown was not caused by me, why it is taking so > > > > > > > > > > > > > > long. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Now there is still one N900 DM related patch > > > > > > > > > > > > > > waiting for review. I'm > > > > > > > > > > > > > > converting code step by step. > > > > > > > > > > > > > > > > > > > > > > > > > > > > So the ball is not on my side. > > > > > > > > > > > > > > > > > > > > > > > > > > So, what patch(es) need to be applied to get DM_USB > > > > > > > > > > > > > enabled? Thanks. > > > > > > > > > > > > > > > > > > > > > > > > I don't see any open patches from you that look related > > > > > > > > > > > > to enabling > > > > > > > > > > > > DM_USB on the platform. If you want to disable USB on > > > > > > > > > > > > the platform for > > > > > > > > > > > > now instead, that's fine too. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I tried to migrate the latest master to DM_USB, but > > > > > > > > > > unfortunately the > > > > > > > > > > results are pretty much sad - adding OF_CONTROL (which is a > > > > > > > > > > prerequisite to > > > > > > > > > > have DM_USB IIUC) and OF_BOARD (so binary to be compiled), > > > > > > > > > > adds ~100k to the > > > > > > > > > > size of the u-boot binary, so it becomes 370284 bytes. > > > > > > > > > > Given that we have > > > > > > > > > > less than 256k of storage space for the u-boot, the > > > > > > > > > > produced binary cannot > > > > > > > > > > be used on n900 the same way current (no DM_USB) binary is > > > > > > > > > > used. > > > > > > > > > > > > > > > > > > > > As I see it, there are not much options left - u-boot on > > > > > > > > > > N900 is not SPL, so > > > > > > > > > > we can't use OF_PLATDATA, which in turn always links libfdt. > > > > > > > > > > Also, if I read the code (usb-uclass.c) correctly, enabling > > > > > > > > > > DM_USB requires > > > > > > > > > > the board to be converted to DT and this is way bigger > > > > > > > > > > change. > > > > > > > > > > > > > > > > > > > > Please advice on how to proceed. > > > > > > > > > > > > > > > > > > Please post your WIP patches, thanks. > > > > > > > > > > > > > > > > > > > > > > > > > Sorry, I am not sure I understand what patches you want me to > > > > > > > > post: > > > > > > > > > > > > > > > > WDT patch has already been sent couple of months ago - > > > > > > > > https://lists.denx.de/pipermail/u-boot/2021-March/443868.html > > > > > > > > Do you want it to be rebased and resend? > > > > > > > > > > > > > > > > DM_USB, I just started writing one and I immediately hit the > > > > > > > > OF_CONTROL > > > > > > > > requirement. Enabling OF_CONTROL requires a full blown > > > > > > > > migration to DT, > > > > > > > > which is a huge task and not really equal to "Please update the > > > > > > > > board to use > > > > > > > > CONFIG_DM_USB...". Without OF_CONTROL, I simply get link > > > > > > > > failures: > > > > > > > > > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > DWARF error: could not find abbrev number 3998 > > > > > > > > /tmp/cc0BOqms.ltrans0.ltrans.o: in function > > > > > > > > `usb_child_post_bind': > > > > > > > > <artificial>:(.text+0x5672): undefined reference to > > > > > > > > `ofnode_read_u32_default' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > <artificial>:(.text+0x568c): undefined reference to > > > > > > > > `ofnode_read_u32_default' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > /tmp/cc0BOqms.ltrans0.ltrans.o: in function `usb_scan_device': > > > > > > > > <artificial>:(.text+0x6c84): undefined reference to > > > > > > > > `ofnode_first_subnode' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > <artificial>:(.text+0x6c96): undefined reference to > > > > > > > > `ofnode_read_u32' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > <artificial>:(.text+0x6ca4): undefined reference to > > > > > > > > `ofnode_next_subnode' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > /tmp/cc0BOqms.ltrans0.ltrans.o:(.u_boot_list_2_uclass_driver_2_usb+0x8): > > > > > > > > undefined reference to `dm_scan_fdt_dev' > > > > > > > > /usr/lib/gcc-cross/arm-linux-gnueabi/8/../../../../arm-linux-gnueabi/bin/ld: > > > > > > > > /tmp/cc0BOqms.ltrans0.ltrans.o:(.u_boot_list_2_uclass_driver_2_usb_hub+0x8): > > > > > > > > undefined reference to `dm_scan_fdt_dev' > > > > > > > > > > > > > > > > Fixing those requires enabling of OF_CONTROL and this in turn > > > > > > > > means the > > > > > > > > board must be migrated to DT, unless I am missing something. > > > > > > > > That's why my > > > > > > > > "please advice..." stance. > > > > > > > > > > > > > > Please post the patches that bring you to the above link errors, > > > > > > > yes, > > > > > > > thanks. > > > > > > > > > > > > To be clearer, finish up a patch that completes the migration but > > > > > > is too > > > > > > large to install on the hardware so that others can take a look. > > > > > > > > > > I am not sure I understand that - a patch that completes the > > > > > migration to > > > > > DM_USB cannot be done ATM as the binary does not link without > > > > > OF_CONTROL. > > > > > And I am not going to enable OF_CONTROL as this means I will have to > > > > > migrate > > > > > everything to DT. That's why I enable DM_USB only - see reply to the > > > > > other > > > > > mail. > > > > > > > > My advise is to provide a linkable but not runnable (or, only runnable > > > > in QEMU, the problem is the fixed layout of the actual device flash, > > > > right?) patch so that we can figure out how and what needs to be tuned > > > > where so that we can see what to do about this platform. > > > > > > > > > > What about the following patch (once you confirm I am on the right track, > > > I > > > will send a proper patch as well). Enabling thumb is a must, otherwise > > > binary does not fit. With the below changes code compiles and runs on a > > > real > > > HW: > > > > Wait, why is thumb disabled? That's something I'd really like to know > > why on as my first guess is there's units with old enough cores that > > have some thumb errata maybe? > > omap3 in N900 suffers from ARM errata 430973, I guess that's why -mthumb was > disabled back then. I implemented N900 specific workaround to set the IBE > bit in ACR in the board code, but I don't see flush BTAC/BTB being executed > on context switch (if there are context switches at all). The only places I > was able to find that flush BTAC/BTB instruction are on startup > (cpu_init_cp15()) and cache invalidate (invalidate_icache_all()). > > BTW the code in cpu_init_cp15() is buggy, as flush is executed before IBE > bit in ACR is set, making that flush effectively a noop on boards where IBE > bit was not set by the previous stage loader, but that's another story not > affecting N900 :).
A separate thread / patch on that would be much appreciated :) > So, as soon as there are no context switches in u-boot that may land > different code on a same physical address, we should be safe. Could you > confirm this is the case? Is there virtual addressing? Multiple processes? The only place I'm not 100% sure we wouldn't have virtual addressing would be when you start to involve the EFI loader and in turn EFI applications are running. But we're specifically disabling this here (and I don't think it's useful on this era hardware anyhow). > > For the second part, how are you binding > > the device? And oh, we didn't disable the EFI loader support already? > > Yeah, looks like selecting libfdt automagically selects a whole bunch of > other things (like CMD_FDT and OF_LIBFDT_OVERLAY) > > > Yeah, that would be another pretty easy choice to win back a bunch of > > space. So yes, this is certainly on the right track, thanks! > > > > Ok, we'll send a proper patch series as soon as it is confirmed that it is > safe to enable -mthumb. Thanks. > In the meanwhile, could you please have a look at the WDT patch Pali posted > back then? It's been merged to -next for a little while now, via the TI tree. -- Tom
signature.asc
Description: PGP signature
