* [U-Boot] [patch] Add display cpuinfo for Atmel at91sam9261 Cores @ 2009-05-02 20:06 Remy Bohmer 2009-05-18 20:25 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 9+ messages in thread From: Remy Bohmer @ 2009-05-02 20:06 UTC (permalink / raw) To: u-boot An embedded and charset-unspecified text was scrubbed... Name: 0001-Add-display-cpuinfo-for-Atmel-at91sam9261-Cores.patch Url: http://lists.denx.de/pipermail/u-boot/attachments/20090502/b1fab66b/attachment.txt ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [patch] Add display cpuinfo for Atmel at91sam9261 Cores 2009-05-02 20:06 [U-Boot] [patch] Add display cpuinfo for Atmel at91sam9261 Cores Remy Bohmer @ 2009-05-18 20:25 ` Jean-Christophe PLAGNIOL-VILLARD 2009-06-01 18:21 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Remy Bohmer 0 siblings, 1 reply; 9+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-05-18 20:25 UTC (permalink / raw) To: u-boot On 22:06 Sat 02 May , Remy Bohmer wrote: > This patch adds support for displaying CPU type information > about Atmel AT91 cores during boot. It currently only configures it for the > at91sam9261ek board, but it can easily be configured for all at91 based cores. please do this in two seperate patch please ask Stelian CC btw please use git > > This patch also provides at91_dbgu.h which is also used by the upcoming > USB CDC-ethernet and USB-gadget support for this board. > > Signed-off-by: Remy B?hmer <linux@bohmer.net> > --- > cpu/arm926ejs/at91/Makefile | 1 > cpu/arm926ejs/at91/cpuinfo.c | 83 ++++++++++++++++++++++++++++ > include/asm-arm/arch-at91/at91_dbgu.h | 69 +++++++++++++++++++++++ > include/asm-arm/arch-at91/cpu.h | 99 ++++++++++++++++++++++++++++++++++ > include/configs/at91sam9261ek.h | 2 > 5 files changed, 254 insertions(+) > create mode 100644 cpu/arm926ejs/at91/cpuinfo.c > create mode 100644 include/asm-arm/arch-at91/at91_dbgu.h > create mode 100644 include/asm-arm/arch-at91/cpu.h > > Index: u-boot-usb.tmp/cpu/arm926ejs/at91/Makefile > =================================================================== > --- u-boot-usb.tmp.orig/cpu/arm926ejs/at91/Makefile 2009-05-02 21:43:41.000000000 +0200 > +++ u-boot-usb.tmp/cpu/arm926ejs/at91/Makefile 2009-05-02 22:01:46.000000000 +0200 > @@ -55,6 +55,7 @@ COBJS-y += at91sam9rl_serial.o > COBJS-$(CONFIG_HAS_DATAFLASH) += at91sam9rl_spi.o > endif > COBJS-$(CONFIG_AT91_LED) += led.o > +COBJS-$(CONFIG_DISPLAY_CPUINFO) +=cpuinfo.o > COBJS-y += clock.o > COBJS-y += cpu.o > COBJS-y += timer.o > Index: u-boot-usb.tmp/cpu/arm926ejs/at91/cpuinfo.c > =================================================================== > --- /dev/null 1970-01-01 00:00:00.000000000 +0000 > +++ u-boot-usb.tmp/cpu/arm926ejs/at91/cpuinfo.c 2009-05-02 22:01:46.000000000 +0200 > @@ -0,0 +1,83 @@ > +#include <command.h> > +#include <common.h> > +#include <arm926ejs.h> > +#include <asm/hardware.h> > +#include <asm/arch/at91_dbgu.h> > +#include <asm/arch/io.h> > + > +int print_cpuinfo(void) > +{ > + unsigned long cidr; > + const char *txt; > + > + cidr = at91_sys_read(AT91_DBGU_CIDR); > + could you use the CONFIG_ARM926EJS and other configs to reduce the size impact > + switch ((cidr & AT91_CIDR_EPROC) >> 5) { > + case 0x1: txt = "ARM946ES"; break; > + case 0x2: txt = "ARM7TDMI"; break; > + case 0x4: txt = "ARM920T"; break; > + case 0x5: txt = "ARM926EJS"; break; > + default: txt = "undefined"; break; > + } > + printf("Embedded Processor: %s\n", txt); otherwise ok Best Regards, J. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores 2009-05-18 20:25 ` Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-01 18:21 ` Remy Bohmer 2009-06-01 18:21 ` [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 Remy Bohmer 2009-06-01 19:59 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Stelian Pop 0 siblings, 2 replies; 9+ messages in thread From: Remy Bohmer @ 2009-06-01 18:21 UTC (permalink / raw) To: u-boot Signed-off-by: Remy Bohmer <linux@bohmer.net> --- V2 reworked comments from Jean-Christophe PLAGNIOL-VILLARD cpu/arm926ejs/at91/Makefile | 1 + cpu/arm926ejs/at91/cpuinfo.c | 65 +++++++++++++++++++++ include/asm-arm/arch-at91/at91_dbgu.h | 69 +++++++++++++++++++++++ include/asm-arm/arch-at91/cpu.h | 99 +++++++++++++++++++++++++++++++++ 4 files changed, 234 insertions(+), 0 deletions(-) create mode 100644 cpu/arm926ejs/at91/cpuinfo.c create mode 100644 include/asm-arm/arch-at91/at91_dbgu.h create mode 100644 include/asm-arm/arch-at91/cpu.h diff --git a/cpu/arm926ejs/at91/Makefile b/cpu/arm926ejs/at91/Makefile index 71acb0b..5c970a1 100644 --- a/cpu/arm926ejs/at91/Makefile +++ b/cpu/arm926ejs/at91/Makefile @@ -32,6 +32,7 @@ COBJS-$(CONFIG_AT91SAM9261) += at91sam9261_devices.o COBJS-$(CONFIG_AT91SAM9263) += at91sam9263_devices.o COBJS-$(CONFIG_AT91SAM9RL) += at91sam9rl_devices.o COBJS-$(CONFIG_AT91_LED) += led.o +COBJS-$(CONFIG_DISPLAY_CPUINFO) +=cpuinfo.o COBJS-y += clock.o COBJS-y += cpu.o COBJS-y += timer.o diff --git a/cpu/arm926ejs/at91/cpuinfo.c b/cpu/arm926ejs/at91/cpuinfo.c new file mode 100644 index 0000000..1c6abad --- /dev/null +++ b/cpu/arm926ejs/at91/cpuinfo.c @@ -0,0 +1,65 @@ +#include <command.h> +#include <common.h> +#include <arm926ejs.h> +#include <asm/hardware.h> +#include <asm/arch/at91_dbgu.h> +#include <asm/arch/io.h> + +int print_cpuinfo(void) +{ + unsigned long cidr; + const char *txt; + + cidr = at91_sys_read(AT91_DBGU_CIDR); + + switch ((cidr & AT91_CIDR_EPROC) >> 5) { + /* 0x1 = ARM946ES, 0x2 = ARM7TDMI, 0x4 = ARM920T */ + case 0x5: txt = "ARM926EJS"; break; + default: txt = "undefined"; break; + } + printf("Embedded Processor: %s\n", txt); + + switch ((cidr & AT91_CIDR_ARCH) >> 20) { + case 0x19: txt = "AT91SAM9xx"; break; + case 0x29: txt = "AT91SAM9XExx"; break; + case 0x39: txt = "CAP9"; break; + default: txt = "undefined"; break; + } + printf("Architecture: %s Series\n", txt); + + printf("CPU-revision: %c\n", (char)('A' + (cidr & AT91_CIDR_VERSION))); + + switch ((cidr & AT91_CIDR_NVPTYP) >> 28) { + case 0x0: txt = "ROM"; break; + case 0x1: txt = "ROMless or onchip flash"; break; + case 0x2: txt = "Embedded flash memory"; break; + case 0x3: txt = "ROM and Embedded flash memory"; break; + case 0x4: txt = "SRAM emulating ROM"; break; + default: txt = "undefined"; break; + } + printf("NonVolatile Program Memory type: %s\n", txt); + + switch (cidr & AT91_CIDR_SRAMSIZ) { + case AT91_CIDR_SRAMSIZ_1K: txt = "1K"; break; + case AT91_CIDR_SRAMSIZ_2K: txt = "2K"; break; + case AT91_CIDR_SRAMSIZ_112K: txt = "112K"; break; + case AT91_CIDR_SRAMSIZ_4K: txt = "4K"; break; + case AT91_CIDR_SRAMSIZ_80K: txt = "80K"; break; + case AT91_CIDR_SRAMSIZ_160K: txt = "160K"; break; + case AT91_CIDR_SRAMSIZ_8K: txt = "8K"; break; + case AT91_CIDR_SRAMSIZ_16K: txt = "16K"; break; + case AT91_CIDR_SRAMSIZ_32K: txt = "32K"; break; + case AT91_CIDR_SRAMSIZ_64K: txt = "64K"; break; + case AT91_CIDR_SRAMSIZ_128K: txt = "128K"; break; + case AT91_CIDR_SRAMSIZ_256K: txt = "256K"; break; + case AT91_CIDR_SRAMSIZ_96K: txt = "96K"; break; + case AT91_CIDR_SRAMSIZ_512K: txt = "512K"; break; + default: txt = "undefined"; break; + } + printf("Internal SRAM size: %s\n", txt); + + if (cidr & AT91_CIDR_EXT) + printf("Extension ID: 0x%x\n", at91_sys_read(AT91_DBGU_EXID)); + + return 0; +} diff --git a/include/asm-arm/arch-at91/at91_dbgu.h b/include/asm-arm/arch-at91/at91_dbgu.h new file mode 100644 index 0000000..55210da --- /dev/null +++ b/include/asm-arm/arch-at91/at91_dbgu.h @@ -0,0 +1,69 @@ +/* + * include/asm-arm/arch-at91/at91_dbgu.h + * + * Copyright (C) 2005 Ivan Kokshaysky + * Copyright (C) SAN People + * + * Debug Unit (DBGU) - System peripherals registers. + * Based on AT91RM9200 datasheet revision E. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#ifndef AT91_DBGU_H +#define AT91_DBGU_H + +#ifdef AT91_DBGU +#define AT91_DBGU_CR (AT91_DBGU + 0x00) /* Control Register */ +#define AT91_DBGU_MR (AT91_DBGU + 0x04) /* Mode Register */ +#define AT91_DBGU_IER (AT91_DBGU + 0x08) /* Interrupt Enable */ +#define AT91_DBGU_TXRDY (1 << 1) /* Transmitter Ready */ +#define AT91_DBGU_TXEMPTY (1 << 9) /* Transmitter Empty */ +#define AT91_DBGU_IDR (AT91_DBGU + 0x0c) /* Interrupt Disable */ +#define AT91_DBGU_IMR (AT91_DBGU + 0x10) /* Interrupt Mask */ +#define AT91_DBGU_SR (AT91_DBGU + 0x14) /* Status Register */ +#define AT91_DBGU_RHR (AT91_DBGU + 0x18) /* Receiver Holding */ +#define AT91_DBGU_THR (AT91_DBGU + 0x1c) /* Transmitter Holding */ +#define AT91_DBGU_BRGR (AT91_DBGU + 0x20) /* Baud Rate Generator */ + +#define AT91_DBGU_CIDR (AT91_DBGU + 0x40) /* Chip ID Register */ +#define AT91_DBGU_EXID (AT91_DBGU + 0x44) /* Chip ID Extension */ +#define AT91_DBGU_FNR (AT91_DBGU + 0x48) /* Force NTRST [SAM9 only] */ +#define AT91_DBGU_FNTRST (1 << 0) /* Force NTRST */ + +#endif /* AT91_DBGU */ + +/* + * Some AT91 parts that don't have full DEBUG units still support the ID + * and extensions register. + */ +#define AT91_CIDR_VERSION (0x1f << 0) /* Version of the Device */ +#define AT91_CIDR_EPROC (7 << 5) /* Embedded Processor */ +#define AT91_CIDR_NVPSIZ (0xf << 8) /* Nonvolatile Program + Memory Size */ +#define AT91_CIDR_NVPSIZ2 (0xf << 12) /* Second Nonvolatile + Program Memory Size */ +#define AT91_CIDR_SRAMSIZ (0xf << 16) /* Internal SRAM Size */ +#define AT91_CIDR_SRAMSIZ_1K (1 << 16) +#define AT91_CIDR_SRAMSIZ_2K (2 << 16) +#define AT91_CIDR_SRAMSIZ_112K (4 << 16) +#define AT91_CIDR_SRAMSIZ_4K (5 << 16) +#define AT91_CIDR_SRAMSIZ_80K (6 << 16) +#define AT91_CIDR_SRAMSIZ_160K (7 << 16) +#define AT91_CIDR_SRAMSIZ_8K (8 << 16) +#define AT91_CIDR_SRAMSIZ_16K (9 << 16) +#define AT91_CIDR_SRAMSIZ_32K (10 << 16) +#define AT91_CIDR_SRAMSIZ_64K (11 << 16) +#define AT91_CIDR_SRAMSIZ_128K (12 << 16) +#define AT91_CIDR_SRAMSIZ_256K (13 << 16) +#define AT91_CIDR_SRAMSIZ_96K (14 << 16) +#define AT91_CIDR_SRAMSIZ_512K (15 << 16) +#define AT91_CIDR_ARCH (0xff << 20) /* Architecture Id */ +#define AT91_CIDR_NVPTYP (7 << 28) /* Nonvolatile Program + Memory Type */ +#define AT91_CIDR_EXT (1 << 31) /* Extension Flag */ + +#endif diff --git a/include/asm-arm/arch-at91/cpu.h b/include/asm-arm/arch-at91/cpu.h new file mode 100644 index 0000000..b5e5715 --- /dev/null +++ b/include/asm-arm/arch-at91/cpu.h @@ -0,0 +1,99 @@ +/* + * include/asm-arm/arch-at91/cpu.h + * + * Copyright (C) 2006 SAN People + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + */ + +#ifndef __ASM_ARCH_CPU_H +#define __ASM_ARCH_CPU_H + +#include <asm/hardware.h> +#include <asm/arch/at91_dbgu.h> +#include <asm/arch/io.h> + + +#define ARCH_ID_AT91RM9200 0x09290780 +#define ARCH_ID_AT91SAM9260 0x019803a0 +#define ARCH_ID_AT91SAM9261 0x019703a0 +#define ARCH_ID_AT91SAM9263 0x019607a0 +#define ARCH_ID_AT91SAM9RL64 0x019b03a0 +#define ARCH_ID_AT91CAP9 0x039A03A0 + +#define ARCH_ID_AT91SAM9XE128 0x329973a0 +#define ARCH_ID_AT91SAM9XE256 0x329a93a0 +#define ARCH_ID_AT91SAM9XE512 0x329aa3a0 + +#define ARCH_ID_AT91M40800 0x14080044 +#define ARCH_ID_AT91R40807 0x44080746 +#define ARCH_ID_AT91M40807 0x14080745 +#define ARCH_ID_AT91R40008 0x44000840 + +static inline unsigned long at91_cpu_identify(void) +{ + return (at91_sys_read(AT91_DBGU_CIDR) & ~AT91_CIDR_VERSION); +} + + +#define ARCH_FAMILY_AT91X92 0x09200000 +#define ARCH_FAMILY_AT91SAM9 0x01900000 +#define ARCH_FAMILY_AT91SAM9XE 0x02900000 + +static inline unsigned long at91_arch_identify(void) +{ + return (at91_sys_read(AT91_DBGU_CIDR) & AT91_CIDR_ARCH); +} + + +#ifdef CONFIG_AT91RM9200 +/* no actual detection for this architecture */ +#define cpu_is_at91rm9200() (1) +#else +#define cpu_is_at91rm9200() (0) +#endif + +#ifdef CONFIG_AT91SAM9261 +#define cpu_is_at91sam9xe() (at91_arch_identify() == ARCH_FAMILY_AT91SAM9XE) +#define cpu_is_at91sam9260() ((at91_cpu_identify() == ARCH_ID_AT91SAM9260) \ + || cpu_is_at91sam9xe()) +#else +#define cpu_is_at91sam9xe() (0) +#define cpu_is_at91sam9260() (0) +#endif + +#ifdef CONFIG_AT91SAM9261 +#define cpu_is_at91sam9261() (at91_cpu_identify() == ARCH_ID_AT91SAM9261) +#else +#define cpu_is_at91sam9261() (0) +#endif + +#ifdef CONFIG_AT91SAM9261 +#define cpu_is_at91sam9263() (at91_cpu_identify() == ARCH_ID_AT91SAM9263) +#else +#define cpu_is_at91sam9263() (0) +#endif + +#ifdef CONFIG_AT91SAM9RL +#define cpu_is_at91sam9rl() (at91_cpu_identify() == ARCH_ID_AT91SAM9RL64) +#else +#define cpu_is_at91sam9rl() (0) +#endif + +#ifdef CONFIG_AT91CAP9 +#define cpu_is_at91cap9() (at91_cpu_identify() == ARCH_ID_AT91CAP9) +#else +#define cpu_is_at91cap9() (0) +#endif + +/* + * Since this is ARM, we will never run on any AVR32 CPU. But these + * definitions may reduce clutter in common drivers. + */ +#define cpu_is_at32ap7000() (0) + +#endif -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 2009-06-01 18:21 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Remy Bohmer @ 2009-06-01 18:21 ` Remy Bohmer 2009-06-01 20:03 ` Stelian Pop 2009-06-01 19:59 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Stelian Pop 1 sibling, 1 reply; 9+ messages in thread From: Remy Bohmer @ 2009-06-01 18:21 UTC (permalink / raw) To: u-boot Signed-off-by: Remy Bohmer <linux@bohmer.net> --- include/configs/at91sam9261ek.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/include/configs/at91sam9261ek.h b/include/configs/at91sam9261ek.h index fdaa71c..9a50347 100644 --- a/include/configs/at91sam9261ek.h +++ b/include/configs/at91sam9261ek.h @@ -45,6 +45,8 @@ #define CONFIG_SKIP_LOWLEVEL_INIT #define CONFIG_SKIP_RELOCATE_UBOOT +#define CONFIG_DISPLAY_CPUINFO 1 + /* * Hardware drivers */ -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 2009-06-01 18:21 ` [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 Remy Bohmer @ 2009-06-01 20:03 ` Stelian Pop 2009-06-01 20:22 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 1 reply; 9+ messages in thread From: Stelian Pop @ 2009-06-01 20:03 UTC (permalink / raw) To: u-boot On Mon, Jun 01, 2009 at 08:21:43PM +0200, Remy Bohmer wrote: > Signed-off-by: Remy Bohmer <linux@bohmer.net> > --- > include/configs/at91sam9261ek.h | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/include/configs/at91sam9261ek.h b/include/configs/at91sam9261ek.h > index fdaa71c..9a50347 100644 > --- a/include/configs/at91sam9261ek.h > +++ b/include/configs/at91sam9261ek.h > @@ -45,6 +45,8 @@ > #define CONFIG_SKIP_LOWLEVEL_INIT > #define CONFIG_SKIP_RELOCATE_UBOOT > > +#define CONFIG_DISPLAY_CPUINFO 1 For homogeneity reasons, since this is a generic AT91 config option, I think it makes sense to define this by default for all the AT91 boards - or not define it at all. Your pick. Stelian. -- Stelian Pop <stelian@popies.net> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 2009-06-01 20:03 ` Stelian Pop @ 2009-06-01 20:22 ` Jean-Christophe PLAGNIOL-VILLARD 2009-06-02 19:17 ` Remy Bohmer 0 siblings, 1 reply; 9+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-01 20:22 UTC (permalink / raw) To: u-boot On 22:03 Mon 01 Jun , Stelian Pop wrote: > On Mon, Jun 01, 2009 at 08:21:43PM +0200, Remy Bohmer wrote: > > > Signed-off-by: Remy Bohmer <linux@bohmer.net> > > --- > > include/configs/at91sam9261ek.h | 2 ++ > > 1 files changed, 2 insertions(+), 0 deletions(-) > > > > diff --git a/include/configs/at91sam9261ek.h b/include/configs/at91sam9261ek.h > > index fdaa71c..9a50347 100644 > > --- a/include/configs/at91sam9261ek.h > > +++ b/include/configs/at91sam9261ek.h > > @@ -45,6 +45,8 @@ > > #define CONFIG_SKIP_LOWLEVEL_INIT > > #define CONFIG_SKIP_RELOCATE_UBOOT > > > > +#define CONFIG_DISPLAY_CPUINFO 1 > > For homogeneity reasons, since this is a generic AT91 config option, I think > it makes sense to define this by default for all the AT91 boards - or not > define it at all. Your pick. for the Atmel's ref boards maybe but for other boards we need to let this choice to each board Maintainer Best Regards, J. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 2009-06-01 20:22 ` Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-02 19:17 ` Remy Bohmer 0 siblings, 0 replies; 9+ messages in thread From: Remy Bohmer @ 2009-06-02 19:17 UTC (permalink / raw) To: u-boot Hello, >> > @@ -45,6 +45,8 @@ >> > ?#define CONFIG_SKIP_LOWLEVEL_INIT >> > ?#define CONFIG_SKIP_RELOCATE_UBOOT >> > >> > +#define CONFIG_DISPLAY_CPUINFO ? ? ? ? ?1 >> >> For homogeneity reasons, since this is a generic AT91 config option, I think >> it makes sense to define this by default for all the AT91 boards - or not >> define it at all. Your pick. Well, I only have the at91sam9261ek board, and as such it is the only one I can test. I do not prefer enabling things for boards I cannot test. Once the framework is there it can be enabled, if desired. > for the Atmel's ref boards maybe but for other boards we need to let this > choice to each board Maintainer I agree on that. Kind Regards, Remy ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores 2009-06-01 18:21 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Remy Bohmer 2009-06-01 18:21 ` [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 Remy Bohmer @ 2009-06-01 19:59 ` Stelian Pop 2009-06-01 20:20 ` Jean-Christophe PLAGNIOL-VILLARD 1 sibling, 1 reply; 9+ messages in thread From: Stelian Pop @ 2009-06-01 19:59 UTC (permalink / raw) To: u-boot Hi R?my, I do have a few comments, see below: On Mon, Jun 01, 2009 at 08:21:42PM +0200, Remy Bohmer wrote: > V2 reworked comments from Jean-Christophe PLAGNIOL-VILLARD [...] > +COBJS-$(CONFIG_DISPLAY_CPUINFO) +=cpuinfo.o missing space ? > + switch ((cidr & AT91_CIDR_EPROC) >> 5) { > + /* 0x1 = ARM946ES, 0x2 = ARM7TDMI, 0x4 = ARM920T */ > + case 0x5: txt = "ARM926EJS"; break; > + default: txt = "undefined"; break; since there are only four choices (until now at least...), why not put the 'txt' for all of them ? and the 'undefined' string is not a good choice: it represents a configuration not known to U-Boot, but its numeric value is well defined. So I suggest you change this to 'unknown', or even better, 'unknown: 0xfoo'. > + } > + printf("Embedded Processor: %s\n", txt); > + > + switch ((cidr & AT91_CIDR_ARCH) >> 20) { > + case 0x19: txt = "AT91SAM9xx"; break; > + case 0x29: txt = "AT91SAM9XExx"; break; > + case 0x39: txt = "CAP9"; break; > + default: txt = "undefined"; break; same comments as above for 'undefined'. > + switch ((cidr & AT91_CIDR_NVPTYP) >> 28) { > + case 0x0: txt = "ROM"; break; > + case 0x1: txt = "ROMless or onchip flash"; break; > + case 0x2: txt = "Embedded flash memory"; break; > + case 0x3: txt = "ROM and Embedded flash memory"; break; > + case 0x4: txt = "SRAM emulating ROM"; break; > + default: txt = "undefined"; break; ditto. > + switch (cidr & AT91_CIDR_SRAMSIZ) { > + case AT91_CIDR_SRAMSIZ_1K: txt = "1K"; break; > + case AT91_CIDR_SRAMSIZ_2K: txt = "2K"; break; > + case AT91_CIDR_SRAMSIZ_112K: txt = "112K"; break; > + case AT91_CIDR_SRAMSIZ_4K: txt = "4K"; break; > + case AT91_CIDR_SRAMSIZ_80K: txt = "80K"; break; > + case AT91_CIDR_SRAMSIZ_160K: txt = "160K"; break; > + case AT91_CIDR_SRAMSIZ_8K: txt = "8K"; break; > + case AT91_CIDR_SRAMSIZ_16K: txt = "16K"; break; > + case AT91_CIDR_SRAMSIZ_32K: txt = "32K"; break; > + case AT91_CIDR_SRAMSIZ_64K: txt = "64K"; break; > + case AT91_CIDR_SRAMSIZ_128K: txt = "128K"; break; > + case AT91_CIDR_SRAMSIZ_256K: txt = "256K"; break; > + case AT91_CIDR_SRAMSIZ_96K: txt = "96K"; break; > + case AT91_CIDR_SRAMSIZ_512K: txt = "512K"; break; > + default: txt = "undefined"; break; bad alignment here (you did align the 'txt =' properly in all other switch/case statements, but not here). ditto for 'undefined'. Stelian. -- Stelian Pop <stelian@popies.net> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores 2009-06-01 19:59 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Stelian Pop @ 2009-06-01 20:20 ` Jean-Christophe PLAGNIOL-VILLARD 0 siblings, 0 replies; 9+ messages in thread From: Jean-Christophe PLAGNIOL-VILLARD @ 2009-06-01 20:20 UTC (permalink / raw) To: u-boot On 21:59 Mon 01 Jun , Stelian Pop wrote: > Hi R?my, > > I do have a few comments, see below: > > On Mon, Jun 01, 2009 at 08:21:42PM +0200, Remy Bohmer wrote: > > > V2 reworked comments from Jean-Christophe PLAGNIOL-VILLARD > [...] > > > +COBJS-$(CONFIG_DISPLAY_CPUINFO) +=cpuinfo.o > > missing space ? > > > + switch ((cidr & AT91_CIDR_EPROC) >> 5) { > > + /* 0x1 = ARM946ES, 0x2 = ARM7TDMI, 0x4 = ARM920T */ > > + case 0x5: txt = "ARM926EJS"; break; > > + default: txt = "undefined"; break; > > since there are only four choices (until now at least...), why not > put the 'txt' for all of them ? We need to use the ifdef to reduce the size impact and not implement non-supported code is important > > and the 'undefined' string is not a good choice: it represents a > configuration not known to U-Boot, but its numeric value is well > defined. So I suggest you change this to 'unknown', or even better, > 'unknown: 0xfoo'. I agree Best Regards, J. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-06-02 19:17 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-02 20:06 [U-Boot] [patch] Add display cpuinfo for Atmel at91sam9261 Cores Remy Bohmer 2009-05-18 20:25 ` Jean-Christophe PLAGNIOL-VILLARD 2009-06-01 18:21 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Remy Bohmer 2009-06-01 18:21 ` [U-Boot] [PATCH] Enable display CPU-Info for at91sam9261 Remy Bohmer 2009-06-01 20:03 ` Stelian Pop 2009-06-01 20:22 ` Jean-Christophe PLAGNIOL-VILLARD 2009-06-02 19:17 ` Remy Bohmer 2009-06-01 19:59 ` [U-Boot] [PATCH V2] Add display cpuinfo for Atmel at91 arm926ejs based cores Stelian Pop 2009-06-01 20:20 ` Jean-Christophe PLAGNIOL-VILLARD
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox