[U-Boot] [PATCH] arch: ifc: update the IFC IP input clock

[york sun]

On 09/07/2016 06:30 PM, Prabhakar Kushwaha wrote:
>
>> -----Original Message-----
>> From: york sun
>> Sent: Wednesday, September 07, 2016 9:17 PM
>> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
>> boot at lists.denx.de
>> Subject: Re: [PATCH] arch: ifc: update the IFC IP input clock
>>
>> On 09/06/2016 07:42 PM, Prabhakar Kushwaha wrote:
>>>
>>>> -----Original Message-----
>>>> From: york sun
>>>> Sent: Tuesday, September 06, 2016 9:10 PM
>>>> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
>>>> boot at lists.denx.de
>>>> Subject: Re: [PATCH] arch: ifc: update the IFC IP input clock
>>>>
>>>> On 09/06/2016 04:15 AM, Prabhakar Kushwaha wrote:
>>>>> IFC IP clock is always a constant divisor of platform clock
>>>>> pre-defined per SoC. Clock Control register (CCR) used in
>>>>> current implementation governs IFC IP output clock.
>>>>>
>>>>> So update IFC IP clock to be defined as per predefined clock
>>>>> divisor of platform clock.
>>>>>
>>>>> Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
>>>>> ---
>>>>>  README                                              |  3 +++
>>>>>  arch/arm/cpu/armv7/ls102xa/clock.c                  | 10 ++--------
>>>>>  arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch2_speed.c | 10 ++--------
>>>>>  arch/arm/cpu/armv8/fsl-layerscape/fsl_lsch3_speed.c | 10 ++--------
>>>>>  arch/arm/include/asm/arch-fsl-layerscape/config.h   |  3 +++
>>>>>  arch/arm/include/asm/arch-ls102xa/config.h          |  1 +
>>>>>  arch/powerpc/cpu/mpc85xx/speed.c                    | 10 ++--------
>>>>>  arch/powerpc/include/asm/config_mpc85xx.h           |  9 +++++++++
>>>>>  8 files changed, 24 insertions(+), 32 deletions(-)
>>>>
>>>> Prabkahar,
>>>>
>>>> Two concerns here
>>>>
>>>> 1, it is not only IFC for powerpc. Older SoCs have local bus. That's why
>>>> the variable is named freq_localbus..
>>>>
>>>
>>> As per my understanding, Issue is valid for eLBC SoC also.
>>> Just wanted to confirm from internal IP team before spinning patch to fix it.
>>>
>>>> 2, what's the reason for this change? Is it wrong to use ccr to
>>>> calculate the clock? Or is it because recent Layerscape SoCs have
>>>> platform PLL different from platform clock? If the latter, can we limit
>>>> the fix to platform clock and not changing powerpc?
>>>>
>>>
>>> CCR governs the IFC output clock.
>>> This clock is used for synchronous NOR, NAND flashes. It is nowhere govern
>> IFC IP internal clock.
>>>
>>> It is true since conception of IFC. Unfortunately code written is wrong since
>> P1010.
>>> It is confusing everyone.
>>>
>>
>> Are you saying the freq_localbus should be the internal clock, not
>> output clock? As far as I can remember, this variable has always be used
>> for output clock since 85xx. I am open to the idea to change to internal
>> clock only if it makes sense. Browsing the code, I see this variable is
>> used for information only, except for setting arch.lbc_clk.
>>
>
> Everyone believe it to be IFC internal clock not the output clock.

Not everyone.

> As I always used to get query from customers about wrong IFC speed.
>
> So better to print IP clock to avoid any confusion.
> IFC output clock will be printed when it is actually being used during synchronous NOR, syn NAND.

I am not against changing it to internal clock. But what are you going 
to print on the console? I think it is confusing to say IFC or local bus 
internal clock speed. Please also check how this clock is used and make 
sure arch.lbc_clk is still correct, after passing to Linux.

York