Sansa E200V2 Bootloader is broken in master

[bahus]

IM saying that buflib commit hasn't affected the others mentioned. And i went thru those bootloaders previously to make them fit and left some wiggle room

OK if you really tested it. I just saw your commit to fit Sansa Clip+ and it was 4 years ago. Way before commit that broke e200V2 bootloader.

I mean https://github.com/Rockbox/rockbox/commit/d78a37676efbc24ce1d5c46b65c6caf45ff3bc27.

I found it as bootloader size related commit and tried it first when master commit didn't work. See
https://github.com/bahusoid/rockbox/commits/e200V2_24.10.2020/

From what I saw all your changes are general to bootloader build and applies for e200v2 too. But it doesn't fit as is. It fits and works after my additional changes. And fails to boot after 3301c5aa (2+ years after your "fit" commit).

[Bilgus]

Looking at that commit I'm guessing that really isn't the smoking gun

likely you can move some stuff around randomly for a favorable alignment and it will work

being that the bootloader is shy by 12k its been a long while since anyone has tried building it

or we done goofed but I'm not seeing anything suspicious

I'll keep messing with it..

[Bilgus]

been going through your changes I think the YUV stuff is superfluous but otherwise looks sane

I'm going through making different changes to get it to fit and maybe we can see if it still crashes at the same commit as yours

ATM I'm leaning towards a pared down buflib for the simulator bootloader but I'd like to try and talk to Amachronic first
but if I don't hear from him will probably move forward anyway

[bahus]

been going through your changes I think the YUV stuff is superfluous but otherwise looks sane

Ok I will prepare change request soon to discuss it further.

I got distracted by other nasty e200v2 issue - it's very unstable with my main rockbox SD card (which works OK with Sansa Clip Zip and Rocker). It fails on SD writes (looks like stuck waiting for transfer state here after  few cycles). But it's another story and should be discussed in separate topic later...

[Bilgus]

sorry I meant to say bootloader not simulator above, re unstable sd card try boosting dcdc voltages or force into non HS mode in the sd driver

[bahus]

I prepared change requests (a bit cleaned up version):
https://gerrit.rockbox.org/r/c/rockbox/+/5611
https://gerrit.rockbox.org/r/c/rockbox/+/5612

try boosting dcdc voltages or force into non HS mode in the sd driver

Thanks. Will look into it.

[Bilgus]

Amachronic found a few thing to allow this to fit check for the patches up on gerrit
it consists of your changes minus the thumb and removing tracebacks
and then defining DEFAULT_CP_STATIC_ALLOC for unicode

and

Code: [Select]

void usb_storage_disconnect(void)
{
#if !defined(BOOTLOADER)
    usb_handle = core_free(usb_handle);
#endif
    (void)usb_handle;
}in usb_storage.c

and

Code: [Select]

/* Priority in bootloader is wanted */
#if !defined(SANSA_E200V2)
#define HAVE_PRIORITY_SCHEDULING
#endifin config.h

Edit:
oh and making lcd_scroll_stop and Lcd_viewport_scroll_stop dummy functions

[amachronic]

Yeah, disabling prio scheduling on top of those two patches is actually all you need to make it fit without --thumb: https://gerrit.rockbox.org/r/c/rockbox/+/5615/2

[bahus]

Modifying 'usb_storage_disconnect` as you suggested fixed it for me. Now bootloader on top of master is fully functional (thumb build with SD boot 119673 out of  allowed 120860 bytes).
Will you make this change or should I add it to my change request?

Yeah, disabling prio scheduling on top of those two patches is actually all you need to make it fit without --thumb: https://gerrit.rockbox.org/r/c/rockbox/+/5615/2

I'm not personally concerned about thumb or no thumb build. Thumb build now works so I see no point disabling priority handling to make no thumb build fit.

UPDATE

Yeah, disabling prio scheduling on top of those two patches is actually all you need to make it fit without --thumb: https://gerrit.rockbox.org/r/c/rockbox/+/5615/2

Out of curiosity I tested this no thumb build without `usb_storage_disconnect` fix. Still doesn't boot. So it doesn't look like some alignment issue to me. `usb_storage_disconnect` is the cause...

[Bilgus]

you have the device. IMO it makes the most sense for you to do a patch

[Bilgus]

Also we don't make new bootloaders often so could you test some of these patches with your builds?

https://gerrit.rockbox.org/r/c/rockbox/+/4948
https://gerrit.rockbox.org/r/c/rockbox/+/4949

I've tested them on clip+ and zip so should work there too I'd guess

[bahus]

you have the device. IMO it makes the most sense for you to do a patch

Ok. I just don't see it as only bootloader related fix and I'm not the one who found the issue. Created change request: https://gerrit.rockbox.org/r/c/rockbox/+/5619

Also we don't make new bootloaders often so could you test some of these patches with your builds?

It works for me and adds around 100 bytes to thumb  size (119676 without, 119759 with those patches)

[Bilgus]

build them on top of his patchsets he isn't going to get upset

that said don't push it yet as he might have other plans with that wip patch..

[amachronic]

Thanks for doing the USB fix bahus. I created patches to enable thumb for the e200v2 bootloader by default, since you confirmed it's working now. Also, we can get rid of the codepage handling completely with this: https://gerrit.rockbox.org/r/c/rockbox/+/5622, saves ~100 bytes off the compressed image vs. your changes to unicode.c.

With all the optimizations + removing logo, we need to remove at least one more thing to make it fit:

• Remove backtraces
• Turn off priority scheduling
• Make filesystem read-only

I would rather keep backtraces unless we have no other choice since they're useful. I don't think priority scheduling is necessary since it only affects performance and not correctness, although there could be some small risk it makes USB a bit less reliable (but only at the initial connection -- transfers should still be stable). Out of the three, I think making the filesystem read-only could give the most gains based on the WIP patch I did; it's kind of a pain, but I'd like to have headroom so we don't have this problem again because of some minor code change somewhere.

Since it's just barely fitting now with SD boot + multiboot v1 enabled, I think we should merge everything and go with disabling backtraces to get things working again.

(btw I don't have plans for the WIP patchset. I just put it up so as not to lose the changes, obviously it needs a lot of rework to be mergeable.)

[bahus]

Also, we can get rid of the codepage handling completely

So it makes my changes in unicode.c unnecessary, doesn't it? Let me know when you are done with your changes so I could test them and do additional clean up.

I'd like to have headroom so we don't have this problem again because of some minor code change somewhere.

Unless something useful is added (exFat support?) I don't think there is a need to build bootloader (it's multiboot in this case). So most likely it won't be about some minor code changes.

I would rather keep backtraces.

I'd rather keep potentially useful for user functionality. There is not much going on in bootloader and it's either work or completely dead with no indications.  Also backtraces are not present in current bootloader release so it's not something we removed from released functionality.

we should merge everything and go with disabling backtraces to get things working again.

Yeah. I'm all in with this plan.


Offtopic: I also tested booting from GPT-partitioned  card (5dc0e4e). It works!