Rockbox

Tasklist

FS#12418 - Merge prototypes from ata-target.h files into one file

Attached to Project: Rockbox
Opened by Boris Gjenero (dreamlayers) - Wednesday, 30 November 2011, 18:13 GMT
Last edited by Boris Gjenero (dreamlayers) - Friday, 09 December 2011, 02:35 GMT
Task Type Patches
Category Operating System/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

Details

I wanted to add STORAGE_INIT_ATTR to some functions, but the prototypes were repeated in all the ata-target.h functions. After a bit of discussion on IRC, I decided to move them into one file. I originally intended to put them in firmware/export/ata.h, but it seems better to put prototypes for users of ata.c into ata.h and prototypes for target-specific functions used by ata.c into a separate file: ata-driver.h.

In ata-driver.h, I include both ata-defines.h and ata-target.h. That is because order matters. Conditionals in ata-driver.h depend on ata-target.h. Also, ata-defines.h contains some generic defines which can be overridden in ata-target.h, and so it must only be included after ata-target.h. It's simplest to just have ata-driver.h deal with this, and have other files only include ata-driver.h. It would be easy to merge ata-driver.h and ata-defines.h.

There's only one function in both ata.h and ata-driver.h: ata_enable(). Maybe some change is needed, because letting other code disable ata without telling ata.c may allow bad things to happen. However, that would be for another patch. This patch also doesn't add any STORAGE_INIT_ATTR, because that can easily be done afterwards.

Bins compile cleanly for all stable targets and creativezvm (which has a "nasty hack"), and the patch doesn't change the resulting rockbox.bin files. I don't like how it changes so many source files. There are a few changes which could be removed from the patch or applied separately if necessary:
- In some ata-target.h, addition of #ifndef to prevent multiple inclusion
- Removal of unneeded inclusion of ata hearder files
- Including config.h in ata-target.h files which clearly depend on it
- Renaming ATA_SET_DEVICE_FEATURES to ATA_SET_PIO_TIMING
- Moving prototypes from ata.c into ata-driver.h

I'm also attaching remove_unused_ata_includes.patch, which removes unused ata includes from files which wouldn't otherwise be touched. I created it as a side-effect of creating the main patch, because I had to review those files to make sure I wasn't breaking anything when I changed those header files.
This task depends upon

Closed by  Boris Gjenero (dreamlayers)
Friday, 09 December 2011, 02:35 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed in r31182

Loading...