Hi Tejun: Re-send this email to you and community. I'm afraid that you may be missed or don't receive it. BTW, can you help to pick up the #2 and #3(reviewed and acked by Shawn) of the v7 patch-set of this serial? Thanks in advance. 2013/7/10 HongXing Zhu <richard.zhuhongxing@xxxxxxxxx> Hi Tejun: There are some differences between generic AHCI controller and imx AHCI controller. The bits definitions of the HBA registers, the Vendor Specific registers, and the AHCI PHY clock, and the PHY signal adjustment window. - CAP_SSS(bit20) of the HOST_CAP is writable, default value is '0', should be configured to be '1' - bit0 (only one AHCI SATA port on imx6q) of the HOST_PORTS_IMPL should be set to be '1'.(default 0) - One Vendor Specific register HOST_TIMER1MS(offset:0xe0) should be configured regarding to the frequency of AHB bus clock. - Configurations of the AHCI PHY clock, and the signal parameters of the GPR13 I'm afraid that the generic ahci_platform driver is not a good place to contain imx6q ahci specific differences. As I know that, the AHCI sata on imx53 doesn't have the PHY signal parameters adjustment window, but the AHCI sata on imx6q does has one. On imx6q, the standard .port_ops = &ahci_platform_ops, is used, but .port_ops = &ahci_platform_retry_srst_ops, would be required by imx53 AHCI controller. We can put all the imx AHCI(imx6q, imx53) specific difference into the sata_imx driver, and keep ahci_platform as clean as possible, if the separate sata_imx driver can be created. How do you think about that? BTW, the version4 patch-set would be sent out a moment later, any suggests and comments are appreciated. Best Regards. 2013/7/6 Tejun Heo <tj@xxxxxxxxxx> Hello, On Sat, Jul 06, 2013 at 02:03:30AM +0000, Zhu Richard-R65037 wrote: > [Richard] Just like what we dicussed in the previous v1/v2 > patch-set. Shawn has the concerns that the IP speicific codes > shouldn't be put in the platform level. So he suggested that setup > a stand-alone driver, contained the imx6q ahci sata specific codes, > and re-use the generic ahci_platform driver as much as possible. > This imx6q standalone ahci sata driver just registers the platform > data, and the others would be handled by ahci_platform driver. Oh, I'm not objecting to the ahci specific part not being in platform code. I'm wondering whether specific handling for imx6q can be included into ahci_platform rather than being in its own driver. It just seems that there aren't too many differences. I could be misreading it so if there are enough differences to warrant a separate driver, please enlighten me. Thanks! -- tejun Best Regards Richard Zhu -----Original Message----- From: Tejun Heo [mailto:htejun@xxxxxxxxx] On Behalf Of Tejun Heo Sent: Saturday, July 06, 2013 10:08 AM To: Zhu Richard-R65037 Cc: Richard Zhu; shawn.guo@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; jgarzik@xxxxxxxxx; avorontsov@xxxxxxxxxxxxx; rob.herring@xxxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; linux-ide@xxxxxxxxxxxxxxx Subject: Re: [v3 3/3] sata: imx: add ahci sata support on imx platforms Hello, On Sat, Jul 06, 2013 at 02:03:30AM +0000, Zhu Richard-R65037 wrote: > [Richard] Just like what we dicussed in the previous v1/v2 patch-set. > Shawn has the concerns that the IP speicific codes shouldn't be put in > the platform level. So he suggested that setup a stand-alone driver, > contained the imx6q ahci sata specific codes, and re-use the generic > ahci_platform driver as much as possible. > This imx6q standalone ahci sata driver just registers the platform > data, and the others would be handled by ahci_platform driver. Oh, I'm not objecting to the ahci specific part not being in platform code. I'm wondering whether specific handling for imx6q can be included into ahci_platform rather than being in its own driver. It just seems that there aren't too many differences. I could be misreading it so if there are enough differences to warrant a separate driver, please enlighten me. Thanks! -- tejun -- 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