FS#10045 - Lyre port patch
Attached to Project:
Rockbox
Opened by jpcasainho (casainho) - Monday, 23 March 2009, 07:28 GMT
Last edited by Daniel Stenberg (bagder) - Monday, 30 March 2009, 12:22 GMT
Opened by jpcasainho (casainho) - Monday, 23 March 2009, 07:28 GMT
Last edited by Daniel Stenberg (bagder) - Monday, 30 March 2009, 12:22 GMT
|
DetailsThis patch id for apply on Rockbox sourceSVN version 20485.
This is the first pacth for the Lyre music player and recorder (were named as Rockbox Player). This patch is for the bootloader and just make the kernel_init() working. I tested it using sleep(4*HZ) on JTAG debug. |
This task depends upon
Closed by Daniel Stenberg (bagder)
Monday, 30 March 2009, 12:22 GMT
Reason for closing: Accepted
Additional comments about closing: Thanks committed to SVN in rev 20574
Monday, 30 March 2009, 12:22 GMT
Reason for closing: Accepted
Additional comments about closing: Thanks committed to SVN in rev 20574
Some cosmetic comments (I mentioned these in IRC as well):
1) "//" shouldn't be used (use /* .... */)
2) No TAB characters.
3) No lines longer than 80 characters
4) Some files have a mixture of DOS and Unix line endings (this shows as ^M characters at the end of some lines in my editor (emacs))
5) For a bootloader patch, no need to include apps/keymaps/keymap-lyre.c
You've created a new target simply called "Lyre". I'm assuming that there will be various different versions of this player - at least a couple of prototypes, and then a "real" device. You may want to call the target that this patch is for something like LYRE_PROTO1 (Prototype 1). If you do that, then you (or other developers) can add further targets for other variations of your device.
In config-lyre.h, you define "BOOTFILE_EXT" to be "rockboxplayerlittle" - this should be changed to "lyre".
Is any code in your patch (e.g. at91sam9260.h) taken from the Atmel SDK? If so, then you should include a header file from that project, including their copyright and license information.
Plus one more cosmetic comment - some of your code is indented with two spaces, not the Rockbox standard four.
The only thing missing is the "at91sam9260.h" because it is big and I need more time to format is style.
Is there anything more that need to be worked?