FS#9545 - Storage cleanup and multi-driver support

Attached to Project: Rockbox
Opened by Frank Gevaerts (fg) - Wednesday, 12 November 2008, 21:42 GMT
Last edited by Frank Gevaerts (fg) - Friday, 17 July 2009, 22:47 GMT
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


This patch add HAVE_MULTIDRIVE as opposed to HAVE_MULTIVOLUME. It also mostly implements multi-driver support.

This has not been tested yet.

Still to do:
- update usb_storage.c properly
- finish multi-driver iplementation (maybe just #define NUM_DRIVES storage_num_drives(), not clear yet
- handle NUM_VOLUMES and NUM_DRIVES properly (allocate NUM_VOLUMES*NUM_DRIVES filesystems, and loop accordingly?)
- test

Longer-term multi-driver open issues:
- storage_*() that don't take a drive argument currently get "broadcasted" to all drivers. It's not clear if that is the right thing to do in all cases
- The current implementation doesn't allow flexibility in deciding which is the primary drive.
This task depends upon

View Dependency Graph

This task blocks these from closing
 FS#10415 - Cowon D2 SD(HC) driver 
Closed by  Frank Gevaerts (fg)
Friday, 17 July 2009, 22:47 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as r21933
Comment by Frank Gevaerts (fg) - Wednesday, 19 November 2008, 21:34 GMT
- usb_storage.c updated
- some small fixes
- still totally untested

This will be split in separate MULTIVOLUME vs MULTIDRIVE and multidriver patches before commit

NUM_VOLUMES in the HAVE_MULTIDRIVE case is still not finalised
Comment by Frank Gevaerts (fg) - Wednesday, 19 November 2008, 23:03 GMT
- Fix stupidity in storage.c
- Some small fixes
- Tested on ipod mini in ATA+RAMDISK configuration. The ramdisk is accessible over USB, but not in the file browser. This could be an issue with it not being formatted on first boot.

Comment by Björn Stenberg (zagor) - Thursday, 15 January 2009, 10:33 GMT
It looks good to me. Just a little question: Do we need this? Are there known upcoming targets that use several different storage devices?

With my Code Police hat on, I don't think storage.c should be "compacted" so much though, even if has many repeated lines. Whitespace is reader friendly.
Comment by Frank Gevaerts (fg) - Thursday, 15 January 2009, 19:51 GMT
D2, Elio, and at least some of the ondas need this. In a more hypothetical future this is also needed by USB host.

Actually I want to split the patch to separate the multivolume confusion from the multi-driver work.
Comment by Maurus Cuelenaere (mcuelenaere) - Monday, 09 March 2009, 20:08 GMT
Sync with r20256 (+ fix sd_sleepnow(), nand_sleepnow() etc warnings).

Confirmed working on Onda VX747.
Comment by Maurus Cuelenaere (mcuelenaere) - Monday, 01 June 2009, 22:48 GMT
Sync with r21163
Comment by Rob Purchase (shotofadds) - Monday, 13 July 2009, 08:24 GMT
Sync to r21835, with the following fixes:

- add storage.c which was missing from v5
- fix HAVE_HOTSWAP typo
- fix warnings in storage.c
- add missing stubs to TCC NAND driver

Tested working on Cowon D2 with SD and NAND drivers (see  FS#10415 ).
Comment by Maurus Cuelenaere (mcuelenaere) - Monday, 13 July 2009, 13:18 GMT
For some reason your patch removed drivers/sd.c from SOURCES; I added it again and now the Onda VX747 build compiles again (I suppose this breaks the D2?)
Comment by Rob Purchase (shotofadds) - Monday, 13 July 2009, 13:50 GMT
Yes, it looks like I made a mistake fixing up the failed hunks. Your v7 patch looks correct.

btw. this only compiled on my D2 build because I had added sd.c elsewhere in SOURCES while hacking around previously... doh!
Comment by Frank Gevaerts (fg) - Tuesday, 14 July 2009, 20:53 GMT
This patch still misses something : the storage system asks the drivers how many drives they have, but it should also tell them what the drive numbers will be. This is needed for e.g. hotswap drives that need to call disk_mount()

If we make *num_drives accept a parameter (e.g. int sd_num_drives(int first_drive)), and storage_init() passes num_drives to all drivers, it should work I think
Comment by Rob Purchase (shotofadds) - Thursday, 16 July 2009, 22:49 GMT
Patch updated with the above change - note that I have NOT yet updated the various hotswap drivers to make use of this functionality. An example of how this can be done is in v4 of the Cowon D2 SD driver patch ( FS#10415 ).

EDIT: Fixed hunk that shouldn't have been present...
Comment by Rafaël Carré (funman) - Friday, 17 July 2009, 00:40 GMT
it would be nice to define void functions to "(void)0" in storage.h and hardcode functions returning a value to 0 or 1, to avoid dummy stubs in target drivers