[U-Boot-Users] [rfc] new spiflash subsystem
this isnt against u-boot mainline, so there will be a few things that are out of date (like the CFG handling), so over look that part. what's up for comments here is the general architecture. ive omitted the env_spiflash.c portion as i think that'll be pretty cheesy.
basically ive laid it out like so: - common/cmd_spiflash.c: provides the user interface of the spiflash subsystem. depends on these functions being implemented elsewhere: * spiflash_info() - ask the spiflash driver for info * spiflash_read() - ask the spiflash driver to read * spiflash_write() - ask the spiflash driver to write * spiflash_erase() - ask the spiflash driver to erase no validation occurs in the common code since it has no idea about sector sizes and such. i could add a spiflash_query() function and have it return a struct describing the spiflash and use that to validate, but i dont think it's really worth the effort. relevant defines: * CFG_CMD_SPIFLASH - include the "spiflash" command * CFG_SPIFLASH_MULTI - support multiple parts (via "cs" parameter)
- drivers/mtd/spiflash_jedec.c: provides the spiflash driver functions needed by the common layer. detects and works with Spansion/ST/Atmel/Winbond parts. i've personally verified at least one part from each family, but obviously not every part ;). it depends on these functions being implemented elsewhere: * spiflash_on() - turn on the SPI * spiflash_off() - turn off the SPI * spiflash_cs_set() - assert/deassert the specified chip select * spiflash_exchange_byte() - send specified byte and return received byte relevant defines: * CFG_SPIFLASH_JEDEC_DRIVER - enable this driver * CFG_SPIFLASH_MULTI - disable a small runtime opt to avoid redetection
- board/$BOARD/spiflash.c: provides the board / cpu specific spiflash functions. ive included the Blackfin one here as an example.
-mike
Unfortunately, this code seems useless, at least for the combination AT91 + SPI flash. Some issues:
I believe that the AT45 is not using the same command set as other SPI flash memories. I think the commands need to be separated. AT45 is much more advanced than other SPI flash. Did you really test your code on the AT45 series?
You assume, incorrectly, that all sector sizes are the same size.
How do you do "byte writes" which is an important feature of the AT45?
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot. Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
Best Regards Ulf Samuelsson
/*
- SPI flash driver user interface
- Copyright (c) 2005-2008 Analog Devices Inc.
- Licensed under the GPL-2 or later.
*/
#include <common.h> #include <config.h> #include <command.h>
#if 1 //(CONFIG_COMMANDS & CFG_CMD_SPIFLASH)
int do_spiflash(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) { { int i = 0; while (i < argc) printf("[%s] ", argv[i++]); printf("\n"); }
if (argc == 1) { usage: printf("Usage:\n%s\n", cmdtp->usage); return 1; }
ulong cs = CFG_SPIFLASH_CS_DEFAULT; #ifdef CFG_SPIFLASH_MULTI cs = simple_strtoul(argv[1], NULL, 10); --argc; ++argv; #endif
if (!strcmp(argv[1], "info")) { if (argc != 2) goto usage; return spiflash_info(cs); }
if (!strcmp(argv[1], "erase")) { size_t off, cnt;
if (argc == 4) { off = simple_strtoul(argv[2], NULL, 16); cnt = simple_strtoul(argv[3], NULL, 16); printf("Erasing SPI flash @CS%lu: off 0x%lx count 0x%lx ... ", cs, off, cnt); } else if (argc == 2) { off = cnt = -1; printf("Erasing entire SPI flash @CS%lu ... ", cs); } else goto usage;
int ret = spiflash_erase(cs, off, cnt); puts(" done\n"); return ret; }
if (argc != 5) goto usage;
size_t (*spiflash_func)(int cs, size_t off, size_t cnt, uchar *buffer); if (!strcmp(argv[1], "read")) spiflash_func = spiflash_read; else if (!strcmp(argv[1], "write")) spiflash_func = spiflash_write; else goto usage;
uchar *addr = (uchar *)simple_strtoul(argv[2], NULL, 16); size_t off = simple_strtoul(argv[3], NULL, 16); size_t cnt = simple_strtoul(argv[4], NULL, 16);
printf("SPI flash @CS%lu %s: addr 0x%p off 0x%lx count 0x%lx ... ", cs, argv[1], addr, off, cnt); size_t ret = spiflash_func(cs, off, cnt, addr); puts(" done\n");
return (ret != 0 ? 0 : 1); }
#ifdef CFG_SPIFLASH_MULTI U_BOOT_CMD(spiflash, 6, 1, do_spiflash, "spiflash - SPI flash sub-system\n", "read cs addr off cnt\n" "spiflash write cs addr off cnt\n" "spiflash erase [off cnt]\n" "spiflash info cs\n" " - read/write 'cnt' bytes from SPI flash with chip-select 'cs' at offset 'off' into/from 'addr'\n" ); #else U_BOOT_CMD(spiflash, 5, 1, do_spiflash, "spiflash - SPI flash sub-system\n", "read addr off cnt\n" "spiflash write addr off cnt\n" "spiflash erase [off cnt]\n" "spiflash info\n" " - read/write 'cnt' bytes from SPI flash at offset 'off' into/from 'addr'\n" ); #endif
#endif
--------------------------------------------------------------------------------
/*
- SPI flash driver for JEDEC compatible parts
- Copyright (c) 2005-2008 Analog Devices Inc.
- Licensed under the GPL-2 or later.
*/
#include <common.h> #include <config.h>
#if defined(CFG_SPIFLASH_JEDEC_DRIVER)
struct flash_info { char *name; uint16_t id; unsigned sector_size; unsigned num_sectors; };
/* SPI Speeds: 50 MHz / 33 MHz */ static struct flash_info flash_spansion_serial_flash[] = { { "S25FL016", 0x0215, 64 * 1024, 32 }, { "S25FL032", 0x0216, 64 * 1024, 64 }, { "S25FL064", 0x0217, 64 * 1024, 128 }, { "S25FL0128", 0x0218, 256 * 1024, 64 }, { NULL, 0, 0, 0 } };
/* SPI Speeds: 50 MHz / 20 MHz */ static struct flash_info flash_st_serial_flash[] = { { "m25p05", 0x2010, 32 * 1024, 2 }, { "m25p10", 0x2011, 32 * 1024, 4 }, { "m25p20", 0x2012, 64 * 1024, 4 }, { "m25p40", 0x2013, 64 * 1024, 8 }, { "m25p16", 0x2015, 64 * 1024, 32 }, { "m25p32", 0x2016, 64 * 1024, 64 }, { "m25p64", 0x2017, 64 * 1024, 128 }, { "m25p128", 0x2018, 256 * 1024, 64 }, { NULL, 0, 0, 0 } };
/* SPI Speeds: 66 MHz / 33 MHz */ static struct flash_info flash_atmel_dataflash[] = { { "AT45DB011x", 0x0c, 264, 512 }, { "AT45DB021x", 0x14, 264, 1025 }, { "AT45DB041x", 0x1c, 264, 2048 }, { "AT45DB081x", 0x24, 264, 4096 }, { "AT45DB161x", 0x2c, 528, 4096 }, { "AT45DB321x", 0x34, 528, 8192 }, { "AT45DB642x", 0x3c, 1056, 8192 }, { NULL, 0, 0, 0 } };
/* SPI Speed: 50 MHz / 25 MHz or 40 MHz / 20 MHz */ static struct flash_info flash_winbond_serial_flash[] = { { "W25X10", 0x3011, 16 * 256, 32 }, { "W25X20", 0x3012, 16 * 256, 64 }, { "W25X40", 0x3013, 16 * 256, 128 }, { "W25X80", 0x3014, 16 * 256, 256 }, { "W25P80", 0x2014, 256 * 256, 16 }, { "W25P16", 0x2015, 256 * 256, 32 }, { NULL, 0, 0, 0 } };
struct flash_ops { uint8_t read, write, erase, status; };
#define OP_READ_SLOW 0x03 #define OP_READ_FAST 0x0B #ifdef CFG_SPIFLASH_SLOW_READ # define OP_READ OP_READ_SLOW #else # define OP_READ OP_READ_FAST #endif static struct flash_ops flash_st_ops = { .read = OP_READ, .write = 0x02, .erase = 0xD8, .status = 0x05, };
static struct flash_ops flash_atmel_ops = { .read = OP_READ, .write = 0x82, .erase = 0x81, .status = 0xD7, };
static struct flash_ops flash_winbond_ops = { .read = OP_READ, .write = 0x02, .erase = 0x20, .status = 0x05, };
struct manufacturer_info { const char *name; uint8_t id; struct flash_info *flashes; struct flash_ops *ops; };
static struct { struct manufacturer_info *manufacturer; struct flash_info *flash; struct flash_ops *ops; uint8_t manufacturer_id, device_id1, device_id2; unsigned int write_length; unsigned long sector_size, num_sectors; } flash;
enum { JED_MANU_SPANSION = 0x01, JED_MANU_ST = 0x20, JED_MANU_ATMEL = 0x1F, JED_MANU_WINBOND = 0xEF, };
static struct manufacturer_info flash_manufacturers[] = { { .name = "Spansion", .id = JED_MANU_SPANSION, .flashes = flash_spansion_serial_flash, .ops = &flash_st_ops, }, { .name = "ST", .id = JED_MANU_ST, .flashes = flash_st_serial_flash, .ops = &flash_st_ops, }, { .name = "Atmel", .id = JED_MANU_ATMEL, .flashes = flash_atmel_dataflash, .ops = &flash_atmel_ops, }, { .name = "Winbond", .id = JED_MANU_WINBOND, .flashes = flash_winbond_serial_flash, .ops = &flash_winbond_ops, }, };
#define TIMEOUT 5000 /* timeout of 5 seconds */
static uint8_t read_status_register(int cs) { uint8_t status_register;
/* send instruction to read status register */ spiflash_cs_set(cs, 1); spiflash_exchange_byte(flash.ops->status); /* send dummy to receive the status register */ status_register = spiflash_exchange_byte(0); spiflash_cs_set(cs, 0);
return status_register; }
/* Request and read the manufacturer and device id of parts which
- are compatible with the JEDEC standard (JEP106) and use that to
- setup other operating conditions.
*/ static int spiflash_detect_part(int cs) { uint16_t dev_id; size_t i;
#ifndef CFG_SPIFLASH_MULTI static char called_init; if (called_init) return 0; #endif
spiflash_cs_set(cs, 1);
/* Send the request for the part identification */ spiflash_exchange_byte(0x9F);
/* Now read in the manufacturer id bytes */ do { flash.manufacturer_id = spiflash_exchange_byte(0); if (flash.manufacturer_id == 0x7F) puts("Warning: unhandled manufacturer continuation byte!\n"); } while (flash.manufacturer_id == 0x7F);
/* Now read in the first device id byte */ flash.device_id1 = spiflash_exchange_byte(0);
/* Now read in the second device id byte */ flash.device_id2 = spiflash_exchange_byte(0);
spiflash_cs_set(cs, 0);
dev_id = (flash.device_id1 << 8) | flash.device_id2;
for (i = 0; i < ARRAY_SIZE(flash_manufacturers); ++i) { if (flash.manufacturer_id == flash_manufacturers[i].id) break; } if (i == ARRAY_SIZE(flash_manufacturers)) goto unknown;
flash.manufacturer = &flash_manufacturers[i]; flash.ops = flash_manufacturers[i].ops;
switch (flash.manufacturer_id) { case JED_MANU_SPANSION: case JED_MANU_ST: case JED_MANU_WINBOND: for (i = 0; flash.manufacturer->flashes[i].name; ++i) { if (dev_id == flash.manufacturer->flashes[i].id) break; } if (!flash.manufacturer->flashes[i].name) goto unknown;
flash.flash = &flash.manufacturer->flashes[i]; flash.sector_size = flash.flash->sector_size; flash.num_sectors = flash.flash->num_sectors; flash.write_length = 256; break;
case JED_MANU_ATMEL: { uint8_t status = read_status_register(cs);
for (i = 0; flash.manufacturer->flashes[i].name; ++i) { if ((status & 0x3c) == flash.manufacturer->flashes[i].id) break; } if (!flash.manufacturer->flashes[i].name) goto unknown;
flash.flash = &flash.manufacturer->flashes[i]; flash.sector_size = flash.flash->sector_size; flash.num_sectors = flash.flash->num_sectors;
/* see if flash is in "power of 2" mode */ if (status & 0x1) flash.sector_size &= ~(1 << (ffs(flash.sector_size) - 1));
flash.write_length = flash.sector_size; break; } }
#ifndef CFG_SPIFLASH_MULTI called_init = 1; #endif return 0;
unknown: printf("Unknown SPI device: 0x%02X 0x%02X 0x%02X\n", flash.manufacturer_id, flash.device_id1, flash.device_id2); return 1; }
static int wait_for_ready_status(int cs) { ulong start = get_timer(0);
while (get_timer(0) - start < TIMEOUT) { switch (flash.manufacturer_id) { case JED_MANU_SPANSION: case JED_MANU_ST: case JED_MANU_WINBOND: if (!(read_status_register(cs) & 0x01)) return 0; break;
case JED_MANU_ATMEL: if (read_status_register(cs) & 0x80) return 0; break; }
if (ctrlc()) { puts("\nAbort\n"); return -1; } }
puts("Timeout\n"); return -1; }
static void transmit_address(size_t addr) { /* Send the highest byte of the 24 bit address at first */ spiflash_exchange_byte(addr >> 16); /* Send the middle byte of the 24 bit address at second */ spiflash_exchange_byte(addr >> 8); /* Send the lowest byte of the 24 bit address finally */ spiflash_exchange_byte(addr); }
static int enable_writing(int cs) { ulong start;
if (flash.manufacturer_id == JED_MANU_ATMEL) return 0;
/* A write enable instruction must previously have been executed */ spiflash_cs_set(cs, 1); spiflash_exchange_byte(0x06); spiflash_cs_set(cs, 0);
/* The status register will be polled to check the write enable latch "WREN" */ start = get_timer(0); while (get_timer(0) - start < TIMEOUT) { if (read_status_register(cs) & 0x02) return 0;
if (ctrlc()) { puts("\nAbort\n"); return -1; } }
puts("Timeout\n"); return -1; }
size_t spiflash_read(int cs, size_t off, size_t cnt, uchar *buffer) { size_t ret = cnt;
spiflash_on(cs);
if (spiflash_detect_part(cs)) ret = 0; else { spiflash_cs_set(cs, 1);
/* Tell the flash we want to read */ spiflash_exchange_byte(flash.ops->read);
/* Tell the flash where to read */ transmit_address(off);
/* Send dummy byte when doing SPI fast reads */ if (flash.ops->read == OP_READ_FAST) spiflash_exchange_byte(0);
/* Now grab the stream of bytes coming back */ size_t i; for (i = 1; i <= cnt; ++i) { *buffer++ = spiflash_exchange_byte(0); if (i % flash.sector_size == 0) puts("."); }
spiflash_cs_set(cs, 0); }
spiflash_off(cs);
return ret; }
static size_t write_flash(int cs, size_t address, size_t cnt, uchar *buffer) { size_t i, write_buffer_size;
if (enable_writing(cs)) return -1;
/* Send write command followed by the 24 bit address */ spiflash_cs_set(cs, 1); spiflash_exchange_byte(flash.ops->write); transmit_address(address);
/* Shoot out a single write buffer */ write_buffer_size = min(cnt, flash.write_length); for (i = 0; i < write_buffer_size; ++i) spiflash_exchange_byte(buffer[i]);
spiflash_cs_set(cs, 0);
/* Wait for the flash to do its thing */ if (wait_for_ready_status(cs)) return -1; else return i; }
static int write_sector(int cs, size_t address, size_t cnt, uchar *buffer) { size_t write_cnt;
while (cnt != 0) { write_cnt = write_flash(cs, address, cnt, buffer); if (write_cnt == -1) return -1;
/* Now that we've sent some bytes out to the flash, update
- our counters a bit
*/ cnt -= write_cnt; address += write_cnt; buffer += write_cnt; }
return 0; }
static int erase_sector(int cs, size_t address) { /* sector gets checked in higher function, so assume it's valid
- here and figure out the offset of the sector in flash
*/ if (enable_writing(cs)) return -1;
/*
- Send the erase block command to the flash followed by the 24 address
- to point to the start of a sector
*/ spiflash_cs_set(cs, 1); spiflash_exchange_byte(flash.ops->erase); transmit_address(address); spiflash_cs_set(cs, 0);
return wait_for_ready_status(cs); }
static size_t write_or_erase(int cs, size_t off, size_t cnt, uchar *buffer, int w_o_e) { int ret = cnt;
spiflash_on(cs);
if (spiflash_detect_part(cs)) ret = 0; else if (off % flash.sector_size || cnt % flash.sector_size) { ret = 0; printf("\n%s: off/cnt not aligned to sector size 0x%x\n", __func__, flash.sector_size); } else { while (off < cnt) { if ((w_o_e == 'w' && write_sector(cs, off, cnt, buffer)) || (w_o_e == 'e' && erase_sector(cs, off))) { ret = 0; break; } off += flash.sector_size; puts("."); } }
spiflash_off(cs);
return ret; }
size_t spiflash_write(int cs, size_t off, size_t cnt, uchar *buffer) { return write_or_erase(cs, off, cnt, buffer, 'w'); }
size_t spiflash_erase(int cs, size_t off, size_t cnt) { return write_or_erase(cs, off, cnt, NULL, 'e'); }
int spiflash_info(int cs) { int ret = 0;
spiflash_on(cs);
if (spiflash_detect_part(cs)) ret = 1; else { printf("SPI Device: %s 0x%02X (%s) 0x%02X 0x%02X\n" "Parameters: num sectors = %i, sector size = %i, write size = %i\n" "Flash Size: %i mbit (%i mbyte)\n" "Status: 0x%02X\n", flash.flash->name, flash.manufacturer_id, flash.manufacturer->name, flash.device_id1, flash.device_id2, flash.num_sectors, flash.sector_size, flash.write_length, (flash.num_sectors * flash.sector_size) >> 17, (flash.num_sectors * flash.sector_size) >> 20, read_status_register(cs));
printf(" Sector Start Addresses:"); unsigned i, off = 0; for (i = 0; i < flash.num_sectors; ++i) { if (i % 5 == 0) puts("\n "); printf("%08x ", off); off += flash.sector_size; } if (i % 5) puts("\n"); }
spiflash_off(cs);
return ret; }
#endif
--------------------------------------------------------------------------------
/*
- SPI flash driver
- Enter bugs at http://blackfin.uclinux.org/
- Copyright (c) 2005-2007 Analog Devices Inc.
- Licensed under the GPL-2 or later.
*/
/* Configuration options:
- CONFIG_SPI_BAUD - value to load into SPI_BAUD (divisor of SCLK to get SPI CLK)
- CONFIG_SPI_FLASH_SLOW_READ - force usage of the slower read
- WARNING: make sure your SCLK + SPI_BAUD is slow enough
*/
#include <common.h> #include <malloc.h> #include <asm/io.h> #include <asm/mach-common/bits/spi.h>
#if defined(CFG_SPIFLASH)
void spiflash_on(int cs) { /* [#3541] This delay appears to be necessary, but not sure
- exactly why as the history behind it is non-existant.
*/ udelay(CONFIG_CCLK_HZ / 25000000);
/* enable SPI pins: SSEL, MOSI, MISO, SCK */ #ifdef __ADSPBF54x__ *pPORTE_FER |= (SPI0_SCK | SPI0_MOSI | SPI0_MISO | SPI0_SEL1); #elif defined(__ADSPBF534__) || defined(__ADSPBF536__) || defined(__ADSPBF537__) *pPORTF_FER |= (PF10 | PF11 | PF12 | PF13); #elif defined(__ADSPBF52x__) bfin_write_PORTG_MUX((bfin_read_PORTG_MUX() & ~PORT_x_MUX_0_MASK) | PORT_x_MUX_0_FUNC_3); bfin_write_PORTG_FER(bfin_read_PORTG_FER() | PG1 | PG2 | PG3 | PG4); #endif
/* initate communication upon write of TDBR */ *pSPI_CTL = (SPE|MSTR|CPHA|CPOL|0x01); *pSPI_BAUD = CONFIG_SPI_BAUD; }
void spiflash_off(int cs) { /* put SPI settings back to reset state */ *pSPI_CTL = 0x0400; *pSPI_BAUD = 0; SSYNC(); }
void spiflash_cs_set(int cs, uint on) { if (on) { cs = 1 << cs;
/* toggle SSEL to reset the device so it'll take a new command */ *pSPI_FLG = 0xFF00 | cs; SSYNC();
*pSPI_FLG = ((0xFF & ~cs) << 8) | cs; } else { /* put SPI settings back to reset state */ *pSPI_FLG = 0xFF00; } SSYNC(); }
uint8_t spiflash_exchange_byte(uint8_t transmit) { *pSPI_TDBR = transmit; SSYNC();
while ((*pSPI_STAT & TXS)) if (ctrlc()) break; while (!(*pSPI_STAT & SPIF)) if (ctrlc()) break; while (!(*pSPI_STAT & RXS)) if (ctrlc()) break;
/* Read dummy to empty the receive register */ return *pSPI_RDBR; }
#endif
--------------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
--------------------------------------------------------------------------------
U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
On Monday 28 January 2008, Ulf Samuelsson wrote:
Unfortunately, this code seems useless, at least for the combination AT91 + SPI flash. Some issues:
I believe that the AT45 is not using the same command set as other SPI flash memories. I think the commands need to be separated.
i already accounted for different command sets based on part family.
AT45 is much more advanced than other SPI flash.
yes, but it still has a basic subset which can be used.
Did you really test your code on the AT45 series?
it was the part i developed the code on originally (the 64mbit one).
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
yes, protection of pages (hardware and software) is missing.
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
ok ? not sure how this is relevant to the topic at hand.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
ok ? u-boot is designed as a monitor to get the system bootstrapped and execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features. what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux. -mike
On Monday 28 January 2008, Ulf Samuelsson wrote:
Unfortunately, this code seems useless, at least for the combination AT91 + SPI flash. Some issues:
I believe that the AT45 is not using the same command set as other SPI flash memories. I think the commands need to be separated.
i already accounted for different command sets based on part family.
AT45 is much more advanced than other SPI flash.
yes, but it still has a basic subset which can be used.
Did you really test your code on the AT45 series?
it was the part i developed the code on originally (the 64mbit one).
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
==> And this is not a good approach, you want the S/W to use the first 256 kB to store initial boot, U-boot and environment. The AT91SAM9 chips will use the first 8 kB sector for the AT91bootstrap, and that should have protection separate from U-Boot code. AVR32 boot ROM will boot from a higher address and use the 8 kB sector at address zero as environment. Both will need to have a S/W partitioning scheme added with separate protection for each partition. This partioning scheme exists today.
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
==> Which means that you want to impose an arbitary limit which currently does not exist in U-Boot. You also increase startup time, which is a critical parameter for many customers.
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
==> No, but U-Boot should make them aware of consequences, so they do not do this by mistake. You normally can't remove the bootloader from a parallel flash without unprotecting it first.
Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
yes, protection of pages (hardware and software) is missing.
==> Exactly my point
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
ok ? not sure how this is relevant to the topic at hand.
==> Just pointing out that the user interface is missing features.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
ok ? u-boot is designed as a monitor to get the system bootstrapped and execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features. what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux. -mike
==> Yes, it is a basic design decision, and with huge flash memories, it is becoming flawed. Thats why I think we should rethink how we do things, before we rewrite the code.
Programming the internal flash memory in an efficient way is really the core of a bootloader, not an extended feature. Programming time is a critical parameter for large volume production so if you can avoid booting Linux, this is probably a good thing.
==> My conclusion remains, that I do not see why anyone using dataflash in the current U-boot would want to switch to the proposed implementation, which will reduce performance and functionality, and increase risk of errors.
Best Regards Ulf Samuelsson ulf@atmel.com Atmel Nordic AB Mail: Box 2033, 174 02 Sundbyberg, Sweden Visit: Kavallerivägen 24, 174 58 Sundbyberg, Sweden Phone +46 (8) 441 54 22 Fax +46 (8) 441 54 29 GSM +46 (706) 22 44 57
On Tue, 29 Jan 2008 08:35:50 +0100 "Ulf Samuelsson" ulf.samuelsson@atmel.com wrote:
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Current implementation is also AT91-only, which is a design flaw that is much worse than anything in Mike's approach.
That said, I think the low-level SPI API should make it possible for a low-level driver to do DMA. The proposed byte-at-a-time API does not allow this, while the existing spi_xfer() interface in include/spi.h does, assuming the buffers are properly aligned.
Haavard
please quote properly in your replies
On Tuesday 29 January 2008, Ulf Samuelsson wrote:
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
==> And this is not a good approach, you want the S/W to use the first 256 kB to store initial boot, U-boot and environment. The AT91SAM9 chips will use the first 8 kB sector for the AT91bootstrap, and that should have protection separate from U-Boot code. AVR32 boot ROM will boot from a higher address and use the 8 kB sector at address zero as environment. Both will need to have a S/W partitioning scheme added with separate protection for each partition. This partioning scheme exists today.
sorry, i didnt mean to imply i thought this limitation was something to be written in stone. i understand perfectly why you would want this flexibility, so i can take a look. rather than making general statements, can you please provide a concrete list of all relevant examples you know of.
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
==> Which means that you want to impose an arbitary limit which currently does not exist in U-Boot. You also increase startup time, which is a critical parameter for many customers.
uhh, what ? you're telling me that customers do a SPI write in u-boot everytime they boot up ? that just sounds stupid and your customers need to rethink their system. "critical boot time" and "spi flash writing" dont belong in the same sentence :P
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
then i'm afraid your implementation is wrong. DMA is board/cpu-specific and there's no way that can be represented in a general framework. in fact, when i was researching how to support SPI dataflash originally, i saw the dataflash code in u-boot and found it to be worthless in terms of re-use. as Haavard points out though, by using a more general SPI transfer layer, you can still (wrongly imo) do DMA transfers.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
==> No, but U-Boot should make them aware of consequences, so they do not do this by mistake. You normally can't remove the bootloader from a parallel flash without unprotecting it first.
providing sector protection is a different topic from removing functionality. nand flash provides the ability to erase the whole flash, so are you going to lobby them to drop that too ?
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
ok ? not sure how this is relevant to the topic at hand.
==> Just pointing out that the user interface is missing features.
what features ? checksums is not the domain of the flash layer unless the flash media itself requires it (like NAND and ECC/bad-blocks). parallel/SPI flash provide no such thing, so adding a completely software checksum layer is inappropriate. if you want checksums, add them yourself in your board code.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
ok ? u-boot is designed as a monitor to get the system bootstrapped and execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features. what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux.
==> Yes, it is a basic design decision, and with huge flash memories, it is becoming flawed. Thats why I think we should rethink how we do things, before we rewrite the code.
Programming the internal flash memory in an efficient way is really the core of a bootloader, not an extended feature. Programming time is a critical parameter for large volumeproduction so if you can avoid booting Linux, this is probably a good thing.
not relevant to the thread. please drop any further comments on the topic. if you want to re-architect u-boot, start a new thread.
==> My conclusion remains, that I do not see why anyone using dataflash in the current U-boot would want to switch to the proposed implementation, which will reduce performance and functionality, and increase risk of errors.
do you not understand how the software development process works ? i post a new framework, asking for feedback. that doesnt mean it is perfect the first time through and that everyone should start using it RIGHT NOW. please exercise some patience. -mike
please quote properly in your replies
==> Mailers can't quote properly when mails are sent as a text attachement. Also quoting does not work when people are sending non-compliant mails allowing long lines.
On Tuesday 29 January 2008, Ulf Samuelsson wrote:
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
==> And this is not a good approach, you want the S/W to use the first 256 kB to store initial boot, U-boot and environment. The AT91SAM9 chips will use the first 8 kB sector for the AT91bootstrap, and that should have protection separate from U-Boot code. AVR32 boot ROM will boot from a higher address and use the 8 kB sector at address zero as environment. Both will need to have a S/W partitioning scheme added with separate protection for each partition. This partioning scheme exists today.
sorry, i didnt mean to imply i thought this limitation was something to be written in stone. i understand perfectly why you would want this flexibility, so i can take a look. rather than making general statements, can you please provide a concrete list of all relevant examples you know of.
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
==> Which means that you want to impose an arbitary limit which currently does not exist in U-Boot. You also increase startup time, which is a critical parameter for many customers.
uhh, what ? you're telling me that customers do a SPI write in u-boot everytime they boot up ? that just sounds stupid and your customers need to rethink their system. "critical boot time" and "spi flash writing" dont belong in the same sentence :P
==> Should have been a line break there, the sentence belongs to the statement below.
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
then i'm afraid your implementation is wrong. DMA is board/cpu-specific and there's no way that can be represented in a general framework. in fact, when i was researching how to support SPI dataflash originally, i saw the dataflash code in u-boot and found it to be worthless in terms of re-use. as Haavard points out though, by using a more general SPI transfer layer, you can still (wrongly imo) do DMA transfers.
==> If a customer measures the boot speed, then slow non-DMA code is wrong and fast code using DMA is right. It may be that this should be in an Atmel specific driver location.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
==> No, but U-Boot should make them aware of consequences, so they do not do this by mistake. You normally can't remove the bootloader from a parallel flash without unprotecting it first.
providing sector protection is a different topic from removing functionality. nand flash provides the ability to erase the whole flash, so are you going to lobby them to drop that too ?
==> If you store the U-boot and environment in the NAND flash then allowing users to destroy the functionality of the board with a simple command is stupid. NAND flash need partitioning and protection as well.
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
ok ? not sure how this is relevant to the topic at hand.
==> Just pointing out that the user interface is missing features.
what features ? checksums is not the domain of the flash layer unless the flash media itself requires it (like NAND and ECC/bad-blocks). parallel/SPI flash provide no such thing, so adding a completely software checksum layer is inappropriate. if you want checksums, add them yourself in your board code.
==> It is your opinion that it is inappropriate, others may disagree. I believe that this should be a user choice. I have seen this problem numerous times and users invariably look in the wrong place to solve the problem, wasting their time and eventually my time as well.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
ok ? u-boot is designed as a monitor to get the system bootstrapped and execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features. what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux.
==> Yes, it is a basic design decision, and with huge flash memories, it is becoming flawed. Thats why I think we should rethink how we do things, before we rewrite the code.
Programming the internal flash memory in an efficient way is really the core of a bootloader, not an extended feature. Programming time is a critical parameter for large volumeproduction so if you can avoid booting Linux, this is probably a good thing.
not relevant to the thread. please drop any further comments on the topic. if you want to re-architect u-boot, start a new thread.
==> My conclusion remains, that I do not see why anyone using dataflash in the current U-boot would want to switch to the proposed implementation, which will reduce performance and functionality, and increase risk of errors.
do you not understand how the software development process works ? i post a new framework, asking for feedback. that doesnt mean it is perfect the first time through and that everyone should start using it RIGHT NOW. please exercise some patience.
==> I *am* providing feedback, and the feedback is that the proposed code will limit functionality compared to the current implementation, and introduce dangerous features which I prefer not having in any boards that I have to support. It is moving u-boot in a direction which I belive is a dead end due to the emergence of huge flash memories. I do not mind that code like this is added to u-boot as long as you do not build this in by default and that the current implemention is available until the deficiencies in the proposed implementation is fixed.
-mike
Best Regards Ulf Samuelsson ulf@atmel.com Atmel Nordic AB Mail: Box 2033, 174 02 Sundbyberg, Sweden Visit: Kavallerivägen 24, 174 58 Sundbyberg, Sweden Phone +46 (8) 441 54 22 Fax +46 (8) 441 54 29 GSM +46 (706) 22 44 57
Technical support when I am not available: AT90 AVR Applications Group: mailto:avr@atmel.com AT91 ARM Applications Group: mailto:at91support@atmel.com AVR32 Applications Group mailto:avr32@atmel.com http://www.avrfreaks.net/; http://avr32linux.org/ http://www.at91.com/ ; ftp://at91dist:distrib@81.80.104.162/ ----- Original Message ----- From: "Mike Frysinger" vapier@gentoo.org To: "Ulf Samuelsson" ulf.samuelsson@atmel.com Cc: u-boot-users@lists.sourceforge.net Sent: Tuesday, January 29, 2008 3:07 PM Subject: Re: [U-Boot-Users] [rfc] new spiflash subsystem
On Tuesday 29 January 2008, Ulf Samuelsson wrote:
please quote properly in your replies
==> Mailers can't quote properly when mails are sent as a text attachement. Also quoting does not work when people are sending non-compliant mails allowing long lines.
funny, no one else has a problem. the body of the original message was not an attachment (the additional files were) nor did it have long lines. the different portions were also properly mime quoted. the code looking for comments did contain long lines, but that is correct behavior when posting code/patches, and line wrapping is wrong. sane e-mail clients can quote perfectly fine in any of these cases. please fix your client. -mike
On Wed, 30 Jan 2008 01:39:56 +0100 "Ulf Samuelsson" ulf@atmel.com wrote:
X-Mailer: Microsoft Outlook Express 6.00.2900.3138
Right...
please quote properly in your replies
==> Mailers can't quote properly when mails are sent as a text attachement. Also quoting does not work when people are sending non-compliant mails allowing long lines.
That's because your mailer is a piece of crap. Get a better one.
Haavard
On Tuesday 29 January 2008, Mike Frysinger wrote:
On Tuesday 29 January 2008, Ulf Samuelsson wrote:
==> And this is not a good approach, you want the S/W to use the first 256 kB to store initial boot, U-boot and environment. The AT91SAM9 chips will use the first 8 kB sector for the AT91bootstrap, and that should have protection separate from U-Boot code. AVR32 boot ROM will boot from a higher address and use the 8 kB sector at address zero as environment. Both will need to have a S/W partitioning scheme added with separate protection for each partition. This partioning scheme exists today.
sorry, i didnt mean to imply i thought this limitation was something to be written in stone. i understand perfectly why you would want this flexibility, so i can take a look. rather than making general statements, can you please provide a concrete list of all relevant examples you know of.
Ulf: can you at least follow up to this so i have a goal to work towards ? maybe i'll demand some sample parts if you want support ! :) -mike
Ulf,
please get your quoting right!!!
In message 007701c86250$2a914b90$050514ac@atmel.com you wrote:
On Monday 28 January 2008, Ulf Samuelsson wrote:
Unfortunately, this code seems useless, at least for the combination AT91 + SPI flash. Some issues:
I believe that the AT45 is not using the same command set as other SPI flash memories. I think the commands need to be separated.
i already accounted for different command sets based on part family.
AT45 is much more advanced than other SPI flash.
yes, but it still has a basic subset which can be used.
Did you really test your code on the AT45 series?
it was the part i developed the code on originally (the 64mbit one).
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
This looks as if you were replying to your own postings, when you actualy reply to Mike's text!!!
Best regards,
Wolfgang Denk
In message 007701c86250$2a914b90$050514ac@atmel.com you wrote:
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Note that you are referring to code that is not part of the public U-Boot tree, and that (as is) will never become a part of it.
Best regards,
Wolfgang Denk
In message 007701c86250$2a914b90$050514ac@atmel.com you wrote:
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Note that you are referring to code that is not part of the public U-Boot tree, and that (as is) will never become a part of it.
No, I am referring to code which does exist in the U-Boot tree.
- Look how proper quoting can be when the sender of the original mail follows mail standards ...
Best Regards Ulf Samuelsson
In message 00be01c862d8$9d61a7e0$070514ac@atmel.com you wrote:
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Note that you are referring to code that is not part of the public U-Boot tree, and that (as is) will never become a part of it.
No, I am referring to code which does exist in the U-Boot tree.
You mean the current dataflash implementation uses DMA? Which part of the code is this?
Best regards,
Wolfgang Denk
In message 00be01c862d8$9d61a7e0$070514ac@atmel.com you wrote:
==> This is not how the dataflash support is implemented in U-Boot today. Current implementation is using DMA.
Note that you are referring to code that is not part of the public U-Boot tree, and that (as is) will never become a part of it.
No, I am referring to code which does exist in the U-Boot tree.
You mean the current dataflash implementation uses DMA? Which part of the code is this?
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@denx.de When the bosses talk about improving productivity, they are never talking about themselves.
Not sure as I do not have a source tree in front of me, but if I remember correctly, it was in the board specific "at45.c" which then was split in two files. One generic "at45.c" and one CPU specific file "spi.c" which is located in the cpu/arm920t atmel specific directory, and the DMA functionality is in the CPU specific part. (Where it should be)
I think a good SPI implementation should (like today) create a command block in a buffer, and request the CPU specific driver to do a block transfer.
Having a generic driver do the small details of access will remove all possibilities for an efficient driver.
There can of course be a driver which sends bytes one by one, as a fallback , but I think this should be an option that can be overridden by a driver focusing on efficiency.
Best Regards Ulf Samuelsson
In message 000301c8634b$5471f2b0$8901a8c0@atmel.com you wrote:
You mean the current dataflash implementation uses DMA? Which part of the code is this?
...
Not sure as I do not have a source tree in front of me, but if I remember correctly, it was in the board specific "at45.c" which then was split in two files. One generic "at45.c" and one CPU specific file "spi.c" which is located in the cpu/arm920t atmel specific directory, and the DMA functionality is in the CPU specific part. (Where it should be)
Could you please look it up once you get access to the source code again? Thanks.
Best regards,
Wolfgang Denk
On 19:41 Mon 28 Jan , Mike Frysinger wrote:
On Monday 28 January 2008, Ulf Samuelsson wrote:
Unfortunately, this code seems useless, at least for the combination AT91 + SPI flash. Some issues:
I believe that the AT45 is not using the same command set as other SPI flash memories. I think the commands need to be separated.
i already accounted for different command sets based on part family.
AT45 is much more advanced than other SPI flash.
yes, but it still has a basic subset which can be used.
Did you really test your code on the AT45 series?
it was the part i developed the code on originally (the 64mbit one).
You assume, incorrectly, that all sector sizes are the same size.
that depends on what level you look at it. sector 0 can be accessed in pieces, but it can also be treated as one big sector the same as all the others. this is how i treated it.
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
I disagree on some product you use a spi flash to store other code and not nessarely store u-boot in it? (you can have 2 falsh)
yes, protection of pages (hardware and software) is missing.
Typically you want to store data with a checksum,since relying on the boot of the linux kernel to produce the error, will in my experience make people confused and they will spend a lot of time barking up the wrong tree.
ok ? not sure how this is relevant to the topic at hand.
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system. How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Today, you have to use 10 lines (U-Boot occupies 1 MB) and that is really bad.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
ok ? u-boot is designed as a monitor to get the system bootstrapped and execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features. what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux.
I also disagree you may need the optimal performance, in some project I need it.
Best Regards, J.
-mike
This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
How do you do "byte writes" which is an important feature of the AT45?
simple: i dont. spi flash writing isnt something to be done constantly nor is it fast, so i dont sweat getting maximum performance.
This is an artificial limitation based on your _opinion_. How, why, or what someone chooses to write into spi flash should not be constrained by design.
Your code does not support DMA transfers, while the current dataflash code runs DMA up to 50 Mbps.
so ? the point of u-boot is to do everything in PIO mode.
This is also an opinion -- that I happen to disagree with.
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
I have valid reasons, better _requirements_, for erasing an entire spi flash and have done so many times. I never realized that I was stupid ... and for all these years! ;-)
Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
I disagree on some product you use a spi flash to store other code and not nessarely store u-boot in it? (you can have 2 falsh)
Ditto -- I also disagree.
execute something else. it isnt an operating system, it doesnt get maximal performance, and it isnt supposed to support all sorts of extended features.
What it's _supposed_ to support can be debated. But I'm quite sure that preventing extended features is not a good thing to design into a subsystem from the outset.
what you describe as deficiency doesnt apply to the topic at hand and really sounds like a basic design decision for u-boot. if you want optimal performance, use Linux.
The pot is calling the kettle black here -- WRT basic design decisions. If I want optimal performance, I shouldn't have to find an alternative to u-boot simply because a subsystem prevents me from doing so by design.
I think many of the comments/suggestions in this email topic have been sound and raise some valid issues/concerns -- this is a good thing.
Regards, --Scott
Erasing the entire SPI flash is generally stupid, since you store the environment there. You typically also store the initial bootloader and U-Boot.
so the user is stupid if they erase the entire flash ? you could say the same thing about any flash type.
I have valid reasons, better _requirements_, for erasing an entire spi flash and have done so many times. I never realized that I was stupid ... and for all these years! ;-)
Very rarely you want to erase the complete flash ,and a protection mechanism is needed to avoid accidental overwrites. The current solution allows dataflash pages to be protected.
I disagree on some product you use a spi flash to store other code and not nessarely store u-boot in it? (you can have 2 falsh)
OK, let me rephrase the statement: It is stupid to allow the user to *accidently* erase the environment sector and the U-boot area The proposed user interface will do this, if you do not give any address parameters to the erase command.
Best Regards Ulf Samuelsson
In message 017901c8620a$d600f0c0$030514ac@atmel.com you wrote:
There is a general problem with U-Boot which seems to assume that there is more RAM than flash in the system.
U-Boot does not make any such assumption - at least none of the ports I know of.
How do you easily copy 256 MB from an SD-Card to an onboard 256 MB NAND flash when the SDRAM is 64 MB?
Please read the archives. We've had this discussion a long time before.
The vanilla way of supporting storage devices is really wasteful in resources, and you cannot compare two memory areas if the memory area is larger than half the SDRAM size.
You can compare *memory* areas of any size that is mapped into the processor's address space. I guess you are talking about storage devices.
Best regards,
Wolfgang Denk
On Mon, 28 Jan 2008 15:00:34 -0500 Mike Frysinger vapier@gentoo.org wrote:
uint8_t spiflash_exchange_byte(uint8_t transmit)
Uh. Did you just propose SPI framework #6 for u-boot?
How about we try to modify one of the existing ones so that it can be useful for everything? That is, spi flash, spi eeproms, LCD panels, sensors, etc.?
In fact, I did post a patch that attempted this not so long ago, but I never got any response:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/33822/focus=33821
I'd really like to see a common layer for SPI flash though, so I don't mean to criticise your effort. But it would be nice if not every new SPI client came with its own low-level SPI functions...
Haavard
On Monday 28 January 2008, Haavard Skinnemoen wrote:
On Mon, 28 Jan 2008 15:00:34 -0500 Mike Frysinger vapier@gentoo.org wrote:
uint8_t spiflash_exchange_byte(uint8_t transmit)
Uh. Did you just propose SPI framework #6 for u-boot?
How about we try to modify one of the existing ones so that it can be useful for everything? That is, spi flash, spi eeproms, LCD panels, sensors, etc.?
In fact, I did post a patch that attempted this not so long ago, but I never got any response:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/33822/focus=33821
I'd really like to see a common layer for SPI flash though, so I don't mean to criticise your effort. But it would be nice if not every new SPI client came with its own low-level SPI functions...
i know exactly where you're coming from, so dont sweat it. my purpose was to get things integrated and then take a look at the existing pieces and unify them. easier i think to have known working code to throw at a common framework than code for hypothetical uses we cant actually break our teeth on. -mike
On Mon, 28 Jan 2008 19:29:38 -0500 Mike Frysinger vapier@gentoo.org wrote:
I'd really like to see a common layer for SPI flash though, so I don't mean to criticise your effort. But it would be nice if not every new SPI client came with its own low-level SPI functions...
i know exactly where you're coming from, so dont sweat it. my purpose was to get things integrated and then take a look at the existing pieces and unify them. easier i think to have known working code to throw at a common framework than code for hypothetical uses we cant actually break our teeth on.
Ok, don't worry about it then. I just wanted to make sure you investigate the API(s) we already have before you propose something new.
So let's just make "please use the include/spi.h API for low-level SPI access" be one of the comments on your RFC :-)
Haavard
On Tuesday 29 January 2008, Haavard Skinnemoen wrote:
Mike Frysinger vapier@gentoo.org wrote:
I'd really like to see a common layer for SPI flash though, so I don't mean to criticise your effort. But it would be nice if not every new SPI client came with its own low-level SPI functions...
i know exactly where you're coming from, so dont sweat it. my purpose was to get things integrated and then take a look at the existing pieces and unify them. easier i think to have known working code to throw at a common framework than code for hypothetical uses we cant actually break our teeth on.
Ok, don't worry about it then. I just wanted to make sure you investigate the API(s) we already have before you propose something new.
So let's just make "please use the include/spi.h API for low-level SPI access" be one of the comments on your RFC :-)
do you have a git tree for this so i can pull it down and test it for Blackfin ? i can see about posting an updated implementation then ... -mike
On Tue, 29 Jan 2008 09:13:32 -0500 Mike Frysinger vapier@gentoo.org wrote:
So let's just make "please use the include/spi.h API for low-level SPI access" be one of the comments on your RFC :-)
do you have a git tree for this so i can pull it down and test it for Blackfin ? i can see about posting an updated implementation then ...
No, it's really just the patch I mentioned, and a follow-up patch with a hardware spi driver that implements the new API. include/spi.h exists today, I just did a couple of tweaks to make it more useful with hardware SPI controllers.
The patch needs to be rebased and possibly updated to fix up more boards and drivers. I'm hoping to get it done this week, but I won't promise anything.
Haavard
Haavard Skinnemoen wrote:
On Mon, 28 Jan 2008 15:00:34 -0500 Mike Frysinger vapier@gentoo.org wrote:
uint8_t spiflash_exchange_byte(uint8_t transmit)
Uh. Did you just propose SPI framework #6 for u-boot?
How about we try to modify one of the existing ones so that it can be useful for everything? That is, spi flash, spi eeproms, LCD panels, sensors, etc.?
In fact, I did post a patch that attempted this not so long ago, but I never got any response:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/33822/focus=33821
Huh, missed that one :-( . Nice work - getting rid of the table is a good thing. If you re-base and post again I can do some testing.
regards, Ben
On Mon, 28 Jan 2008 19:44:55 -0500 Ben Warren biggerbadderben@gmail.com wrote:
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/33822/focus=33821
Huh, missed that one :-( . Nice work - getting rid of the table is a good thing. If you re-base and post again I can do some testing.
That would be great. I'm currently a bit busy trying to extend the Linux DMA Engine framework, but I'll see if I can get it rebased before this week ends.
Haavard
participants (8)
-
Ben Warren -
Haavard Skinnemoen -
Jean-Christophe PLAGNIOL-VILLARD -
Mike Frysinger -
Scott McNutt -
Ulf Samuelsson -
Ulf Samuelsson -
Wolfgang Denk