On Thu, Jan 06, 2011 at 06:17:56PM -0500, Jeff Garzik wrote: > >[...] > >>>It is overkill to rename the entirety of ahci_platform just for one override > >>>function. > >>>This sort of thing I would have expected to be added directly to > >>>ahci_platform.c. > >>It might be overkill for only one controller. but it is more clean and > >>readable to have different SoC specific changes in separate files, > >>especially when more SoCs need to make similar changes. > > > >I think that renaming the file is not necessary. You can just > >rename the module in the makefile. > > > >Personally I like the current approach more than putting > >controller-specific fixups directly into ahci_platform. > > My main objection is the renaming. If ahci_platform wants to export > some symbols for SoC modules like ahci_platform_cns, that's ok. > > In general, follow the library approach rather than linking modules > together in strange ways. In ahci_platform case there's almost nothing to factor out to a library. Each platform just needs its own small "fixups" that we normally pass via platform data. We would happily pass them via platform data, and initially Mac did exactly this, see: http://ns3.spinics.net/lists/linux-ide/msg39554.html You may notice that all platform-specific quirks are in the platform code, which is neat. But there is a problem: once the platform code needs to use libata, then we have to build libata into the kernel (no modules possible), i.e. http://ns3.spinics.net/lists/linux-ide/msg39656.html It's not something new, we have the same "problem" with SDHCI, USB and plenty other drivers. But the solution is simple, for example see drivers/mmc/host/sdhci-pltfm.c and sdhci-cns3xxx.c (and sdhci-esdhc-imx.c as another example of platform-specific hooks for sdhci-pltfm). Of course, we could make full-fledged platform driver just for CNS3xxx, but that's kind of overkill. IMO, it's better to make ahci_platform flexible and suitable for generic use. As for renaming, I agree that ahci_platform.c file does not need to be renamed, I still think that we could just rename the module (so far it is used only on CNS3xxx platforms, so no big deal even if anybody rely on the module name, which I doubt.) Thanks, -- Anton Vorontsov Email: cbouatmailru@xxxxxxxxx -- 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
