Skip to contentSkip to conversation

SST25L (non JEDEC) SPI Flash driver

PATCH In review Ryan Mallon 5 participants 16 messages Jun 22, 2009 – Jun 25, 2009

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 SST25L (non JEDEC) SPI Flash driver

Add support for the non JEDEC SST25L SPI Flash devices.

---

drivers/mtd/devices/Kconfig +10 -0
index 325fab9..c222514 100644
@@ -104,6 +104,16 @@ config M25PXX_USE_FAST_READ
 	help
 	  This option enables FAST_READ access supported by ST M25Pxx.
 
+config MTD_SST25L
+	tristate "Support SST25L (non JEDEC) SPI Flash chips"
+	depends on SPI_MASTER
+	help
+	  This enables access to the non JEDEC SST25L SPI flash chips, used
+	  for program and data storage.
+
+	  Set up your spi devices with the right board-specific platform data,
+	  if you want to specify device partitioning.
+
 config MTD_SLRAM
 	tristate "Uncached system RAM"
 	help
drivers/mtd/devices/Makefile +1 -0
index 0993d5c..ab5c9b9 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_MTD_LART)		+= lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
+obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
drivers/mtd/devices/sst25l.c new file +508 -0

508 lines not on this page · raw message

#1 lore raw

Hi Ryan,

drivers/mtd/devices/sst25l.c line 143

Quoted 6 lines · Ryan Mallon
drivers/mtd/devices/sst25l.c new file +6 -0
+static int sst25l_erase_block(struct sst25l_flash *flash, u32 offset)
+{
+	u8 command[4];
+	int err;
+
+	err = sst25l_write_enable(flash, 1);

Is this enable/disable strictly required for each write/erase? Can't we just enable write once?

Quoted 7 lines
drivers/mtd/devices/sst25l.c new file +7 -0
+	if (err)
+		return err;
+
+	command[0] = SST25L_CMD_BLOCK_ERASE;
+	command[1] = offset >> 16;
+	command[2] = offset >> 8;
+	command[3] = offset;

drivers/mtd/devices/sst25l.c line 160

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+	err = spi_write(flash->spi, command, 4);
+	if (err)
+		return err;
+
+	err = sst25l_wait_till_ready(flash);
+	if (err)
+		return err;
+
+	return sst25l_write_enable(flash, 0);
+}

baruch

Signature · 3 lines
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -
#2 lore raw
Quoted 12 lines · Baruch Siach

Hi Ryan,

drivers/mtd/devices/sst25l.c line 143

Quoted 6 lines · Ryan Mallon
drivers/mtd/devices/sst25l.c new file +6 -0
+static int sst25l_erase_block(struct sst25l_flash *flash, u32 offset)
+{
+	u8 command[4];
+	int err;
+
+	err = sst25l_write_enable(flash, 1);

Is this enable/disable strictly required for each write/erase? Can't we just enable write once?

The datasheet says that prior to any write/erase instruction the write enable instruction must be sent.

~Ryan

Signature · 8 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: [email protected]             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934
#3 lore raw
In reply to Ryan Mallon

drivers/mtd/devices/Kconfig config M25PXX_USE_FAST_READ · line 105

Quoted 13 lines · Ryan Mallon

Add support for the non JEDEC SST25L SPI Flash devices.

---

drivers/mtd/devices/Kconfig +0 -0
index 325fab9..c222514 100644
@@ -104,6 +104,16 @@ config M25PXX_USE_FAST_READ
 	help
 	  This option enables FAST_READ access supported by ST M25Pxx.

Would it please be possible to get rid of this warning:

CC [M] drivers/mtd/devices/sst25l.o
drivers/mtd/devices/sst25l.c: In function ‘sst25l_write’:
drivers/mtd/devices/sst25l.c:306: warning: comparison of distinct pointer types lacks a cast

I see it on my x86_64.

Signature · 2 lines
Best regards,
Artem Bityutskiy (Битюцкий Артём)
#4 lore raw
In reply to Ryan Mallon

2009/6/22 Ryan Mallon <[email protected]>:

Quoted 54 lines

Add support for the non JEDEC SST25L SPI Flash devices.

---

drivers/mtd/devices/Kconfig +10 -0
index 325fab9..c222514 100644
@@ -104,6 +104,16 @@ config M25PXX_USE_FAST_READ
       help
         This option enables FAST_READ access supported by ST M25Pxx.
 
+config MTD_SST25L
+       tristate "Support SST25L (non JEDEC) SPI Flash chips"
+       depends on SPI_MASTER
+       help
+         This enables access to the non JEDEC SST25L SPI flash chips, used
+         for program and data storage.
+
+         Set up your spi devices with the right board-specific platform data,
+         if you want to specify device partitioning.
+
 config MTD_SLRAM
       tristate "Uncached system RAM"
       help
drivers/mtd/devices/Makefile +1 -0
index 0993d5c..ab5c9b9 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_MTD_LART)                += lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)    += block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)    += mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)       += m25p80.o
+obj-$(CONFIG_MTD_SST25L)       += sst25l.o
drivers/mtd/devices/sst25l.c new file +12 -0
new file mode 100644
index 0000000..8e62479
@@ -0,0 +1,508 @@
+/*
+ * drivers/mtd/sst25l.c
+ *
+ * Driver for SST25L SPI Flash chips
+ *
+ * Copyright (C) 2009 Bluewater Systems Ltd
+ * Author: Andre Renaud <[email protected]>
+ * Author: Ryan Mallon <[email protected]>
+ *
+ * Based on m25p80.c
+ *
+ * This code is free software; you can redistribute it and/or modify

drivers/mtd/devices/sst25l.c line 22

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/math64.h>

I'm always suspicious about drivers doing maths, so I think this can be removed, see below...

Quoted 36 lines
drivers/mtd/devices/sst25l.c new file +36 -0
+#include <linux/interrupt.h>
+
+#include <linux/mtd/mtd.h>
+#include <linux/mtd/partitions.h>
+
+#include <linux/spi/spi.h>
+#include <linux/spi/flash.h>
+
+/* Erases can take up to 3 seconds! */
+#define MAX_READY_WAIT_JIFFIES msecs_to_jiffies(3000)
+
+#define SST25L_CMD_WRSR                0x01    /* Write status register */
+#define SST25L_CMD_WRDI                0x04    /* Write disable */
+#define SST25L_CMD_RDSR                0x05    /* Read status register */
+#define SST25L_CMD_WREN                0x06    /* Write enable */
+#define SST25L_CMD_READ                0x03    /* High speed read */
+
+#define SST25L_CMD_EWSR                0x50    /* Enable write status register */
+#define SST25L_CMD_BLOCK_ERASE 0x52    /* Erase sector */
+#define SST25L_CMD_READ_ID     0x90    /* Read device ID */
+#define SST25L_CMD_AAI_PROGRAM 0xaf    /* Auto address increment */
+
+#define SST25L_STATUS_BUSY     (1 << 0)        /* Chip is busy */
+#define SST25L_STATUS_WREN     (1 << 1)        /* Write enabled */
+#define SST25L_STATUS_BP0      (1 << 2)        /* Block protection 0 */
+#define SST25L_STATUS_BP1      (1 << 3)        /* Block protection 1 */
+
+struct sst25l_flash {
+       struct spi_device       *spi;
+       struct mutex            lock;
+       struct mtd_info         mtd;
+
+       int                     partitioned;
+};
+
+struct flash_info {

drivers/mtd/devices/sst25l.c line 68

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+       const char              *name;
+       u16                     device_id;
+       unsigned                page_size;
+       unsigned                nr_pages;
+       unsigned                erase_size;
+};
+
+#define to_sst25l_flash(x) container_of(x, struct sst25l_flash, mtd)
+
+static struct flash_info __devinitdata sst25l_flash_info[] = {

__devinitdata or __initdata?
Check the definitions from include/linux/init.h. If it's hot-pluggable then it's fine!

Quoted 94 lines
drivers/mtd/devices/sst25l.c new file +94 -0
+       {"sst25lf020a", 0xbf43, 256, 1024, 32 * 1024},
+       {"sst25lf040a", 0xbf44, 256, 2048, 32 * 1024},
+};
+
+static int sst25l_status(struct sst25l_flash *flash, int *status)
+{
+       u8 command, response;
+       int err;
+
+       command = SST25L_CMD_RDSR;
+       err = spi_write_then_read(flash->spi, &command, 1, &response, 1);
+       if (err < 0)
+               return err;
+
+       *status = response;
+       return 0;
+}
+
+static int sst25l_write_enable(struct sst25l_flash *flash, int enable)
+{
+       u8 command[2];
+       int status, err;
+
+       command[0] = enable ? SST25L_CMD_WREN : SST25L_CMD_WRDI;
+       err = spi_write(flash->spi, command, 1);
+       if (err)
+               return err;
+
+       command[0] = SST25L_CMD_EWSR;
+       err = spi_write(flash->spi, command, 1);
+       if (err)
+               return err;
+
+       command[0] = SST25L_CMD_WRSR;
+       command[1] = enable ? 0 : SST25L_STATUS_BP0 | SST25L_STATUS_BP1;
+       err = spi_write(flash->spi, command, 2);
+       if (err)
+               return err;
+
+       if (enable) {
+               err = sst25l_status(flash, &status);
+               if (err)
+                       return err;
+               if (!(status & SST25L_STATUS_WREN))
+                       return -EROFS;
+       }
+
+       return 0;
+}
+
+static int sst25l_wait_till_ready(struct sst25l_flash *flash)
+{
+       unsigned long deadline;
+       int status, err;
+
+       deadline = jiffies + MAX_READY_WAIT_JIFFIES;
+       do {
+               err = sst25l_status(flash, &status);
+               if (err)
+                       return err;
+               if (!(status & SST25L_STATUS_BUSY))
+                       return 0;
+
+               cond_resched();
+       } while (!time_after_eq(jiffies, deadline));
+
+       return -ETIMEDOUT;
+}
+
+static int sst25l_erase_block(struct sst25l_flash *flash, u32 offset)
+{
+       u8 command[4];
+       int err;
+
+       err = sst25l_write_enable(flash, 1);
+       if (err)
+               return err;
+
+       command[0] = SST25L_CMD_BLOCK_ERASE;
+       command[1] = offset >> 16;
+       command[2] = offset >> 8;
+       command[3] = offset;
+       err = spi_write(flash->spi, command, 4);
+       if (err)
+               return err;
+
+       err = sst25l_wait_till_ready(flash);
+       if (err)
+               return err;
+
+       return sst25l_write_enable(flash, 0);
+}
+
+static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr)

drivers/mtd/devices/sst25l.c line 172

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+{
+       struct sst25l_flash *flash = to_sst25l_flash(mtd);
+       u32 addr, end, rem;
+       int err;
+
+       /* Sanity checks */
+       if (instr->addr + instr->len > flash->mtd.size)
+               return -EINVAL;
+
+       div_u64_rem(instr->len, mtd->erasesize, &rem);

if ((instr->len & 0xFFFFFFFFU) % mtd->erasesize)
return -EINVAL;

drivers/mtd/devices/sst25l.c line 176

Quoted 4 lines
drivers/mtd/devices/sst25l.c new file +4 -0
+       if (rem)
+               return -EINVAL;
+
+       div_u64_rem(instr->addr, mtd->erasesize, &rem);

Same thing again.

Quoted 90 lines
drivers/mtd/devices/sst25l.c new file +90 -0
+       if (rem)
+               return -EINVAL;
+
+       addr = instr->addr;
+       end = addr + instr->len;
+
+       mutex_lock(&flash->lock);
+
+       err = sst25l_wait_till_ready(flash);
+       if (err)
+               return err;
+
+       while (addr < end) {
+               err = sst25l_erase_block(flash, addr);
+               if (err) {
+                       mutex_unlock(&flash->lock);
+                       instr->state = MTD_ERASE_FAILED;
+                       dev_err(&flash->spi->dev, "Erase failed\n");
+                       return err;
+               }
+
+               addr += mtd->erasesize;
+       }
+
+       mutex_unlock(&flash->lock);
+
+       instr->state = MTD_ERASE_DONE;
+       mtd_erase_callback(instr);
+       return 0;
+}
+
+static int sst25l_read(struct mtd_info *mtd, loff_t from, size_t len,
+                      size_t *retlen, u_char *buf)
+{
+       struct sst25l_flash *flash = to_sst25l_flash(mtd);
+       struct spi_transfer transfer[2];
+       struct spi_message message;
+       u8 command[4];
+       int ret;
+
+       /* Sanity checking */
+       if (len == 0)
+               return 0;
+
+       if (from + len > flash->mtd.size)
+               return -EINVAL;
+
+       if (retlen)
+               *retlen = 0;
+
+       spi_message_init(&message);
+       memset(&transfer, 0, sizeof(transfer));
+
+       command[0] = SST25L_CMD_READ;
+       command[1] = from >> 16;
+       command[2] = from >> 8;
+       command[3] = from;
+
+       transfer[0].tx_buf = command;
+       transfer[0].len = sizeof(command);
+       spi_message_add_tail(&transfer[0], &message);
+
+       transfer[1].rx_buf = buf;
+       transfer[1].len = len;
+       spi_message_add_tail(&transfer[1], &message);
+
+       mutex_lock(&flash->lock);
+
+       /* Wait for previous write/erase to complete */
+       ret = sst25l_wait_till_ready(flash);
+       if (ret) {
+               mutex_unlock(&flash->lock);
+               return ret;
+       }
+
+       spi_sync(flash->spi, &message);
+
+       if (retlen && message.actual_length > sizeof(command))
+               *retlen += message.actual_length - sizeof(command);
+
+       mutex_unlock(&flash->lock);
+       return 0;
+}
+
+static int sst25l_write(struct mtd_info *mtd, loff_t to, size_t len,
+                       size_t *retlen, const u_char *buf)
+{
+       struct sst25l_flash *flash = to_sst25l_flash(mtd);
+       int i, j, ret, bytes, copied = 0;
+       u8 command[5];

drivers/mtd/devices/sst25l.c line 276

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+       u32 rem;
+
+       /* Sanity checks */
+       if (!len)
+               return 0;
+
+       if (to + len > flash->mtd.size)
+               return -EINVAL;
+
+       div_u64_rem(to, mtd->writesize, &rem);

I see. But do you really want to do that with maths? Can't you just do some simple arithmetics since mtd->writesize is u32 anything in the higher 32 bits can be ignored (it will be divisible with any u32 value anyway!), so

/* Need to be evenly divisible by writesize */
if ((to & 0xFFFFFFFFU) % mtd->writesize)

    return -EINVAL;

does this work?

Quoted 43 lines
drivers/mtd/devices/sst25l.c new file +43 -0
+       if (rem)
+               return -EINVAL;
+
+       mutex_lock(&flash->lock);
+
+       ret = sst25l_write_enable(flash, 1);
+       if (ret)
+               goto out;
+
+       for (i = 0; i < len; i += mtd->writesize) {
+               ret = sst25l_wait_till_ready(flash);
+               if (ret)
+                       goto out;
+
+               /* Write the first byte of the page */
+               command[0] = SST25L_CMD_AAI_PROGRAM;
+               command[1] = (to + i) >> 16;
+               command[2] = (to + i) >> 8;
+               command[3] = (to + i);
+               command[4] = buf[i];
+               ret = spi_write(flash->spi, command, 5);
+               if (ret < 0)
+                       goto out;
+               copied++;
+
+               /*
+                * Write the remaining bytes using auto address
+                * increment mode
+                */
+               bytes = min(mtd->writesize, len - i);
+               for (j = 1; j < bytes; j++, copied++) {
+                       ret = sst25l_wait_till_ready(flash);
+                       if (ret)
+                               goto out;
+
+                       command[1] = buf[i + j];
+                       ret = spi_write(flash->spi, command, 2);
+                       if (ret)
+                               goto out;
+               }
+       }
+
+out:

drivers/mtd/devices/sst25l.c line 329

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+       ret = sst25l_write_enable(flash, 0);
+
+       if (retlen)
+               *retlen = copied;
+
+       mutex_unlock(&flash->lock);
+       return ret;
+}
+
+static struct flash_info *__devinit sst25l_match_device(struct spi_device *spi)

Or is it __init really? NB: this will save (very little) memory on your system.

Quoted 31 lines
drivers/mtd/devices/sst25l.c new file +31 -0
+{
+       struct flash_info *flash_info = NULL;
+       u8 command[4], response;
+       int i, err;
+       u16 id;
+
+       command[0] = SST25L_CMD_READ_ID;
+       command[1] = 0;
+       command[2] = 0;
+       command[3] = 0;
+       err = spi_write_then_read(spi, command, sizeof(command), &response, 1);
+       if (err < 0) {
+               dev_err(&spi->dev, "error reading device id msb\n");
+               return NULL;
+       }
+
+       id = response << 8;
+
+       command[0] = SST25L_CMD_READ_ID;
+       command[1] = 0;
+       command[2] = 0;
+       command[3] = 1;
+       err = spi_write_then_read(spi, command, sizeof(command), &response, 1);
+       if (err < 0) {
+               dev_err(&spi->dev, "error reading device id lsb\n");
+               return NULL;
+       }
+
+       id |= response;
+
+       for (i = 0; i < ARRAY_SIZE(sst25l_flash_info); i++)

drivers/mtd/devices/sst25l.c line 370

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+               if (sst25l_flash_info[i].device_id == id)
+                       flash_info = &sst25l_flash_info[i];
+
+       if (!flash_info)
+               dev_err(&spi->dev, "unknown id %.4x\n", id);
+
+       return flash_info;
+}
+
+static int __devinit sst25l_probe(struct spi_device *spi)

Or __init?

Quoted 88 lines
drivers/mtd/devices/sst25l.c new file +88 -0
+{
+       struct flash_info *flash_info;
+       struct sst25l_flash *flash;
+       struct flash_platform_data *data;
+       int i;
+
+       flash_info = sst25l_match_device(spi);
+       if (!flash_info)
+               return -ENODEV;
+
+       flash = kzalloc(sizeof(struct sst25l_flash), GFP_KERNEL);
+       if (!flash)
+               return -ENOMEM;
+
+       flash->spi = spi;
+       mutex_init(&flash->lock);
+       dev_set_drvdata(&spi->dev, flash);
+
+       data = spi->dev.platform_data;
+       if (data && data->name)
+               flash->mtd.name = data->name;
+       else
+               flash->mtd.name = dev_name(&spi->dev);
+
+       flash->mtd.type         = MTD_NORFLASH;
+       flash->mtd.flags        = MTD_CAP_NORFLASH;
+       flash->mtd.erasesize    = flash_info->erase_size;
+       flash->mtd.writesize    = flash_info->page_size;
+       flash->mtd.size         = flash_info->page_size * flash_info->nr_pages;
+       flash->mtd.erase        = sst25l_erase;
+       flash->mtd.read         = sst25l_read;
+       flash->mtd.write        = sst25l_write;
+
+       dev_info(&spi->dev, "%s (%lld Kbytes)\n", flash_info->name,
+                (long long)flash->mtd.size >> 10);
+
+       DEBUG(MTD_DEBUG_LEVEL2,
+             "mtd .name = %s, .size = 0x%llx (%lldMiB) "
+             ".erasesize = 0x%.8x (%uKiB) .numeraseregions = %d\n",
+             flash->mtd.name,
+             (long long)flash->mtd.size, (long long)(flash->mtd.size >> 20),
+             flash->mtd.erasesize, flash->mtd.erasesize / 1024,
+             flash->mtd.numeraseregions);
+
+       if (flash->mtd.numeraseregions)
+               for (i = 0; i < flash->mtd.numeraseregions; i++)
+                       DEBUG(MTD_DEBUG_LEVEL2,
+                             "mtd.eraseregions[%d] = { .offset = 0x%llx, "
+                             ".erasesize = 0x%.8x (%uKiB), "
+                             ".numblocks = %d }\n",
+                             i, (long long)flash->mtd.eraseregions[i].offset,
+                             flash->mtd.eraseregions[i].erasesize,
+                             flash->mtd.eraseregions[i].erasesize / 1024,
+                             flash->mtd.eraseregions[i].numblocks);
+
+       if (mtd_has_partitions()) {
+               struct mtd_partition *parts = NULL;
+               int nr_parts = 0;
+
+               if (mtd_has_cmdlinepart()) {
+                       static const char *part_probes[] =
+                               {"cmdlinepart", NULL};
+
+                       nr_parts = parse_mtd_partitions(&flash->mtd,
+                                                       part_probes,
+                                                       &parts, 0);
+               }
+
+               if (nr_parts <= 0 && data && data->parts) {
+                       parts = data->parts;
+                       nr_parts = data->nr_parts;
+               }
+
+               if (nr_parts > 0) {
+                       for (i = 0; i < nr_parts; i++) {
+                               DEBUG(MTD_DEBUG_LEVEL2, "partitions[%d] = "
+                                     "{.name = %s, .offset = 0x%llx, "
+                                     ".size = 0x%llx (%lldKiB) }\n",
+                                     i, parts[i].name,
+                                     (long long)parts[i].offset,
+                                     (long long)parts[i].size,
+                                     (long long)(parts[i].size >> 10));
+                       }
+
+                       flash->partitioned = 1;
+                       return add_mtd_partitions(&flash->mtd,
+                                                 parts, nr_parts);
+               }

drivers/mtd/devices/sst25l.c line 468

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+
+       } else if (data->nr_parts) {
+               dev_warn(&spi->dev, "ignoring %d default partitions on %s\n",
+                        data->nr_parts, data->name);
+       }
+
+       return add_mtd_device(&flash->mtd) == 1 ? -ENODEV : 0;
+}
+
+static int __devexit sst25l_remove(struct spi_device *spi)

Or __exit?

Quoted 11 lines
drivers/mtd/devices/sst25l.c new file +11 -0
+{
+       struct sst25l_flash *flash = dev_get_drvdata(&spi->dev);
+       int ret;
+
+       if (mtd_has_partitions() && flash->partitioned)
+               ret = del_mtd_partitions(&flash->mtd);
+       else
+               ret = del_mtd_device(&flash->mtd);
+       if (ret == 0)
+               kfree(flash);
+       return ret;

drivers/mtd/devices/sst25l.c line 489

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+}
+
+static struct spi_driver sst25l_driver = {
+       .driver = {
+               .name   = "sst25l",
+               .bus    = &spi_bus_type,
+               .owner  = THIS_MODULE,
+       },
+       .probe          = sst25l_probe,
+       .remove         = __devexit_p(sst25l_remove),

Or __exit_p()?

Quoted 21 lines
drivers/mtd/devices/sst25l.c new file +19 -1
+};
+
+static int __init sst25l_init(void)
+{
+       return spi_register_driver(&sst25l_driver);
+}
+
+static void __exit sst25l_exit(void)
+{
+       spi_unregister_driver(&sst25l_driver);
+}
+
+module_init(sst25l_init);
+module_exit(sst25l_exit);
+
+MODULE_DESCRIPTION("MTD SPI driver for SST25L Flash chips");
+MODULE_AUTHOR("Andre Renaud <[email protected]>, "
+             "Ryan Mallon <[email protected]>");
+MODULE_LICENSE("GPL");
 
--
Quoted 5 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Yours,
Linus Walleij

#5 lore raw

Hi Linus,

Thanks for the comments. I have removed the dependency on math64.h, I changed the instances to u32 casts and mod operator (the sst25l flash chips are quite small so it won't be a problem).

I have also replaced the __devinit/__devexit markers with the __init/__exit equivalents. In our particular instance the chip is soldered onto the board so its not hot-pluggable. I think this will be the case for most people who are using these chips.

Out of curiosity: I'm not too clear on what makes a particular chip hot-pluggable. I think technically the sst25l chip could be put onto a hot-pluggable board or power domain. Can't most devices be made hot-pluggable? Is the general rule to make devices non hot-pluggable if most/all boards in the mainline do not allow it to be hot-plugged?

Also fixed a warning on the arguments to the min macro pointed out by Artem Bityitskiy.

Updated patch below:

---
Add support for SST25L (non JEDEC) SPI Flash chips

---

drivers/mtd/devices/Kconfig +10 -0
index 325fab9..c222514 100644
@@ -104,6 +104,16 @@ config M25PXX_USE_FAST_READ
 	help
 	  This option enables FAST_READ access supported by ST M25Pxx.
 
+config MTD_SST25L
+	tristate "Support SST25L (non JEDEC) SPI Flash chips"
+	depends on SPI_MASTER
+	help
+	  This enables access to the non JEDEC SST25L SPI flash chips, used
+	  for program and data storage.
+
+	  Set up your spi devices with the right board-specific platform data,
+	  if you want to specify device partitioning.
+
 config MTD_SLRAM
 	tristate "Uncached system RAM"
 	help
drivers/mtd/devices/Makefile +1 -0
index 0993d5c..ab5c9b9 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_MTD_LART)		+= lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
+obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
drivers/mtd/devices/sst25l.c new file +503 -0

503 lines not on this page · raw message

#6 lore raw

2009/6/22 Ryan Mallon <[email protected]>:

Quoted 5 lines

Out of curiosity: I'm not too clear on what makes a particular chip hot-pluggable. I think technically the sst25l chip could be put onto a hot-pluggable board or power domain. Can't most devices be made hot-pluggable? Is the general rule to make devices non hot-pluggable if most/all boards in the mainline do not allow it to be hot-plugged?

I haven't seen any rule about it, but nominally the drivers support the configurations found in the kernel tree, so if some board physically existing and soon-to-run linux hotplugs chips like this, then have it __dev{init|exit} else __{init|exit} IMHO.

Of course you can design for all plausible use cases but it will pile up indefinitely. Personally I try to not design software for a hardware until it exists, and I like the IETF catch-phrase "rough consensus and running code" and this sort of fits that :-)

Yours,
Linus Walleij

#7 lore raw
Quoted 13 lines · Linus Walleij

2009/6/22 Ryan Mallon <[email protected]>:

Quoted 5 lines

Out of curiosity: I'm not too clear on what makes a particular chip hot-pluggable. I think technically the sst25l chip could be put onto a hot-pluggable board or power domain. Can't most devices be made hot-pluggable? Is the general rule to make devices non hot-pluggable if most/all boards in the mainline do not allow it to be hot-plugged?

I haven't seen any rule about it, but nominally the drivers support the configurations found in the kernel tree, so if some board physically existing and soon-to-run linux hotplugs chips like this, then have it __dev{init|exit} else __{init|exit} IMHO.

Quoted 4 lines

Of course you can design for all plausible use cases but it will pile up indefinitely. Personally I try to not design software for a hardware until it exists, and I like the IETF catch-phrase "rough consensus and running code" and this sort of fits that :-)

Okay, thanks. Thats basically the conclusion we came to as well: if someone else wants to hotplug this chip in the future they can post a patch for it.

BTW, most of my other patches go via the ARM list, which uses the patch system. Whats the procedure for most other lists, do patches just get collected by the subsection maintainer when they are okay?

~Ryan

Signature · 8 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: [email protected]             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934
#8 lore raw

2009/6/23 Ryan Mallon <[email protected]>:

Quoted 3 lines

BTW, most of my other patches go via the ARM list, which uses the patch system. Whats the procedure for most other lists, do patches just get collected by the subsection maintainer when they are okay?

That's the most typical procedure, however I think the SPI maintainer (David) has been busy with other things so I merged the PL022 driver through the ARM tree instead. Another possibility is to send it off to Andrew Mortons tree I guess.

Yours,
Linus Walleij

#9 lore raw
Quoted 10 lines · Linus Walleij

2009/6/23 Ryan Mallon <[email protected]>:

Quoted 3 lines

BTW, most of my other patches go via the ARM list, which uses the patch system. Whats the procedure for most other lists, do patches just get collected by the subsection maintainer when they are okay?

That's the most typical procedure, however I think the SPI maintainer (David) has been busy with other things so I merged the PL022 driver through the ARM tree instead. Another possibility is to send it off to Andrew Mortons tree I guess.

Can I get your acked-by and then just CC Andrew Morton?

~Ryan

Signature · 8 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: [email protected]             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934
Acked-by
#10 lore raw

2009/6/25 Ryan Mallon <[email protected]>:

Quoted 7 lines
Quoted 6 lines · Linus Walleij

2009/6/23 Ryan Mallon <[email protected]>:

Quoted 3 lines

BTW, most of my other patches go via the ARM list, which uses the patch system. Whats the procedure for most other lists, do patches just get collected by the subsection maintainer when they are okay?

Quoted 6 lines
Quoted 4 lines

That's the most typical procedure, however I think the SPI maintainer (David) has been busy with other things so I merged the PL022 driver through the ARM tree instead. Another possibility is to send it off to Andrew Mortons tree I guess.

Can I get your acked-by and then just CC Andrew Morton?

I'm not much of a stakeholder in the SPI subsystem, but FWIW:

Yours,
Linus Walleij

#11 lore raw
In reply to Ryan Mallon
Quoted 11 lines · Ryan Mallon

Add support for the non JEDEC SST25L SPI Flash devices.

drivers/mtd/devices/sst25l.c new file +5 -0
+struct flash_info {
+	const char		*name;
+	u16			device_id;
+	unsigned		page_size;
+	unsigned		nr_pages;

drivers/mtd/devices/sst25l.c line 72

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+	unsigned		erase_size;
+};
+
+#define to_sst25l_flash(x) container_of(x, struct sst25l_flash, mtd)
+
+static struct flash_info __devinitdata sst25l_flash_info[] = {
+	{"sst25lf020a", 0xbf43, 256, 1024, 32 * 1024},
+	{"sst25lf040a",	0xbf44,	256, 2048, 32 * 1024},
+};
+

Ryan,

I finally got a chance to test this on my hardware. Previously I was using a hacked version on the m25p80 driver to access the SST23LF020A on my board.

The driver works fine but you are only supporting the block erase, SST25L_CMD_BLOCK_ERASE (opcode 0x52). The SST25L chips also can do a sector erase (opcode 0x20) and a full chip erase (opcode 0x60).

The full chip erase is not that important but might give a slight performance increase on larger devices. Max block erase time is listed as 25ms at 25MHz in the datasheet. The SST25LF020A contains 8 blocks so it's "typical" full chip erase time should be around 200ms. For comparison, the chip erase time is listed as 100ms.

The sector erase is a bigger deal. On the EDB93xx boards one of the SPI flash uses is used to store the MAC address at offset 0x2000 (4K offset). The board can also be configured to boot from offset 0x0000 in the SPI flash. This basically gives the user a three partition setup:

static struct mtd_partition edb93xx_spi_flash_partitions[] = {
	{
		.name	= "SPI bootstrap",
		.offset	= 0,
		.size	= SZ_4K,
	}, {
		.name	= "Bootstrap config",
		.offset	= MTDPART_OFS_APPEND,
		.size	= SZ_4K,
	}, {
		.name	= "unallocated",
		.offset	= MTDPART_OFS_APPEND,
		.size	= MTDPART_SIZ_FULL,
	},
};

Without the sector erase this partition setup results in all of the partitions being forced read-only.

Just my two cents...

Regards,
Hartley

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

#12 lore raw
Quoted 28 lines · H Hartley Sweeten
Quoted 13 lines · Ryan Mallon

Add support for the non JEDEC SST25L SPI Flash devices.

(quoted diff) +5 -0
+struct flash_info {
+	const char		*name;
+	u16			device_id;
+	unsigned		page_size;
+	unsigned		nr_pages;

Review

Quoted 10 lines
(quoted diff) +10 -0
+	unsigned		erase_size;
+};
+
+#define to_sst25l_flash(x) container_of(x, struct sst25l_flash, mtd)
+
+static struct flash_info __devinitdata sst25l_flash_info[] = {
+	{"sst25lf020a", 0xbf43, 256, 1024, 32 * 1024},
+	{"sst25lf040a",	0xbf44,	256, 2048, 32 * 1024},
+};
+

Ryan,

Quoted 8 lines

I finally got a chance to test this on my hardware. Previously I was using a hacked version on the m25p80 driver to access the SST23LF020A on my board.

The driver works fine but you are only supporting the block erase, SST25L_CMD_BLOCK_ERASE (opcode 0x52). The SST25L chips also can do a sector erase (opcode 0x20) and a full chip erase (opcode 0x60).

The MTD sub-system doesn't support different types of erases (as far as I am aware). We chose to use block erases since they are faster than sector erases and the trade-off of having 32k aligned partitions didn't seem to bad.

Quoted 6 lines

The full chip erase is not that important but might give a slight performance increase on larger devices. Max block erase time is listed as 25ms at 25MHz in the datasheet. The SST25LF020A contains 8 blocks so it's "typical" full chip erase time should be around 200ms. For comparison, the chip erase time is listed as 100ms.

Quoted 8 lines

The sector erase is a bigger deal. On the EDB93xx boards one of the SPI flash uses is used to store the MAC address at offset 0x2000 (4K offset). The board can also be configured to boot from offset 0x0000 in the SPI flash.

Without the sector erase this partition setup results in all of the partitions being forced read-only.

The updated patch below uses sector erases instead to allow for smaller partition sizes.

Linus, is this still okay by you?

~Ryan

----
Add support for SST25L (non JEDEC) SPI Flash chips. Although the SSTL25 chips support a quick 32kb block erase, we use the slower 4k sector erase to allow for smaller 4kb aligned MTD partitions.

----

drivers/mtd/devices/Kconfig +10 -0
index 325fab9..c222514 100644
@@ -104,6 +104,16 @@ config M25PXX_USE_FAST_READ
 	help
 	  This option enables FAST_READ access supported by ST M25Pxx.
 
+config MTD_SST25L
+	tristate "Support SST25L (non JEDEC) SPI Flash chips"
+	depends on SPI_MASTER
+	help
+	  This enables access to the non JEDEC SST25L SPI flash chips, used
+	  for program and data storage.
+
+	  Set up your spi devices with the right board-specific platform data,
+	  if you want to specify device partitioning.
+
 config MTD_SLRAM
 	tristate "Uncached system RAM"
 	help
drivers/mtd/devices/Makefile +1 -0
index 0993d5c..ab5c9b9 100644
@@ -16,3 +16,4 @@ obj-$(CONFIG_MTD_LART)		+= lart.o
 obj-$(CONFIG_MTD_BLOCK2MTD)	+= block2mtd.o
 obj-$(CONFIG_MTD_DATAFLASH)	+= mtd_dataflash.o
 obj-$(CONFIG_MTD_M25P80)	+= m25p80.o
+obj-$(CONFIG_MTD_SST25L)	+= sst25l.o
drivers/mtd/devices/sst25l.c new file +503 -0

503 lines not on this page · raw message

#13 lore raw
Quoted 33 lines · Ryan Mallon
Quoted 32 lines · H Hartley Sweeten
Quoted 13 lines · Ryan Mallon

Add support for the non JEDEC SST25L SPI Flash devices.

drivers/mtd/devices/sst25l.c new file +5 -0
+struct flash_info {
+	const char		*name;
+	u16			device_id;
+	unsigned		page_size;
+	unsigned		nr_pages;

drivers/mtd/devices/sst25l.c line 71

Quoted 10 lines
drivers/mtd/devices/sst25l.c new file +10 -0
+	unsigned		erase_size;
+};
+
+#define to_sst25l_flash(x) container_of(x, struct sst25l_flash, mtd)
+
+static struct flash_info __devinitdata sst25l_flash_info[] = {
+	{"sst25lf020a", 0xbf43, 256, 1024, 32 * 1024},
+	{"sst25lf040a",	0xbf44,	256, 2048, 32 * 1024},
+};
+

Ryan,

I finally got a chance to test this on my hardware. Previously I was using a hacked version on the m25p80 driver to access the SST23LF020A on my board.

Quoted 8 lines
Quoted 4 lines

The driver works fine but you are only supporting the block erase, SST25L_CMD_BLOCK_ERASE (opcode 0x52). The SST25L chips also can do a sector erase (opcode 0x20) and a full chip erase (opcode 0x60).

The MTD sub-system doesn't support different types of erases (as far as I am aware). We chose to use block erases since they are faster than sector erases and the trade-off of having 32k aligned partitions didn't seem to bad.

True. The mtd.erasesize is supposed to be the smallest erasable region on the chip. Thus, the sector size should be used.

Quoted 11 lines
Quoted 11 lines

The full chip erase is not that important but might give a slight performance increase on larger devices. Max block erase time is listed as 25ms at 25MHz in the datasheet. The SST25LF020A contains 8 blocks so it's "typical" full chip erase time should be around 200ms. For comparison, the chip erase time is listed as 100ms.

The sector erase is a bigger deal. On the EDB93xx boards one of the SPI flash uses is used to store the MAC address at offset 0x2000 (4K offset). The board can also be configured to boot from offset 0x0000 in the SPI flash.

Quoted 7 lines
Quoted 3 lines

Without the sector erase this partition setup results in all of the partitions being forced read-only.

The updated patch below uses sector erases instead to allow for smaller partition sizes.

Linus, is this still okay by you?

I think you could still support the block erase and chip erase internal to the driver for performance. Maybe something like the following?

static int __sst25l_erase(struct sst25l_flash *flash, u32 offset, u8 cmd)

{
	u8 command[4];
	int err;

	err = sst25l_write_enable(flash, 1);
	if (err)
		return err;

	command[0] = cmd;
	if (cmd != SST25L_CMD_CHIP_ERASE) {
		command[1] = offset >> 16;
		command[2] = offset >> 8;
		command[3] = offset;
	}
	err = spi_write(flash->spi, command,
			cmd == SST25L_CMD_CHIP_ERASE ? 1 : 4);
	if (err)
		return err;

	err = sst25l_wait_till_ready(flash);
	if (err)
		return err;

	return sst25l_write_enable(flash, 0);
}

static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr)

{
	struct sst25l_flash *flash = to_sst25l_flash(mtd);
	u32 block = flash->block_size;
	u32 sector = flash->sector_size;
	u32 addr, end;
	int err;

	/* Sanity checks */
	if (instr->addr + instr->len > flash->mtd.size)
		return -EINVAL;

	if ((u32)instr->len % mtd->erasesize)
		return -EINVAL;

	if ((u32)instr->addr % mtd->erasesize)
		return -EINVAL;

	addr = instr->addr;
	end = addr + instr->len;

	mutex_lock(&flash->lock);

	err = sst25l_wait_till_ready(flash);
	if (err)
		return err;

	if (instr->len == mtd->size) {
		err = __sst25l_erase(flash, addr, SST25L_CMD_CHIP_ERASE);
		if (err) {
			mutex_unlock(&flash->lock);
			instr->state = MTD_ERASE_FAILED;
			dev_err(&flash->spi->dev, "Erase failed\n");
			return err;
		}
	} else {
		while (addr < end) {
			u32 erasesize;

			if (addr % block || addr + block > end) {
				erasesize = sector;
				err = __sst25l_erase(flash, addr,
						SST25L_CMD_SECTOR_ERASE);
			} else {
				erasesize = block;
				err = __sst25l_erase(flash, addr,
						SST25L_CMD_BLOCK_ERASE);
			}
			if (err) {
				mutex_unlock(&flash->lock);
				instr->state = MTD_ERASE_FAILED;
				dev_err(&flash->spi->dev, "Erase failed\n");
				return err;
			}

			addr += erasesize;
		}
	}

	mutex_unlock(&flash->lock);

	instr->state = MTD_ERASE_DONE;
	mtd_erase_callback(instr);
	return 0;
}

You would need to add sector_size and block_size to the ss25l_flash and flash_info structs and modify flash_info as appropriate. Then copy those over during the probe and use sector_size for mtd.erasesize.

That way we get the 4K erase size but still get the performance increase for block and chip erase.

What do you think?

Regards,
Hartley

#14 lore raw
Quoted 9 lines · H Hartley Sweeten
Quoted 2 lines · Ryan Mallon
Quoted 1 line · H Hartley Sweeten

On Sunday, June 21, 2009 8:59 PM, Ryan Mallon wrote:

I think you could still support the block erase and chip erase internal to the driver for performance. Maybe something like the following?

static int sst25l_erase(struct mtd_info *mtd, struct erase_info *instr) {

...

Quoted 33 lines
	if (instr->len == mtd->size) {
		err = __sst25l_erase(flash, addr, SST25L_CMD_CHIP_ERASE);
		if (err) {
			mutex_unlock(&flash->lock);
			instr->state = MTD_ERASE_FAILED;
			dev_err(&flash->spi->dev, "Erase failed\n");
			return err;
		}
	} else {
		while (addr < end) {
			u32 erasesize;

			if (addr % block || addr + block > end) {
				erasesize = sector;
				err = __sst25l_erase(flash, addr,
						SST25L_CMD_SECTOR_ERASE);
			} else {
				erasesize = block;
				err = __sst25l_erase(flash, addr,
						SST25L_CMD_BLOCK_ERASE);
			}
			if (err) {
				mutex_unlock(&flash->lock);
				instr->state = MTD_ERASE_FAILED;
				dev_err(&flash->spi->dev, "Erase failed\n");
				return err;
			}

			addr += erasesize;
		}
	}
Quoted 6 lines

You would need to add sector_size and block_size to the ss25l_flash and flash_info structs and modify flash_info as appropriate. Then copy those over during the probe and use sector_size for mtd.erasesize.

That way we get the 4K erase size but still get the performance increase for block and chip erase.

We discussed this, but it adds additional complexity to what is otherwise quite a simple driver. The 4K sector erase will work in all cases, and the speed difference shouldn't be a massive problem. I would venture a guess that most people using the chips have reasonably non-volatile storage on them, and don't get erased very often. We use ours for the Bootloader and boot environment.

~Ryan

Signature · 8 lines
Bluewater Systems Ltd - ARM Technology Solution Centre

       Ryan Mallon                              Unit 5, Amuri Park
       Phone: +64 3 3779127                     404 Barbadoes St
       Fax:   +64 3 3779135                     PO Box 13 889
       Email: [email protected]             Christchurch, 8013
       Web:   http://www.bluewatersys.com       New Zealand
       Freecall Australia  1800 148 751         USA 1800 261 2934