Hi Heiko, On Tuesday 25 June 2013 12:35 PM, Heiko Schocher wrote:
Hello Lokesh,
Am 25.06.2013 07:39, schrieb 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@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 speak with the customer ...
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
No, that is not good. We should prevent such board specific defines in common code. I think this define is not necessary, as, if we have a scale_vcore() function, I can set CONFIG_SYS_MPUCLK to the end value ! I try this out! Thanks!
My idea here is to populate data according to the board. Its good if you use the same value.
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..:)
Ok, i try it ...
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..
Yes, that looks good. Hmm... have we access to an pmic connected over i2c at this time?
you can do an i2c_init() here. Thanks and regards, Lokesh
bye, Heiko