Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: X5 dual boot revisited

X5 dual boot revisited

From: RaeNye <raenye_at_netvision.net.il>
Date: Mon, 08 May 2006 18:42:13 +0200

Instead of continuing this over flyspray comments, let's hear what you have
to say about it.
My last comment (not in the tracker), is below.

Background:
The first patch works but relies on fixed memory addresses in the Cowon
preloader (the part of firmware that never changes). My attempts at making
it work with either RB code or my own have all failed.

(Warning: long discussion ahead)

-- Bagder:
Ah, this approach looks much better to me!
...
except that the second patch makes my X5 start in "all black" mode that I
can't escape. I can see the LED go red and I believe there's a disk spinup
but then nothing until I use my paperclip.
I tried the first patch then and it worked.
Two requests about this (except the bug). I'm aware this might be a bit
premature but I thought I'd let you know anyway:

1. please provide diffs with -u
2. I would appreciate if you could move this fw-patch code to the mkboot
source code, as that's already used for the h100 and h300 players to "patch
in" bootloaders. Then I'd also like to see you use all three file names
given as command line arguments and not assume a particular file in "current
directory".

-- RaeNye:
The reason for it is probably the horrible i2c-generic.
I tried to fix the 2nd patch by including I2C routines from the preloader
and now it just doesn't work (i.e., boots RB).
Maybe you could help me find the bug (I left the debugging screens in this
patch).
All I know is that for some unknown reason it wouldn't set RxAk after
writing the device byte (0x10) to MBDR2.

About mkboot, I renamed x5-patch-original-firmware.c to mkboot-x5.c (no
common structure is shared between iriver's mkboot and mkboot-x5, so they
shouldn't reside in the same file) and added it to
tools/{Makefile,configure}. I also added the OF as a command line parameter.

everything except crt0.S works flawlessly, please commit these to the cvs
and crt0.S will be added when it's bug-free.

P.S.
I'm still sure that the first patch is much safer.

-- Bagder:
1. Why is i2c-generic horrible for this use case? And even it it was, I
don't see how you fix it by avoiding it.
2. I would still prefer using mkboot since it doesn't add another tool and
the use case is the _exact_ same as the existing mkboot tool, only another
platform. And mkboot already takes an argument specifying target.
3. How can the first patch be "much safer"? Please explain.

-- RaeNye:
1. I need pcf50606 read/write functions, and I can either use the
preloader's, rockbox's or my own.
i2c-generic introduces global variables, which I prefer not to have before
rockbox is initialized. You've seen it crashing the device for no good
reason.
2. mkboot (which is actually mkboot-iriver) has a very different structure,
and you can't easily isolate the platform dependent code. Your best shot
would be to rename main() there to main_iriver() and have mkboot-x5's main()
renamed to main_iaudio().
Do you really think it's the right thing to do?
3. The first patch wraps tested and valid code with small asm patches. The
last patch is 10 times the size of it, and probably contains 10 times the
bugs (it obviously contains at least one whereas the first patch works).
I have no more leads in the debugging of it, so unless you have any idea,
I'm pretty much stuck there.
(read: After spending a few hours in debugging it, I've concluded that I
prefer to add support for more hardware over fixing a non-existant bug).

-- Bagder:
1. Ok, so your program bugs and possibly the i2c generic can be improved. I
still think fixing the problems is the right way rather than to adding the
very obscure and hard-to-track dependency on a fixed address in the cowon
firmware. I rather depende on our code.
2. I do. The situation we have is an ever increasing amount of platforms we
support and without carefully doing things like this we will also add a new
tool for every new target, for no good reason IMHO. Yes, the x5 approach is
totally different and by all means keep the code in a separate .c file if
you think that is the correct way. Users that have BOTH irivers and x5 will
indeed benefit from using the same tool (with only a modified option) and
manual writers will cheer having to document the single approch and so on.
And again, when we add support for more platforms we extend the tool to
support them too...
3. You actually consider the Rockbox code to be "much" more unstable than
relying on an absolute address to the cowon code?

Yes, if we *REALLY* cannot make it work the "right" way (according to me) I
could be persuaded to apply the one I disagree with. Of course we can also
improve it to right in the future.

-- RaeNye:
We might as well move this discussion to the rockbox-dev list for more
opinions...

1+3. I make no claims about the stability of Rockbox code in general;
I know that I prefer using Cowon's code -- which I have inspected thoroughly
-- than using /my/ obviously buggy implementation. Indeed, I tried to call
RB's well-tested PCF50606 code, alas it fails in this case.

From my point of view, Cowon's functions are API calls into a shared library
loaded before my code.
Would you be less reluctant to use fixed addresses if before using Cowon's
code I'll verify its checksum?

2. Let's say I add an option to choose whether RB or the OF is booted by
default (many users have asked for it).
Now you require all mkboot-<platform> to support it?
Should I refrain from adding this functionality just because mkboot-iriver
is broken without it?

In fact, I don't want even to *compile* mkboot-iriver if my build is any
non-iriver platform.
It's a useless dependency in my makefile.

See fsck for a good example (IMHO) for how it should be done.
I suggest mkboot to be a shell script calling the right mkboot-<platform>.

BTW,
Could you think of a non-volatile place to store a single bit other than
flash/HDD ?
(for saving the 'which firmware should boot by default' variable)

-- Bagder:
Yes, I wouldn't mind seeing this taken to the dev list.

Yes if the firmware code would somehow be checked in a rudimentary way that
it is likely to be the one we expect, at least we can provide a way to
signal if the startup fails due to this. Like with some special led-flashing
combo or something.

I can't stop you from doing whatever you want, but users all over always
whined about the bootloader order and so far we haven't made any bootloader
boot in the reversed order mainly because A) it is rockbox bootloader it
loads Rockbox B) if you really really want the OF primarily then you should
probably not have the bootloader in the first place and C) we really should
work and focus on making Rockbox stable and good enough so that user's won't
feel the need for a reversed boot preference.

Ok, so if we consider fsck a "good" way of solving the problem then how's
your approach to making the mkboot use that approach? Of course you don't
get tools built that you won't need, but according to me the mkboot is the
tool that would be used for firmware bootloader patching and then you need
to compile it...

-- Linus:
Besides, if the Rockbox pcf50606 driver doesn't work for the bootloader,
then I suggest we fix that bug instead of using the cowon functions.

-- Luckz:
Wouldn't it in any case be easier to have the whole thing always boot from
Rockbox? Waiting for that (three seconds?) and pressing REC and so on a bit
(two seconds?) really isn't that horrible, while, on the other side, it
sounds like a total nightmare to make it NOT automatically load the OF
anymore once you start making it do that.

-- RaeNye: (new comment)
Luckz: Cowon's OF might rely on what the preloader has done, and RB
bootloader thrashes everything. Thus, I must decide ASAP which firmware to
load. If you're talking about launching the OF from within RB (e.g., as a
plugin) then that's a completely different feature (which, BTW, might be
implemented in the future).

Linus: As I said before, I suspect the problem originates from i2c-generic
using global variables before initialized by RB-bootloader. Having a more
efficient PCF50606 driver is important (I nagged about it before) but
irrelevant to the progress of the dual-bootloader. After PCF50606 driver is
re-implmented (possibly by yours truly), it makes perfect sense to adjust
the dual-bootloader to use it, but it is not a prerequisite IMHO.

Bagder: If so, I intend to add an additive dword-sizes checksum over the
preloader's code. I cannot signal it's not working with either leds, the lcd
or any hardware (since that would require PCF50606 funcs...) but I can
fallback to booting RB.

OSS is all about choice, hence I don't intend to tell users anything like
"The dual-bootloader will only boot RB by default because I'm a RB dev" as
much as grub won't tell you "I ain't booting your Windows by default". Even
if the official version supports only RB-first, it is easy to reverse the
order (as people in iaudiophile.net's forum have already done).

Finally, you always run fsck, which calls the needed worker (e.g.,
fsck.ext2). If your system has ReiserFS, you'll have fsck.reiser; if not,
you won't need it or even have it compiled. The very same should be done
with mkboot.
Received on 2006-05-08

Page template was last modified "Tue Sep 7 00:00:02 2021" The Rockbox Crew -- Privacy Policy