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

[Michael Trimarchi]

Hi

On Tue, Nov 19, 2013 at 12:10 PM, Igor Grinberg <grinberg at 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 at amarulasolutions.com> wrote:
>>> Hi Igor
>>>
>>> On Tue, Nov 19, 2013 at 9:24 AM, Igor Grinberg <grinberg at 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 at 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

> As a consequence of this patch, there might be boards that
> are not correctly configured and thus broken.
> In other words, this patch breaks boards that rely on those clocks
> being enabled.

Michael

>


>>>
>>
>> To be more clear, this change just enable clocks that are supposed
>> to be used.
>
> Well, if I read the patch correctly, it does not enable anything.
> It removes the unconditional configuration of some clocks...
>
>> Now that code was supposed to be removed long
>> time ago.
>
> Were there any discussions on this meter?
> Can you please point me to a discussion on this?
> Otherwise, the 'supposed' word here does not have a basis.
>
>>
>> Michael
>>
>>> Michael
>>>
>>>>>
>>>>>       sdelay(1000);
>>>>>  }
>>>>> diff --git a/arch/arm/include/asm/arch-omap3/clock.h b/arch/arm/include/asm/arch-omap3/clock.h
>>>>> index be669c1..1912cc9 100644
>>>>> --- a/arch/arm/include/asm/arch-omap3/clock.h
>>>>> +++ b/arch/arm/include/asm/arch-omap3/clock.h
>>>>> @@ -27,8 +27,6 @@
>>>>>  #define ICK_DSS_ON   0x00000001
>>>>>  #define FCK_CAM_ON   0x00000001
>>>>>  #define ICK_CAM_ON   0x00000001
>>>>> -#define FCK_PER_ON   0x0003ffff
>>>>> -#define ICK_PER_ON   0x0003ffff
>>>>>
>>>>>  /* Used to index into DPLL parameter tables */
>>>>>  typedef struct {
>>>>>
>>>>
>>>> --
>>>> Regards,
>>>> Igor.
>>>
>>>
>>>
>>> --
>>> | Michael Nazzareno Trimarchi                     Amarula Solutions BV |
>>> | COO  -  Founder
>>> Cruquiuskade 47 |
>>> | +31(0)851119172                                   Amsterdam 1018 AM NL |
>>
>
> --
> Regards,
> Igor.



-- 
| Michael Nazzareno Trimarchi                     Amarula Solutions BV |
| COO  -  Founder
Cruquiuskade 47 |
| +31(0)851119172                                   Amsterdam 1018 AM NL |