| Message ID | 1446168261-4102-1-git-send-email-computersforpeace@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Oct 29, 2015 at 06:24:20PM -0700, Brian Norris wrote: > Some spi-nor drivers perform sector erase by duplicating their > write_reg() command. Let's not require that the driver fill this out, > and provide a default instead. > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 30 ++++++++++++++++++++++++++---- > include/linux/mtd/spi-nor.h | 3 ++- > 2 files changed, 28 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 49883905a434..58a435372be7 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -313,6 +313,29 @@ static void spi_nor_unlock_and_unprep(struct spi_nor *nor, enum spi_nor_ops ops) > } > > /* > + * Initiate the erasure of a single sector > + */ > +static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) > +{ > + u8 buf[nor->addr_width]; Hmm, I see some warnings about variable length arrays here. Probably better to just set a MAX macro with the value of 4, since the extra byte really doesn't matter that much. I'll send v2. Brian
On Friday, October 30, 2015 at 10:05:33 PM, Brian Norris wrote: > On Thu, Oct 29, 2015 at 06:24:20PM -0700, Brian Norris wrote: > > Some spi-nor drivers perform sector erase by duplicating their > > write_reg() command. Let's not require that the driver fill this out, > > and provide a default instead. > > > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > > --- > > > > drivers/mtd/spi-nor/spi-nor.c | 30 ++++++++++++++++++++++++++---- > > include/linux/mtd/spi-nor.h | 3 ++- > > 2 files changed, 28 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c > > b/drivers/mtd/spi-nor/spi-nor.c index 49883905a434..58a435372be7 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -313,6 +313,29 @@ static void spi_nor_unlock_and_unprep(struct spi_nor > > *nor, enum spi_nor_ops ops) > > > > } > > > > /* > > > > + * Initiate the erasure of a single sector > > + */ > > +static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) > > +{ > > + u8 buf[nor->addr_width]; > > Hmm, I see some warnings about variable length arrays here. Probably > better to just set a MAX macro with the value of 4, since the extra byte > really doesn't matter that much. Hm yeah, this dynamic allocation is not great. You might want to put some sort of assertion in place, to check whether addr_width isn't > MAX . > I'll send v2. > > Brian Best regards, Marek Vasut
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 49883905a434..58a435372be7 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -313,6 +313,29 @@ static void spi_nor_unlock_and_unprep(struct spi_nor *nor, enum spi_nor_ops ops) } /* + * Initiate the erasure of a single sector + */ +static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr) +{ + u8 buf[nor->addr_width]; + int i; + + if (nor->erase) + return nor->erase(nor, addr); + + /* + * Default implementation, if driver doesn't have a specialized HW + * control + */ + for (i = nor->addr_width - 1; i >= 0; i--) { + buf[i] = addr & 0xff; + addr >>= 8; + } + + return nor->write_reg(nor, nor->erase_opcode, buf, nor->addr_width); +} + +/* * Erase an address range on the nor chip. The address range may extend * one or more erase sectors. Return an error is there is a problem erasing. */ @@ -371,10 +394,9 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr) while (len) { write_enable(nor); - if (nor->erase(nor, addr)) { - ret = -EIO; + ret = spi_nor_erase_sector(nor, addr); + if (ret) goto erase_err; - } addr += mtd->erasesize; len -= mtd->erasesize; @@ -1138,7 +1160,7 @@ static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info) static int spi_nor_check(struct spi_nor *nor) { if (!nor->dev || !nor->read || !nor->write || - !nor->read_reg || !nor->write_reg || !nor->erase) { + !nor->read_reg || !nor->write_reg) { pr_err("spi-nor: please fill all the necessary fields!\n"); return -EINVAL; } diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h index c8723b62c4cd..69898b675ade 100644 --- a/include/linux/mtd/spi-nor.h +++ b/include/linux/mtd/spi-nor.h @@ -144,7 +144,8 @@ struct mtd_info; * @read: [DRIVER-SPECIFIC] read data from the SPI NOR * @write: [DRIVER-SPECIFIC] write data to the SPI NOR * @erase: [DRIVER-SPECIFIC] erase a sector of the SPI NOR - * at the offset @offs + * at the offset @offs; if not provided by the driver, + * spi-nor will send the erase opcode via write_reg() * @flash_lock: [FLASH-SPECIFIC] lock a region of the SPI NOR * @flash_unlock: [FLASH-SPECIFIC] unlock a region of the SPI NOR * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
Some spi-nor drivers perform sector erase by duplicating their write_reg() command. Let's not require that the driver fill this out, and provide a default instead. Signed-off-by: Brian Norris <computersforpeace@gmail.com> --- drivers/mtd/spi-nor/spi-nor.c | 30 ++++++++++++++++++++++++++---- include/linux/mtd/spi-nor.h | 3 ++- 2 files changed, 28 insertions(+), 5 deletions(-)