[maemo-leste] [PATCH] arm: Remove nokia_rx51 board

[Ivaylo Dimitrov]

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

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?

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

In the meanwhile, could you please have a look at the WDT patch Pali 
posted back then?

Thanks,
Ivo