Patchwork [U-Boot,V2] i.MX6: mx6q_sabrelite: add SATA bindings

login
register
mail settings
Submitter Eric Nelson
Date March 13, 2012, 4:59 p.m.
Message ID <1331657978-19270-1-git-send-email-eric.nelson@boundarydevices.com>
Download mbox | patch
Permalink /patch/146447/
State Superseded
Headers show

Comments

Eric Nelson - March 13, 2012, 4:59 p.m.
Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
This patch requires Stefano's driver for MX5/MX6.
	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html

V2 updated based on comments from the ML:
	- uses data structures imx_ccm_reg and iomuxc_base_regs 
	for register offsets and constants from imx-regs.h and iomux-v3.h
	- moves setup_sata() from board_early_init_f() into board_init()

 arch/arm/include/asm/arch-mx6/imx-regs.h      |    9 ++
 arch/arm/include/asm/arch-mx6/iomux-v3.h      |  101 +++++++++++++++++++++++++
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |   59 ++++++++++++++
 include/configs/mx6qsabrelite.h               |   13 +++
 4 files changed, 182 insertions(+), 0 deletions(-)
Dirk Behme - March 14, 2012, 2:53 p.m.
On 13.03.2012 17:59, Eric Nelson wrote:
...
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
...
> +	/* Enable sata clock */
> +	reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
> +	reg |= MXC_CCM_CCGR5_CG2_MASK;
> +	writel(reg, &imx_ccm->CCGR5);

We touch the CCGR5 already in the imximage.cfg. So we could drop this 
code completely and just add the MXC_CCM_CCGR5_CG2_MASK to the imximage.cfg.

What are the advantages/disadvantages of this?

Advantages:

Less code, touch the register only once in imximage.cfg.

Disadvantages:

Less readability, doing it in setup_sata() instead of imximage.cfg is 
easier to understand and disable (by removing CONFIG_CMD_SATA).

Opinions?

It sounds like this results in the basic question: Which registers 
should be touched in imximage.cfg, and which by explicit code in 
drivers/board files?

Best regards

Dirk
Stefano Babic - March 15, 2012, 8:28 a.m.
On 14/03/2012 15:53, Dirk Behme wrote:
> On 13.03.2012 17:59, Eric Nelson wrote:
> ...
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> ...
>> +    /* Enable sata clock */
>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>> +    writel(reg, &imx_ccm->CCGR5);
> 

Hi Dirk,

> We touch the CCGR5 already in the imximage.cfg. So we could drop this
> code completely and just add the MXC_CCM_CCGR5_CG2_MASK to the
> imximage.cfg.
> 
> What are the advantages/disadvantages of this?
> 
> Advantages:
> 
> Less code, touch the register only once in imximage.cfg.
> 
> Disadvantages:
> 
> Less readability, doing it in setup_sata() instead of imximage.cfg is
> easier to understand and disable (by removing CONFIG_CMD_SATA).
> 
> Opinions?
> 
> It sounds like this results in the basic question: Which registers
> should be touched in imximage.cfg, and which by explicit code in
> drivers/board files?

Of course the board maintainer can decide which is the best for its
custom board. My personal rule of thumb is to put into imximage.cfg only
the initialization of the DRAM controller, so that the SOC is able to
copy u-boot into RAM, while the rest is done in code.

Best regards,
Stefano
Liu Hui-R64343 - March 15, 2012, 9:34 a.m.
>-----Original Message-----
>From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@lists.denx.de]
>On Behalf Of Stefano Babic
>Sent: Thursday, March 15, 2012 4:28 PM
>To: Dirk Behme
>Cc: u-boot@lists.denx.de; wg@denx.de
>Subject: Re: [U-Boot] [PATCH V2] i.MX6: mx6q_sabrelite: add SATA bindings
>
>On 14/03/2012 15:53, Dirk Behme wrote:
>> On 13.03.2012 17:59, Eric Nelson wrote:
>> ...
>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> ...
>>> +    /* Enable sata clock */
>>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>>> +    writel(reg, &imx_ccm->CCGR5);
>>
>
>Hi Dirk,
>
>> We touch the CCGR5 already in the imximage.cfg. So we could drop this
>> code completely and just add the MXC_CCM_CCGR5_CG2_MASK to the
>> imximage.cfg.
>>
>> What are the advantages/disadvantages of this?
>>
>> Advantages:
>>
>> Less code, touch the register only once in imximage.cfg.
>>
>> Disadvantages:
>>
>> Less readability, doing it in setup_sata() instead of imximage.cfg is
>> easier to understand and disable (by removing CONFIG_CMD_SATA).
>>
>> Opinions?
>>
>> It sounds like this results in the basic question: Which registers
>> should be touched in imximage.cfg, and which by explicit code in
>> drivers/board files?
>
>Of course the board maintainer can decide which is the best for its custom
>board. My personal rule of thumb is to put into imximage.cfg only the
>initialization of the DRAM controller, so that the SOC is able to copy u-boot
>into RAM, while the rest is done in code.

As the comments in the imximage.cfg,

# set the default clock gate to save power
DATA 4 0x020c4068 0x00C03F3F
DATA 4 0x020c406c 0x0030FC03
DATA 4 0x020c4070 0x0FFFC000
DATA 4 0x020c4074 0x3FF00000
DATA 4 0x020c4078 0x00FFF300
DATA 4 0x020c407c 0x0F0000C3
DATA 4 0x020c4080 0x000003FF

This is my intention for the default setting to gate clock as much as possible in
order to save power. This also means that when other periph added you need
Turn on the clock explicitly. So, my suggestion is:
- don't change this imximage.cfg,
- turn on the clock you need in the driver code,

Jason Liu
>
>Best regards,
>Stefano
>
>
>--
>=================================================================
>====
>DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
>HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de
>=================================================================
>====
>_______________________________________________
>U-Boot mailing list
>U-Boot@lists.denx.de
>http://lists.denx.de/mailman/listinfo/u-boot
Stefano Babic - March 15, 2012, 9:58 a.m.
On 15/03/2012 10:34, Liu Hui-R64343 wrote:

> As the comments in the imximage.cfg,
> 
> # set the default clock gate to save power
> DATA 4 0x020c4068 0x00C03F3F
> DATA 4 0x020c406c 0x0030FC03
> DATA 4 0x020c4070 0x0FFFC000
> DATA 4 0x020c4074 0x3FF00000
> DATA 4 0x020c4078 0x00FFF300
> DATA 4 0x020c407c 0x0F0000C3
> DATA 4 0x020c4080 0x000003FF
> 
> This is my intention for the default setting to gate clock as much as possible in
> order to save power. This also means that when other periph added you need
> Turn on the clock explicitly.

Indeed, this is a good idea and respects U-boot principles: enable
something only when it is needed.

> So, my suggestion is:
> - don't change this imximage.cfg,
> - turn on the clock you need in the driver code,

Agree

Best regards,
Stefano Babic
Dirk Behme - March 24, 2012, 7:09 a.m.
Hi Stefano,

On 13.03.2012 17:59, Eric Nelson wrote:
> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>

Should this patch go into your u-boot-imx.git -next?

Best regards

Dirk

> ---
> This patch requires Stefano's driver for MX5/MX6.
> 	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html
>
> V2 updated based on comments from the ML:
> 	- uses data structures imx_ccm_reg and iomuxc_base_regs
> 	for register offsets and constants from imx-regs.h and iomux-v3.h
> 	- moves setup_sata() from board_early_init_f() into board_init()
>
>   arch/arm/include/asm/arch-mx6/imx-regs.h      |    9 ++
>   arch/arm/include/asm/arch-mx6/iomux-v3.h      |  101 +++++++++++++++++++++++++
>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |   59 ++++++++++++++
>   include/configs/mx6qsabrelite.h               |   13 +++
>   4 files changed, 182 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
> index 3e5c4c2..c450afb 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -297,5 +297,14 @@ struct aipstz_regs {
>   	u32	opacr4;
>   };
>
> +struct iomuxc_base_regs {
> +	u32     gpr[14];        /* 0x000 */
> +	u32     obsrv[5];       /* 0x038 */
> +	u32     swmux_ctl[197]; /* 0x04c */
> +	u32     swpad_ctl[250]; /* 0x360 */
> +	u32     swgrp[26];      /* 0x748 */
> +	u32     daisy[104];     /* 0x7b0..94c */
> +};
> +
>   #endif /* __ASSEMBLER__*/
>   #endif /* __ASM_ARCH_MX6_IMX_REGS_H__ */
> diff --git a/arch/arm/include/asm/arch-mx6/iomux-v3.h b/arch/arm/include/asm/arch-mx6/iomux-v3.h
> index 4558f4f..a3e0169 100644
> --- a/arch/arm/include/asm/arch-mx6/iomux-v3.h
> +++ b/arch/arm/include/asm/arch-mx6/iomux-v3.h
> @@ -100,4 +100,105 @@ typedef u64 iomux_v3_cfg_t;
>   int imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
>   int imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t *pad_list, unsigned count);
>
> +/*
> + * IOMUXC_GPR13 bit fields
> + */
> +#define IOMUXC_GPR13_SDMA_STOP_REQ	(1<<30)
> +#define IOMUXC_GPR13_CAN2_STOP_REQ	(1<<29)
> +#define IOMUXC_GPR13_CAN1_STOP_REQ	(1<<28)
> +#define IOMUXC_GPR13_ENET_STOP_REQ	(1<<27)
> +#define IOMUXC_GPR13_SATA_PHY_8_MASK	(7<<24)
> +#define IOMUXC_GPR13_SATA_PHY_7_MASK	(0x1f<<19)
> +#define IOMUXC_GPR13_SATA_PHY_6_SHIFT	16
> +#define IOMUXC_GPR13_SATA_PHY_6_MASK	(7<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
> +#define IOMUXC_GPR13_SATA_SPEED_MASK	(1<<15)
> +#define IOMUXC_GPR13_SATA_PHY_5_MASK	(1<<14)
> +#define IOMUXC_GPR13_SATA_PHY_4_MASK	(7<<11)
> +#define IOMUXC_GPR13_SATA_PHY_3_MASK	(0x1f<<7)
> +#define IOMUXC_GPR13_SATA_PHY_2_MASK	(0x1f<<2)
> +#define IOMUXC_GPR13_SATA_PHY_1_MASK	(3<<0)
> +
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_0P5DB	(0b000<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P0DB	(0b001<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P5DB	(0b010<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P0DB	(0b011<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P5DB	(0b100<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB	(0b101<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P5DB	(0b110<<24)
> +#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_4P0DB	(0b111<<24)
> +
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA1I	(0b10000<<19)
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA1M	(0b10000<<19)
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA1X	(0b11010<<19)
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA2I	(0b10010<<19)
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA2M	(0b10010<<19)
> +#define IOMUXC_GPR13_SATA_PHY_7_SATA2X	(0b11010<<19)
> +
> +#define IOMUXC_GPR13_SATA_SPEED_1P5G	(0<<15)
> +#define IOMUXC_GPR13_SATA_SPEED_3G	(1<<15)
> +
> +#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED	(0<<14)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_ENABLED		(1<<14)
> +
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_16_16	(0<<11)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_14_16	(1<<11)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_12_16	(2<<11)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_10_16	(3<<11)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16		(4<<11)
> +#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_8_16		(5<<11)
> +
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB	(0b0000<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P37_DB	(0b0001<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P74_DB	(0b0010<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P11_DB	(0b0011<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P48_DB	(0b0100<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P85_DB	(0b0101<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P22_DB	(0b0110<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P59_DB	(0b0111<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P96_DB	(0b1000<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P33_DB	(0b1001<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P70_DB	(0b1010<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P07_DB	(0b1011<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P44_DB	(0b1100<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P81_DB	(0b1101<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P28_DB	(0b1110<<7)
> +#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P75_DB	(0b1111<<7)
> +
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P937V	(0b00000<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P947V	(0b00001<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P957V	(0b00010<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P966V	(0b00011<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P976V	(0b00100<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P986V	(0b00101<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_0P996V	(0b00110<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P005V	(0b00111<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P015V	(0b01000<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P025V	(0b01001<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P035V	(0b01010<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P045V	(0b01011<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P054V	(0b01100<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P064V	(0b01101<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P074V	(0b01110<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P084V	(0b01111<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P094V	(0b10000<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P104V	(0b10001<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P113V	(0b10010<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P123V	(0b10011<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P133V	(0b10100<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P143V	(0b10101<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P152V	(0b10110<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P162V	(0b10111<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P172V	(0b11000<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P182V	(0b11001<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P191V	(0b11010<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P201V	(0b11011<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P211V	(0b11100<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P221V	(0b11101<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P230V	(0b11110<<2)
> +#define IOMUXC_GPR13_SATA_PHY_2_TX_1P240V	(0b11111<<2)
> +
> +#define IOMUXC_GPR13_SATA_PHY_1_FAST	0
> +#define IOMUXC_GPR13_SATA_PHY_1_MEDIUM	1
> +#define IOMUXC_GPR13_SATA_PHY_1_SLOW	2
> +
>   #endif	/* __MACH_IOMUX_V3_H__*/
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index 1d09a72..5915159 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -23,6 +23,7 @@
>   #include<common.h>
>   #include<asm/io.h>
>   #include<asm/arch/imx-regs.h>
> +#include<asm/arch/ccm_regs.h>
>   #include<asm/arch/mx6x_pins.h>
>   #include<asm/arch/iomux-v3.h>
>   #include<asm/errno.h>
> @@ -267,6 +268,60 @@ int board_eth_init(bd_t *bis)
>   	return 0;
>   }
>
> +#ifdef CONFIG_CMD_SATA
> +
> +int setup_sata(void)
> +{
> +	u32 reg = 0;
> +	s32 timeout = 100000;
> +	struct imx_ccm_reg *const imx_ccm
> +		= (struct imx_ccm_reg *) CCM_BASE_ADDR;
> +	struct iomuxc_base_regs *const iomuxc_regs
> +		= (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
> +
> +	/* Enable sata clock */
> +	reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
> +	reg |= MXC_CCM_CCGR5_CG2_MASK;
> +	writel(reg,&imx_ccm->CCGR5);
> +
> +	/* Enable PLLs */
> +	reg = readl(&imx_ccm->analog_pll_enet);
> +	reg&= ~BM_ANADIG_PLL_SYS_POWERDOWN;
> +	writel(reg,&imx_ccm->analog_pll_enet);
> +	reg |= BM_ANADIG_PLL_SYS_ENABLE;
> +	while (timeout--) {
> +		if (readl(&imx_ccm->analog_pll_enet)&  BM_ANADIG_PLL_SYS_LOCK)
> +			break;
> +	}
> +	if (timeout<= 0)
> +		return -1;
> +	reg&= ~BM_ANADIG_PLL_SYS_BYPASS;
> +	writel(reg,&imx_ccm->analog_pll_enet);
> +	reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
> +	writel(reg,&imx_ccm->analog_pll_enet);
> +
> +	/* Enable sata phy */
> +	reg = readl(&iomuxc_regs->gpr[13])
> +		&  (IOMUXC_GPR13_SDMA_STOP_REQ
> +		   |IOMUXC_GPR13_CAN2_STOP_REQ
> +		   |IOMUXC_GPR13_CAN1_STOP_REQ
> +		   |IOMUXC_GPR13_ENET_STOP_REQ);
> +
> +	reg |= IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
> +		|IOMUXC_GPR13_SATA_PHY_7_SATA2M
> +		|IOMUXC_GPR13_SATA_SPEED_3G
> +		|(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
> +		|IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
> +		|IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
> +		|IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
> +		|IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
> +		|IOMUXC_GPR13_SATA_PHY_1_SLOW;
> +	writel(reg,&iomuxc_regs->gpr[13]);
> +
> +	return 0;
> +}
> +#endif
> +
>   int board_early_init_f(void)
>   {
>          setup_iomux_uart();
> @@ -283,6 +338,10 @@ int board_init(void)
>   	setup_spi();
>   #endif
>
> +#ifdef CONFIG_CMD_SATA
> +	setup_sata();
> +#endif
> +
>          return 0;
>   }
>
> diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
> index c851559..a3c97c6 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -71,6 +71,19 @@
>   #define CONFIG_CMD_FAT
>   #define CONFIG_DOS_PARTITION
>
> +#define CONFIG_CMD_SATA
> +/*
> + * SATA Configs
> + */
> +#ifdef CONFIG_CMD_SATA
> +#define CONFIG_DWC_AHSATA
> +#define CONFIG_SYS_SATA_MAX_DEVICE	1
> +#define CONFIG_DWC_AHSATA_PORT_ID	0
> +#define CONFIG_DWC_AHSATA_BASE_ADDR	SATA_ARB_BASE_ADDR
> +#define CONFIG_LBA48
> +#define CONFIG_LIBATA
> +#endif
> +
>   #define CONFIG_CMD_PING
>   #define CONFIG_CMD_DHCP
>   #define CONFIG_CMD_MII
Stefano Babic - March 24, 2012, 8:19 a.m.
Am 24/03/2012 08:09, schrieb Dirk Behme:
> Hi Stefano,

Hi Dirk,

> 
> On 13.03.2012 17:59, Eric Nelson wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> 
> Should this patch go into your u-boot-imx.git -next?

mmhh...in patchworks this is marked as "Changes requested" - I do not
remember now why, but I have a couple of open questions rereading the
patch...

>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> index 1d09a72..5915159 100644
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> @@ -23,6 +23,7 @@
>>   #include<common.h>
>>   #include<asm/io.h>
>>   #include<asm/arch/imx-regs.h>
>> +#include<asm/arch/ccm_regs.h>
>>   #include<asm/arch/mx6x_pins.h>
>>   #include<asm/arch/iomux-v3.h>
>>   #include<asm/errno.h>
>> @@ -267,6 +268,60 @@ int board_eth_init(bd_t *bis)
>>       return 0;
>>   }
>>
>> +#ifdef CONFIG_CMD_SATA
>> +
>> +int setup_sata(void)
>> +{
>> +    u32 reg = 0;
>> +    s32 timeout = 100000;
>> +    struct imx_ccm_reg *const imx_ccm
>> +        = (struct imx_ccm_reg *) CCM_BASE_ADDR;
>> +    struct iomuxc_base_regs *const iomuxc_regs
>> +        = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
>> +
>> +    /* Enable sata clock */
>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>> +    writel(reg,&imx_ccm->CCGR5);
>> +
>> +    /* Enable PLLs */
>> +    reg = readl(&imx_ccm->analog_pll_enet);
>> +    reg&= ~BM_ANADIG_PLL_SYS_POWERDOWN;
>> +    writel(reg,&imx_ccm->analog_pll_enet);
>> +    reg |= BM_ANADIG_PLL_SYS_ENABLE;
>> +    while (timeout--) {
>> +        if (readl(&imx_ccm->analog_pll_enet)&  BM_ANADIG_PLL_SYS_LOCK)
>> +            break;
>> +    }
>> +    if (timeout<= 0)
>> +        return -1;
>> +    reg&= ~BM_ANADIG_PLL_SYS_BYPASS;
>> +    writel(reg,&imx_ccm->analog_pll_enet);
>> +    reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
>> +    writel(reg,&imx_ccm->analog_pll_enet);

Is it all this part really board specific or mx6 specific ? I would like
to split this part into a common and a board specific parts, and making
the common part available for other boards.

>> +
>> +    /* Enable sata phy */
>> +    reg = readl(&iomuxc_regs->gpr[13])
>> +        &  (IOMUXC_GPR13_SDMA_STOP_REQ
>> +           |IOMUXC_GPR13_CAN2_STOP_REQ
>> +           |IOMUXC_GPR13_CAN1_STOP_REQ
>> +           |IOMUXC_GPR13_ENET_STOP_REQ);

Why do you need to touch CAN bits ?

Best regards,
Stefano
Eric Nelson - March 24, 2012, 8:41 p.m.
On 03/24/2012 12:09 AM, Dirk Behme wrote:
> Hi Stefano,
>
> On 13.03.2012 17:59, Eric Nelson wrote:
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>
> Should this patch go into your u-boot-imx.git -next?
>
> Best regards
>
> Dirk
>
>> ---
>> This patch requires Stefano's driver for MX5/MX6.
>> http://lists.denx.de/pipermail/u-boot/2012-February/118530.html
>>
>> V2 updated based on comments from the ML:
>> - uses data structures imx_ccm_reg and iomuxc_base_regs
>> for register offsets and constants from imx-regs.h and iomux-v3.h
>> - moves setup_sata() from board_early_init_f() into board_init()
>>

Thanks Dirk!

I've been struggling to get some time this week to go through patchwork
and re-ping based on Graeme's request and you beat me to it.
Eric Nelson - March 24, 2012, 9:01 p.m.
Hi Stefano,

On 03/24/2012 01:19 AM, stefano babic wrote:
> Am 24/03/2012 08:09, schrieb Dirk Behme:
>> Hi Stefano,
>
> Hi Dirk,
>
>> On 13.03.2012 17:59, Eric Nelson wrote:
>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>
>> Should this patch go into your u-boot-imx.git -next?
>
> mmhh...in patchworks this is marked as "Changes requested" - I do not
> remember now why, but I have a couple of open questions rereading the
> patch...
>
Funny how that happens with time.

>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> index 1d09a72..5915159 100644
>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>
 >>> <snip>
 >>>
>>> +    u32 reg = 0;
>>> +    s32 timeout = 100000;
>>> +    struct imx_ccm_reg *const imx_ccm
>>> +        = (struct imx_ccm_reg *) CCM_BASE_ADDR;
>>> +    struct iomuxc_base_regs *const iomuxc_regs
>>> +        = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
>>> +
>>> +    /* Enable sata clock */
>>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>>> +    writel(reg,&imx_ccm->CCGR5);
>>> +
>>> +    /* Enable PLLs */
>>> +    reg = readl(&imx_ccm->analog_pll_enet);
>>> +    reg&= ~BM_ANADIG_PLL_SYS_POWERDOWN;
>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>> +    reg |= BM_ANADIG_PLL_SYS_ENABLE;
>>> +    while (timeout--) {
>>> +        if (readl(&imx_ccm->analog_pll_enet)&   BM_ANADIG_PLL_SYS_LOCK)
>>> +            break;
>>> +    }
>>> +    if (timeout<= 0)
>>> +        return -1;
>>> +    reg&= ~BM_ANADIG_PLL_SYS_BYPASS;
>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>> +    reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>
> Is it all this part really board specific or mx6 specific ? I would like
> to split this part into a common and a board specific parts, and making
> the common part available for other boards.
>

This is all generic MX6 SATA enablement and could be moved into
common code.

Is cpu/armv7/mx6/clock.c the right place?

How about routine named 'enable_sata_clock()'?

>>> +
>>> +    /* Enable sata phy */
>>> +    reg = readl(&iomuxc_regs->gpr[13])
>>> +&   (IOMUXC_GPR13_SDMA_STOP_REQ
>>> +           |IOMUXC_GPR13_CAN2_STOP_REQ
>>> +           |IOMUXC_GPR13_CAN1_STOP_REQ
>>> +           |IOMUXC_GPR13_ENET_STOP_REQ);
>
> Why do you need to touch CAN bits ?
>
Not touching them, just keeping them the way they are and
clearing out the rest (which are all SATA fields).

Perhaps this would be clearer:
	reg = readl(&iomuxc->gpr[13])
		& ~(IOMUXC_GPR13_SATA_PHY_8_MASK
		   |IOMUXC_GPR13_SATA_PHY_7_MASK
		   |IOMUXC_GPR13_SATA_PHY_6_MASK
                    |IOMUXC_GPR13_SATA_SPEED_MASK
		   |IOMUXC_GPR13_SATA_PHY_5_MASK
		   |IOMUXC_GPR13_SATA_PHY_4_MASK
		   |IOMUXC_GPR13_SATA_PHY_3_MASK
		   |IOMUXC_GPR13_SATA_PHY_2_MASK
		   |IOMUXC_GPR13_SATA_PHY_1_MASK);

Or better yet, maybe a macro in iomux-v3.h:

#define IOMUXC_GPR13_SATA_MASK (IOMUXC_GPR13_SATA_PHY_8_MASK \
				|IOMUXC_GPR13_SATA_PHY_7_MASK \
				|IOMUXC_GPR13_SATA_PHY_6_MASK \
                                 |IOMUXC_GPR13_SATA_SPEED_MASK \
				|IOMUXC_GPR13_SATA_PHY_5_MASK \
				|IOMUXC_GPR13_SATA_PHY_4_MASK \
				|IOMUXC_GPR13_SATA_PHY_3_MASK \
				|IOMUXC_GPR13_SATA_PHY_2_MASK \
				|IOMUXC_GPR13_SATA_PHY_1_MASK)

and this code in setup_sata():
	reg = readl(&iomuxc->gpr[13])
		& ~(IOMUXC_GPR13_SATA_MASK)
Stefano Babic - March 25, 2012, 10:17 a.m.
On 24/03/2012 22:01, Eric Nelson wrote:
> Hi Stefano,
> 
> On 03/24/2012 01:19 AM, stefano babic wrote:
>> Am 24/03/2012 08:09, schrieb Dirk Behme:
>>> Hi Stefano,
>>
>> Hi Dirk,
>>
>>> On 13.03.2012 17:59, Eric Nelson wrote:
>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>
>>> Should this patch go into your u-boot-imx.git -next?
>>
>> mmhh...in patchworks this is marked as "Changes requested" - I do not
>> remember now why, but I have a couple of open questions rereading the
>> patch...
>>
> Funny how that happens with time.

Well, it is not a case ;-). I scan regularly patchwork and I check for
not worked patches - however, if a patch is in "Changes requested", I
expect a new version and I do not care about it. Dirk's message let me
convince that the patch was not deeply reviewed and I checked it again.
So many thanks to Dirk.

> 
>>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> index 1d09a72..5915159 100644
>>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>>
>>>> <snip>
>>>>
>>>> +    u32 reg = 0;
>>>> +    s32 timeout = 100000;
>>>> +    struct imx_ccm_reg *const imx_ccm
>>>> +        = (struct imx_ccm_reg *) CCM_BASE_ADDR;
>>>> +    struct iomuxc_base_regs *const iomuxc_regs
>>>> +        = (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
>>>> +
>>>> +    /* Enable sata clock */
>>>> +    reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
>>>> +    reg |= MXC_CCM_CCGR5_CG2_MASK;
>>>> +    writel(reg,&imx_ccm->CCGR5);
>>>> +
>>>> +    /* Enable PLLs */
>>>> +    reg = readl(&imx_ccm->analog_pll_enet);
>>>> +    reg&= ~BM_ANADIG_PLL_SYS_POWERDOWN;
>>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>>> +    reg |= BM_ANADIG_PLL_SYS_ENABLE;
>>>> +    while (timeout--) {
>>>> +        if (readl(&imx_ccm->analog_pll_enet)&  
>>>> BM_ANADIG_PLL_SYS_LOCK)
>>>> +            break;
>>>> +    }
>>>> +    if (timeout<= 0)
>>>> +        return -1;
>>>> +    reg&= ~BM_ANADIG_PLL_SYS_BYPASS;
>>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>>> +    reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
>>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>
>> Is it all this part really board specific or mx6 specific ? I would like
>> to split this part into a common and a board specific parts, and making
>> the common part available for other boards.
>>
> 
> This is all generic MX6 SATA enablement and could be moved into
> common code.
> 
> Is cpu/armv7/mx6/clock.c the right place?
> 
> How about routine named 'enable_sata_clock()'?

ok, agree.

> 
>>>> +
>>>> +    /* Enable sata phy */
>>>> +    reg = readl(&iomuxc_regs->gpr[13])
>>>> +&   (IOMUXC_GPR13_SDMA_STOP_REQ
>>>> +           |IOMUXC_GPR13_CAN2_STOP_REQ
>>>> +           |IOMUXC_GPR13_CAN1_STOP_REQ
>>>> +           |IOMUXC_GPR13_ENET_STOP_REQ);
>>
>> Why do you need to touch CAN bits ?
>>
> Not touching them, just keeping them the way they are and
> clearing out the rest (which are all SATA fields).

Then these three are superfluous, are not they ? You read the register,
and putting in "or" with IOMUXC_GPR13_CANx_STOP_REQ does nothing.

Why do you not use clrsetbits to change the sata bits ?

> 
> Perhaps this would be clearer:
>     reg = readl(&iomuxc->gpr[13])
>         & ~(IOMUXC_GPR13_SATA_PHY_8_MASK
>            |IOMUXC_GPR13_SATA_PHY_7_MASK
>            |IOMUXC_GPR13_SATA_PHY_6_MASK
>                    |IOMUXC_GPR13_SATA_SPEED_MASK
>            |IOMUXC_GPR13_SATA_PHY_5_MASK
>            |IOMUXC_GPR13_SATA_PHY_4_MASK
>            |IOMUXC_GPR13_SATA_PHY_3_MASK
>            |IOMUXC_GPR13_SATA_PHY_2_MASK
>            |IOMUXC_GPR13_SATA_PHY_1_MASK);
> 
> Or better yet, maybe a macro in iomux-v3.h:

clrsetbits does exactly what you want, I think.

Best regards,
Stefano Babic
Eric Nelson - March 25, 2012, 3:33 p.m.
Hi Stefano,

On 03/25/2012 03:17 AM, Stefano Babic wrote:
> On 24/03/2012 22:01, Eric Nelson wrote:
>> Hi Stefano,
>>
>> On 03/24/2012 01:19 AM, stefano babic wrote:
>>> Am 24/03/2012 08:09, schrieb Dirk Behme:
>>>> Hi Stefano,
>>>
>>> Hi Dirk,
>>>
>>>> On 13.03.2012 17:59, Eric Nelson wrote:
>>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>>
>>>> Should this patch go into your u-boot-imx.git -next?
>>>
>>> mmhh...in patchworks this is marked as "Changes requested" - I do not
>>> remember now why, but I have a couple of open questions rereading the
>>> patch...
>>>
>> Funny how that happens with time.
>
> Well, it is not a case ;-). I scan regularly patchwork and I check for
> not worked patches - however, if a patch is in "Changes requested", I
> expect a new version and I do not care about it. Dirk's message let me
> convince that the patch was not deeply reviewed and I checked it again.
> So many thanks to Dirk.
>
Ack that thanks to Dirk!

>>
>>>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>>> index 1d09a72..5915159 100644
>>>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>>>
>>>>> <snip>
>>>>>
>>>>> +    if (timeout<= 0)
>>>>> +        return -1;
>>>>> +    reg&= ~BM_ANADIG_PLL_SYS_BYPASS;
>>>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>>>> +    reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
>>>>> +    writel(reg,&imx_ccm->analog_pll_enet);
>>>
>>> Is it all this part really board specific or mx6 specific ? I would like
>>> to split this part into a common and a board specific parts, and making
>>> the common part available for other boards.
>>>
>>
>> This is all generic MX6 SATA enablement and could be moved into
>> common code.
>>
>> Is cpu/armv7/mx6/clock.c the right place?
>>
>> How about routine named 'enable_sata_clock()'?
>
> ok, agree.
>
Will do.

>>
>>>>> +
>>>>> +    /* Enable sata phy */
>>>>> +    reg = readl(&iomuxc_regs->gpr[13])
>>>>> +&    (IOMUXC_GPR13_SDMA_STOP_REQ
>>>>> +           |IOMUXC_GPR13_CAN2_STOP_REQ
>>>>> +           |IOMUXC_GPR13_CAN1_STOP_REQ
>>>>> +           |IOMUXC_GPR13_ENET_STOP_REQ);
>>>
>>> Why do you need to touch CAN bits ?
>>>
>> Not touching them, just keeping them the way they are and
>> clearing out the rest (which are all SATA fields).
>
> Then these three are superfluous, are not they ? You read the register,
> and putting in "or" with IOMUXC_GPR13_CANx_STOP_REQ does nothing.
>
> Why do you not use clrsetbits to change the sata bits ?
>
There's a simple answer for this: I hadn't heard of 'clrsetbits' before.

>>
>> Perhaps this would be clearer:
>>      reg = readl(&iomuxc->gpr[13])
>>          &  ~(IOMUXC_GPR13_SATA_PHY_8_MASK
>>             |IOMUXC_GPR13_SATA_PHY_7_MASK
>>             |IOMUXC_GPR13_SATA_PHY_6_MASK
>>                     |IOMUXC_GPR13_SATA_SPEED_MASK
>>             |IOMUXC_GPR13_SATA_PHY_5_MASK
>>             |IOMUXC_GPR13_SATA_PHY_4_MASK
>>             |IOMUXC_GPR13_SATA_PHY_3_MASK
>>             |IOMUXC_GPR13_SATA_PHY_2_MASK
>>             |IOMUXC_GPR13_SATA_PHY_1_MASK);
>>
>> Or better yet, maybe a macro in iomux-v3.h:
>
> clrsetbits does exactly what you want, I think.
>
Ok. I'll address in a V2.

Patch

diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h b/arch/arm/include/asm/arch-mx6/imx-regs.h
index 3e5c4c2..c450afb 100644
--- a/arch/arm/include/asm/arch-mx6/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
@@ -297,5 +297,14 @@  struct aipstz_regs {
 	u32	opacr4;
 };
 
+struct iomuxc_base_regs {
+	u32     gpr[14];        /* 0x000 */
+	u32     obsrv[5];       /* 0x038 */
+	u32     swmux_ctl[197]; /* 0x04c */
+	u32     swpad_ctl[250]; /* 0x360 */
+	u32     swgrp[26];      /* 0x748 */
+	u32     daisy[104];     /* 0x7b0..94c */
+};
+
 #endif /* __ASSEMBLER__*/
 #endif /* __ASM_ARCH_MX6_IMX_REGS_H__ */
diff --git a/arch/arm/include/asm/arch-mx6/iomux-v3.h b/arch/arm/include/asm/arch-mx6/iomux-v3.h
index 4558f4f..a3e0169 100644
--- a/arch/arm/include/asm/arch-mx6/iomux-v3.h
+++ b/arch/arm/include/asm/arch-mx6/iomux-v3.h
@@ -100,4 +100,105 @@  typedef u64 iomux_v3_cfg_t;
 int imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
 int imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t *pad_list, unsigned count);
 
+/*
+ * IOMUXC_GPR13 bit fields
+ */
+#define IOMUXC_GPR13_SDMA_STOP_REQ	(1<<30)
+#define IOMUXC_GPR13_CAN2_STOP_REQ	(1<<29)
+#define IOMUXC_GPR13_CAN1_STOP_REQ	(1<<28)
+#define IOMUXC_GPR13_ENET_STOP_REQ	(1<<27)
+#define IOMUXC_GPR13_SATA_PHY_8_MASK	(7<<24)
+#define IOMUXC_GPR13_SATA_PHY_7_MASK	(0x1f<<19)
+#define IOMUXC_GPR13_SATA_PHY_6_SHIFT	16
+#define IOMUXC_GPR13_SATA_PHY_6_MASK	(7<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
+#define IOMUXC_GPR13_SATA_SPEED_MASK	(1<<15)
+#define IOMUXC_GPR13_SATA_PHY_5_MASK	(1<<14)
+#define IOMUXC_GPR13_SATA_PHY_4_MASK	(7<<11)
+#define IOMUXC_GPR13_SATA_PHY_3_MASK	(0x1f<<7)
+#define IOMUXC_GPR13_SATA_PHY_2_MASK	(0x1f<<2)
+#define IOMUXC_GPR13_SATA_PHY_1_MASK	(3<<0)
+
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_0P5DB	(0b000<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P0DB	(0b001<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_1P5DB	(0b010<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P0DB	(0b011<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_2P5DB	(0b100<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB	(0b101<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P5DB	(0b110<<24)
+#define IOMUXC_GPR13_SATA_PHY_8_RXEQ_4P0DB	(0b111<<24)
+
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1I	(0b10000<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1M	(0b10000<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA1X	(0b11010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2I	(0b10010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2M	(0b10010<<19)
+#define IOMUXC_GPR13_SATA_PHY_7_SATA2X	(0b11010<<19)
+
+#define IOMUXC_GPR13_SATA_SPEED_1P5G	(0<<15)
+#define IOMUXC_GPR13_SATA_SPEED_3G	(1<<15)
+
+#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED	(0<<14)
+#define IOMUXC_GPR13_SATA_SATA_PHY_5_SS_ENABLED		(1<<14)
+
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_16_16	(0<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_14_16	(1<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_12_16	(2<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_10_16	(3<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16		(4<<11)
+#define IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_8_16		(5<<11)
+
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB	(0b0000<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P37_DB	(0b0001<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P74_DB	(0b0010<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P11_DB	(0b0011<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P48_DB	(0b0100<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_1P85_DB	(0b0101<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P22_DB	(0b0110<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P59_DB	(0b0111<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_2P96_DB	(0b1000<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P33_DB	(0b1001<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_3P70_DB	(0b1010<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P07_DB	(0b1011<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P44_DB	(0b1100<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_4P81_DB	(0b1101<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P28_DB	(0b1110<<7)
+#define IOMUXC_GPR13_SATA_PHY_3_TXBOOST_5P75_DB	(0b1111<<7)
+
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P937V	(0b00000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P947V	(0b00001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P957V	(0b00010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P966V	(0b00011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P976V	(0b00100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P986V	(0b00101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_0P996V	(0b00110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P005V	(0b00111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P015V	(0b01000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P025V	(0b01001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P035V	(0b01010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P045V	(0b01011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P054V	(0b01100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P064V	(0b01101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P074V	(0b01110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P084V	(0b01111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P094V	(0b10000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P104V	(0b10001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P113V	(0b10010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P123V	(0b10011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P133V	(0b10100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P143V	(0b10101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P152V	(0b10110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P162V	(0b10111<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P172V	(0b11000<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P182V	(0b11001<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P191V	(0b11010<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P201V	(0b11011<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P211V	(0b11100<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P221V	(0b11101<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P230V	(0b11110<<2)
+#define IOMUXC_GPR13_SATA_PHY_2_TX_1P240V	(0b11111<<2)
+
+#define IOMUXC_GPR13_SATA_PHY_1_FAST	0
+#define IOMUXC_GPR13_SATA_PHY_1_MEDIUM	1
+#define IOMUXC_GPR13_SATA_PHY_1_SLOW	2
+
 #endif	/* __MACH_IOMUX_V3_H__*/
diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 1d09a72..5915159 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -23,6 +23,7 @@ 
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/imx-regs.h>
+#include <asm/arch/ccm_regs.h>
 #include <asm/arch/mx6x_pins.h>
 #include <asm/arch/iomux-v3.h>
 #include <asm/errno.h>
@@ -267,6 +268,60 @@  int board_eth_init(bd_t *bis)
 	return 0;
 }
 
+#ifdef CONFIG_CMD_SATA
+
+int setup_sata(void)
+{
+	u32 reg = 0;
+	s32 timeout = 100000;
+	struct imx_ccm_reg *const imx_ccm
+		= (struct imx_ccm_reg *) CCM_BASE_ADDR;
+	struct iomuxc_base_regs *const iomuxc_regs
+		= (struct iomuxc_base_regs *) IOMUXC_BASE_ADDR;
+
+	/* Enable sata clock */
+	reg = readl(&imx_ccm->CCGR5); /* CCGR5 */
+	reg |= MXC_CCM_CCGR5_CG2_MASK;
+	writel(reg, &imx_ccm->CCGR5);
+
+	/* Enable PLLs */
+	reg = readl(&imx_ccm->analog_pll_enet);
+	reg &= ~BM_ANADIG_PLL_SYS_POWERDOWN;
+	writel(reg, &imx_ccm->analog_pll_enet);
+	reg |= BM_ANADIG_PLL_SYS_ENABLE;
+	while (timeout--) {
+		if (readl(&imx_ccm->analog_pll_enet) & BM_ANADIG_PLL_SYS_LOCK)
+			break;
+	}
+	if (timeout <= 0)
+		return -1;
+	reg &= ~BM_ANADIG_PLL_SYS_BYPASS;
+	writel(reg, &imx_ccm->analog_pll_enet);
+	reg |= BM_ANADIG_PLL_ENET_ENABLE_SATA;
+	writel(reg, &imx_ccm->analog_pll_enet);
+
+	/* Enable sata phy */
+	reg = readl(&iomuxc_regs->gpr[13])
+		& (IOMUXC_GPR13_SDMA_STOP_REQ
+		   |IOMUXC_GPR13_CAN2_STOP_REQ
+		   |IOMUXC_GPR13_CAN1_STOP_REQ
+		   |IOMUXC_GPR13_ENET_STOP_REQ);
+
+	reg |= IOMUXC_GPR13_SATA_PHY_8_RXEQ_3P0DB
+		|IOMUXC_GPR13_SATA_PHY_7_SATA2M
+		|IOMUXC_GPR13_SATA_SPEED_3G
+		|(3<<IOMUXC_GPR13_SATA_PHY_6_SHIFT)
+		|IOMUXC_GPR13_SATA_SATA_PHY_5_SS_DISABLED
+		|IOMUXC_GPR13_SATA_SATA_PHY_4_ATTEN_9_16
+		|IOMUXC_GPR13_SATA_PHY_3_TXBOOST_0P00_DB
+		|IOMUXC_GPR13_SATA_PHY_2_TX_1P104V
+		|IOMUXC_GPR13_SATA_PHY_1_SLOW;
+	writel(reg, &iomuxc_regs->gpr[13]);
+
+	return 0;
+}
+#endif
+
 int board_early_init_f(void)
 {
        setup_iomux_uart();
@@ -283,6 +338,10 @@  int board_init(void)
 	setup_spi();
 #endif
 
+#ifdef CONFIG_CMD_SATA
+	setup_sata();
+#endif
+
        return 0;
 }
 
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index c851559..a3c97c6 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -71,6 +71,19 @@ 
 #define CONFIG_CMD_FAT
 #define CONFIG_DOS_PARTITION
 
+#define CONFIG_CMD_SATA
+/*
+ * SATA Configs
+ */
+#ifdef CONFIG_CMD_SATA
+#define CONFIG_DWC_AHSATA
+#define CONFIG_SYS_SATA_MAX_DEVICE	1
+#define CONFIG_DWC_AHSATA_PORT_ID	0
+#define CONFIG_DWC_AHSATA_BASE_ADDR	SATA_ARB_BASE_ADDR
+#define CONFIG_LBA48
+#define CONFIG_LIBATA
+#endif
+
 #define CONFIG_CMD_PING
 #define CONFIG_CMD_DHCP
 #define CONFIG_CMD_MII