[U-Boot] [PATCH] arm: omap3: Enable clocks for peripherals only if they are used

[Tom Rini <trini@ti.com>]

On Tue, Nov 19, 2013 at 12:56:13PM +0100, Michael Trimarchi wrote:
> Hi
> 
> On Tue, Nov 19, 2013 at 12:10 PM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> > Hi Michael,
> >
> > On 11/19/13 10:59, Michael Trimarchi wrote:
> >> Hi Igor
> >>
> >> On Tue, Nov 19, 2013 at 9:49 AM, Michael Trimarchi
> >> <michael@amarulasolutions.com> wrote:
> >>> Hi Igor
> >>>
> >>> On Tue, Nov 19, 2013 at 9:24 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> >>>> Hi Michael,
> >>>>
> >>>> On 11/18/13 16:10, Michael Trimarchi wrote:
> >>>>> Enable clocks for the peripherals only if they are used
> >
> > This is not exactly what the patch does, right?
> > I would expect a better description of what you do in the patch
> > and what are the consequences of this patch.
> 
> It does the right. If you open the files you can find that clock
> are enabled if the peripheral is used.
> 
> > May be some of the consequences can be avoided?
> >
> 
> Yes, this is correct. Most of the consequences come if
> you use gpio bank and you don't active them
> 
> >>>>>
> >>>>> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> >>>>> ---
> >>>>>  arch/arm/cpu/armv7/omap3/clock.c        | 2 --
> >>>>>  arch/arm/include/asm/arch-omap3/clock.h | 2 --
> >>>>>  2 files changed, 4 deletions(-)
> >>>>>
> >>>>> diff --git a/arch/arm/cpu/armv7/omap3/clock.c b/arch/arm/cpu/armv7/omap3/clock.c
> >>>>> index 14fc7e8..1bc27bd 100644
> >>>>> --- a/arch/arm/cpu/armv7/omap3/clock.c
> >>>>> +++ b/arch/arm/cpu/armv7/omap3/clock.c
> >>>>> @@ -730,8 +730,6 @@ void per_clocks_enable(void)
> >>>>>               sr32(&prcm_base->fclken_cam, 0, 32, FCK_CAM_ON);
> >>>>>               sr32(&prcm_base->iclken_cam, 0, 32, ICK_CAM_ON);
> >>>>>       }
> >>>>> -     sr32(&prcm_base->fclken_per, 0, 32, FCK_PER_ON);
> >>>>> -     sr32(&prcm_base->iclken_per, 0, 32, ICK_PER_ON);
> >>>>
> >>>> Hmmm...
> >>>> Am I missing something or is this change breaks boards that
> >>>> currently rely on the peripheral clocks being enabled here?
> >>>>
> >>>
> >>> This change break boards that are not correctly configured.
> >
> > I don't agree on this one.
> > This change breaks boards that are _correctly_ configured
> > prior to the patch.
> 
> If you think that if the boards are correct they should boot

I think this is an extension of the CONFIG_SYS_ENABLE_PADS_ALL /
CONFIG_SYS_CLOCKS_ENABLE_ALL problem.  The long stated design of U-Boot
is we enable what we need.  The long stated design of the Linux Kernel,
in general, is that it doesn't rely on the bootloader to have enabled
all of this.  And then various vendors (mine included) broke both rules
for a while when people weren't paying enough attention.  So yes, it's
likely that some boards will break because they assume the clock was
enabled for them.

The question is, are there boards where it's reasonable to assume people
would upgrade U-Boot but not upgrade the kernel and see this problem?
That I don't know the answer to.

The second question is, are there problems with leaving the clock
enablement on?  I suspect there is, which is how this was run into in
the first place.

At some level the platforms people use today and their needs outweigh
the problems of the platforms people used to use, but still maybe could,
if they wanted to dust it off and plug it in, and it didn't just up and
die on them.

-- 
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/20131120/b399b431/attachment.pgp>