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

[Tom Rini]

On Fri, Oct 19, 2012 at 08:25:46PM -0400, Andrew Bradford wrote:
> 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?

Lets just match the rest of the SoC code and have the empty loop.  We
can have the discussion about what kind of delay or macro makes most
sense another time.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121020/f07b420b/attachment.pgp>