[PATCH 0/3] ARM: OMAP: AM35xx: fix some boot warnings with AM3505/3517

[PATCH 0/3] ARM: OMAP: AM35xx: fix some boot warnings with AM3505/3517

[Paul Walmsley]

Hello,

this series fixes some of the warnings observed at boot with the AM35xx
boards.  Intended for either the 3.5 or 3.5-rc merge windows.

Particularly appreciated would be some feedback on the "ARM: OMAP:
AM35xx: fix UART4 softreset" patch from people who know the AM35xx SoC
hardware.


- Paul

---

am35xx_hwmod_data_fixes_a_3.5
   text	   data	    bss	    dec	    hex	filename
6601274	 724684	5587616	12913574	 c50ba6	vmlinux.omap2plus_defconfig.orig
6601274	 724748	5587616	12913638	 c50be6	vmlinux.omap2plus_defconfig

Paul Walmsley (3):
      ARM: OMAP AM35xx: clock and hwmod data: fix AM35xx HSOTGUSB hwmod
      ARM: OMAP AM35xx: clock and hwmod data: fix UART4 data
      ARM: OMAP: AM35xx: fix UART4 softreset


 arch/arm/mach-omap2/clock3xxx_data.c       |   26 +++++++++++--------
 arch/arm/mach-omap2/cm-regbits-34xx.h      |    4 +--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   38 ++++++++++++++++++++--------
 arch/arm/mach-omap2/prcm-common.h          |    4 +--
 4 files changed, 46 insertions(+), 26 deletions(-)

[PATCH 1/3] ARM: OMAP AM35xx: clock and hwmod data: fix AM35xx HSOTGUSB hwmod

[Paul Walmsley]

Partially fix the hwmod data for the AM35xx USB OTG hwmod.  This
should resolve the following boot warning on AM35xx platforms:

omap_hwmod: am35x_otg_hs: cannot be enabled for reset (3)

While here, also fix the clkdev records, to avoid warnings about
duplicate clock aliases.

The hwmod is also connected to the wrong interconnect.  It should be
connected to the IPSS, not the L4 CORE.  But that is left for a future
fix, since it probably has a dependency on some hwmod core changes.

Cc: Felipe Balbi <balbi@ti.com>
Cc: Hema HK <hemahk@ti.com>
Cc: Mark A. Greer <mgreer@animalcreek.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock3xxx_data.c       |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   14 +++++---------
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 4e1a3b0..13102b9 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3478,8 +3478,8 @@ static struct omap_clk omap3xxx_clks[] = {
 	CLK("davinci_mdio.0",	NULL,	&emac_fck,	CK_AM35XX),
 	CLK("vpfe-capture",	"master",	&vpfe_ick,	CK_AM35XX),
 	CLK("vpfe-capture",	"slave",	&vpfe_fck,	CK_AM35XX),
-	CLK("musb-am35x",	"ick",		&hsotgusb_ick_am35xx,	CK_AM35XX),
-	CLK("musb-am35x",	"fck",		&hsotgusb_fck_am35xx,	CK_AM35XX),
+	CLK(NULL,	"hsotgusb_ick",		&hsotgusb_ick_am35xx,	CK_AM35XX),
+	CLK(NULL,	"hsotgusb_fck",		&hsotgusb_fck_am35xx,	CK_AM35XX),
 	CLK(NULL,	"hecc_ck",	&hecc_ck,	CK_AM35XX),
 	CLK(NULL,	"uart4_ick",	&uart4_ick_am35xx,	CK_AM35XX),
 	CLK("omap_timer.1",	"32k_ck",	&omap_32k_fck,  CK_3XXX),
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 0c65079..678a3aa 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -1637,25 +1637,20 @@ static struct omap_hwmod omap3xxx_usbhsotg_hwmod = {
 
 /* usb_otg_hs */
 static struct omap_hwmod_irq_info am35xx_usbhsotg_mpu_irqs[] = {
-
 	{ .name = "mc", .irq = 71 },
 	{ .irq = -1 }
 };
 
 static struct omap_hwmod_class am35xx_usbotg_class = {
 	.name = "am35xx_usbotg",
-	.sysc = NULL,
 };
 
 static struct omap_hwmod am35xx_usbhsotg_hwmod = {
 	.name		= "am35x_otg_hs",
 	.mpu_irqs	= am35xx_usbhsotg_mpu_irqs,
-	.main_clk	= NULL,
-	.prcm = {
-		.omap2 = {
-		},
-	},
+	.main_clk	= "hsotgusb_fck",
 	.class		= &am35xx_usbotg_class,
+	.flags		= HWMOD_NO_IDLEST,
 };
 
 /* MMC/SD/SDIO common */
@@ -2046,9 +2041,10 @@ static struct omap_hwmod_ocp_if omap3xxx_usbhsotg__l3 = {
 static struct omap_hwmod_ocp_if am35xx_usbhsotg__l3 = {
 	.master		= &am35xx_usbhsotg_hwmod,
 	.slave		= &omap3xxx_l3_main_hwmod,
-	.clk		= "core_l3_ick",
+	.clk		= "hsotgusb_ick",
 	.user		= OCP_USER_MPU,
 };
+
 /* L4_CORE -> L4_WKUP interface */
 static struct omap_hwmod_ocp_if omap3xxx_l4_core__l4_wkup = {
 	.master	= &omap3xxx_l4_core_hwmod,
@@ -2342,7 +2338,7 @@ static struct omap_hwmod_addr_space am35xx_usbhsotg_addrs[] = {
 static struct omap_hwmod_ocp_if am35xx_l4_core__usbhsotg = {
 	.master		= &omap3xxx_l4_core_hwmod,
 	.slave		= &am35xx_usbhsotg_hwmod,
-	.clk		= "l4_ick",
+	.clk		= "hsotgusb_ick",
 	.addr		= am35xx_usbhsotg_addrs,
 	.user		= OCP_USER_MPU,
 };

[PATCH 2/3] ARM: OMAP AM35xx: clock and hwmod data: fix UART4 data

[Paul Walmsley]

Add missing terminators to the arrays of IRQ, DMA, and address space
structure records in the AM35xx UART4 hwmod data.  Without these
terminators, the following warnings appear on boot:

omap_uart.3: failed to claim resource 58
omap_device: omap_uart: build failed (-16)
WARNING: at /home/paul/linux/arch/arm/mach-omap2/serial.c:375 omap_serial_init_port+0x198/0x284()
Could not build omap_device for omap_uart: uart4.

Also, AM35xx uart4_fck has an incorrect parent clock pointer.  Fix it
and clean up a whitespace issue.

Fix some incorrectly-named macros related to AM35xx UART4.

Cc: Kyle Manna <kyle.manna@fuel7.com>
Cc: Mark A. Greer <mgreer@animalcreek.com>
Cc: Ranjith Lohithakshan <ranjithl@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock3xxx_data.c       |   14 +++++++-------
 arch/arm/mach-omap2/cm-regbits-34xx.h      |    4 ++--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |    7 +++++--
 arch/arm/mach-omap2/prcm-common.h          |    4 ++--
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 13102b9..11644bf 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -2490,13 +2490,13 @@ static struct clk uart4_fck = {
 };
 
 static struct clk uart4_fck_am35xx = {
-	.name           = "uart4_fck",
-	.ops            = &clkops_omap2_dflt_wait,
-	.parent         = &per_48m_fck,
-	.enable_reg     = OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
-	.enable_bit     = OMAP3430_EN_UART4_SHIFT,
-	.clkdm_name     = "core_l4_clkdm",
-	.recalc         = &followparent_recalc,
+	.name		= "uart4_fck",
+	.ops		= &clkops_omap2_dflt_wait,
+	.parent		= &core_48m_fck,
+	.enable_reg	= OMAP_CM_REGADDR(CORE_MOD, CM_FCLKEN1),
+	.enable_bit	= AM35XX_EN_UART4_SHIFT,
+	.clkdm_name	= "core_l4_clkdm",
+	.recalc		= &followparent_recalc,
 };
 
 static struct clk gpt2_fck = {
diff --git a/arch/arm/mach-omap2/cm-regbits-34xx.h b/arch/arm/mach-omap2/cm-regbits-34xx.h
index 8083a8c..766338f 100644
--- a/arch/arm/mach-omap2/cm-regbits-34xx.h
+++ b/arch/arm/mach-omap2/cm-regbits-34xx.h
@@ -169,8 +169,6 @@
 /* AM35XX specific CM_ICLKEN1_CORE bits */
 #define AM35XX_EN_IPSS_MASK				(1 << 4)
 #define AM35XX_EN_IPSS_SHIFT				4
-#define AM35XX_EN_UART4_MASK				(1 << 23)
-#define AM35XX_EN_UART4_SHIFT				23
 
 /* CM_ICLKEN2_CORE */
 #define OMAP3430_EN_PKA_MASK				(1 << 4)
@@ -207,6 +205,8 @@
 #define OMAP3430_ST_DES2_MASK				(1 << 26)
 #define OMAP3430_ST_MSPRO_SHIFT				23
 #define OMAP3430_ST_MSPRO_MASK				(1 << 23)
+#define AM35XX_ST_UART4_SHIFT				23
+#define AM35XX_ST_UART4_MASK				(1 << 23)
 #define OMAP3430_ST_HDQ_SHIFT				22
 #define OMAP3430_ST_HDQ_MASK				(1 << 22)
 #define OMAP3430ES1_ST_FAC_SHIFT			8
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index 678a3aa..c939131 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -526,11 +526,13 @@ static struct omap_hwmod omap36xx_uart4_hwmod = {
 
 static struct omap_hwmod_irq_info am35xx_uart4_mpu_irqs[] = {
 	{ .irq = INT_35XX_UART4_IRQ, },
+	{ .irq = -1 }
 };
 
 static struct omap_hwmod_dma_info am35xx_uart4_sdma_reqs[] = {
 	{ .name = "rx", .dma_req = AM35XX_DMA_UART4_RX, },
 	{ .name = "tx", .dma_req = AM35XX_DMA_UART4_TX, },
+	{ .dma_req = -1 }
 };
 
 static struct omap_hwmod am35xx_uart4_hwmod = {
@@ -542,9 +544,9 @@ static struct omap_hwmod am35xx_uart4_hwmod = {
 		.omap2 = {
 			.module_offs = CORE_MOD,
 			.prcm_reg_id = 1,
-			.module_bit = OMAP3430_EN_UART4_SHIFT,
+			.module_bit = AM35XX_EN_UART4_SHIFT,
 			.idlest_reg_id = 1,
-			.idlest_idle_bit = OMAP3430_EN_UART4_SHIFT,
+			.idlest_idle_bit = AM35XX_ST_UART4_SHIFT,
 		},
 	},
 	.class		= &omap2_uart_class,
@@ -2188,6 +2190,7 @@ static struct omap_hwmod_addr_space am35xx_uart4_addr_space[] = {
 		.pa_end		= OMAP3_UART4_AM35XX_BASE + SZ_1K - 1,
 		.flags		= ADDR_MAP_ON_INIT | ADDR_TYPE_RT,
 	},
+	{ }
 };
 
 static struct omap_hwmod_ocp_if am35xx_l4_core__uart4 = {
diff --git a/arch/arm/mach-omap2/prcm-common.h b/arch/arm/mach-omap2/prcm-common.h
index 6da3ba4..cc1398e 100644
--- a/arch/arm/mach-omap2/prcm-common.h
+++ b/arch/arm/mach-omap2/prcm-common.h
@@ -203,8 +203,8 @@
 #define OMAP3430_EN_MMC2_SHIFT				25
 #define OMAP3430_EN_MMC1_MASK				(1 << 24)
 #define OMAP3430_EN_MMC1_SHIFT				24
-#define OMAP3430_EN_UART4_MASK				(1 << 23)
-#define OMAP3430_EN_UART4_SHIFT				23
+#define AM35XX_EN_UART4_MASK				(1 << 23)
+#define AM35XX_EN_UART4_SHIFT				23
 #define OMAP3430_EN_MCSPI4_MASK				(1 << 21)
 #define OMAP3430_EN_MCSPI4_SHIFT			21
 #define OMAP3430_EN_MCSPI3_MASK				(1 << 20)

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Paul Walmsley]

During kernel init, the AM3505/AM3517 UART4 cannot complete its softreset:

omap_hwmod: uart4: softreset failed (waited 10000 usec)

This also results in another warning later in the boot process:

omap_hwmod: uart4: enabled state can only be entered from initialized, idle, or disabled state

>From empirical observation, the AM35xx UART4 IP block requires either
uart1_fck or uart2_fck to be enabled while UART4 resets.  Otherwise
the reset will never complete.  So this patch adds uart1_fck as an
optional clock for UART4 and adds the appropriate hwmod flag to cause
uart1_fck to be enabled during the reset process.  (The choice of
uart1_fck over uart2_fck was arbitrary.)

Unfortunately this observation raises many questions.  Is it necessary
for uart1_fck or uart2_fck to be controlled with uart4_fck for the
UART4 to work correctly?  What exactly do the AM35xx UART4 clock
tree and the related PRCM idle management FSMs look like?  If anyone
has the ability to answer these questions through empirical functional
testing, or hardware information from the AM35xx designers, it would
be greatly appreciated.

Cc: Beno?t Cousson <b-cousson@ti.com>
Cc: Kyle Manna <kyle.manna@fuel7.com>
Cc: Mark A. Greer <mgreer@animalcreek.com>
Cc: Ranjith Lohithakshan <ranjithl@ti.com>
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock3xxx_data.c       |    8 ++++++--
 arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   17 +++++++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
index 11644bf..12c64db 100644
--- a/arch/arm/mach-omap2/clock3xxx_data.c
+++ b/arch/arm/mach-omap2/clock3xxx_data.c
@@ -3201,8 +3201,12 @@ static struct clk vpfe_fck = {
 };
 
 /*
- * The UART1/2 functional clock acts as the functional
- * clock for UART4. No separate fclk control available.
+ * The UART1/2 functional clock acts as the functional clock for
+ * UART4. No separate fclk control available.  XXX Well now we have a
+ * uart4_fck that is apparently used as the UART4 functional clock,
+ * but it also seems that uart1_fck or uart2_fck are still needed, at
+ * least for UART4 softresets to complete.  This really needs
+ * clarification.
  */
 static struct clk uart4_ick_am35xx = {
 	.name		= "uart4_ick",
diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
index c939131..c6653a80 100644
--- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
@@ -535,6 +535,20 @@ static struct omap_hwmod_dma_info am35xx_uart4_sdma_reqs[] = {
 	{ .dma_req = -1 }
 };
 
+/*
+ * XXX AM35xx UART4 cannot complete its softreset without uart1_fck or
+ * uart2_fck being enabled.  So we add uart1_fck as an optional clock,
+ * below, and set the HWMOD_CONTROL_OPT_CLKS_IN_RESET.  This really
+ * should not be needed.  The functional clock structure of the AM35xx
+ * UART4 is extremely unclear and opaque; it is unclear what the role
+ * of uart1/2_fck is for the UART4.  Any clarification from either
+ * empirical testing or the AM3505/3517 hardware designers would be
+ * most welcome.
+ */
+static struct omap_hwmod_opt_clk am35xx_uart4_opt_clks[] = {
+	{ .role = "softreset_uart1_fck", .clk = "uart1_fck" },
+};
+
 static struct omap_hwmod am35xx_uart4_hwmod = {
 	.name		= "uart4",
 	.mpu_irqs	= am35xx_uart4_mpu_irqs,
@@ -549,6 +563,9 @@ static struct omap_hwmod am35xx_uart4_hwmod = {
 			.idlest_idle_bit = AM35XX_ST_UART4_SHIFT,
 		},
 	},
+	.opt_clks	= am35xx_uart4_opt_clks,
+	.opt_clks_cnt	= ARRAY_SIZE(am35xx_uart4_opt_clks),
+	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
 	.class		= &omap2_uart_class,
 };
 

[PATCH 0/3] ARM: OMAP: AM35xx: fix some boot warnings with AM3505/3517

[Mark A. Greer]

On Thu, May 10, 2012 at 11:29:13AM -0600, Paul Walmsley wrote:
> Hello,
> 
> this series fixes some of the warnings observed at boot with the AM35xx
> boards.  Intended for either the 3.5 or 3.5-rc merge windows.
> 
> Particularly appreciated would be some feedback on the "ARM: OMAP:
> AM35xx: fix UART4 softreset" patch from people who know the AM35xx SoC
> hardware.

Hi Paul.

What repo/branch are these patches based on?

Thanks,

Mark

[PATCH 0/3] ARM: OMAP: AM35xx: fix some boot warnings with AM3505/3517

[Paul Walmsley]

Hi Mark

On Thu, 10 May 2012, Mark A. Greer wrote:

> On Thu, May 10, 2012 at 11:29:13AM -0600, Paul Walmsley wrote:
> > Hello,
> > 
> > this series fixes some of the warnings observed at boot with the AM35xx
> > boards.  Intended for either the 3.5 or 3.5-rc merge windows.
> > 
> > Particularly appreciated would be some feedback on the "ARM: OMAP:
> > AM35xx: fix UART4 softreset" patch from people who know the AM35xx SoC
> > hardware.
> 
> What repo/branch are these patches based on?

They are based on the 'omap-devel-b-for-3.5' tag in my omap-pending tree 
on kernel.org:

http://git.kernel.org/?p=linux/kernel/git/pjw/omap-pending.git;a=tag;h=8b14f48f6a985a71e250feb3e8595d9400829a18

The series is also available via git from git://git.pwsan.com/linux-2.6 in 
the branch "am35xx_hwmod_data_fixes_a_3.5".


- Paul

[PATCH 1/3] ARM: OMAP AM35xx: clock and hwmod data: fix AM35xx HSOTGUSB hwmod

[Mark A. Greer]

On Thu, May 10, 2012 at 11:29:17AM -0600, Paul Walmsley wrote:
> Partially fix the hwmod data for the AM35xx USB OTG hwmod.  This
> should resolve the following boot warning on AM35xx platforms:
> 
> omap_hwmod: am35x_otg_hs: cannot be enabled for reset (3)
> 
> While here, also fix the clkdev records, to avoid warnings about
> duplicate clock aliases.
> 
> The hwmod is also connected to the wrong interconnect.  It should be
> connected to the IPSS, not the L4 CORE.  But that is left for a future
> fix, since it probably has a dependency on some hwmod core changes.
> 
> Cc: Felipe Balbi <balbi@ti.com>
> Cc: Hema HK <hemahk@ti.com>
> Cc: Mark A. Greer <mgreer@animalcreek.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

Acked-by: Mark A. Greer <mgreer@animalcreek.com>
(on an am3517evm)

[PATCH 2/3] ARM: OMAP AM35xx: clock and hwmod data: fix UART4 data

[Mark A. Greer]

On Thu, May 10, 2012 at 11:29:18AM -0600, Paul Walmsley wrote:
> Add missing terminators to the arrays of IRQ, DMA, and address space
> structure records in the AM35xx UART4 hwmod data.  Without these
> terminators, the following warnings appear on boot:
> 
> omap_uart.3: failed to claim resource 58
> omap_device: omap_uart: build failed (-16)
> WARNING: at /home/paul/linux/arch/arm/mach-omap2/serial.c:375 omap_serial_init_port+0x198/0x284()
> Could not build omap_device for omap_uart: uart4.
> 
> Also, AM35xx uart4_fck has an incorrect parent clock pointer.  Fix it
> and clean up a whitespace issue.
> 
> Fix some incorrectly-named macros related to AM35xx UART4.
> 
> Cc: Kyle Manna <kyle.manna@fuel7.com>
> Cc: Mark A. Greer <mgreer@animalcreek.com>
> Cc: Ranjith Lohithakshan <ranjithl@ti.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

Acked-by: Mark A. Greer <mgreer@animalcreek.com>
(on an am3517evm)

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Mark A. Greer]

On Thu, May 10, 2012 at 11:29:19AM -0600, Paul Walmsley wrote:
> During kernel init, the AM3505/AM3517 UART4 cannot complete its softreset:
> 
> omap_hwmod: uart4: softreset failed (waited 10000 usec)
> 
> This also results in another warning later in the boot process:
> 
> omap_hwmod: uart4: enabled state can only be entered from initialized, idle, or disabled state
> 
> >From empirical observation, the AM35xx UART4 IP block requires either
> uart1_fck or uart2_fck to be enabled while UART4 resets.  Otherwise
> the reset will never complete.  So this patch adds uart1_fck as an
> optional clock for UART4 and adds the appropriate hwmod flag to cause
> uart1_fck to be enabled during the reset process.  (The choice of
> uart1_fck over uart2_fck was arbitrary.)
> 
> Unfortunately this observation raises many questions.  Is it necessary
> for uart1_fck or uart2_fck to be controlled with uart4_fck for the
> UART4 to work correctly?  What exactly do the AM35xx UART4 clock
> tree and the related PRCM idle management FSMs look like?  If anyone
> has the ability to answer these questions through empirical functional
> testing, or hardware information from the AM35xx designers, it would
> be greatly appreciated.
> 
> Cc: Beno?t Cousson <b-cousson@ti.com>
> Cc: Kyle Manna <kyle.manna@fuel7.com>
> Cc: Mark A. Greer <mgreer@animalcreek.com>
> Cc: Ranjith Lohithakshan <ranjithl@ti.com>
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

Acked-by: Mark A. Greer <mgreer@animalcreek.com>
(on an am3517evm)

Mark

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Cousson, Benoit]

Hi Paul,

On 5/10/2012 7:29 PM, Paul Walmsley wrote:
> During kernel init, the AM3505/AM3517 UART4 cannot complete its softreset:
>
> omap_hwmod: uart4: softreset failed (waited 10000 usec)
>
> This also results in another warning later in the boot process:
>
> omap_hwmod: uart4: enabled state can only be entered from initialized, idle, or disabled state
>
>> From empirical observation, the AM35xx UART4 IP block requires either
> uart1_fck or uart2_fck to be enabled while UART4 resets.  Otherwise
> the reset will never complete.  So this patch adds uart1_fck as an
> optional clock for UART4 and adds the appropriate hwmod flag to cause
> uart1_fck to be enabled during the reset process.  (The choice of
> uart1_fck over uart2_fck was arbitrary.)
>
> Unfortunately this observation raises many questions.  Is it necessary
> for uart1_fck or uart2_fck to be controlled with uart4_fck for the
> UART4 to work correctly?  What exactly do the AM35xx UART4 clock
> tree and the related PRCM idle management FSMs look like?  If anyone
> has the ability to answer these questions through empirical functional
> testing, or hardware information from the AM35xx designers, it would
> be greatly appreciated.

I do not have any clue about that chip, but is this clock really what it 
is supposed to be? I mean, isn't the uart1_fck the parent of all the 
UART fck or something like that. Don't we just have an issue becasue the 
clock names are not accurate?

Regards,
Benoit

>
> Cc: Beno?t Cousson<b-cousson@ti.com>
> Cc: Kyle Manna<kyle.manna@fuel7.com>
> Cc: Mark A. Greer<mgreer@animalcreek.com>
> Cc: Ranjith Lohithakshan<ranjithl@ti.com>
> Signed-off-by: Paul Walmsley<paul@pwsan.com>
> ---
>   arch/arm/mach-omap2/clock3xxx_data.c       |    8 ++++++--
>   arch/arm/mach-omap2/omap_hwmod_3xxx_data.c |   17 +++++++++++++++++
>   2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/clock3xxx_data.c b/arch/arm/mach-omap2/clock3xxx_data.c
> index 11644bf..12c64db 100644
> --- a/arch/arm/mach-omap2/clock3xxx_data.c
> +++ b/arch/arm/mach-omap2/clock3xxx_data.c
> @@ -3201,8 +3201,12 @@ static struct clk vpfe_fck = {
>   };
>
>   /*
> - * The UART1/2 functional clock acts as the functional
> - * clock for UART4. No separate fclk control available.
> + * The UART1/2 functional clock acts as the functional clock for
> + * UART4. No separate fclk control available.  XXX Well now we have a
> + * uart4_fck that is apparently used as the UART4 functional clock,
> + * but it also seems that uart1_fck or uart2_fck are still needed, at
> + * least for UART4 softresets to complete.  This really needs
> + * clarification.
>    */
>   static struct clk uart4_ick_am35xx = {
>   	.name		= "uart4_ick",
> diff --git a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> index c939131..c6653a80 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_3xxx_data.c
> @@ -535,6 +535,20 @@ static struct omap_hwmod_dma_info am35xx_uart4_sdma_reqs[] = {
>   	{ .dma_req = -1 }
>   };
>
> +/*
> + * XXX AM35xx UART4 cannot complete its softreset without uart1_fck or
> + * uart2_fck being enabled.  So we add uart1_fck as an optional clock,
> + * below, and set the HWMOD_CONTROL_OPT_CLKS_IN_RESET.  This really
> + * should not be needed.  The functional clock structure of the AM35xx
> + * UART4 is extremely unclear and opaque; it is unclear what the role
> + * of uart1/2_fck is for the UART4.  Any clarification from either
> + * empirical testing or the AM3505/3517 hardware designers would be
> + * most welcome.
> + */
> +static struct omap_hwmod_opt_clk am35xx_uart4_opt_clks[] = {
> +	{ .role = "softreset_uart1_fck", .clk = "uart1_fck" },
> +};
> +
>   static struct omap_hwmod am35xx_uart4_hwmod = {
>   	.name		= "uart4",
>   	.mpu_irqs	= am35xx_uart4_mpu_irqs,
> @@ -549,6 +563,9 @@ static struct omap_hwmod am35xx_uart4_hwmod = {
>   			.idlest_idle_bit = AM35XX_ST_UART4_SHIFT,
>   		},
>   	},
> +	.opt_clks	= am35xx_uart4_opt_clks,
> +	.opt_clks_cnt	= ARRAY_SIZE(am35xx_uart4_opt_clks),
> +	.flags		= HWMOD_CONTROL_OPT_CLKS_IN_RESET,
>   	.class		=&omap2_uart_class,
>   };
>
>
>

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Paul Walmsley]

Hi Beno?t

On Fri, 11 May 2012, Cousson, Benoit wrote:

> I do not have any clue about that chip, but is this clock really what it is
> supposed to be? I mean, isn't the uart1_fck the parent of all the UART fck or
> something like that. Don't we just have an issue becasue the clock names are
> not accurate?

I guess that's what I'm trying to find out.

According to the AM3517 TRM rev. B (SPRUGR0B) Figure 14-20 "UART 
Functional Integration" and Table 14-11 "UART Clocks", all of the UARTs 
appear to have independent functional clocks.  The table even mentions a 
CM_FCLKEN1_CORE.EN_UART4 bit.  But the PRCM chapter of this TRM doesn't 
mention that at all.  So the documentation is not too useful here.

On the rest of the OMAPs, as far as I know, the UART clocks are all 
separate.


- Paul

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Cousson, Benoit]

On 5/11/2012 11:22 AM, Paul Walmsley wrote:
> Hi Beno?t
>
> On Fri, 11 May 2012, Cousson, Benoit wrote:
>
>> I do not have any clue about that chip, but is this clock really what it is
>> supposed to be? I mean, isn't the uart1_fck the parent of all the UART fck or
>> something like that. Don't we just have an issue becasue the clock names are
>> not accurate?
>
> I guess that's what I'm trying to find out.
>
> According to the AM3517 TRM rev. B (SPRUGR0B) Figure 14-20 "UART
> Functional Integration" and Table 14-11 "UART Clocks", all of the UARTs
> appear to have independent functional clocks.  The table even mentions a
> CM_FCLKEN1_CORE.EN_UART4 bit.  But the PRCM chapter of this TRM doesn't
> mention that at all.  So the documentation is not too useful here.
>
> On the rest of the OMAPs, as far as I know, the UART clocks are all
> separate.

In fact, not really. The same PER_48M_GFCLK clock is used for every UART 
instances in OMAP4. We do have a separate modulemode for each UART but 
as far as clocks are concerned, the source clock is the same.

Regards,
Benoit

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Paul Walmsley]

On Fri, 11 May 2012, Cousson, Benoit wrote:

> On 5/11/2012 11:22 AM, Paul Walmsley wrote:
> 
> > On the rest of the OMAPs, as far as I know, the UART clocks are all
> > separate.
> 
> In fact, not really. The same PER_48M_GFCLK clock is used for every UART
> instances in OMAP4. We do have a separate modulemode for each UART but as far
> as clocks are concerned, the source clock is the same.

By "separate" I mean that they are all independently gated. This does not 
appear to be the case with AM3517.  Or perhaps it is the case and 
something isn't right with the idle management FSMs.


- Paul

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Cousson, Benoit]

On 5/11/2012 11:35 AM, Paul Walmsley wrote:
> On Fri, 11 May 2012, Cousson, Benoit wrote:
>
>> On 5/11/2012 11:22 AM, Paul Walmsley wrote:
>>
>>> On the rest of the OMAPs, as far as I know, the UART clocks are all
>>> separate.
>>
>> In fact, not really. The same PER_48M_GFCLK clock is used for every UART
>> instances in OMAP4. We do have a separate modulemode for each UART but as far
>> as clocks are concerned, the source clock is the same.
>
> By "separate" I mean that they are all independently gated. This does not
> appear to be the case with AM3517.  Or perhaps it is the case and
> something isn't right with the idle management FSMs.

But they are not separately gated for OMAP4 either. This is the 
modulemode trick that make us think that, but it just means that the 
PRCM should ensure that this clock is needed when at least one UART 
modulemode is enabled. In fact the SPI modules are using the same clock.

The modulemode is used to do some reference counting, but the gating is 
done globally. It was the same with the FCK_EN and ICK_EN in OMAP2&3.

The PRCM spec show only one global gating between FUNC_48M_FCLK and 
PER_48M_GFCLK. All the modules are sharing the exact same clock.

Regards,
Benoit

[PATCH 3/3] ARM: OMAP: AM35xx: fix UART4 softreset

[Paul Walmsley]

On Fri, 11 May 2012, Cousson, Benoit wrote:

> But they are not separately gated for OMAP4 either. This is the modulemode
> trick that make us think that, but it just means that the PRCM should ensure
> that this clock is needed when at least one UART modulemode is enabled. In
> fact the SPI modules are using the same clock.
> 
> The modulemode is used to do some reference counting, but the gating is done
> globally. It was the same with the FCK_EN and ICK_EN in OMAP2&3.
> 
> The PRCM spec show only one global gating between FUNC_48M_FCLK and
> PER_48M_GFCLK. All the modules are sharing the exact same clock.

Well, on OMAP3 and AM3517, they do not all share the same clock.  If you 
look at the 34xx TRM vZU Table 17-11 "UART Clocks", you can see that the 
UART1 and 2 functional clocks are sourced from clocks managed by the CORE 
clockdomain, while UART3 is sourced from clocks managed by the PER 
clockdomain.  This is also expressed in our OMAP3 clock tree.

I realize this is slightly tangential to your point.  As to where the 
clock gating nodes are actually located on a given piece of silicon, 
unfortunately I have no way of confirming that, aside from power 
measurement.  You might be correct about what's going on with the AM3517 
but it would be nice to get some confirmation from the AM35xx PRCM 
engineer(s).  At this point one of two possibilities seem likely: either 
CM_FCLKEN1_CORE.EN_UART4 does nothing, or CM_FCLKEN1_CORE.EN_UART4 is 
necessary but not sufficient to get the UART4 to work.  If 
CM_FCLKEN1_CORE.EN_UART4 does not actually do anything then we need to 
remove it from the kernel and ideally file a documentation bug to get it 
removed from the TRM.  On the other hand, if we do have to enable 
CM_FCLKEN1_CORE.EN_UART4 to deassert the IdleAck to UART4, but the PRCM 
usecounting is broken, then I guess we'll need to add a workaround to the 
hwmod code to essentially support multiple "main clocks".


- Paul