On Mon, Jul 01, 2013 at 08:34:38PM +0530, Girish K S wrote: > On 1 July 2013 20:18, Shawn Guo <shawn.guo@xxxxxxxxxx> wrote: > > On Mon, Jul 01, 2013 at 04:57:35PM +0530, Girish K S wrote: > >> Hello Richard, > >> > >> Instead of writing a separate driver for the changes you mentioned in > >> the commit message. you can just add those changes to > >> the platform data (pdata-> init). > > > > No. The platform init hook is not used to do IP block related setup. > > If we're mapping ahci block and setting up ahci registers in platform > > code, we are generally not doing the right thing. > > Yes I understand (anything specific to driver should be part of > driver). I need to touch few platforms that are already doing this. > Then how about keeping setup as part of driver data for the specific controller. I was too rush in writing the reply. Here is what I meant. We should reuse the generic ahci_platform driver. And that means we have to use pdata->init() hook to do vendor specific setup. But the hook shouldn't be implemented in arch code (e.g. arch/arm/mach-imx/mach-imx6q.c), and it should be implemented in a vendor specific driver like sata_imx.c. IOW, we reuse everything implemented in the generic ahci_platform driver, and handle differences in vendor specific ahci driver via pdata->init() hook. Does that make the most sense? Shawn -- 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