[U-Boot-Users] [rfc] new spiflash subsystem

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/13afe397/attachment.pgp 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cmd_spiflash.c
Type: text/x-csrc
Size: 2389 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/13afe397/attachment.c 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spiflash_jedec.c
Type: text/x-csrc
Size: 11469 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/13afe397/attachment-0001.c 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spiflash.c
Type: text/x-csrc
Size: 2052 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/13afe397/attachment-0002.c 

[U-Boot-Users] [rfc] new spiflash subsystem

[Ulf Samuelsson]

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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users
>

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/7ae3f643/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080128/df5c772b/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Ben Warren]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Ulf Samuelsson]

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 at 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Jean-Christophe PLAGNIOL-VILLARD]

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 at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/u-boot-users

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Scott McNutt]

>>> 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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 volume
> production 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080129/bd8300f9/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080129/a4fc0fe7/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[Wolfgang Denk]

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

-- 
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 at denx.de
Committee, n.:  A group of men who individually can do nothing but as
a group decide that nothing can be done.                 - Fred Allen

[U-Boot-Users] [rfc] new spiflash subsystem

[Wolfgang Denk]

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

-- 
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 at denx.de
Intel told us the Pentium would have "RISK" features...

[U-Boot-Users] [rfc] new spiflash subsystem

[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

-- 
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 at denx.de
Without freedom of choice there is no creativity.
	-- Kirk, "The return of the Archons", stardate 3157.4

[U-Boot-Users] [rfc] new spiflash subsystem

[Ulf Samuelsson]

>>>> 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Ulf Samuelsson]

> 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Ulf Samuelsson]

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 volume
> production 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 at 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 at atmel.com
AT91 ARM Applications Group: mailto:at91support at atmel.com
AVR32 Applications Group        mailto:avr32 at atmel.com
http://www.avrfreaks.net/;            http://avr32linux.org/
http://www.at91.com/ ;                ftp://at91dist:distrib at 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080129/c7407044/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Haavard Skinnemoen]

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

[U-Boot-Users] [rfc] new spiflash subsystem

[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 at denx.de
When the bosses talk about improving  productivity,  they  are  never
talking about themselves.

[U-Boot-Users] [rfc] new spiflash subsystem

[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
> 
> -- 
> 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 at 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

[U-Boot-Users] [rfc] new spiflash subsystem

[Mike Frysinger]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 827 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20080130/07f29032/attachment.pgp 

[U-Boot-Users] [rfc] new spiflash subsystem

[Wolfgang Denk]

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

-- 
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 at denx.de
He'd heard her use that sweet, innocent  tone  of  voice  before.  It
meant that, pretty soon, there was going to be trouble.
                                        - Terry Pratchett, _Truckers_