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

2013-06-25 Thread Heiko Schocher 
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 lokeshvu...@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 000..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!

 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?

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-25 Thread Lokesh Vutla 

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 lokeshvu...@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 000..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



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-25 Thread Heiko Schocher 
Hello Lokesh,

Am 25.06.2013 10:17, schrieb Lokesh Vutla:
 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 lokeshvu...@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 000..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 ...

It seems, we can make this static, no need for
doing this dynamically ... I try it ...

 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
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-25 Thread Tom Rini 
On Tue, Jun 25, 2013 at 11:09:41AM +0530, Lokesh Vutla wrote:
 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 lokeshvu...@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 000..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();
 }

Keep in mind the OPP50 advisory (errata 1.0.24) as well.  The first
fix/work-around for this I've seen drops us down to start with, and then
raises things up.

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-25 Thread Heiko Schocher 
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 lokeshvu...@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


Acked-by: Heiko Schocher h...@denx.de

Tested on 3 am335x boards (no need anymore to set mpu_pll twice
on this boards :-), so:

Tested-by: Heiko Schocher h...@denx.de

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-25 Thread Heiko Schocher 
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 lokeshvu...@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

Acked-by: Heiko Schocher h...@denx.de

Tested on 3 am335x boards so:

Tested-by: Heiko Schocher h...@denx.de

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-24 Thread Heiko Schocher 
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 lokeshvu...@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 000..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 ...

[...]
 diff --git a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c 
 b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
 index 9c4d0b4..e878b25 100644
 --- a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
 +++ b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
 @@ -26,56 +26,53 @@
  #define PRCM_FORCE_WAKEUP0x2
  #define PRCM_FUNCTL  0x0
  
 -#define PRCM_EMIF_CLK_ACTIVITY   BIT(2)
 -#define PRCM_L3_GCLK_ACTIVITYBIT(4)
 -
 -#define PLL_BYPASS_MODE  0x4
 -#define ST_MN_BYPASS 0x0100
 -#define ST_DPLL_CLK  0x0001
 -#define CLK_SEL_MASK 0x7
 -#define CLK_DIV_MASK 0x1f
 -#define CLK_DIV2_MASK0x7f
 -#define CLK_SEL_SHIFT0x8
 -#define CLK_MODE_SEL 0x7
 -#define CLK_MODE_MASK0xfff8
 -#define CLK_DIV_SEL  0xFFE0
  #define CPGMAC0_IDLE 0x3
 -#define DPLL_CLKDCOLDO_GATE_CTRL0x300
 -
  #define OSC  (V_OSCK/100)

and could we move this define then to
arch/arm/include/asm/arch-am33xx/clock.h
too?

Thnaks!

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-24 Thread 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 lokeshvu...@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 000..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.

Thanks and regards,
Lokesh


[...]

diff --git a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c 
b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
index 9c4d0b4..e878b25 100644
--- a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
+++ b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
@@ -26,56 +26,53 @@
  #define PRCM_FORCE_WAKEUP 0x2
  #define PRCM_FUNCTL   0x0

-#define PRCM_EMIF_CLK_ACTIVITY BIT(2)
-#define PRCM_L3_GCLK_ACTIVITY  BIT(4)
-
-#define PLL_BYPASS_MODE0x4
-#define ST_MN_BYPASS   0x0100
-#define ST_DPLL_CLK0x0001
-#define CLK_SEL_MASK   0x7
-#define CLK_DIV_MASK   0x1f
-#define CLK_DIV2_MASK  0x7f
-#define CLK_SEL_SHIFT  0x8
-#define CLK_MODE_SEL   0x7
-#define CLK_MODE_MASK  0xfff8
-#define CLK_DIV_SEL0xFFE0
  #define CPGMAC0_IDLE  0x3
-#define DPLL_CLKDCOLDO_GATE_CTRL0x300
-
  #define OSC   (V_OSCK/100)


and could we move this define then to
arch/arm/include/asm/arch-am33xx/clock.h
too?

Thnaks!

bye,
Heiko



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-24 Thread Heiko Schocher 
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 lokeshvu...@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 000..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 ...

bye,
Heiko
-- 
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

2013-06-24 Thread 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 lokeshvu...@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 000..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



___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot