[U-Boot] [PATCH V2 10/14] ARM: AM43xx: clocks: Update DPLL details for EPOS EVM

[Lokesh Vutla <lokeshvutla@ti.com>]

On Friday 22 November 2013 02:07 AM, Vaibhav Bedia wrote:
> On Thu, Nov 21, 2013 at 1:18 AM, Lokesh Vutla <lokeshvutla@ti.com> wrote:
>> Updating the Multiplier and Dividers values for all DPLLs for EPOS EVM.
>> Following are the DPLL locking frequencies at OPP NOM:
>> MPU locks at 600MHz
>> Core locks at 1000MHz
>> Per locks at 960MHz
>> DDR locks at 266MHz
>>
> 
> As mentioned earlier, this hardcoded frequency approach is really not
> scalable when you have
> more device variants coming in. Just look at the AM335x changes on how
> this gets complicated.
We already had a discussion on this during V1 of this series.
Sekhar and Tom replied to you comments. What is the point in asking the same question again?

Since you are very concerned here. Why are you feeling it so complicated when new variants come?
We can always differentiate the new variant from the old ones and can pass dpll structure accordingly. It is just a matter a one if condition.
It ll be better to look at the current code and see how cleanly it is done.

Thanks
Lokesh

> 
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  arch/arm/cpu/armv7/am33xx/clock.c        |   12 +++++++---
>>  arch/arm/cpu/armv7/am33xx/clock_am33xx.c |   15 ++++++++++++
>>  arch/arm/cpu/armv7/am33xx/clock_am43xx.c |    8 +------
>>  arch/arm/include/asm/arch-am33xx/clock.h |    7 +++---
>>  board/ti/am43xx/board.c                  |   38 +++++++++++++++++++++++++++---
>>  board/ti/am43xx/board.h                  |    1 +
>>  board/ti/am43xx/mux.c                    |    5 ++++
>>  7 files changed, 69 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/arm/cpu/armv7/am33xx/clock.c b/arch/arm/cpu/armv7/am33xx/clock.c
>> index 8e5f3c6..0672798 100644
>> --- a/arch/arm/cpu/armv7/am33xx/clock.c
>> +++ b/arch/arm/cpu/armv7/am33xx/clock.c
>> @@ -101,9 +101,15 @@ void do_setup_dpll(const struct dpll_regs *dpll_regs,
>>  static void setup_dplls(void)
>>  {
>>         const struct dpll_params *params;
>> -       do_setup_dpll(&dpll_core_regs, &dpll_core);
>> -       do_setup_dpll(&dpll_mpu_regs, &dpll_mpu);
>> -       do_setup_dpll(&dpll_per_regs, &dpll_per);
>> +
>> +       params = get_dpll_core_params();
>> +       do_setup_dpll(&dpll_core_regs, params);
>> +
>> +       params = get_dpll_mpu_params();
>> +       do_setup_dpll(&dpll_mpu_regs, params);
>> +
>> +       params = get_dpll_per_params();
>> +       do_setup_dpll(&dpll_per_regs, params);
>>         writel(0x300, &cmwkup->clkdcoldodpllper);
>>
>>         params = get_dpll_ddr_params();
>> diff --git a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
>> index fabe259..92142c8 100644
>> --- a/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
>> +++ b/arch/arm/cpu/armv7/am33xx/clock_am33xx.c
>> @@ -62,6 +62,21 @@ const struct dpll_params dpll_core = {
>>  const struct dpll_params dpll_per = {
>>                 960, OSC-1, 5, -1, -1, -1, -1};
>>
>> +const struct dpll_params *get_dpll_mpu_params(void)
>> +{
>> +       return &dpll_mpu;
>> +}
>> +
>> +const struct dpll_params *get_dpll_core_params(void)
>> +{
>> +       return &dpll_core;
>> +}
>> +
>> +const struct dpll_params *get_dpll_per_params(void)
>> +{
>> +       return &dpll_per;
>> +}
>> +
>>  void setup_clocks_for_console(void)
>>  {
>>         clrsetbits_le32(&cmwkup->wkclkstctrl, CD_CLKCTRL_CLKTRCTRL_MASK,
>> diff --git a/arch/arm/cpu/armv7/am33xx/clock_am43xx.c b/arch/arm/cpu/armv7/am33xx/clock_am43xx.c
>> index 22963b7..97c00b4 100644
>> --- a/arch/arm/cpu/armv7/am33xx/clock_am43xx.c
>> +++ b/arch/arm/cpu/armv7/am33xx/clock_am43xx.c
>> @@ -48,15 +48,9 @@ const struct dpll_regs dpll_ddr_regs = {
>>         .cm_idlest_dpll         = CM_WKUP + 0x5A4,
>>         .cm_clksel_dpll         = CM_WKUP + 0x5AC,
>>         .cm_div_m2_dpll         = CM_WKUP + 0x5B0,
>> +       .cm_div_m4_dpll         = CM_WKUP + 0x5B8,
>>  };
>>
>> -const struct dpll_params dpll_mpu = {
>> -               -1, -1, -1, -1, -1, -1, -1};
>> -const struct dpll_params dpll_core = {
>> -               -1, -1, -1, -1, -1, -1, -1};
>> -const struct dpll_params dpll_per = {
>> -               -1, -1, -1, -1, -1, -1, -1};
>> -
>>  void setup_clocks_for_console(void)
>>  {
>>         /* Do not add any spl_debug prints in this function */
>> diff --git a/arch/arm/include/asm/arch-am33xx/clock.h b/arch/arm/include/asm/arch-am33xx/clock.h
>> index 519249e..7637457 100644
>> --- a/arch/arm/include/asm/arch-am33xx/clock.h
>> +++ b/arch/arm/include/asm/arch-am33xx/clock.h
>> @@ -98,13 +98,12 @@ extern const struct dpll_regs dpll_mpu_regs;
>>  extern const struct dpll_regs dpll_core_regs;
>>  extern const struct dpll_regs dpll_per_regs;
>>  extern const struct dpll_regs dpll_ddr_regs;
>> -extern const struct dpll_params dpll_mpu;
>> -extern const struct dpll_params dpll_core;
>> -extern const struct dpll_params dpll_per;
>> -extern const struct dpll_params dpll_ddr;
>>
>>  extern struct cm_wkuppll *const cmwkup;
>>
>> +const struct dpll_params *get_dpll_mpu_params(void);
>> +const struct dpll_params *get_dpll_core_params(void);
>> +const struct dpll_params *get_dpll_per_params(void);
>>  const struct dpll_params *get_dpll_ddr_params(void);
>>  void do_setup_dpll(const struct dpll_regs *, const struct dpll_params *);
>>  void prcm_init(void);
>> diff --git a/board/ti/am43xx/board.c b/board/ti/am43xx/board.c
>> index 723d0ca..2d1b8f9 100644
>> --- a/board/ti/am43xx/board.c
>> +++ b/board/ti/am43xx/board.c
>> @@ -65,12 +65,44 @@ static int read_eeprom(struct am43xx_board_id *header)
>>
>>  #ifdef CONFIG_SPL_BUILD
>>
>> -const struct dpll_params dpll_ddr = {
>> -               -1, -1, -1, -1, -1, -1, -1};
>> +const struct dpll_params epos_evm_dpll_ddr = {
>> +               266, 24, 1, -1, 1, -1, -1};
>> +const struct dpll_params epos_evm_dpll_mpu = {
>> +               600, 24, 1, -1, -1, -1, -1};
>> +const struct dpll_params epos_evm_dpll_core = {
>> +               1000, 24, -1, -1, 10, 8, 4};
>> +const struct dpll_params epos_evm_dpll_per = {
>> +               960, 24, 5, -1, -1, -1, -1};
>>
>>  const struct dpll_params *get_dpll_ddr_params(void)
>>  {
>> -       return &dpll_ddr;
>> +       if (board_is_eposevm())
>> +               return &epos_evm_dpll_ddr;
>> +}
>> +
>> +const struct dpll_params *get_dpll_mpu_params(void)
>> +{
>> +       if (board_is_eposevm())
>> +               return &epos_evm_dpll_mpu;
>> +}
>> +
>> +const struct dpll_params *get_dpll_core_params(void)
>> +{
>> +       struct am43xx_board_id header;
>> +
>> +       enable_i2c0_pin_mux();
>> +       i2c_init(CONFIG_SYS_OMAP24_I2C_SPEED, CONFIG_SYS_OMAP24_I2C_SLAVE);
>> +       if (read_eeprom(&header) < 0)
>> +               puts("Could not get board ID.\n");
>> +
> 
> Reading the EEPROM again? Why don't you just read it once and update some
> common structure?
> 
> Regards,
> Vaibhav
>