[U-Boot] [PATCH 1/4] ARM: AM33xx: Cleanup dplls data

[Lokesh Vutla]

Hi Heiko,
On Tuesday 25 June 2013 10:24 AM, Heiko Schocher wrote:
> Hello Lokesh,
>
> Am 25.06.2013 05:48, schrieb Lokesh Vutla:
>> Hi Heiko,
>> On Tuesday 25 June 2013 12:42 AM, Heiko Schocher wrote:
>>> Hello Lokesh,
>>>
>>> Am 24.06.2013 15:15, schrieb Lokesh Vutla:
>>>> Locking sequence for all the dplls is same.
>>>> In the current code same sequence is done repeatedly
>>>> for each dpll. Instead have a generic function
>>>> for locking dplls and pass dpll data to that function.
>>>>
>>>> This is derived from OMAP4 boards.
>>>>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
>>>> ---
>>>>    arch/arm/cpu/armv7/am33xx/Makefile           |    1 +
>>>>    arch/arm/cpu/armv7/am33xx/clock.c            |  116 ++++++++++++++
>>>>    arch/arm/cpu/armv7/am33xx/clock_am33xx.c     |  222 +++++---------------------
>>>>    arch/arm/cpu/armv7/am33xx/emif4.c            |    4 +
>>>>    arch/arm/include/asm/arch-am33xx/clock.h     |   68 ++++++++
>>>>    arch/arm/include/asm/arch-am33xx/ddr_defs.h  |    2 +
>>>>    arch/arm/include/asm/arch-am33xx/sys_proto.h |    1 +
>>>>    7 files changed, 232 insertions(+), 182 deletions(-)
>>>>    create mode 100644 arch/arm/cpu/armv7/am33xx/clock.c
>>>>
>>> [...]
>>>> diff --git a/arch/arm/cpu/armv7/am33xx/clock.c b/arch/arm/cpu/armv7/am33xx/clock.c
>>>> new file mode 100644
>>>> index 0000000..a7f1d83
>>>> --- /dev/null
>>>> +++ b/arch/arm/cpu/armv7/am33xx/clock.c
>>>> @@ -0,0 +1,116 @@
>>> [...]
>>>> +static void do_setup_dpll(const struct dpll_regs *dpll_regs,
>>>> +			  const struct dpll_params *params)
>>>> +{
>>>
>>> Could we have this function not only static? I posted a patch:
>>>
>>> [U-Boot] arm, am335x: make mpu pll config configurable
>>> http://patchwork.ozlabs.org/patch/248509/
>>>
>>> which uses mpu_pll_config() for switching mpu pll clock
>>> from board code ... you delete this function later in this patch,
>>> so I think, I can switch to do_setup_pll() ... if this is not
>>> static code ...
>> Yes I saw that patch. No need to make this non-static.
>> Please have your own struct "const struct dpll_params dpll_mpu"
>> and update your values accordingly.
>
> Hmm.. maybe I miss something here. You call setup_dplls()
> in arch/arm/cpu/armv7/am33xx/clock.c using &dpll_mpu defined
> in arch/arm/cpu/armv7/am33xx/clock_am33xx.c ... so how to
> make here a board specific struct?
>
> The MPUCLK is configurable through the define CONFIG_SYS_MPUCLK
> which is good, but I have on this board a PMIC, which in board SPL
> code change MPU and core voltage ... and after that I change
> the MPU clock again ...
Ohk.
Can't we scale the voltages before calling setup_dplls()
(Why do you want to configure the MPU clocks twice?
I don't know much about your board, so I am just asking..:) )
What I meant is something like below:
void __weak scale_vcores(void)
{}

void prcm_init()
{
	enable_basic_clocks();
	scale_vcores();
	setup_dplls();
}

have your own scale_vcores in your board file.
and for dpll_mpu have something like this:
#ifdef CONFIG_<BOARD>
const struct dpll_params dpll_mpu = {
		M, N, 1, -1, -1, -1, -1};
#else
const struct dpll_params dpll_mpu = {
		CONFIG_SYS_MPUCLK, OSC-1, 1, -1, -1, -1, -1};
#endif

I hope this should be possible on your board.
I am telling this because it will be easy for me during my next cleanup 
during
which I planned to combine omap-common and am33xx code..:)

This is the exactly what is done for omap( program voltages and then 
setup dplls)
You can refer to arch/arm/cpu/armv7/omap-common/clocks-common.c
prcm_init() function.

Please correct me if I am wrong..

Thanks and regards,
Lokesh

>
> bye,
> Heiko
>