Resend: i.MX6: add enable_sata_clock()

PATCH Superseded Eric Nelson 4 participants 7 messages Apr 25, 2012 – May 6, 2012 1 message not in the archive

Subsystems (2)

Share & tools

A message that is not in the archive (sent off-list or not yet received): http://lists.denx.de/pipermail/u-boot/2012-March/121135.html

Eric Nelson

author

2012-04-25 23:48 UTC #1 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

This patch requires Stefano's driver for MX5/MX6 to be useful.

	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html

This is the first and board-independent part of what's needed to enable SATA on an i.MX6 board as discussed in this thread:

	http://lists.denx.de/pipermail/u-boot/2012-March/120919.html

Signed-off-by Eric Nelson <[email protected]>

---
V2 updated to return -EIO instead of -1.

 arch/arm/cpu/armv7/mx6/clock.c           |   31 ++++++++
 arch/arm/include/asm/arch-mx6/clock.h    |    1 +
 arch/arm/include/asm/arch-mx6/imx-regs.h |    9 +++
 arch/arm/include/asm/arch-mx6/iomux-v3.h |  111 ++++++++++++++++++++++++++++++
 4 files changed, 152 insertions(+), 0 deletions(-)

arch/arm/cpu/armv7/mx6/clock.c +31 -0

index ef98563..de91cac 100644
@@ -303,6 +303,37 @@ u32 imx_get_fecclk(void)
 	return decode_pll(PLL_ENET, CONFIG_SYS_MX6_HCLK);
 }
 
+int enable_sata_clock(void)
+{
+	u32 reg = 0;
+	s32 timeout = 100000;
+	struct imx_ccm_reg *const imx_ccm
+		= (struct imx_ccm_reg *) CCM_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 -EIO;
+	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);
+
+	return 0 ;
+}
+
 unsigned int mxc_get_clock(enum mxc_clock clk)
 {
 	switch (clk) {

arch/arm/include/asm/arch-mx6/clock.h +1 -0

index 613809b..b91d8bf 100644
@@ -47,5 +47,6 @@ u32 imx_get_uartclk(void);
 u32 imx_get_fecclk(void);
 unsigned int mxc_get_clock(enum mxc_clock clk);
 void enable_usboh3_clk(unsigned char enable);
+int enable_sata_clock(void);
 
 #endif /* __ASM_ARCH_CLOCK_H */

arch/arm/include/asm/arch-mx6/imx-regs.h +9 -0

index 6d25c8d..e165810 100644
@@ -436,5 +436,14 @@ struct anatop_regs {
 	u32	digprog;		/* 0x260 */
 };
 
+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__ */

arch/arm/include/asm/arch-mx6/iomux-v3.h +111 -0

115 lines not on this page · raw message

Signature · 1 line

1.7.9

Dirk Behme

2012-04-28 14:42 UTC #2 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Hi Eric,

first, do you really want the "Resend" in the subject?

Quoted 23 lines · Eric Nelson

This patch requires Stefano's driver for MX5/MX6 to be useful.
	http://lists.denx.de/pipermail/u-boot/2012-February/118530.html
This is the first and board-independent part of what's needed to enable SATA on an i.MX6 board as discussed in this thread:
	http://lists.denx.de/pipermail/u-boot/2012-March/120919.html
Signed-off-by Eric Nelson<[email protected]>
---
V2 updated to return -EIO instead of -1.
  arch/arm/cpu/armv7/mx6/clock.c           |   31 ++++++++
  arch/arm/include/asm/arch-mx6/clock.h    |    1 +
  arch/arm/include/asm/arch-mx6/imx-regs.h |    9 +++
  arch/arm/include/asm/arch-mx6/iomux-v3.h |  111 ++++++++++++++++++++++++++++++
  4 files changed, 152 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/mx6/clock.c +0 -0
index ef98563..de91cac 100644

arch/arm/cpu/armv7/mx6/clock.c u32 imx_get_fecclk(void) · line 311

Quoted 9 lines

arch/arm/cpu/armv7/mx6/clock.c +6 -0

@@ -303,6 +303,37 @@ u32 imx_get_fecclk(void)
  	return decode_pll(PLL_ENET, CONFIG_SYS_MX6_HCLK);
  }
 
+int enable_sata_clock(void)
+{
+	u32 reg = 0;
+	s32 timeout = 100000;
+	struct imx_ccm_reg *const imx_ccm
+		= (struct imx_ccm_reg *) CCM_BASE_ADDR;

Could you check if the file clock.c has already a global

struct imx_ccm_reg *imx_ccm = (struct imx_ccm_reg *)CCM_BASE_ADDR;

definition, so this redefinition of "struct imx_ccm_reg *const imx_ccm" should (must?) be removed here? See

http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/clock.c;h=ef98563ff7639a3895e5dc00acd9cc30bb3c6772;hb=HEAD#l37

Trying to compile this I had to remove this on my test branch [1].

Best regards

Dirk

[1]
https://github.com/dirkbehme/u-boot-imx6/commit/543964ad861b8143c142df3bb0f914ef2aee7558#L0R309

Eric Nelson

author

2012-04-28 15:29 UTC #3 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Quoted 40 lines · Dirk Behme

Hi Eric,
first, do you really want the "Resend" in the subject?
Quoted 23 lines · Eric Nelson
This patch requires Stefano's driver for MX5/MX6 to be useful.
http://lists.denx.de/pipermail/u-boot/2012-February/118530.html
This is the first and board-independent part of what's needed to enable SATA on an i.MX6 board as discussed in this thread:
http://lists.denx.de/pipermail/u-boot/2012-March/120919.html
Signed-off-by Eric Nelson<[email protected]>
---
V2 updated to return -EIO instead of -1.
arch/arm/cpu/armv7/mx6/clock.c | 31 ++++++++ arch/arm/include/asm/arch-mx6/clock.h | 1 + arch/arm/include/asm/arch-mx6/imx-regs.h | 9 +++ arch/arm/include/asm/arch-mx6/iomux-v3.h | 111 ++++++++++++++++++++++++++++++
4 files changed, 152 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/mx6/clock.c +0 -0
index ef98563..de91cac 100644
arch/arm/cpu/armv7/mx6/clock.c u32 imx_get_fecclk(void) · line 311
Quoted 9 lines
arch/arm/cpu/armv7/mx6/clock.c +6 -0
@@ -303,6 +303,37 @@ u32 imx_get_fecclk(void)
 return decode_pll(PLL_ENET, CONFIG_SYS_MX6_HCLK);
 }
 
+int enable_sata_clock(void)
+{
+ u32 reg = 0;
+ s32 timeout = 100000;
+ struct imx_ccm_reg *const imx_ccm
+ = (struct imx_ccm_reg *) CCM_BASE_ADDR;
Could you check if the file clock.c has already a global

Quoted 9 lines

struct imx_ccm_reg *imx_ccm = (struct imx_ccm_reg *)CCM_BASE_ADDR;
definition, so this redefinition of "struct imx_ccm_reg *const imx_ccm" should (must?) be removed here? See
http://git.denx.de/cgi-bin/gitweb.cgi?p=u-boot.git;a=blob;f=arch/arm/cpu/armv7/mx6/clock.c;h=ef98563ff7639a3895e5dc00acd9cc30bb3c6772;hb=HEAD#l37
Trying to compile this I had to remove this on my test branch [1].

Hi Dirk,

I tried to address this global/non-const by making it explicitly public, but Stefano nacked the patch:

	http://patchwork.ozlabs.org/patch/144712/

As it stands, the compiler won't generate an external reference for the local above, so it should be good.

The global/writable/not published variable imx_ccm should be fixed though. At least it should be static if it's not exposed through a header and const so it isn't placed in the BSS segment.

I'll generate a patch later today.

Regards,

Eric

Stefano Babic

2012-05-06 17:04 UTC #4 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Quoted 8 lines · Eric Nelson

Hi Dirk,
I tried to address this global/non-const by making it explicitly public, but Stefano nacked the patch:
    http://patchwork.ozlabs.org/patch/144712/
As it stands, the compiler won't generate an external reference for the local above, so it should be good.

It seems ok. However, this conflicts with

imx-common: Factor out get_ahb_clk()

sent by Fabio Estevam.

Reason is only the modified structure name : I made these changes to your patch:

arch/arm/cpu/armv7/mx6/clock.c +2 -2

index 3255771..52d5dc4 100644
@@ -296,8 +296,8 @@ int enable_sata_clock(void)
 {
        u32 reg = 0;
        s32 timeout = 100000;
-       struct imx_ccm_reg *const imx_ccm
-               = (struct imx_ccm_reg *) CCM_BASE_ADDR;
+       struct mxc_ccm_reg *const imx_ccm
+               = (struct mxc_ccm_reg *) CCM_BASE_ADDR;
 
        /* Enable sata clock */

Change is trivial, but please take a look.

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

Eric Nelson

author

2012-05-06 17:42 UTC #5 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Hi Stefano,

Quoted 35 lines · Stefano Babic

Quoted 8 lines · Eric Nelson
Hi Dirk,
I tried to address this global/non-const by making it explicitly public, but Stefano nacked the patch:
     http://patchwork.ozlabs.org/patch/144712/
As it stands, the compiler won't generate an external reference for the local above, so it should be good.
It seems ok. However, this conflicts with
imx-common: Factor out get_ahb_clk()
sent by Fabio Estevam.
Reason is only the modified structure name : I made these changes to your patch:
arch/arm/cpu/armv7/mx6/clock.c +2 -2
index 3255771..52d5dc4 100644
@@ -296,8 +296,8 @@ int enable_sata_clock(void)
  {
         u32 reg = 0;
         s32 timeout = 100000;
-       struct imx_ccm_reg *const imx_ccm
-               = (struct imx_ccm_reg *) CCM_BASE_ADDR;
+       struct mxc_ccm_reg *const imx_ccm
+               = (struct mxc_ccm_reg *) CCM_BASE_ADDR;
 
         /* Enable sata clock */
 

Quoted 2 lines

Change is trivial, but please take a look.

This is clearly fine, but it has me laughing because I never remember what 'mxc' stands for.

Regards,

Eric

Fabio Estevam

reviewer

2012-05-06 18:08 UTC #6 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Hi Eric,

Quoted 2 lines · Eric Nelson

This is clearly fine, but it has me laughing because I never remember what 'mxc' stands for.

Yes, this can cause confunsion. The 'mxc' term is frequently used in the kernel/u-boot and has the same meaning as 'imx'.

Regards,

Fabio Estevam

Eric Nelson

author

2012-05-06 18:33 UTC #7 lore raw

PATCH v2 1/1 Resend: i.MX6: add enable_sata_clock()

Quoted 11 lines · Fabio Estevam

Hi Eric,
Quoted 2 lines · Eric Nelson
This is clearly fine, but it has me laughing because I never remember what 'mxc' stands for.
Yes, this can cause confunsion. The 'mxc' term is frequently used in the kernel/u-boot and has the same meaning as 'imx'.

Mobile Extreme Confusion? :)