From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Nelson Date: Sat, 03 Mar 2012 09:09:11 -0700 Subject: [U-Boot] [PATCH] i.MX6: mx6q_sabrelite: add SATA bindings In-Reply-To: <4F51DDBF.5090407@denx.de> References: <1330735585-16514-1-git-send-email-eric.nelson@boundarydevices.com> <4F51DDBF.5090407@denx.de> Message-ID: <4F524227.4040006@boundarydevices.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> --- >> 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 >