|
Rockbox mail archiveSubject: X5 dual boot revisitedX5 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 |