FS#12418 - Merge prototypes from ata-target.h files into one file
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
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.
Friday, 09 December 2011, 02:35 GMT
Reason for closing: Accepted
Additional comments about closing: Committed in r31182