Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To No-one
  • Operating System All players
  • 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 dreamlayers - 2011-11-30
Last edited by dreamlayers - 2011-12-09

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

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.

Closed by  dreamlayers
2011-12-09 02:35
Reason for closing:  Accepted
Additional comments about closing:  

Committed in r31182

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing