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

2012-03-03 Thread Eric Nelson

On 03/03/2012 12:35 PM, Wolfgang Grandegger wrote:

On 03/03/2012 05:09 PM, Eric Nelson wrote:

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 0x8000
+#define ANATOP_PLL_ENABLE_MASK  0x2000
+#define ANATOP_PLL_BYPASS_MASK  0x0001
+#define ANATOP_PLL_LOCK 0x8000
+#define ANATOP_PLL_PWDN_MASK0x1000
+#define ANATOP_PLL_HOLD_RING_OFF_MASK   0x0800
+#define ANATOP_SATA_CLK_ENABLE_MASK 0x0010


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.


Most, if not all, definitions above are already defined in
arch/arm/include/asm/arch-mx6/ccm_regs.h including a struct member.
Therefore there's also no need for magic offsets.

Wolfgang.


Thanks Wolfgang.

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-03-03 Thread Wolfgang Grandegger
On 03/03/2012 05:09 PM, Eric Nelson wrote:
> 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 0x8000
>>> +#define ANATOP_PLL_ENABLE_MASK  0x2000
>>> +#define ANATOP_PLL_BYPASS_MASK  0x0001
>>> +#define ANATOP_PLL_LOCK 0x8000
>>> +#define ANATOP_PLL_PWDN_MASK0x1000
>>> +#define ANATOP_PLL_HOLD_RING_OFF_MASK   0x0800
>>> +#define ANATOP_SATA_CLK_ENABLE_MASK 0x0010
>>
>> 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.

Most, if not all, definitions above are already defined in
arch/arm/include/asm/arch-mx6/ccm_regs.h including a struct member.
Therefore there's also no need for magic offsets.

Wolfgang.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-03-03 Thread 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
---
  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 0x8000
+#define ANATOP_PLL_ENABLE_MASK  0x2000
+#define ANATOP_PLL_BYPASS_MASK  0x0001
+#define ANATOP_PLL_LOCK 0x8000
+#define ANATOP_PLL_PWDN_MASK0x1000
+#define ANATOP_PLL_HOLD_RING_OFF_MASK   0x0800
+#define ANATOP_SATA_CLK_ENABLE_MASK 0x0010


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_SIZE0x0004
  #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 = 10;
+
+   /* 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


___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2012-03-03 Thread Stefano Babic
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 0x8000
> +#define ANATOP_PLL_ENABLE_MASK  0x2000
> +#define ANATOP_PLL_BYPASS_MASK  0x0001
> +#define ANATOP_PLL_LOCK 0x8000
> +#define ANATOP_PLL_PWDN_MASK0x1000
> +#define ANATOP_PLL_HOLD_RING_OFF_MASK   0x0800
> +#define ANATOP_SATA_CLK_ENABLE_MASK 0x0010

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

> +
>  #define CHIP_REV_1_0 0x10
>  #define IRAM_SIZE0x0004
>  #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 = 10;
> +
> + /* 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/

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


>  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 ?

Best regards,
Stefano Babic

-- 
=
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: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot