[U-Boot] [PATCH 1/6] am33xx: Enable UART{1,2,4,5} clocks

[Andrew Bradford]

Tom & Marek,

On Thu, 27 Sep 2012 10:53:05 -0700
Tom Rini <trini at ti.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 09/27/12 10:27, Marek Vasut wrote:
> > Dear Tom Rini,
> > 
> >> On 09/27/12 10:11, Marek Vasut wrote:
> >>> Dear Tom Rini,
> >>> 
> >>>> On 09/27/12 09:45, Marek Vasut wrote:
> >>>>> Dear Tom Rini,
> >>>>> 
> >>>>>> On Thu, Sep 27, 2012 at 06:13:36PM +0200, Marek Vasut 
> >>>>>> wrote:
> >>>>>>> Dear Andrew Bradford,
> >>>>>>> 
> >>>>>>>> If configured to use UART{1,2,4,5}, such as on the 
> >>>>>>>> Beaglebone RS232 cape, enable the required clocks
> >>>>>>>> for the UART in use.
> >>>>>>>> 
> >>>>>>>> Signed-off-by: Andrew Bradford 
> >>>>>>>> <andrew at bradfordembedded.com> ---
> >>>>>>>> 
> >>>>>>>> arch/arm/cpu/armv7/am33xx/clock.c |   28 
> >>>>>>>> ++++++++++++++++++++++++++++ 1 file changed, 28 
> >>>>>>>> insertions(+)
> >>>>>>>> 
> >>>>>>>> diff --git a/arch/arm/cpu/armv7/am33xx/clock.c 
> >>>>>>>> b/arch/arm/cpu/armv7/am33xx/clock.c index 
> >>>>>>>> 2b19506..4eb9226 100644 --- 
> >>>>>>>> a/arch/arm/cpu/armv7/am33xx/clock.c +++ 
> >>>>>>>> b/arch/arm/cpu/armv7/am33xx/clock.c @@ -114,6
> >>>>>>>> +114,34 @@ static void enable_per_clocks(void)
> >>>>>>>> 
> >>>>>>>> while (readl(&cmwkup->wkup_uart0ctrl) !=
> >>>>>>>> PRCM_MOD_EN) ;
> >>>>>>>> 
> >>>>>>>> +	/* UART1 */ +#ifdef CONFIG_SERIAL2 + 
> >>>>>>>> writel(PRCM_MOD_EN, &cmper->uart1clkctrl); +	while 
> >>>>>>>> (readl(&cmper->uart1clkctrl) != PRCM_MOD_EN)
> >>>>>>>> +		;
> >>>>>>> 
> >>>>>>> Call WATCHDOG_RESET() here, fix glboally
> >>>>>> 
> >>>>>> We don't have WATCHDOG_RESET...
> >>>>> 
> >>>>> You do, and it opts-out to udelay(1) is most cases.
> >>>> 
> >>>> It looks like it opts-out to {} in most cases, in 
> >>>> <watchdog.h>
> >>> 
> >>> Correct, we use it to retrigger watchdog timer if implemented.
> >> 
> >> Which the SoC support isn't doing and the rest of the code also 
> >> isn't trying to use.  Arguably the whole file should be doing 
> >> udelay(1) in each of these instances and a clean up patch which 
> >> this series depends on might be useful.
> > 
> > So we're changing the practice from doing WATCHDOG_RESET() to 
> > udelay(1) ? And we're doing so in generic code?
> 
> I think we should use WATCHDOG_RESET where it makes sense and udelay
> where we're just delaying.  I don't see WATCHDOG_RESET() being used
> for enable this-or-that clock.  But maybe I'm just really missing
> something about how we use WATCHDOG_RESET in the case where it's not a
> nop.

Is there a consensus on the use of WATCHDOG_RESET vs. udelay(1) in this
instance?

Looking through the arch/arm/cpu/armv7/{omap-common,omap3}/clock files
shows no use of either WATCHDOG_RESET or udelay(1) in the ways
mentioned. In fact, I can't easily find a use of WATCHDOG_RESET at all
within arch/arm/cpu/armv7 code.

What is the goal of using either udelay(1) or WATCHDOG_RESET as
recommended here?

Thanks,
Andrew