[U-Boot] [PATCH v2 13/22] omap4: add clock support

Wolfgang Denk
2011-05-15T19:00:29Z

Dear Aneesh V,

In message <1305472900-4004-14-git-send-email-aneesh@ti.com> you wrote:
> Add support for:
> 1. DPLL locking
> 2. Initialization of clock domains and clock modules
>
> This work draws upon previous work done for x-loader mainly by:
> Santosh Shilimkar <santosh.shilimkar@ti.com>
> Rajendra Nayak <rnayak@ti.com>
>
> Signed-off-by: Aneesh V <aneesh@ti.com>
> ---
> V2:
> * Use pre-calculated M & N values instead of calculated ones
> * Changes due to make file changes
> * Some corrections
> * Do all clock initialization in SPL itself instead of differing some
> work to u-boot
> ---
> arch/arm/cpu/armv7/omap4/Makefile | 1 +
> arch/arm/cpu/armv7/omap4/board.c | 1 +
> arch/arm/cpu/armv7/omap4/clocks.c | 731 +++++++++++++++++++++++++++
> arch/arm/cpu/armv7/omap4/clocks_get_m_n.c | 154 ++++++
> arch/arm/include/asm/arch-omap4/clocks.h | 506 ++++++++++++++++++
> arch/arm/include/asm/arch-omap4/sys_proto.h | 6 +
> arch/arm/include/asm/omap_common.h | 3 +
> spl/board/ti/omap4.mk | 7 +-
> 8 files changed, 1408 insertions(+), 1 deletions(-)
> create mode 100644 arch/arm/cpu/armv7/omap4/clocks.c
> create mode 100644 arch/arm/cpu/armv7/omap4/clocks_get_m_n.c
> create mode 100644 arch/arm/include/asm/arch-omap4/clocks.h

It appears this might be part of or taken from some bigger scope
clocks framework. Otherwise it's diffcult for me to understand why
OMAP4 needs 1400+ lines of code, when other SoCs appear to do with
considerably less. Please comment.

> diff --git a/arch/arm/cpu/armv7/omap4/Makefile b/arch/arm/cpu/armv7/omap4/Makefile
> index 987dc9d..6154e86 100644
> --- a/arch/arm/cpu/armv7/omap4/Makefile
> +++ b/arch/arm/cpu/armv7/omap4/Makefile
> @@ -30,6 +30,7 @@ SOBJS += lowlevel_init.o
> COBJS += board.o
> COBJS += mem.o
> COBJS += sys_info.o
> +COBJS += clocks.o

Please keep lists sorted.

...
> +static inline void do_bypass_dpll(u32 base)
> +{
> + struct dpll_regs *dpll_regs = (struct dpll_regs *)base;
> +
> + modify_reg_32(&dpll_regs->cm_clkmode_dpll,
> + CM_CLKMODE_DPLL_DPLL_EN_SHIFT,
> + CM_CLKMODE_DPLL_DPLL_EN_MASK, DPLL_EN_FAST_RELOCK_BYPASS);

NAK, please use standard macros (see previous messages).

> +static void do_setup_dpll(u32 base, const struct dpll_params *params, u8 lock)
> +{
> + u32 temp;
> + struct dpll_regs *dpll_regs = (struct dpll_regs *)base;
> +
> + bypass_dpll(base);
> +
> + /* Set M & N */
> + temp = readl(&dpll_regs->cm_clksel_dpll);
> + set_bit_field(temp, CM_CLKSEL_DPLL_M_SHIFT, CM_CLKSEL_DPLL_M_MASK,
> + params->m);
> + set_bit_field(temp, CM_CLKSEL_DPLL_N_SHIFT, CM_CLKSEL_DPLL_N_MASK,
> + params->n);
> + writel(temp, &dpll_regs->cm_clksel_dpll);

NAK, please use standard macros (see previous messages).

...
> diff --git a/arch/arm/cpu/armv7/omap4/clocks_get_m_n.c b/arch/arm/cpu/armv7/omap4/clocks_get_m_n.c
> new file mode 100644
> index 0000000..777ec11
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/omap4/clocks_get_m_n.c
> @@ -0,0 +1,154 @@
...
> +void main(void)

It appears this is a host program, not part of U-Boot. I don't think
that arch/arm/cpu/armv7/omap4/ is the right place for this program.

...
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-omap4/clocks.h
> @@ -0,0 +1,506 @@
...
> +#define CM_WKUP_CLKSTCTRL (OMAP44XX_L4_WKUP_BASE + 0x7800)
> +#define CM_WKUP_L4WKUP_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7820)
> +#define CM_WKUP_WDTIMER1_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7828)
> +#define CM_WKUP_WDTIMER2_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7830)
> +#define CM_WKUP_GPIO1_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7838)
> +#define CM_WKUP_GPTIMER1_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7840)
> +#define CM_WKUP_GPTIMER12_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7848)
> +#define CM_WKUP_SYNCTIMER_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7850)
> +#define CM_WKUP_USIM_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7858)
> +#define CM_WKUP_SARRAM_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7860)
> +#define CM_WKUP_KEYBOARD_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7878)
> +#define CM_WKUP_RTC_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7880)
> +#define CM_WKUP_BANDGAP_CLKCTRL (OMAP44XX_L4_WKUP_BASE + 0x7888)
> +
> +/* DEVICE_PRM Registers */
> +#define PRM_VC_VAL_BYPASS (OMAP44XX_L4_WKUP_BASE + 0x7BA0)
> +#define PRM_VC_CFG_CHANNEL (OMAP44XX_L4_WKUP_BASE + 0x7BA4)
> +#define PRM_VC_CFG_I2C_MODE (OMAP44XX_L4_WKUP_BASE + 0x7BA8)
> +#define PRM_VC_CFG_I2C_CLK (OMAP44XX_L4_WKUP_BASE + 0x7BAC)
> +
> +/* CM1.CKGEN module registers */
> +#define CM_CLKSEL_CORE (OMAP44XX_L4_CORE_BASE + 0x4100)
> +#define CM_CLKSEL_ABE (OMAP44XX_L4_CORE_BASE + 0x4108)
> +#define CM_DLL_CTRL (OMAP44XX_L4_CORE_BASE + 0x4110)
> +#define CM_CLKMODE_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4120)
> +#define CM_IDLEST_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4124)
> +#define CM_AUTOIDLE_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4128)
> +#define CM_CLKSEL_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x412c)
> +#define CM_DIV_M2_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4130)
> +#define CM_DIV_M3_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4134)
> +#define CM_DIV_M4_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4138)
> +#define CM_DIV_M5_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x413c)
> +#define CM_DIV_M6_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4140)
> +#define CM_DIV_M7_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4144)
> +#define CM_SSC_DELTAMSTEP_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4148)
> +#define CM_SSC_MODFREQDIV_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x414c)
> +#define CM_EMU_OVERRIDE_DPLL_CORE (OMAP44XX_L4_CORE_BASE + 0x4150)
> +#define CM_CLKMODE_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x4160)
> +#define CM_IDLEST_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x4164)
> +#define CM_AUTOIDLE_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x4168)
> +#define CM_CLKSEL_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x416c)
> +#define CM_DIV_M2_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x4170)
> +#define CM_SSC_DELTAMSTEP_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x4188)
> +#define CM_SSC_MODFREQDIV_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x418c)
> +#define CM_BYPCLK_DPLL_MPU (OMAP44XX_L4_CORE_BASE + 0x419c)
> +#define CM_CLKMODE_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41a0)
> +#define CM_IDLEST_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41a4)
> +#define CM_AUTOIDLE_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41a8)
> +#define CM_CLKSEL_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41ac)
> +#define CM_DIV_M4_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41b8)
> +#define CM_DIV_M5_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41bc)
> +#define CM_SSC_DELTAMSTEP_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41c8)
> +#define CM_SSC_MODFREQDIV_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41cc)
> +#define CM_BYPCLK_DPLL_IVA (OMAP44XX_L4_CORE_BASE + 0x41dc)
> +#define CM_CLKMODE_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41e0)
> +#define CM_IDLEST_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41e4)
> +#define CM_AUTOIDLE_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41e8)
> +#define CM_CLKSEL_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41ec)
> +#define CM_DIV_M2_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41f0)
> +#define CM_DIV_M3_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x41f4)
> +#define CM_SSC_DELTAMSTEP_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x4208)
> +#define CM_SSC_MODFREQDIV_DPLL_ABE (OMAP44XX_L4_CORE_BASE + 0x420c)
> +#define CM_CLKMODE_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4220)
> +#define CM_IDLEST_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4224)
> +#define CM_AUTOIDLE_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4228)
> +#define CM_CLKSEL_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x422c)
> +#define CM_DIV_M2_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4230)
> +#define CM_DIV_M4_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4238)
> +#define CM_DIV_M5_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x423c)
> +#define CM_DIV_M6_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4240)
> +#define CM_SSC_DELTAMSTEP_DPLL_DDRPHY (OMAP44XX_L4_CORE_BASE + 0x4248)
> +#define CM_SHADOW_FREQ_CONFIG1 (OMAP44XX_L4_CORE_BASE + 0x4260)

NAK. We do not accept base address plus offset notation. Please
declare C structs instead.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I think animal testing is a terrible idea; they get all nervous and
give the wrong answers.