Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Applications
  • Assigned To No-one
  • Operating System Another
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by funman - 2008-10-07
Last edited by linuxstb - 2008-10-12

FS#9467 - Clip port with LCD driver

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.

Closed by  linuxstb
2008-10-12 16:46
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

Committed as r18782

Project Manager

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?

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.

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.

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.

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)

Did you upload the right patch? This one is identical to the first patch, with the exception of crt0.S being removed.

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.

Here is the right patch, with the message in show_logo.c being conditional on LCD_WIDTH

hum here is the patch

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing