Comments:
* I am concerned at the unmodified duplication of so much code. Given
that so little core AHCI modification was done, I would be interested in
exploring alternatives to wholesale code duplication.
* Avoid global variables. Use a private struct and hpriv rather than
creating globals such as 'amba_base'
* calls to ioremap() must handle NULL return values. You can
ahci_save_initial_config() from a simple table lookup to an actual map
* Don't add magic numbers like '0' to replace AHCI_PCI_BAR. Create your
own symbol AMBA_IOMAP_IDX or somesuch, with value zero.
* ahci_configure_dma_masks() is wrong. You must call generic DMA
mapping API or amba bus DMA mapping API (if exists), since you cannot
call PCI DMA mapping API.
* amba_request_regions() should pass the driver name ("ahci"), not the
more-generic "sata"
* we put spaces between all function parameters. You incorrect elide
spaces such as in this example:
- rc = ahci_reset_controller(host);
+ rc = ahci_reset_controller(host,pdev);
* there is nothing AMBA-specific about the implementation of function
ata_amba_remove_one(). I would rather ata_amba_remove_one() be a
one-line call to a function that works on generic struct device-based
devices.
Jeff
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
