diff mbox

[RFC,2/2] MX53 Enable the AHCI SATA on MX53 LOCO

Message ID 1300096544-1946-2-git-send-email-Hong-Xing.Zhu@freescale.com
State Not Applicable
Delegated to: David Miller
Headers show

Commit Message

Richard Zhu March 14, 2011, 9:55 a.m. UTC
Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
---
 arch/arm/mach-mx5/board-mx53_loco.c |  120 +++++++++++++++++++++++++++++++++++
 1 files changed, 120 insertions(+), 0 deletions(-)

Comments

Jeff Garzik March 14, 2011, 2:50 p.m. UTC | #1
On 03/14/2011 05:55 AM, Richard Zhu wrote:
> Signed-off-by: Richard Zhu<Hong-Xing.Zhu@freescale.com>

Acked-by: Jeff Garzik <jgarzik@redhat.com>


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sascha Hauer March 15, 2011, 9:02 a.m. UTC | #2
On Mon, Mar 14, 2011 at 05:55:44PM +0800, Richard Zhu wrote:
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> ---
>  arch/arm/mach-mx5/board-mx53_loco.c |  120 +++++++++++++++++++++++++++++++++++
>  1 files changed, 120 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
> index 0a18f8d..9a7bbea 100644
> --- a/arch/arm/mach-mx5/board-mx53_loco.c
> +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> @@ -23,11 +23,13 @@
>  #include <linux/fec.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>
> +#include <linux/ahci_platform.h>
>  
>  #include <mach/common.h>
>  #include <mach/hardware.h>
>  #include <mach/imx-uart.h>
>  #include <mach/iomux-mx53.h>
> +#include <mach/ahci_sata.h>
>  
>  #include <asm/mach-types.h>
>  #include <asm/mach/arch.h>
> @@ -203,6 +205,123 @@ static const struct imxi2c_platform_data mx53_loco_i2c_data __initconst = {
>  	.bitrate = 100000,
>  };
>  
> +/* HW Initialization, if return 0, initialization is successful. */
> +static int sata_init(struct device *dev, void __iomem *addr)
> +{
> +	void __iomem *mmio;
> +	struct clk *clk;
> +	int ret = 0;
> +	u32 tmpdata;
> +
> +	clk = clk_get(dev, "imx_sata_clk");
> +	ret = IS_ERR(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't get clock.\n");
> +		return ret;
> +	}

IS_ERR returns 0 or 1 which is not a valid return value here.

> +	ret = clk_enable(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't enable clock.\n");

You have a struct device *, so you should use dev_err.

> +		clk_put(clk);
> +		return ret;
> +	}
> +
> +	/* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
> +	clk = clk_get(dev, "usb_phy1");
> +	ret = IS_ERR(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> +		goto no_input_clk;
> +	}
> +	ret = clk_enable(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI Can't enable USB PHY1 clock.\n");
> +		clk_put(clk);
> +		goto no_input_clk;
> +	}
> +
> +	/* Get the AHB clock rate, and configure the TIMER1MS reg later */
> +	clk = clk_get(NULL, "ahb_clk");
> +	ret = IS_ERR(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't get AHB clock.\n");
> +		goto no_clk;
> +	}
> +
> +	mmio = ioremap(MX53_SATA_BASE_ADDR, SZ_2K);
> +	if (mmio == NULL) {
> +		printk(KERN_ERR "Failed to map SATA REGS\n");
> +		goto no_clk;
> +	}
> +
> +	tmpdata = readl(mmio + HOST_CAP);
> +	if (!(tmpdata & HOST_CAP_SSS)) {
> +		tmpdata |= HOST_CAP_SSS;
> +		writel(tmpdata, mmio + HOST_CAP);
> +	}
> +
> +	if (!(readl(mmio + HOST_PORTS_IMPL) & 0x1))
> +		writel((readl(mmio + HOST_PORTS_IMPL) | 0x1),
> +			mmio + HOST_PORTS_IMPL);
> +
> +	tmpdata = clk_get_rate(clk) / 1000;
> +	writel(tmpdata, mmio + HOST_TIMER1MS);
> +
> +	iounmap(mmio);
> +
> +	return ret;
> +
> +no_clk:
> +	clk = clk_get(dev, "usb_phy1");

While technically not strictly necessary at the moment you should
balance your clk_get / clk_put calls. Everything else is bad taste.


> +	if (IS_ERR(clk)) {
> +		clk = NULL;
> +		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> +	} else {
> +		clk_disable(clk);
> +		clk_put(clk);
> +	}
> +
> +no_input_clk:
> +	clk = clk_get(dev, "imx_sata_clk");
> +	if (IS_ERR(clk)) {
> +		clk = NULL;
> +		printk(KERN_ERR "IMX AHCI can't get clock.\n");
> +	} else {
> +		clk_disable(clk);
> +		clk_put(clk);
> +	}
> +
> +	return ret;
> +}
> +
> +static void sata_exit(struct device *dev)
> +{
> +	struct clk *clk;
> +
> +	clk = clk_get(dev, "usb_phy1");
> +	if (IS_ERR(clk)) {
> +		clk = NULL;
> +		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> +	} else {
> +		clk_disable(clk);
> +		clk_put(clk);
> +	}
> +
> +	clk = clk_get(dev, "imx_sata_clk");
> +	if (IS_ERR(clk)) {
> +		clk = NULL;
> +		printk(KERN_ERR "IMX AHCI can't get clock.\n");
> +	} else {
> +		clk_disable(clk);
> +		clk_put(clk);
> +	}
> +}
> +
> +static struct ahci_platform_data sata_data = {
> +	.init = sata_init,
> +	.exit = sata_exit,
> +};
> +
>  static void __init mx53_loco_board_init(void)
>  {
>  	mxc_iomux_v3_setup_multiple_pads(mx53_loco_pads,
> @@ -215,6 +334,7 @@ static void __init mx53_loco_board_init(void)
>  	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
>  	imx53_add_sdhci_esdhc_imx(0, NULL);
>  	imx53_add_sdhci_esdhc_imx(2, NULL);
> +	imx53_add_ahci_imx(0, &sata_data);
>  }
>  
>  static void __init mx53_loco_timer_init(void)
> -- 
> 1.7.1
> 
> 
>
Sascha Hauer March 15, 2011, 9:32 a.m. UTC | #3
On Mon, Mar 14, 2011 at 05:55:44PM +0800, Richard Zhu wrote:
> Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> ---
>  arch/arm/mach-mx5/board-mx53_loco.c |  120 +++++++++++++++++++++++++++++++++++
>  1 files changed, 120 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
> index 0a18f8d..9a7bbea 100644
> --- a/arch/arm/mach-mx5/board-mx53_loco.c
> +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> @@ -23,11 +23,13 @@
>  #include <linux/fec.h>
>  #include <linux/delay.h>
>  #include <linux/gpio.h>
> +#include <linux/ahci_platform.h>
>  
>  #include <mach/common.h>
>  #include <mach/hardware.h>
>  #include <mach/imx-uart.h>
>  #include <mach/iomux-mx53.h>
> +#include <mach/ahci_sata.h>
>  
>  #include <asm/mach-types.h>
>  #include <asm/mach/arch.h>
> @@ -203,6 +205,123 @@ static const struct imxi2c_platform_data mx53_loco_i2c_data __initconst = {
>  	.bitrate = 100000,
>  };
>  
> +/* HW Initialization, if return 0, initialization is successful. */
> +static int sata_init(struct device *dev, void __iomem *addr)

Isn't this addr pointer exactly the address you remap again below?

> +{
> +	void __iomem *mmio;
> +	struct clk *clk;
> +	int ret = 0;
> +	u32 tmpdata;
> +
> +	clk = clk_get(dev, "imx_sata_clk");

This one is associated with the device, so passing in dev here is ok,
but...

> +	ret = IS_ERR(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't get clock.\n");
> +		return ret;
> +	}
> +	ret = clk_enable(clk);
> +	if (ret) {
> +		printk(KERN_ERR "IMX AHCI can't enable clock.\n");
> +		clk_put(clk);
> +		return ret;
> +	}
> +
> +	/* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
> +	clk = clk_get(dev, "usb_phy1");

... this clock is not associated with the device, it just happens to be
used for this purpose on LOCO. Or is this this really not LOCO specific
but i.MX53 specific? If this is LOCO specific you should pass NULL as
the device pointer. If this is i.MX53 specific we should discuss about
adding a imx53_init_sata as then all of this function is not LOCO
specific.


Have these patches undergone some Linaro internal review? These are
really not ready for the wider world.

Sascha
Zhu Richard-R65037 March 16, 2011, 9:49 a.m. UTC | #4
Hi Sascha:

Best Regards
Richard Zhu


> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> Sent: Tuesday, March 15, 2011 5:03 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@lists.infradead.org; jgarzik@pobox.com;
> kernel@pengutronix.de; linux-ide@vger.kernel.org;
> avorontsov@ru.mvista.com; eric@eukrea.com; eric.miao@linaro.org
> Subject: Re: [RFC PATCH 2/2] MX53 Enable the AHCI SATA on MX53 LOCO
>
> On Mon, Mar 14, 2011 at 05:55:44PM +0800, Richard Zhu wrote:
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> > ---
> >  arch/arm/mach-mx5/board-mx53_loco.c |  120
> > +++++++++++++++++++++++++++++++++++
> >  1 files changed, 120 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-mx5/board-mx53_loco.c
> > b/arch/arm/mach-mx5/board-mx53_loco.c
> > index 0a18f8d..9a7bbea 100644
> > --- a/arch/arm/mach-mx5/board-mx53_loco.c
> > +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> > @@ -23,11 +23,13 @@
> >  #include <linux/fec.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio.h>
> > +#include <linux/ahci_platform.h>
> >
> >  #include <mach/common.h>
> >  #include <mach/hardware.h>
> >  #include <mach/imx-uart.h>
> >  #include <mach/iomux-mx53.h>
> > +#include <mach/ahci_sata.h>
> >
> >  #include <asm/mach-types.h>
> >  #include <asm/mach/arch.h>
> > @@ -203,6 +205,123 @@ static const struct imxi2c_platform_data
> mx53_loco_i2c_data __initconst = {
> >     .bitrate = 100000,
> >  };
> >
> > +/* HW Initialization, if return 0, initialization is successful. */
> > +static int sata_init(struct device *dev, void __iomem *addr) {
> > +   void __iomem *mmio;
> > +   struct clk *clk;
> > +   int ret = 0;
> > +   u32 tmpdata;
> > +
> > +   clk = clk_get(dev, "imx_sata_clk");
> > +   ret = IS_ERR(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't get clock.\n");
> > +           return ret;
> > +   }
>
> IS_ERR returns 0 or 1 which is not a valid return value here.
Accepted.

>
> > +   ret = clk_enable(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't enable clock.\n");
>
> You have a struct device *, so you should use dev_err.
Accepted.

>
> > +           clk_put(clk);
> > +           return ret;
> > +   }
> > +
> > +   /* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
> > +   clk = clk_get(dev, "usb_phy1");
> > +   ret = IS_ERR(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> > +           goto no_input_clk;
> > +   }
> > +   ret = clk_enable(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI Can't enable USB PHY1 clock.\n");
> > +           clk_put(clk);
> > +           goto no_input_clk;
> > +   }
> > +
> > +   /* Get the AHB clock rate, and configure the TIMER1MS reg later */
> > +   clk = clk_get(NULL, "ahb_clk");
> > +   ret = IS_ERR(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't get AHB clock.\n");
> > +           goto no_clk;
> > +   }
> > +
> > +   mmio = ioremap(MX53_SATA_BASE_ADDR, SZ_2K);
> > +   if (mmio == NULL) {
> > +           printk(KERN_ERR "Failed to map SATA REGS\n");
> > +           goto no_clk;
> > +   }
> > +
> > +   tmpdata = readl(mmio + HOST_CAP);
> > +   if (!(tmpdata & HOST_CAP_SSS)) {
> > +           tmpdata |= HOST_CAP_SSS;
> > +           writel(tmpdata, mmio + HOST_CAP);
> > +   }
> > +
> > +   if (!(readl(mmio + HOST_PORTS_IMPL) & 0x1))
> > +           writel((readl(mmio + HOST_PORTS_IMPL) | 0x1),
> > +                   mmio + HOST_PORTS_IMPL);
> > +
> > +   tmpdata = clk_get_rate(clk) / 1000;
> > +   writel(tmpdata, mmio + HOST_TIMER1MS);
> > +
> > +   iounmap(mmio);
> > +
> > +   return ret;
> > +
> > +no_clk:
> > +   clk = clk_get(dev, "usb_phy1");
>
> While technically not strictly necessary at the moment you should balance
> your clk_get / clk_put calls. Everything else is bad taste.
>
Yeah, you are right.
How do you think about add two clk pointers member into ahci_host_priv struct to balance
 clk_get/clk_put pair calls?
Or
Added one private pointer into ahci_host_priv to make a more flexible method for kinds of different platforms to
balance the hw resource(clk, power and so on) balance?


>
> > +   if (IS_ERR(clk)) {
> > +           clk = NULL;
> > +           printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> > +   } else {
> > +           clk_disable(clk);
> > +           clk_put(clk);
> > +   }
> > +
> > +no_input_clk:
> > +   clk = clk_get(dev, "imx_sata_clk");
> > +   if (IS_ERR(clk)) {
> > +           clk = NULL;
> > +           printk(KERN_ERR "IMX AHCI can't get clock.\n");
> > +   } else {
> > +           clk_disable(clk);
> > +           clk_put(clk);
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> > +static void sata_exit(struct device *dev) {
> > +   struct clk *clk;
> > +
> > +   clk = clk_get(dev, "usb_phy1");
> > +   if (IS_ERR(clk)) {
> > +           clk = NULL;
> > +           printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
> > +   } else {
> > +           clk_disable(clk);
> > +           clk_put(clk);
> > +   }
> > +
> > +   clk = clk_get(dev, "imx_sata_clk");
> > +   if (IS_ERR(clk)) {
> > +           clk = NULL;
> > +           printk(KERN_ERR "IMX AHCI can't get clock.\n");
> > +   } else {
> > +           clk_disable(clk);
> > +           clk_put(clk);
> > +   }
> > +}
> > +
> > +static struct ahci_platform_data sata_data = {
> > +   .init = sata_init,
> > +   .exit = sata_exit,
> > +};
> > +
> >  static void __init mx53_loco_board_init(void)  {
> >     mxc_iomux_v3_setup_multiple_pads(mx53_loco_pads,
> > @@ -215,6 +334,7 @@ static void __init mx53_loco_board_init(void)
> >     imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
> >     imx53_add_sdhci_esdhc_imx(0, NULL);
> >     imx53_add_sdhci_esdhc_imx(2, NULL);
> > +   imx53_add_ahci_imx(0, &sata_data);
> >  }
> >
> >  static void __init mx53_loco_timer_init(void)
> > --
> > 1.7.1
> >
> >
> >
>
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555
> |


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zhu Richard-R65037 March 16, 2011, 9:56 a.m. UTC | #5
Hi Sascha:

Best Regards
Richard Zhu


> -----Original Message-----
> From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
> Sent: Tuesday, March 15, 2011 5:33 PM
> To: Zhu Richard-R65037
> Cc: linux-arm-kernel@lists.infradead.org; jgarzik@pobox.com;
> kernel@pengutronix.de; linux-ide@vger.kernel.org;
> avorontsov@ru.mvista.com; eric@eukrea.com; eric.miao@linaro.org
> Subject: Re: [RFC PATCH 2/2] MX53 Enable the AHCI SATA on MX53 LOCO
>
> On Mon, Mar 14, 2011 at 05:55:44PM +0800, Richard Zhu wrote:
> > Signed-off-by: Richard Zhu <Hong-Xing.Zhu@freescale.com>
> > ---
> >  arch/arm/mach-mx5/board-mx53_loco.c |  120
> > +++++++++++++++++++++++++++++++++++
> >  1 files changed, 120 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mach-mx5/board-mx53_loco.c
> > b/arch/arm/mach-mx5/board-mx53_loco.c
> > index 0a18f8d..9a7bbea 100644
> > --- a/arch/arm/mach-mx5/board-mx53_loco.c
> > +++ b/arch/arm/mach-mx5/board-mx53_loco.c
> > @@ -23,11 +23,13 @@
> >  #include <linux/fec.h>
> >  #include <linux/delay.h>
> >  #include <linux/gpio.h>
> > +#include <linux/ahci_platform.h>
> >
> >  #include <mach/common.h>
> >  #include <mach/hardware.h>
> >  #include <mach/imx-uart.h>
> >  #include <mach/iomux-mx53.h>
> > +#include <mach/ahci_sata.h>
> >
> >  #include <asm/mach-types.h>
> >  #include <asm/mach/arch.h>
> > @@ -203,6 +205,123 @@ static const struct imxi2c_platform_data
> mx53_loco_i2c_data __initconst = {
> >     .bitrate = 100000,
> >  };
> >
> > +/* HW Initialization, if return 0, initialization is successful. */
> > +static int sata_init(struct device *dev, void __iomem *addr)
>
> Isn't this addr pointer exactly the address you remap again below?
Yes, it is.
Would re-use this parameter later.

>
> > +{
> > +   void __iomem *mmio;
> > +   struct clk *clk;
> > +   int ret = 0;
> > +   u32 tmpdata;
> > +
> > +   clk = clk_get(dev, "imx_sata_clk");
>
> This one is associated with the device, so passing in dev here is ok,
> but...
>
> > +   ret = IS_ERR(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't get clock.\n");
> > +           return ret;
> > +   }
> > +   ret = clk_enable(clk);
> > +   if (ret) {
> > +           printk(KERN_ERR "IMX AHCI can't enable clock.\n");
> > +           clk_put(clk);
> > +           return ret;
> > +   }
> > +
> > +   /* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
> > +   clk = clk_get(dev, "usb_phy1");
>
> ... this clock is not associated with the device, it just happens to be
> used for this purpose on LOCO. Or is this this really not LOCO specific
> but i.MX53 specific? If this is LOCO specific you should pass NULL as the
> device pointer. If this is i.MX53 specific we should discuss about adding
> a imx53_init_sata as then all of this function is not LOCO specific.
>
This is related to the HW design.
On MX53 SOC, the AHCI bus clk can be sourced from internal USB_PHY1 clk, or one external
crystal oscillator.
I would create external two functions later, one used for internal clk initialization, and
the other would be used when external crystal oscillator is used.
 and move these codes into the internal clk initialization function.
How do you think about it?

>
> Have these patches undergone some Linaro internal review? These are
> really not ready for the wider world.
>
> Sascha
>
> --
> Pengutronix e.K.                           |
> |
> Industrial Linux Solutions                 | http://www.pengutronix.de/
> |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0
> |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555
> |


--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-mx5/board-mx53_loco.c b/arch/arm/mach-mx5/board-mx53_loco.c
index 0a18f8d..9a7bbea 100644
--- a/arch/arm/mach-mx5/board-mx53_loco.c
+++ b/arch/arm/mach-mx5/board-mx53_loco.c
@@ -23,11 +23,13 @@ 
 #include <linux/fec.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
+#include <linux/ahci_platform.h>
 
 #include <mach/common.h>
 #include <mach/hardware.h>
 #include <mach/imx-uart.h>
 #include <mach/iomux-mx53.h>
+#include <mach/ahci_sata.h>
 
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
@@ -203,6 +205,123 @@  static const struct imxi2c_platform_data mx53_loco_i2c_data __initconst = {
 	.bitrate = 100000,
 };
 
+/* HW Initialization, if return 0, initialization is successful. */
+static int sata_init(struct device *dev, void __iomem *addr)
+{
+	void __iomem *mmio;
+	struct clk *clk;
+	int ret = 0;
+	u32 tmpdata;
+
+	clk = clk_get(dev, "imx_sata_clk");
+	ret = IS_ERR(clk);
+	if (ret) {
+		printk(KERN_ERR "IMX AHCI can't get clock.\n");
+		return ret;
+	}
+	ret = clk_enable(clk);
+	if (ret) {
+		printk(KERN_ERR "IMX AHCI can't enable clock.\n");
+		clk_put(clk);
+		return ret;
+	}
+
+	/* FSL IMX AHCI SATA uses the internal usb phy1 clk on loco */
+	clk = clk_get(dev, "usb_phy1");
+	ret = IS_ERR(clk);
+	if (ret) {
+		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
+		goto no_input_clk;
+	}
+	ret = clk_enable(clk);
+	if (ret) {
+		printk(KERN_ERR "IMX AHCI Can't enable USB PHY1 clock.\n");
+		clk_put(clk);
+		goto no_input_clk;
+	}
+
+	/* Get the AHB clock rate, and configure the TIMER1MS reg later */
+	clk = clk_get(NULL, "ahb_clk");
+	ret = IS_ERR(clk);
+	if (ret) {
+		printk(KERN_ERR "IMX AHCI can't get AHB clock.\n");
+		goto no_clk;
+	}
+
+	mmio = ioremap(MX53_SATA_BASE_ADDR, SZ_2K);
+	if (mmio == NULL) {
+		printk(KERN_ERR "Failed to map SATA REGS\n");
+		goto no_clk;
+	}
+
+	tmpdata = readl(mmio + HOST_CAP);
+	if (!(tmpdata & HOST_CAP_SSS)) {
+		tmpdata |= HOST_CAP_SSS;
+		writel(tmpdata, mmio + HOST_CAP);
+	}
+
+	if (!(readl(mmio + HOST_PORTS_IMPL) & 0x1))
+		writel((readl(mmio + HOST_PORTS_IMPL) | 0x1),
+			mmio + HOST_PORTS_IMPL);
+
+	tmpdata = clk_get_rate(clk) / 1000;
+	writel(tmpdata, mmio + HOST_TIMER1MS);
+
+	iounmap(mmio);
+
+	return ret;
+
+no_clk:
+	clk = clk_get(dev, "usb_phy1");
+	if (IS_ERR(clk)) {
+		clk = NULL;
+		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
+	} else {
+		clk_disable(clk);
+		clk_put(clk);
+	}
+
+no_input_clk:
+	clk = clk_get(dev, "imx_sata_clk");
+	if (IS_ERR(clk)) {
+		clk = NULL;
+		printk(KERN_ERR "IMX AHCI can't get clock.\n");
+	} else {
+		clk_disable(clk);
+		clk_put(clk);
+	}
+
+	return ret;
+}
+
+static void sata_exit(struct device *dev)
+{
+	struct clk *clk;
+
+	clk = clk_get(dev, "usb_phy1");
+	if (IS_ERR(clk)) {
+		clk = NULL;
+		printk(KERN_ERR "IMX AHCI can't get USB PHY1 CLK.\n");
+	} else {
+		clk_disable(clk);
+		clk_put(clk);
+	}
+
+	clk = clk_get(dev, "imx_sata_clk");
+	if (IS_ERR(clk)) {
+		clk = NULL;
+		printk(KERN_ERR "IMX AHCI can't get clock.\n");
+	} else {
+		clk_disable(clk);
+		clk_put(clk);
+	}
+}
+
+static struct ahci_platform_data sata_data = {
+	.init = sata_init,
+	.exit = sata_exit,
+};
+
 static void __init mx53_loco_board_init(void)
 {
 	mxc_iomux_v3_setup_multiple_pads(mx53_loco_pads,
@@ -215,6 +334,7 @@  static void __init mx53_loco_board_init(void)
 	imx53_add_imx_i2c(1, &mx53_loco_i2c_data);
 	imx53_add_sdhci_esdhc_imx(0, NULL);
 	imx53_add_sdhci_esdhc_imx(2, NULL);
+	imx53_add_ahci_imx(0, &sata_data);
 }
 
 static void __init mx53_loco_timer_init(void)