| Message ID | 1305058102-26936-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | Changes Requested |
| Headers | show |
Dear Mike Frysinger, In message <1305058102-26936-1-git-send-email-vapier@gentoo.org> you wrote: > This adds a simple parallel nor flash test to automatically verify > erasing/writing/reading of sectors. The code is based on existing > Blackfin tests but generalized for everyone to use. What exactly does the "parallel" in "parallel nor flash test" mean? > Signed-off-by: Mike Frysinger <vapier@gentoo.org> > --- > include/post.h | 1 + > post/drivers/Makefile | 2 +- > post/drivers/flash.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ > post/tests.c | 15 +++++++- > 4 files changed, 112 insertions(+), 2 deletions(-) > create mode 100644 post/drivers/flash.c ... > -COBJS-$(CONFIG_HAS_POST) += i2c.o memory.o rtc.o > +COBJS-$(CONFIG_HAS_POST) += flash.o i2c.o memory.o rtc.o This has always been a thorn in my side. can we not change this such that for example flash.o gets only built when CONFIG_SYS_POST_FLASH is set in CONFIG_POST ? ... > +static void *seed_src_data(ulong *old_len, ulong new_len) > +{ > + unsigned char *ret = (void *)CONFIG_SYS_SDRAM_BASE; This will not work on many systems. For example, on PPC you will overwrite the exception vectors in low memory, thus crashing the system. Best regards, Wolfgang Denk
On Tuesday, May 10, 2011 16:17:00 Wolfgang Denk wrote: > Mike Frysinger wrote: > > This adds a simple parallel nor flash test to automatically verify > > erasing/writing/reading of sectors. The code is based on existing > > Blackfin tests but generalized for everyone to use. > > What exactly does the "parallel" in "parallel nor flash test" mean? how it is connected to the processor > > -COBJS-$(CONFIG_HAS_POST) += i2c.o memory.o rtc.o > > +COBJS-$(CONFIG_HAS_POST) += flash.o i2c.o memory.o rtc.o > > This has always been a thorn in my side. can we not change this such > that for example flash.o gets only built when CONFIG_SYS_POST_FLASH is > set in CONFIG_POST ? no, not atm. the current POST design has CONFIG_POST as a bit field. that should probably be fixed more generally since we seem to be hitting the bit field limit. but this would have to be fixed for the entire post tree and not individual tests. > > +static void *seed_src_data(ulong *old_len, ulong new_len) > > +{ > > + unsigned char *ret = (void *)CONFIG_SYS_SDRAM_BASE; > > This will not work on many systems. For example, on PPC you will > overwrite the exception vectors in low memory, thus crashing the > system. this is how the default memory post test works. it writes all memory from CONFIG_SYS_SDRAM_BASE to CONFIG_SYS_SDRAM_BASE+MIN(1GiB,bd->memsize). so how does this work on ppc systems ? i didnt think calling malloc() would be OK since i need enough memory to hold one sector ... -mike
Dear Mike Frysinger, In message <201105101711.07625.vapier@gentoo.org> you wrote: > > > > This adds a simple parallel nor flash test to automatically verify > > > erasing/writing/reading of sectors. The code is based on existing > > > Blackfin tests but generalized for everyone to use. > > > > What exactly does the "parallel" in "parallel nor flash test" mean? > > how it is connected to the processor Are there any other types of NOR flashes? I mean, I have never heard anybody use this term before. Maybe we can just omit it - I find it more confusing than helpful. > > > + unsigned char *ret = (void *)CONFIG_SYS_SDRAM_BASE; > > > > This will not work on many systems. For example, on PPC you will > > overwrite the exception vectors in low memory, thus crashing the > > system. > > this is how the default memory post test works. it writes all memory from > CONFIG_SYS_SDRAM_BASE to CONFIG_SYS_SDRAM_BASE+MIN(1GiB,bd->memsize). so how > does this work on ppc systems ? The RAM test runs before relocation to RAM, i. e. whenRAM is still not used by anything. > i didnt think calling malloc() would be OK since i need enough memory to hold > one sector ... Just grabbing a random address in RAM is certainly not a proper way to allocate I/O buffers. If you need them, and if your code design makes it even necessary to pass these buffers around (so you can not just put them on the stack which is what I would try to do) you have to use malloc(). Best regards, Wolfgang Denk
On Tuesday, May 10, 2011 17:59:42 Wolfgang Denk wrote: > Mike Frysinger wrote: > > > > This adds a simple parallel nor flash test to automatically verify > > > > erasing/writing/reading of sectors. The code is based on existing > > > > Blackfin tests but generalized for everyone to use. > > > > > > What exactly does the "parallel" in "parallel nor flash test" mean? > > > > how it is connected to the processor > > Are there any other types of NOR flashes? "parallel" is the connection type to the processor meaning it may be directly addressed (has dedicated address and data lines). "NOR" is simply the technology type. serial flashes tend to be NOR flashes connected via a SPI bus. then there are NAND flashes which tend to be connected via a dedicated NAND flash controller (NFC), but that isnt a hard line anymore either. ive seen serial NAND flashes which means they connect via SPI. > I mean, I have never heard anybody use this term before. Maybe we can just > omit it - I find it more confusing than helpful. *shrug* i cant say i found that to be the same, but doesnt matter to me. the point of saying "parallel nor" was to be explicit ... this post test is only for nor flashes connected directly to the processor using the u-boot include/flash.h standard. it is not for spi nor flashes, nor is it for nand flashes of any type. > > i didnt think calling malloc() would be OK since i need enough memory to > > hold one sector ... > > Just grabbing a random address in RAM is certainly not a proper way > to allocate I/O buffers. If you need them, and if your code design > makes it even necessary to pass these buffers around (so you can not > just put them on the stack which is what I would try to do) you have > to use malloc(). considering the current test design blasts away at entire sectors, i didnt think putting a 128KiB buffer on the stack was kosher. should be trivial to change it to malloc() ... -mike
diff --git a/include/post.h b/include/post.h index c9ec2f4..19991de 100644 --- a/include/post.h +++ b/include/post.h @@ -186,6 +186,7 @@ extern int post_hotkeys_pressed(void); #define CONFIG_SYS_POST_BSPEC5 0x00100000 #define CONFIG_SYS_POST_CODEC 0x00200000 #define CONFIG_SYS_POST_COPROC 0x00400000 +#define CONFIG_SYS_POST_FLASH 0x00800000 #endif /* CONFIG_POST */ diff --git a/post/drivers/Makefile b/post/drivers/Makefile index 0d87ae0..85d6c03 100644 --- a/post/drivers/Makefile +++ b/post/drivers/Makefile @@ -24,6 +24,6 @@ include $(TOPDIR)/config.mk LIB = libpostdrivers.o -COBJS-$(CONFIG_HAS_POST) += i2c.o memory.o rtc.o +COBJS-$(CONFIG_HAS_POST) += flash.o i2c.o memory.o rtc.o include $(TOPDIR)/post/rules.mk diff --git a/post/drivers/flash.c b/post/drivers/flash.c new file mode 100644 index 0000000..5ecb853 --- /dev/null +++ b/post/drivers/flash.c @@ -0,0 +1,96 @@ +/* + * Parallel NOR Flash tests + * + * Copyright (c) 2005-2011 Analog Devices Inc. + * + * Licensed under the GPL-2 or later. + */ + +#include <common.h> +#include <post.h> +#include <flash.h> + +#if CONFIG_POST & CONFIG_SYS_POST_FLASH + +/* + * This code will walk over the declared sectors erasing them, + * then programming them, then verifying the written contents. + * Possible future work: + * - verify sectors before/after are not erased/written + * - verify partial writes (e.g. programming only middle of sector) + * - verify the contents of the erased sector + * - better seed pattern than 0x00..0xff + */ + +#ifndef CONFIG_SYS_POST_FLASH_NUM +# define CONFIG_SYS_POST_FLASH_NUM 0 +#endif +#if CONFIG_SYS_POST_FLASH_START >= CONFIG_SYS_POST_FLASH_END +# error "invalid flash block start/end" +#endif + +extern flash_info_t flash_info[]; + +static void *seed_src_data(ulong *old_len, ulong new_len) +{ + unsigned char *ret = (void *)CONFIG_SYS_SDRAM_BASE; + ulong i; + + for (i = *old_len; i < new_len; ++i) + ret[i] = i; + + *old_len = new_len; + + return ret; +} + +int flash_post_test(int flags) +{ + ulong len; + void *src; + int ret, n, n_start, n_end; + flash_info_t *info; + + /* the output from the common flash layers needs help */ + puts("\n"); + + len = 0; + info = &flash_info[CONFIG_SYS_POST_FLASH_NUM]; + n_start = CONFIG_SYS_POST_FLASH_START; + n_end = CONFIG_SYS_POST_FLASH_END; + + for (n = n_start; n < n_end; ++n) { + ulong s_start, s_len, s_off; + + s_start = info->start[n]; + s_len = flash_sector_size(info, n); + s_off = s_start - info->start[0]; + + src = seed_src_data(&len, s_len); + + printf("\tsector %i: %#lx +%#lx", + n, s_start, s_len); + + ret = flash_erase(info, n, n + 1); + if (ret) { + flash_perror(ret); + break; + } + + ret = write_buff(info, src, s_start, s_len); + if (ret) { + flash_perror(ret); + break; + } + + ret = memcmp(src, (void *)s_start, s_len); + if (ret) { + printf(" verify failed with %i\n", ret); + break; + } + } + + return ret; +} + +#endif diff --git a/post/tests.c b/post/tests.c index 71437b6..bfb9cb5 100644 --- a/post/tests.c +++ b/post/tests.c @@ -46,6 +46,7 @@ extern int sysmon_post_test (int flags); extern int dsp_post_test (int flags); extern int codec_post_test (int flags); extern int ecc_post_test (int flags); +extern int flash_post_test(int flags); extern int dspic_init_post_test (int flags); extern int dspic_post_test (int flags); @@ -301,7 +302,19 @@ struct post_test post_list[] = NULL, NULL, CONFIG_SYS_POST_COPROC - } + }, +#endif +#if CONFIG_POST & CONFIG_SYS_POST_FLASH + { + "Parallel NOR flash test", + "flash", + "This test verifies parallel flash operations.", + POST_RAM | POST_SLOWTEST | POST_MANUAL, + &flash_post_test, + NULL, + NULL, + CONFIG_SYS_POST_FLASH + }, #endif };
This adds a simple parallel nor flash test to automatically verify erasing/writing/reading of sectors. The code is based on existing Blackfin tests but generalized for everyone to use. Signed-off-by: Mike Frysinger <vapier@gentoo.org> --- include/post.h | 1 + post/drivers/Makefile | 2 +- post/drivers/flash.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++ post/tests.c | 15 +++++++- 4 files changed, 112 insertions(+), 2 deletions(-) create mode 100644 post/drivers/flash.c