Rockbox

Tasklist

FS#9467 - Clip port with LCD driver

Attached to Project: Rockbox
Opened by Rafaël Carré (funman) - Tuesday, 07 October 2008, 23:00 GMT
Last edited by Dave Chapman (linuxstb) - Sunday, 12 October 2008, 16:46 GMT
Task Type Patches
Category Applications
Status Closed
Assigned To No-one
Operating System Another
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Basic port with only bootloader support.

LCD driver for SSD1303 controller

Note that you need a modified version of mkamsboot to run this bootloader.

The patch also shortens a bit the string in show_logo.c to make it fit on the Clip screen.
This task depends upon

Closed by  Dave Chapman (linuxstb)
Sunday, 12 October 2008, 16:46 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as r18782
Comment by Daniel Stenberg (bagder) - Friday, 10 October 2008, 07:40 GMT
Is your plan to make the firmware/target/arm/crt0.S a generic ARM version or how come it isn't in the 3525 dir?
Comment by Rafaël Carré (funman) - Friday, 10 October 2008, 13:17 GMT
Yes that's the plan, in fact there is a reference to this non-existing (in svn) file in firmware/SOURCES if CPU_ARM is defined.

I copied the file from firmware/target/arm/s5l8700 and remove some commented code, and a word after the ARM vectors ('DFUC')

The FIQ and IRQ handler are defined in system-xx.c, so they can be specific to each port using this C runtime.

However I am not sure about the software interrupt handler, maybe it need to call a C function, which would just return or do something useful depending on the port.
Comment by Dave Chapman (linuxstb) - Saturday, 11 October 2008, 18:27 GMT
A few comments...

1) What is the source of the as3525.h file, and have you made any changes to it? If it's been changed, then I would like to commit the original version to SVN first, and then the version included in this patch, so we have a history of changes.

2) You mentioned in IRC that the LCD driver has parts copied from firmware/target/arm/tcc77x/lcd-ssd1815.c and parts which have been written from scratch. This means the driver is similar to the existing one, but unnecessarily (IMO) different in lots of places. It would be nicer to start with a copy of lcd-ssd1815.c and then modify that as required - so a "diff" between the two versions clearly shows what is actually different.

You also mentioned in IRC that the inits in lcd-ssd1303.c were taken from the original firmware - it would be nice to document this fact.

3) I've just moved the s5l8700/crt0.S file in SVN up one level - so we can share it rather than add a new file.

4) Similarly, if there are any other files you've copied/pasted from, it would be nice to list them, so an "svn cp" can be done when committing.

Sorry if this seems like hard work instead of fun hacking, but a little effort now makes things more maintainable in the future - when people look back to try and work out why things were done a certain way.
Comment by Rafaël Carré (funman) - Saturday, 11 October 2008, 19:09 GMT
a few answers ;)

1) the source of this file is  FS#8843  and the modifications I have made are cosmetics and registers addition

2) When I started this, I was not sure if the commands used in SSD1815 and SSD1303 controllers were the same, but it looks like they are exactly the same, so this can be done.

True there should be a comment which says that the init routine is greatly inspired from the OF (i.e. some code was removed, so the version here is more compact and understandable)

3) Ok

4) I copied just every .h file in as3525/clip/ but only to get the 'skeleton', i.e. they don't have any actual content, besides the symbol necessary for compilation.
Comment by Rafaël Carré (funman) - Saturday, 11 October 2008, 20:23 GMT
New tested patch:

* I removed crt0.S
* I started over lcd-ssd1815.c (I removed TCC77x specific code, as well as commands not shared between SSD1815 and SSD1303, and added requested comments)
* I added button code for the Clip (keypad scanning)
Comment by Dave Chapman (linuxstb) - Sunday, 12 October 2008, 06:30 GMT
Did you upload the right patch? This one is identical to the first patch, with the exception of crt0.S being removed.
Comment by Dave Chapman (linuxstb) - Sunday, 12 October 2008, 07:07 GMT
One comment I forgot to make about your original patch - the change in "showlogo.c" affects targets with larger LCDs, so I think it would be nicer to make the displayed string depend on LCD_WIDTH - e.g. use your text if LCD_WIDTH <= 128, and the existing text otherwise.
Comment by Rafaël Carré (funman) - Sunday, 12 October 2008, 14:11 GMT
Here is the right patch, with the message in show_logo.c being conditional on LCD_WIDTH
Comment by Rafaël Carré (funman) - Sunday, 12 October 2008, 14:13 GMT
hum here is the patch

Loading...