Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#10045 - Lyre port patch

Attached to Project: Rockbox
Opened by jpcasainho (casainho) - Monday, 23 March 2009, 08:28 GMT+2
Last edited by Daniel Stenberg (bagder) - Monday, 30 March 2009, 14:22 GMT+2
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To No-one
Player Type Another
Severity Low
Priority Normal
Reported Version Version 3.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This 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.
   lyre_bootloader.patch (287.3 KiB)
 bootloader/SOURCES                                  |    3 
 bootloader/lyre.c                                   |   36 
 tools/configure                                     |   25 
 apps/keymaps/keymap-lyre.c                          |  334 ++
 firmware/export/at91sam9260.h                       | 2521 ++++++++++++++++++++
 firmware/export/config.h                            |   13 
 firmware/export/config-lyre.h                       |   95 
 firmware/SOURCES                                    |   12 
 firmware/target/arm/at91sam/app.lds                 |  107 
 firmware/target/arm/at91sam/boot.lds                |   77 
 firmware/target/arm/at91sam/lyre/debug-lyre.c       |   34 
 firmware/target/arm/at91sam/lyre/adc-lyre.c         |   31 
 firmware/target/arm/at91sam/lyre/backlight-lyre.c   |   45 
 firmware/target/arm/at91sam/lyre/button-target.h    |   48 
 firmware/target/arm/at91sam/lyre/timer-target.h     |   42 
 firmware/target/arm/at91sam/lyre/lcd-lyre.c         |   27 
 firmware/target/arm/at91sam/lyre/system-target.h    |   29 
 firmware/target/arm/at91sam/lyre/debug-target.h     |   26 
 firmware/target/arm/at91sam/lyre/adc-target.h       |   28 
 firmware/target/arm/at91sam/lyre/backlight-target.h |   26 
 firmware/target/arm/at91sam/lyre/button-lyre.c      |   99 
 firmware/target/arm/at91sam/lyre/crt0.S             |  247 +
 firmware/target/arm/at91sam/lyre/timer-lyre.c       |  122 
 firmware/target/arm/at91sam/lyre/kernel-lyre.c      |   78 
 firmware/target/arm/at91sam/lyre/lcd-target.h       |   24 
 firmware/target/arm/at91sam/lyre/system-lyre.c      |  152 +
 26 files changed, 4278 insertions(+), 3 deletions(-)

This task depends upon

Closed by  Daniel Stenberg (bagder)
Monday, 30 March 2009, 14:22 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  Thanks committed to SVN in rev 20574
Comment by Dominik Riebeling (bluebrother) - Monday, 23 March 2009, 08:44 GMT+2
The patch contains heaps of C++-style comments. Rockbox doesn't use C++-style comments in C files. Furthermore, all keyword lines ($Id$) are broken -- the closing $ is missing.
Comment by Dave Chapman (linuxstb) - Monday, 23 March 2009, 08:47 GMT+2
Nice work on getting this far.

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
Comment by Dave Chapman (linuxstb) - Monday, 23 March 2009, 09:10 GMT+2
Some more comments:

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.
Comment by Daniel Stenberg (bagder) - Monday, 23 March 2009, 09:19 GMT+2
The comment for LCD_DEPTH in the config file says 65535 colours, but that's not right is it? AFAIU, you have 444 on the actual LCD with gives *64* colours. The 16 bits 565 (and thus 64K colours) is only what you keep internally in the frame buffer.
Comment by jpcasainho (casainho) - Monday, 23 March 2009, 23:14 GMT+2
Here is the patch again, for revision 20502.

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?
   lyre_bootloader-r20502.patch (267.7 KiB)
 bootloader/SOURCES                                  |    3 
 bootloader/lyre.c                                   |   36 
 tools/configure                                     |   25 
 firmware/export/at91sam9260.h                       | 2553 ++++++++++++++++++++
 firmware/export/config.h                            |   13 
 firmware/export/config-lyre.h                       |   98 
 firmware/SOURCES                                    |   12 
 firmware/target/arm/at91sam/boot.lds                |   81 
 firmware/target/arm/at91sam/lyre/debug-lyre.c       |   34 
 firmware/target/arm/at91sam/lyre/adc-lyre.c         |   31 
 firmware/target/arm/at91sam/lyre/backlight-lyre.c   |   45 
 firmware/target/arm/at91sam/lyre/button-target.h    |   48 
 firmware/target/arm/at91sam/lyre/timer-target.h     |   42 
 firmware/target/arm/at91sam/lyre/lcd-lyre.c         |   27 
 firmware/target/arm/at91sam/lyre/system-target.h    |   29 
 firmware/target/arm/at91sam/lyre/debug-target.h     |   26 
 firmware/target/arm/at91sam/lyre/adc-target.h       |   28 
 firmware/target/arm/at91sam/lyre/backlight-target.h |   26 
 firmware/target/arm/at91sam/lyre/button-lyre.c      |   99 
 firmware/target/arm/at91sam/lyre/crt0.S             |  274 ++
 firmware/target/arm/at91sam/lyre/timer-lyre.c       |  123 
 firmware/target/arm/at91sam/lyre/kernel-lyre.c      |   78 
 firmware/target/arm/at91sam/lyre/lcd-target.h       |   24 
 firmware/target/arm/at91sam/lyre/system-lyre.c      |  150 +
 24 files changed, 3902 insertions(+), 3 deletions(-)

Comment by Dave Chapman (linuxstb) - Wednesday, 25 March 2009, 08:29 GMT+2
That last patch is looking better, but you didn't respond to my comments about renaming this target to something like "Lyre Prototype 1", or the BOOTFILE_EXT change.
Comment by jpcasainho (casainho) - Wednesday, 25 March 2009, 12:27 GMT+2
Dave Chapman (linuxstb), I will work on the changes you said. Right now I am busy working the "at91sam9260.h". As soon as I have it I will put here the patch again :-) Thank you.
Comment by jpcasainho (casainho) - Thursday, 26 March 2009, 21:33 GMT+2
Here is :-) I hope is all ok.
   lyre_proto1-bootloader-r20543.patch (92.9 KiB)
 bootloader/SOURCES                                              |    3 
 bootloader/lyre_proto1.c                                        |   36 
 tools/configure                                                 |   25 
 firmware/export/at91sam9260.h                                   |  678 ++++++++++
 firmware/export/config.h                                        |   14 
 firmware/export/config-lyre_proto1.h                            |  100 +
 firmware/SOURCES                                                |   12 
 firmware/target/arm/at91sam/lyre_proto1/system-lyre_proto1.c    |  150 ++
 firmware/target/arm/at91sam/lyre_proto1/debug-lyre_proto1.c     |   34 
 firmware/target/arm/at91sam/lyre_proto1/button-target.h         |   48 
 firmware/target/arm/at91sam/lyre_proto1/adc-lyre_proto1.c       |   31 
 firmware/target/arm/at91sam/lyre_proto1/backlight-lyre_proto1.c |   45 
 firmware/target/arm/at91sam/lyre_proto1/timer-target.h          |   42 
 firmware/target/arm/at91sam/lyre_proto1/system-target.h         |   29 
 firmware/target/arm/at91sam/lyre_proto1/lcd-lyre_proto1.c       |   27 
 firmware/target/arm/at91sam/lyre_proto1/debug-target.h          |   26 
 firmware/target/arm/at91sam/lyre_proto1/adc-target.h            |   28 
 firmware/target/arm/at91sam/lyre_proto1/backlight-target.h      |   26 
 firmware/target/arm/at91sam/lyre_proto1/crt0.S                  |  274 ++++
 firmware/target/arm/at91sam/lyre_proto1/button-lyre_proto1.c    |   99 +
 firmware/target/arm/at91sam/lyre_proto1/lcd-target.h            |   24 
 firmware/target/arm/at91sam/lyre_proto1/timer-lyre_proto1.c     |  123 +
 firmware/target/arm/at91sam/lyre_proto1/kernel-lyre_proto1.c    |   78 +
 firmware/target/arm/at91sam/boot.lds                            |   81 +
 24 files changed, 2030 insertions(+), 3 deletions(-)

Comment by Maurus Cuelenaere (mcuelenaere) - Friday, 27 March 2009, 10:53 GMT+2
You added tool="$rootdir/tools/scramble -add=lyp1" but seem to have forgotten the tools/scramble.c part.
Comment by jpcasainho (casainho) - Friday, 27 March 2009, 11:20 GMT+2
Ok, thanks. So I changed that line to: tool="cp" . I think there should be no problem since the main task for now is having the kernel_init().
   lyre_proto1-bootloader-r20552.patch (92.9 KiB)
 bootloader/SOURCES                                              |    3 
 bootloader/lyre_proto1.c                                        |   36 
 tools/configure                                                 |   25 
 firmware/export/at91sam9260.h                                   |  678 ++++++++++
 firmware/export/config.h                                        |   14 
 firmware/export/config-lyre_proto1.h                            |  100 +
 firmware/SOURCES                                                |   12 
 firmware/target/arm/at91sam/lyre_proto1/system-lyre_proto1.c    |  150 ++
 firmware/target/arm/at91sam/lyre_proto1/debug-lyre_proto1.c     |   34 
 firmware/target/arm/at91sam/lyre_proto1/button-target.h         |   48 
 firmware/target/arm/at91sam/lyre_proto1/adc-lyre_proto1.c       |   31 
 firmware/target/arm/at91sam/lyre_proto1/backlight-lyre_proto1.c |   45 
 firmware/target/arm/at91sam/lyre_proto1/timer-target.h          |   42 
 firmware/target/arm/at91sam/lyre_proto1/system-target.h         |   29 
 firmware/target/arm/at91sam/lyre_proto1/lcd-lyre_proto1.c       |   27 
 firmware/target/arm/at91sam/lyre_proto1/debug-target.h          |   26 
 firmware/target/arm/at91sam/lyre_proto1/adc-target.h            |   28 
 firmware/target/arm/at91sam/lyre_proto1/backlight-target.h      |   26 
 firmware/target/arm/at91sam/lyre_proto1/crt0.S                  |  274 ++++
 firmware/target/arm/at91sam/lyre_proto1/button-lyre_proto1.c    |   99 +
 firmware/target/arm/at91sam/lyre_proto1/lcd-target.h            |   24 
 firmware/target/arm/at91sam/lyre_proto1/timer-lyre_proto1.c     |  123 +
 firmware/target/arm/at91sam/lyre_proto1/kernel-lyre_proto1.c    |   78 +
 firmware/target/arm/at91sam/boot.lds                            |   81 +
 24 files changed, 2030 insertions(+), 3 deletions(-)

Loading...