[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 at ti.com>
> Cc: Kyle Manna<kyle.manna at fuel7.com>
> Cc: Mark A. Greer<mgreer at animalcreek.com>
> Cc: Ranjith Lohithakshan<ranjithl at ti.com>
> Signed-off-by: Paul Walmsley<paul at 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,
>   };
>
>
>