[U-Boot] [PATCH] i.MX6: mx6q_sabrelite: add SATA bindings

[Eric Nelson]

On 03/03/2012 02:00 AM, Stefano Babic wrote:
> On 03/03/2012 01:46, Eric Nelson wrote:
>> This patch requires Stefano's driver for MX5/MX6.
>> 	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html
>
> This is helpful, but should be not part of the commit message that is
> stored in git. Move all additional info after the "---" line.
>
>>
>> Signed-off-by: Eric Nelson<eric.nelson at boundarydevices.com>
>> ---
>>   arch/arm/include/asm/arch-mx6/imx-regs.h      |   11 +++++
>>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |   54 +++++++++++++++++++++++++
>>   include/configs/mx6qsabrelite.h               |   13 ++++++
>>   3 files changed, 78 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..2441434 100644
>> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
>> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
>> @@ -165,6 +165,17 @@
>>   #define IP2APB_USBPHY1_BASE_ADDR    (AIPS2_OFF_BASE_ADDR + 0x78000)
>>   #define IP2APB_USBPHY2_BASE_ADDR    (AIPS2_OFF_BASE_ADDR + 0x7C000)
>>
>> +/*
>> + * ANATOP register definitions
>> + */
>> +#define ANATOP_PLL_LOCK                 0x80000000
>> +#define ANATOP_PLL_ENABLE_MASK          0x00002000
>> +#define ANATOP_PLL_BYPASS_MASK          0x00010000
>> +#define ANATOP_PLL_LOCK                 0x80000000
>> +#define ANATOP_PLL_PWDN_MASK            0x00001000
>> +#define ANATOP_PLL_HOLD_RING_OFF_MASK   0x00000800
>> +#define ANATOP_SATA_CLK_ENABLE_MASK     0x00100000
>
> There is already a thread initiated by Wolfgang regarding the ANATOP
> registers:
>
> http://lists.denx.de/pipermail/u-boot/2012-February/117942.html
>
> I think we need in any case a way to consolitate this stuff, even if
> decided to postpone this activity
>

Thanks. I saw that but seem to have forgotten it.

>> +
>>   #define CHIP_REV_1_0                 0x10
>>   #define IRAM_SIZE                    0x00040000
>>   #define IMX_IIM_BASE                 OCOTP_BASE_ADDR
>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> index 2786482..e0ba6a4 100644
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> @@ -273,10 +273,64 @@ int board_eth_init(bd_t *bis)
>>   	return 0;
>>   }
>>
>> +#ifdef CONFIG_CMD_SATA
>> +
>> +int setup_sata(void)
>> +{
>> +	u32 reg = 0;
>> +	s32 timeout = 100000;
>> +
>> +	/* Enable sata clock */
>> +	reg = readl(CCM_BASE_ADDR + 0x7c); /* CCGR5 */
>> +	reg |= 0x30;
>> +	writel(reg, CCM_BASE_ADDR + 0x7c);
>
> We have not the Reference Manual, and using hexadecimal values makes
> things ready for the Obfuscated C Code Contest:
>
> http://www.ioccc.org/
>

I'll try to obfuscate more in a V2. We can win this thing! ;)

> Do not use offsets. Instead of that, provide structures to access the
> internal register. This must be fixed globally.
>
>
>> +
>> +	/* Enable PLLs */
>> +	reg = readl(ANATOP_BASE_ADDR + 0xe0); /* ENET PLL */
>> +	reg&= ~ANATOP_PLL_PWDN_MASK;
>> +	writel(reg, ANATOP_BASE_ADDR + 0xe0);
>> +	reg |= ANATOP_PLL_ENABLE_MASK;
>> +	while (timeout--) {
>> +		if (readl(ANATOP_BASE_ADDR + 0xe0)&  ANATOP_PLL_LOCK)
>> +			break;
>> +	}
>
> Ditto
>
>
>> +	if (timeout<= 0)
>> +		return -1;
>> +	reg&= ~ANATOP_PLL_BYPASS_MASK;
>> +	writel(reg, ANATOP_BASE_ADDR + 0xe0);
>> +	reg |= ANATOP_SATA_CLK_ENABLE_MASK;
>> +	writel(reg, ANATOP_BASE_ADDR + 0xe0);
>> +
>> +	/* Enable sata phy */
>> +	reg = readl(IOMUXC_BASE_ADDR + 0x34); /* GPR13 */
>
> We have functions to manage pinmux, use them
>
>

Right.

>>   int board_early_init_f(void)
>>   {
>>          setup_iomux_uart();
>>
>> +#ifdef CONFIG_CMD_SATA
>> +	setup_sata();
>
> Why do we need in early ? Is it really needed before relocation ?
>

Nope. Not needed in early.

> Best regards,
> Stefano Babic
>