[U-Boot] [PATCH V2 3/5] ARM: OMAP5: clocks: Add OPP settings required for OMAP543X ES2.0 soc

Nishanth Menon nm at ti.com
Tue Feb 5 16:29:59 CET 2013


On 17:32-20130205, R Sricharan wrote:
> Hi Nishanth,
> 
> 
> On Tuesday 05 February 2013 01:46 AM, Nishanth Menon wrote:
> >On 19:59-20130204, R Sricharan wrote:
> >>Change OPP settings as per the latest 0.4 version of
> >>addendum for OMAP5430 ES2.0
> >-->please be clear that these are for OPP_NOM. FYI, latest documentation
> >is 0.5 rev which was released in Jan, considering this patch was send in
> >Feb, might be good to cross verify any updates.
> >
>  Thanks for the info, i was not aware of 0.5 version. I will recheck
>  if something is different here.
> 
> >in addition, I suspect TRM was referred for certain DPLLs as well?
> >>
> >>Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
> >>Signed-off-by: R Sricharan <r.sricharan at ti.com>
> >>---
> >>  arch/arm/cpu/armv7/omap-common/clocks-common.c |    4 +
> >>  arch/arm/cpu/armv7/omap4/hw_data.c             |  142 +++++++-------
> >>  arch/arm/cpu/armv7/omap5/hw_data.c             |  242 +++++++++++++++---------
> >>  arch/arm/include/asm/arch-omap5/clocks.h       |    4 +
> >>  arch/arm/include/asm/omap_common.h             |    6 +-
> >>  5 files changed, 241 insertions(+), 157 deletions(-)
> >>
> >>diff --git a/arch/arm/cpu/armv7/omap-common/clocks-common.c b/arch/arm/cpu/armv7/omap-common/clocks-common.c
> >>index 88e5336..164253c 100644
> >>--- a/arch/arm/cpu/armv7/omap-common/clocks-common.c
> >>+++ b/arch/arm/cpu/armv7/omap-common/clocks-common.c
> >>@@ -103,10 +103,14 @@ void setup_post_dividers(u32 const base, const struct dpll_params *params)
> >>  		writel(params->m6_h13, &dpll_regs->cm_div_m6_h13_dpll);
> >>  	if (params->m7_h14 >= 0)
> >>  		writel(params->m7_h14, &dpll_regs->cm_div_m7_h14_dpll);
> >>+	if (params->h21 >= 0)
> >>+		writel(params->h21, &dpll_regs->cm_div_h21_dpll);
> >>  	if (params->h22 >= 0)
> >>  		writel(params->h22, &dpll_regs->cm_div_h22_dpll);
> >>  	if (params->h23 >= 0)
> >>  		writel(params->h23, &dpll_regs->cm_div_h23_dpll);
> >>+	if (params->h24 >= 0)
> >>+		writel(params->h24, &dpll_regs->cm_div_h24_dpll);
> >>  }
> >>
> >>  static inline void do_bypass_dpll(u32 const base)
> >>diff --git a/arch/arm/cpu/armv7/omap4/hw_data.c b/arch/arm/cpu/armv7/omap4/hw_data.c
> >>index 892d016..b01bf5d 100644
> >>--- a/arch/arm/cpu/armv7/omap4/hw_data.c
> >>+++ b/arch/arm/cpu/armv7/omap4/hw_data.c
> >>@@ -51,113 +51,113 @@ struct omap_sys_ctrl_regs const **ctrl =
> >>
> >>  /* dpll locked at 1400 MHz MPU clk at 700 MHz(OPP100) - DCC OFF */
> >this: equivalent to OPP_SB?
> >>  static const struct dpll_params mpu_dpll_params_1400mhz[NUM_SYS_CLKS] = {
> >>-	{175, 2, 1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>-	{700, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>-	{125, 2, 1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>-	{401, 10, 1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>-	{350, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>-	{700, 26, 1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>-	{638, 34, 1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>+	{175, 2, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{700, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{125, 2, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{401, 10, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{350, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{700, 26, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{638, 34, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>  };
> >>
> >>  /* dpll locked at 1584 MHz - MPU clk at 792 MHz(OPP Turbo 4430) */
> >Do we even support this? there is no OPP Turbo for OMAP5, we have
> >OPP_HIGH which needs DCC and AVS etc..
> >>  static const struct dpll_params mpu_dpll_params_1600mhz[NUM_SYS_CLKS] = {
> >>-	{200, 2, 1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>-	{800, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>-	{619, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>-	{125, 2, 1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>-	{400, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>-	{800, 26, 1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>-	{125, 5, 1, -1, -1, -1, -1, -1, -1, -1}		/* 38.4 MHz */
> >>+	{200, 2, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{800, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{619, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{125, 2, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{400, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{800, 26, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{125, 5, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1}		/* 38.4 MHz */
> >>  };
> >>
> >>  /* dpll locked at 1200 MHz - MPU clk at 600 MHz */
> >Please correct this.
> >>  static const struct dpll_params mpu_dpll_params_1200mhz[NUM_SYS_CLKS] = {
> >>-	{50, 0, 1, -1, -1, -1, -1, -1, -1, -1},		/* 12 MHz   */
> >>-	{600, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>-	{250, 6, 1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>-	{125, 3, 1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>-	{300, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>-	{200, 8, 1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>-	{125, 7, 1, -1, -1, -1, -1, -1, -1, -1}		/* 38.4 MHz */
> >Follow on question: if we are dropping ES1.0 support entirely - we
> >should drop it's support in id detection as well! ES1.0 and ES2.0 DPLL
> >configurations are different unfortunately.
> 
> >>+	{50, 0, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},		/* 12 MHz   */
> >>+	{600, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{250, 6, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{125, 3, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{300, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{200, 8, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{125, 7, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1}		/* 38.4 MHz */
> >>  };
> >>
> >>  static const struct dpll_params core_dpll_params_1600mhz[NUM_SYS_CLKS] = {
> >OPP_LOW is not supported on OMAP5 es2.0? Supported core DPLL locked frequency is
> >2127.36MHz?
> >>-	{200, 2, 1, 5, 8, 4, 6, 5, -1, -1},	/* 12 MHz   */
> >>-	{800, 12, 1, 5, 8, 4, 6, 5, -1, -1},	/* 13 MHz   */
> >>-	{619, 12, 1, 5, 8, 4, 6, 5, -1, -1},	/* 16.8 MHz */
> >>-	{125, 2, 1, 5, 8, 4, 6, 5, -1, -1},	/* 19.2 MHz */
> >>-	{400, 12, 1, 5, 8, 4, 6, 5, -1, -1},	/* 26 MHz   */
> >>-	{800, 26, 1, 5, 8, 4, 6, 5, -1, -1},	/* 27 MHz   */
> >>-	{125, 5, 1, 5, 8, 4, 6, 5, -1, -1}	/* 38.4 MHz */
> >>+	{200, 2, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{800, 12, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{619, 12, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{125, 2, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{400, 12, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{800, 26, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{125, 5, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1}	/* 38.4 MHz */
> >>  };
> >>
> >>  static const struct dpll_params core_dpll_params_es1_1524mhz[NUM_SYS_CLKS] = {
> >>-	{127, 1, 1, 5, 8, 4, 6, 5, -1, -1},	/* 12 MHz   */
> >>-	{762, 12, 1, 5, 8, 4, 6, 5, -1, -1},	/* 13 MHz   */
> >>-	{635, 13, 1, 5, 8, 4, 6, 5, -1, -1},	/* 16.8 MHz */
> >>-	{635, 15, 1, 5, 8, 4, 6, 5, -1, -1},	/* 19.2 MHz */
> >>-	{381, 12, 1, 5, 8, 4, 6, 5, -1, -1},	/* 26 MHz   */
> >>-	{254, 8, 1, 5, 8, 4, 6, 5, -1, -1},	/* 27 MHz   */
> >>-	{496, 24, 1, 5, 8, 4, 6, 5, -1, -1}	/* 38.4 MHz */
> >>+	{127, 1, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{762, 12, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{635, 13, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{635, 15, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{381, 12, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{254, 8, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{496, 24, 1, 5, 8, 4, 6, 5, -1, -1, -1, -1}	/* 38.4 MHz */
> >>  };
> >>
>    Answering all of your above questions here.
>    The above data is for OMAP4 and not OMAP5. This file was modified
>    here just to include dummy dividers. Because we were now using a
>    common dpll_params structure, there was no functional change in this
>    file. I think the problem was commit log was missing this info.
>     I will update to clarify this.
I dont think updating commit log alone is the right solution -> renaming the
variables make more sense - making ones meant for OMAP4 as _omap4430, 60
etc help readability of code.
[...]
> >>  /* ABE M & N values with 32K clock as source */
> >>  static const struct dpll_params abe_dpll_params_32k_196608khz = {
> >We do not intend to support 32K ABE source except when doing DPLL
> >cascading - so this is in effect an configuration which is
> >un-used in any s/w line.
> 
>   1) In fact we not locking ABE, IVA, USB DPLLs any more in bootloader
>      unless some one enable a CLOCKS_ENABLE_ALL config.
>      But we still have there data, so that we can quickly experiment
>      when needed.
it is no longer supported on OMAP5. If you have doubts on the silicon
usecase specification, I recommend you do read the TI internal
documentation for the same.
> 
>   2) I remember that locking with 32K source was considered better
>      because, we can cut sys_clk when audio is still running.
>      Also i think that the jitter was not a real problem for audio
>      even when running with 32Khz clock. I have to check this point
>      again.
please check again.
> 
>      The reason we had both the sources supported was just from an
>      experiment motive. I have seen that during pre silicon, we have
As a rule, experimentation code should NOT be in mainline. only
recommended operation code should ever be put in mainline - real
products are supposed to ramp with this code.

>      to switch the clock source from default to get locking. So having
>      support for both helps in those cases.
not if the silicon is neither validated NOR expected to be supported at
this.
[...]
> >>  static const struct dpll_params mpu_dpll_params_1_5ghz[NUM_SYS_CLKS] = {
> >I am not sure we'd like to do this.
>     This file is common for ES1.0 as well. Also i have added frequency
>     tables for OPP_HIGH and OPP_LOW as well. We some times find it
>     nessecary to boot with a different than OPP_NOM to do experiments.
>     So this is useful from that point.  But what is missing is the VDDs
>     which I usually add manually while testing. But now to complete
>    this, i will add that as well.
Why? Es1.0 will never be publically released, it never was meant to be
public. ES2.0 is the only one meant to go out. Supporting ES1.0 and
ES2.0 means a ton of redundant code being maintained in mainline with no
product lines meant to actively using it - ever! Do we really want to do
this??

> 
> >>-	{125, 0, 1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>-	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>-	{625, 6, 1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>-	{625, 7, 1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>-	{750, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>-	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>-	{625, 15, 1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>+	{125, 0, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{625, 6, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{625, 7, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{750, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{625, 15, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>  };
> >>
> >>  static const struct dpll_params mpu_dpll_params_2ghz[NUM_SYS_CLKS] = {
> >again, frequency is wrong for es2.0?
>    This file is common for ES1.0 as well.
ES1.0 DOES NOT run at 2GHz!
> 
> >>-	{500, 2, 1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>-	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>-	{2024, 16, 1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>-	{625, 5, 1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>-	{1000, 12, 1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>-	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>-	{625, 11, 1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>+	{500, 2, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 12 MHz   */
> >>+	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 13 MHz   */
> >>+	{2024, 16, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 16.8 MHz */
> >>+	{625, 5, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 19.2 MHz */
> >>+	{1000, 12, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 26 MHz   */
> >>+	{-1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1},	/* 27 MHz   */
> >>+	{625, 11, 1, -1, -1, -1, -1, -1, -1, -1, -1, -1}	/* 38.4 MHz */
> >>  };
> >>
> >>  static const struct dpll_params mpu_dpll_params_1100mhz[NUM_SYS_CLKS] = {
> >again, frequency is wrong for es2.0?
>    MPU runs at 1100 MHZ in OPP_NOM on 2.0
1099MHz - I suggest latest spec to be clear on this - i suggest
reviewing further based on update.
[...]
> >>+	.iva = iva_dpll_params_2330mhz_es2,
> >>+#ifdef CONFIG_SYS_OMAP_ABE_SYSCK
> >>+	.abe = abe_dpll_params_sysclk_196608khz,
> >>+#else
> >>+	.abe = &abe_dpll_params_32k_196608khz,
> >>+#endif
> >I strongly suggest dumping ABE sourcing from 32K clock - the code is
> >just a nuisance waiting to exercise an unsupported silicon feature
> >waiting to happen.
[...]
> >>  	default:
> >>diff --git a/arch/arm/include/asm/arch-omap5/clocks.h b/arch/arm/include/asm/arch-omap5/clocks.h
> >>index 15362ae..685aad5 100644
> >>--- a/arch/arm/include/asm/arch-omap5/clocks.h
> >>+++ b/arch/arm/include/asm/arch-omap5/clocks.h
> >>@@ -208,6 +208,10 @@
> >>  #define VDD_MM_5432	1150
> >>  #define VDD_CORE_5432	1150
> >>
> >>+#define VDD_MPU_ES2	1060
> >>+#define VDD_MM_ES2	1025
> >>+#define VDD_CORE_ES2	1040
> >^^^ these are meant only for OPP_NOM - considering that you have DPLLs
> >for OPP_SB added in, do you even expect these devices to bootup at
> >anything *but* OPP_NOM??
>   I agree. I add it manually while doing experiments. I will add these
>   here.
As a rule we want maintainable s/w in mainline, not experimental code.
maintainable s/w implies s/w that adheres to SoC's specified operational
ranges unfortunately :(.

-- 
Regards,
Nishanth Menon


More information about the U-Boot mailing list