From mboxrd@z Thu Jan 1 00:00:00 1970 From: Prafulla Wadaskar Date: Thu, 28 Jul 2011 12:14:18 -0700 Subject: [U-Boot] RFC [PATCH 2/5] arm/kirkwood: print speeds with cpu info. In-Reply-To: <20110728013157.GH11758@titan.lakedaemon.net> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de > -----Original Message----- > From: Jason [mailto:u-boot at lakedaemon.net] > Sent: Thursday, July 28, 2011 7:02 AM > To: Prafulla Wadaskar > Cc: clint at debian.org; wd at denx.de; u-boot at lists.denx.de; Prabhanjan > Sarnaik; Ashish Karkare; Siddarth Gore; bdale at gag.com > Subject: Re: RFC [PATCH 2/5] arm/kirkwood: print speeds with cpu info. > > On Wed, Jul 27, 2011 at 11:21:16AM -0700, Prafulla Wadaskar wrote: > > > > > > > -----Original Message----- > > > From: Jason Cooper [mailto:u-boot at lakedaemon.net] > > > Sent: Wednesday, July 27, 2011 2:49 AM > > > To: clint at debian.org; wd at denx.de; Prafulla Wadaskar > > > Cc: u-boot at lists.denx.de; Prabhanjan Sarnaik; Ashish Karkare; > Siddarth > > > Gore; bdale at gag.com; Jason Cooper > > > Subject: RFC [PATCH 2/5] arm/kirkwood: print speeds with cpu info. > > > > > > > > > Signed-off-by: Jason Cooper > > > --- > > > arch/arm/cpu/arm926ejs/kirkwood/cpu.c | 46 > > > ++++++++++++++++++++++++++++++ > > > arch/arm/include/asm/arch-kirkwood/cpu.h | 1 + > > > 2 files changed, 47 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > > > b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > > > index b4a4c04..a69f9f2 100644 > > > --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > > > +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c > > > @@ -270,11 +270,26 @@ static void kw_sysrst_check(void) > > > } > > > > > > #if defined(CONFIG_DISPLAY_CPUINFO) > > > +#define MSAR_CPUCLCK_EXTRACT(X) (((X & 0x2) >> 1) | ((X & > 0x400000) >> > > > 21) | \ > > > + ((X & 0x18) >> 1)) > > > +#define MSAR_L2CLCK_EXTRACT(X) (((X & 0x600) >> 9) | ((X & > 0x80000) >> > > > 17)) > > > +#define MSAR_DDRCLCK_RTIO_MASK (0xf << 5) > > > + > > > +#define MSAR_TCLCK_OFFS 21 > > > +#define MSAR_TCLCK_MASK (0x1 << MSAR_TCLCK_OFFS) > > > +#define MV_BOARD_TCLK_166MHZ 166666667 > > > +#define MV_BOARD_TCLK_200MHZ 200000000 > > > +#define MSAR_TCLCK_167 (0x1 << MSAR_TCLCK_OFFS) > > > +#define MSAR_TCLCK_200 (0x0 << MSAR_TCLCK_OFFS) > > > > There are one time used macros, can they be optimized? > > Sure, do you prefer a static assignment: > > #define MSAR_TCLCK_166 0x00200000 This is okay with some comments. > > or, just do away with it altogether and use the number? > > > > + > > > int print_cpuinfo(void) > > > { > > > char *rev; > > > u16 devid = (readl(KW_REG_PCIE_DEVID) >> 16) & 0xffff; > > > u8 revid = readl(KW_REG_PCIE_REVID) & 0xff; > > > + u32 cpu_clk, t_clk, tmp; > > > + u32 sys_clk, l2_clk; > > > + u32 l2_ratio, ddr_ratio; > > > > > > if ((readl(KW_REG_DEVICE_ID) & 0x03) > 2) { > > > printf("Error.. %s:Unsupported Kirkwood SoC 88F%04x\n", > > > __FUNCTION__, devid); > > > @@ -297,6 +312,37 @@ int print_cpuinfo(void) > > > } > > > > > > printf("SoC: Kirkwood 88F%04x_%s\n", devid, rev); > > > + > > > + tmp = readl(MPP_SAMPLE_AT_RESET); > > > + cpu_clk = MSAR_CPUCLCK_EXTRACT(tmp); > > > + if (cpu_clk == 0x9) > > > + cpu_clk = 1200; > > > + > > > + l2_ratio = MSAR_L2CLCK_EXTRACT(tmp); > > > + l2_clk = cpu_clk / l2_ratio; > > > + > > > + ddr_ratio = tmp & MSAR_DDRCLCK_RTIO_MASK; > > > + ddr_ratio = ddr_ratio >> 5; > > > + if (ddr_ratio == 4) > > > + sys_clk = 400; > > > + > > > + switch (tmp & MSAR_TCLCK_MASK) { > > > + case MSAR_TCLCK_167: > > > + t_clk = MV_BOARD_TCLK_166MHZ; > > > + break; > > > + case MSAR_TCLCK_200: > > > + t_clk = MV_BOARD_TCLK_200MHZ; > > > + break; > > > + default: > > > + t_clk = MV_BOARD_TCLK_200MHZ; > > > + break; > > > + } > > > + > > > + printf("CPU running @ %dMHz L2 running @ %dMHz\n", > > > + cpu_clk, l2_clk); > > > + printf("SysClock = %dMHz, TClock = %dMHz\n", > > > + sys_clk, t_clk / 1000000); > > > > It is good to encapsulate this in #ifdef CONFIG_SYS_XXX > > It's already encapsulated in CONFIG_DISPLAY_CPUINFO, so if enabled, on > boot it looks like: > > SoC: Kirkwood 88F6281_A1 > CPU running @ 1200MHz L2 running @ 400MHz > SysClock = 400MHz, TClock = 200MHz > > I can encapsulate it separately if you want, but it seems a little too > fine grained for me. Either I want cpu info, or I don't. ymmv. I think you are right, this should be okay, anyway it displays useful information. Sorry for this noisy comments. Regards.. Prafulla . .