This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#6929 - Gigabeat bootloader improvements
Attached to Project:
Rockbox
Opened by Barry Wardell (barrywardell) - Thursday, 29 March 2007, 01:44 GMT+2
Opened by Barry Wardell (barrywardell) - Thursday, 29 March 2007, 01:44 GMT+2
|
DetailsThis patch brings some improvements for the Gigabeat bootloader:
1) Large amount of code simplification 2) Only show boot messages if there is either an error or a button is pressed 3) Remove non-functional dual-boot code. Print a warning about it not working instead. 4) Restore the logo to the main Rockbox binary - otherwise we just see a blue screen while Rockbox is loading The code for the bootsplash is still present, although I wonder how useful it is given the splash is shown for less than 1 second. The only benefit I see is if the bootsplash matched the rockbox splash, so that it's a seamless transition. The main problem currently is that now that boot messages are not shown, the bootloader doesn't update the framebuffer at all. However, lcd_init() seems to cause the LCD to update. Since there has been no updates to the framebuffer it updates the LCD with what was last in the framebuffer, which is normally the "Shutting down..." message from the previous shutdown. The options I see for fixing this are: 1) Make lcd_init() not do a LCD update. 2) Make the address of the LCD framebuffer the same as in the Gigabeat bootloader (the one with the progress bar). I think this would make the LCD be updated with what's currently on screen, so there would be no noticable update. 3) Don't do lcd_init() in the bootloader unless there is an error or a button is pressed or the bootsplash is to be shown. I haven't been successful in getting any of the three solutions working satisfactorily, but that was mainly due to lac of time rather than major problems encountered. 
gigabeat_boot.patch
(7.2 KiB)
apps/main.c | 5 - bootloader/gigabeat.c | 184 +++++++++++++++++++------------------------------- bootloader/common.c | 2 3 files changed, 72 insertions(+), 119 deletions(-) |
This task depends upon
Closed by Karl Kurbjun (kkurbjun)
Saturday, 21 April 2007, 18:20 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed to svn
Saturday, 21 April 2007, 18:20 GMT+2
Reason for closing: Accepted
Additional comments about closing: Committed to svn
Did you by any chance look into the making the gigabeat use the 'proper' rockbox format (with checksum and version) instead of a raw image?
I looked into using the 'proper' rockbox format. It's an easy change to do. The only problem is backwards compaibility. How do we know whether the rockbox.gigabeat we are loading is in the correct format? One suggestion on IRC was to change the extension, but there were no suggestions for what that extension should be. Another option would be to check bytes 4-7 for the model ID. In the 'proper' format, these bytes would be present. In the older files, I guess it would be at least highly unlikely that these 4 bytes would happen to match the model ID. In fact, we should probably do this for all bootloaders anyway.
Either option would be an easy fix to do, requiring only a couple of extra lines of code.
Some comments about the Gigabeat bootloader in general (unrelated to your patch). I'm pretty sure that "Vol+ button to restore original kernel" is slightly too long for the Gigabeat's screen. Perhaps we should reword the message since having it trail off the screen doesn't look very good. Also, is there any reason we have to hold Menu for rescue mode? Is there anything different about Rescue Mode and if not, why don't we treat USB insertion like Bootloader USB mode on other targets (H300 etc)?
I'd also agree with roolku that bootsplash should be removed and the plan for a "proper" Rockbox sounds good to me.
Yes, especially the case of using an old bootloader (which obviously can't be changed) with a new image. But I don't think there is much we can do (apart from renaming the image so it is a "file not found" error rather than a crash, but I consider this cosmetic) and the earlier it gets changed the less people are affected.
> I'm pretty sure that "Vol+ button to restore original kernel" is slightly too long for the Gigabeat's screen.
I don't see the need for any of the strings at all. No other units have them (afaik) and you have to be a fast reader to read them.
The original kernel boot is not working anyway.
> Also, is there any reason we have to hold Menu for rescue mode? Is there anything different about Rescue Mode and if not, why don't we treat USB insertion like Bootloader USB mode on other targets (H300 etc)?
If I understand correctly the rescue mode doesn't work for the X series. So this would result in a crash if they switch the unit on with USB plugged in. But I agree it would be neater without the key press.
I pulled out the code for bootsplash, and copying FWIMG01.DAT.ORIG to FWIMG01.DAT (it's one direction anyway, and if you have applied the minimal zip in the wiki and ran that code then you can't boot your player and have to do the recovery procedure).
This patch also removes the toggling of GPG12 as this pin does not seem to be attached to anything. Eventually I would like to add in full initialization of all the GPIO ports so that we are not relying on a previous state set in the OF bootloader.
It would be great if a Gigabeat X user could try the bootloader USB mode in this patch to make sure everything is ok. I don't believe this code should crash the X series.
Also as a note, I think the gigabeat booatloader should do an all or nothing approach to the proper format. If you are using the new bootloader you need to use a newer build. I don't think that code should be added in for loading an old raw format image - chances are that if you are using a new BL you will be using the latest builds anyway. It is extremely unlikely that an updated bootloader will confuse a raw image for the proper format as well (from what I have seen the images all start with a bunch of zeros anyway which won't pass the checksum). If they are using the old bootloader there's a chance that the new proper images will still run, but if they don't they can go into recovery mode and update their BL.
Let me know if the patch does not apply - I had to pull out alot of other code I'm working on and I may have messed something up.
Comments are greatly appreciated.
1) What is the purpose of the changes to enable_mmu()?
2) Why do you change map_memory() to memory_init()?
3) Can you explain what coldstart detection does?
1) I changed enable_mmu in an attempt to make rolo work more reliably. Right now it invalidates the cache and I was hoping to make sure it didn't destroy any writes to the memory done by rolo. I may move the cache cleaning call to within rolo itself though.
2) I changed it to memory_init as I was playing around with having it called within the system_init function so that the bootloader and the main file(rockbox.gigabeat) were more consistent between eachother. I was doing this because eventually I would like to play around with a rombox type of setup for the gigabeat. Right now I don't think the code is ready to be put in flash, but hopefully one day it will be suitable. I felt that memory_init was a more accurate function name as it's doing a bit more then just mapping memory.
3) Coldstart detection checks if the player is in reset (the pin is low) as was done on the Irivers. It reduces the clicks on startup as the code doesn't think it needs to initialize and reset the drive twice between the bootloader and the main code. I am guessing it is designed for when the player is initially powered on as the GPIO pins should be initially set in high impedance or their function, once they are configured as outputs they may output a zero initially (depending on how the port configuration is done, or if the function output does this already) which is a reset on the ATA lines (in this case it's also connected to the ARESET on the C7C68300 as well)
I am not seeing old FB data displayed on my player with the patch I have. Is it showing on yours? It would be nice if we didn't have to have #ifdefs for bootloaders in the gigabeat port if avoidable for the above mentioned possibility of a rombox for the gigabeat.
1) Remove unrelated whitespace change in apps/main.c
2) #include mmu-meg-fx.h in bootloader/gigabeat.c - this also means we can remove the extern declaration of memory_init()
3) Use the SYSFONT_WIDTH and SYSFONT_HEIGHT constants for the USB Bootloader mode message
4) Remove the wait for button press on ATA error. I don't see what purpose this serves. Instead, show the error, then shut down.
5) Remove the wait for USB on disk error. I don't see what purpose this serves. Instead, show the error, then shut down.
6) Use the error() function when there is a problem with the BOOTFILE.
7) Remove the declaration of memory_init() in mmu-meg-fx.c because it's already done in the header.
The only thing I'm wondering about now is the ALARM_MASK. Is that something that will come later?
I was going to hold off on the alarm_mask until we can actually write code to test against. Right now the OF bootloader interferes with booting from the alarm. The change when it comes also should not be as intrusive as this patch in that users should not have to update the bootloader unless they want to use the alarm clock feature(although they will have to flash). There is still alot of initialization that needs to be written in Rockbox before we should play around with flashing. Right now I've just been working on pin initialization, and I think the ata and usb interfaces are safe, but I don't know about the lcd, the pins that are not part of the ata/usb interface, etc..