Re: [U-Boot] [PATCH 1/1] MTD: SPI: revert removing SST26* flash IC protection ops
>On Wed, Sep 4, 2019 at 11:37 PM Eugeniy Paltsev > wrote: >> >> Commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) >> performs switch from previous 'spi_flash' infrastructure without >> proper testing/investigations which results in regressions. >> >> Fix regression related to SST26 flash IC series which is lacking >> protection ops implementation which were introduced previously by >> Commit 3d4fed87a5fa (mtd: sf: Add support of sst26wf* flash ICs >> protection ops) >> >> Signed-off-by: Eugeniy Paltsev >> --- >> Tom, could you please pick this patch to 2019.10? >> [snip] >> + { INFO("sst26wf032", 0xbf2622, 0, 64 * 1024, 64, SECT_4K | >> SPI_NOR_HAS_SST26LOCK) }, >> + { INFO("sst26wf064", 0xbf2643, 0, 64 * 1024, 128, SECT_4K | >> SPI_NOR_HAS_SST26LOCK) }, > >Thought the commit message says it is revert, the patch contents seems >adding new changes. Revert of removing functionality is adding functionality, isn't it? ;) > So, better add them by saying missing >functionalities. If possible please break the patches into multiple >patches. Ok, I'll split this patch for adding SST26 protection ops part and applying this ops to corresponding flash ICs. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 1/1] MTD: SPI: revert removing SST26* flash IC protection ops
On Sat, Sep 07, 2019 at 09:10:39AM +0530, Jagan Teki wrote: > On Wed, Sep 4, 2019 at 11:37 PM Eugeniy Paltsev > wrote: > > > > Commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) > > performs switch from previous 'spi_flash' infrastructure without > > proper testing/investigations which results in regressions. > > > > Fix regression related to SST26 flash IC series which is lacking > > protection ops implementation which were introduced previously by > > Commit 3d4fed87a5fa (mtd: sf: Add support of sst26wf* flash ICs > > protection ops) > > > > Signed-off-by: Eugeniy Paltsev > > --- > > Tom, could you please pick this patch to 2019.10? > > > > drivers/mtd/spi/sf_internal.h | 1 + > > drivers/mtd/spi/spi-nor-core.c | 181 + > > drivers/mtd/spi/spi-nor-ids.c | 8 +- > > include/linux/mtd/spi-nor.h| 4 + > > 4 files changed, 190 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > > index a6bf734830a..e6da768bf36 100644 > > --- a/drivers/mtd/spi/sf_internal.h > > +++ b/drivers/mtd/spi/sf_internal.h > > @@ -65,6 +65,7 @@ struct flash_info { > > #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase > > */ > > #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */ > > #define USE_CLSR BIT(14) /* use CLSR command */ > > +#define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via > > BPR */ > > }; > > > > extern const struct flash_info spi_nor_ids[]; > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > > index 1acff745d1a..990e39d7c2f 100644 > > --- a/drivers/mtd/spi/spi-nor-core.c > > +++ b/drivers/mtd/spi/spi-nor-core.c > > @@ -945,6 +945,177 @@ read_err: > > } > > > > #ifdef CONFIG_SPI_FLASH_SST > > +/* > > + * sst26 flash series has its own block protection implementation: > > + * 4x - 8 KByte blocks - read & write protection bits - upper addresses > > + * 1x - 32 KByte blocks - write protection bits > > + * rest - 64 KByte blocks - write protection bits > > + * 1x - 32 KByte blocks - write protection bits > > + * 4x - 8 KByte blocks - read & write protection bits - lower addresses > > + * > > + * We'll support only per 64k lock/unlock so lower and upper 64 KByte > > region > > + * will be treated as single block. > > + */ > > +#define SST26_BPR_8K_NUM 4 > > +#define SST26_MAX_BPR_REG_LEN (18 + 1) > > +#define SST26_BOUND_REG_SIZE ((32 + SST26_BPR_8K_NUM * 8) * > > SZ_1K) > > + > > +enum lock_ctl { > > + SST26_CTL_LOCK, > > + SST26_CTL_UNLOCK, > > + SST26_CTL_CHECK > > +}; > > + > > +static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum > > lock_ctl ctl) > > +{ > > + switch (ctl) { > > + case SST26_CTL_LOCK: > > + cmd[bpr_size - (bit / 8) - 1] |= BIT(bit % 8); > > + break; > > + case SST26_CTL_UNLOCK: > > + cmd[bpr_size - (bit / 8) - 1] &= ~BIT(bit % 8); > > + break; > > + case SST26_CTL_CHECK: > > + return !!(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); > > + } > > + > > + return false; > > +} > > + > > +/* > > + * Lock, unlock or check lock status of the flash region of the flash > > (depending > > + * on the lock_ctl value) > > + */ > > +static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, > > enum lock_ctl ctl) > > +{ > > + struct mtd_info *mtd = >mtd; > > + u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size; > > + bool lower_64k = false, upper_64k = false; > > + u8 bpr_buff[SST26_MAX_BPR_REG_LEN] = {}; > > + int ret; > > + > > + /* Check length and offset for 64k alignment */ > > + if ((ofs & (SZ_64K - 1)) || (len & (SZ_64K - 1))) { > > + dev_err(nor->dev, "length or offset is not 64KiB > > allighned\n"); > > + return -EINVAL; > > + } > > + > > + if (ofs + len > mtd->size) { > > + dev_err(nor->dev, "range is more than device size: %#llx + > > %#llx > %#llx\n", > > + ofs, len, mtd->size); > > + return -EINVAL; > > + } > > + > > + /* SST26 family has only 16 Mbit, 32 Mbit and 64 Mbit IC */ > > + if (mtd->size != SZ_2M && > > + mtd->size != SZ_4M && > > + mtd->size != SZ_8M) > > + return -EINVAL; > > + > > + bpr_size = 2 + (mtd->size / SZ_64K / 8); > > + > > + ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, bpr_buff, bpr_size); > > + if (ret < 0) { > > + dev_err(nor->dev, "fail to read block-protection > > register\n"); > > + return ret; > > + } > > + > > + rptr_64k = min_t(u32, ofs + len, mtd->size - SST26_BOUND_REG_SIZE); > > + lptr_64k = max_t(u32, ofs, SST26_BOUND_REG_SIZE); > > + > > + upper_64k = ((ofs + len) > (mtd->size -
Re: [U-Boot] [PATCH 1/1] MTD: SPI: revert removing SST26* flash IC protection ops
On Wed, Sep 4, 2019 at 11:37 PM Eugeniy Paltsev wrote: > > Commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) > performs switch from previous 'spi_flash' infrastructure without > proper testing/investigations which results in regressions. > > Fix regression related to SST26 flash IC series which is lacking > protection ops implementation which were introduced previously by > Commit 3d4fed87a5fa (mtd: sf: Add support of sst26wf* flash ICs > protection ops) > > Signed-off-by: Eugeniy Paltsev > --- > Tom, could you please pick this patch to 2019.10? > > drivers/mtd/spi/sf_internal.h | 1 + > drivers/mtd/spi/spi-nor-core.c | 181 + > drivers/mtd/spi/spi-nor-ids.c | 8 +- > include/linux/mtd/spi-nor.h| 4 + > 4 files changed, 190 insertions(+), 4 deletions(-) > > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h > index a6bf734830a..e6da768bf36 100644 > --- a/drivers/mtd/spi/sf_internal.h > +++ b/drivers/mtd/spi/sf_internal.h > @@ -65,6 +65,7 @@ struct flash_info { > #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */ > #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */ > #define USE_CLSR BIT(14) /* use CLSR command */ > +#define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via BPR > */ > }; > > extern const struct flash_info spi_nor_ids[]; > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 1acff745d1a..990e39d7c2f 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -945,6 +945,177 @@ read_err: > } > > #ifdef CONFIG_SPI_FLASH_SST > +/* > + * sst26 flash series has its own block protection implementation: > + * 4x - 8 KByte blocks - read & write protection bits - upper addresses > + * 1x - 32 KByte blocks - write protection bits > + * rest - 64 KByte blocks - write protection bits > + * 1x - 32 KByte blocks - write protection bits > + * 4x - 8 KByte blocks - read & write protection bits - lower addresses > + * > + * We'll support only per 64k lock/unlock so lower and upper 64 KByte region > + * will be treated as single block. > + */ > +#define SST26_BPR_8K_NUM 4 > +#define SST26_MAX_BPR_REG_LEN (18 + 1) > +#define SST26_BOUND_REG_SIZE ((32 + SST26_BPR_8K_NUM * 8) * SZ_1K) > + > +enum lock_ctl { > + SST26_CTL_LOCK, > + SST26_CTL_UNLOCK, > + SST26_CTL_CHECK > +}; > + > +static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl > ctl) > +{ > + switch (ctl) { > + case SST26_CTL_LOCK: > + cmd[bpr_size - (bit / 8) - 1] |= BIT(bit % 8); > + break; > + case SST26_CTL_UNLOCK: > + cmd[bpr_size - (bit / 8) - 1] &= ~BIT(bit % 8); > + break; > + case SST26_CTL_CHECK: > + return !!(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); > + } > + > + return false; > +} > + > +/* > + * Lock, unlock or check lock status of the flash region of the flash > (depending > + * on the lock_ctl value) > + */ > +static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, > enum lock_ctl ctl) > +{ > + struct mtd_info *mtd = >mtd; > + u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size; > + bool lower_64k = false, upper_64k = false; > + u8 bpr_buff[SST26_MAX_BPR_REG_LEN] = {}; > + int ret; > + > + /* Check length and offset for 64k alignment */ > + if ((ofs & (SZ_64K - 1)) || (len & (SZ_64K - 1))) { > + dev_err(nor->dev, "length or offset is not 64KiB > allighned\n"); > + return -EINVAL; > + } > + > + if (ofs + len > mtd->size) { > + dev_err(nor->dev, "range is more than device size: %#llx + > %#llx > %#llx\n", > + ofs, len, mtd->size); > + return -EINVAL; > + } > + > + /* SST26 family has only 16 Mbit, 32 Mbit and 64 Mbit IC */ > + if (mtd->size != SZ_2M && > + mtd->size != SZ_4M && > + mtd->size != SZ_8M) > + return -EINVAL; > + > + bpr_size = 2 + (mtd->size / SZ_64K / 8); > + > + ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, bpr_buff, bpr_size); > + if (ret < 0) { > + dev_err(nor->dev, "fail to read block-protection register\n"); > + return ret; > + } > + > + rptr_64k = min_t(u32, ofs + len, mtd->size - SST26_BOUND_REG_SIZE); > + lptr_64k = max_t(u32, ofs, SST26_BOUND_REG_SIZE); > + > + upper_64k = ((ofs + len) > (mtd->size - SST26_BOUND_REG_SIZE)); > + lower_64k = (ofs < SST26_BOUND_REG_SIZE); > + > + /* Lower bits in block-protection register are about 64k region */ > + bpr_ptr = lptr_64k / SZ_64K - 1; > + > + /* Process 64K blocks region */ > + while (lptr_64k < rptr_64k) { > + if (sst26_process_bpr(bpr_size,
[U-Boot] [PATCH 1/1] MTD: SPI: revert removing SST26* flash IC protection ops
Commit c4e8862308d4 (mtd: spi: Switch to new SPI NOR framework) performs switch from previous 'spi_flash' infrastructure without proper testing/investigations which results in regressions. Fix regression related to SST26 flash IC series which is lacking protection ops implementation which were introduced previously by Commit 3d4fed87a5fa (mtd: sf: Add support of sst26wf* flash ICs protection ops) Signed-off-by: Eugeniy Paltsev --- Tom, could you please pick this patch to 2019.10? drivers/mtd/spi/sf_internal.h | 1 + drivers/mtd/spi/spi-nor-core.c | 181 + drivers/mtd/spi/spi-nor-ids.c | 8 +- include/linux/mtd/spi-nor.h| 4 + 4 files changed, 190 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h index a6bf734830a..e6da768bf36 100644 --- a/drivers/mtd/spi/sf_internal.h +++ b/drivers/mtd/spi/sf_internal.h @@ -65,6 +65,7 @@ struct flash_info { #define NO_CHIP_ERASE BIT(12) /* Chip does not support chip erase */ #define SPI_NOR_SKIP_SFDP BIT(13) /* Skip parsing of SFDP tables */ #define USE_CLSR BIT(14) /* use CLSR command */ +#define SPI_NOR_HAS_SST26LOCK BIT(15) /* Flash supports lock/unlock via BPR */ }; extern const struct flash_info spi_nor_ids[]; diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 1acff745d1a..990e39d7c2f 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -945,6 +945,177 @@ read_err: } #ifdef CONFIG_SPI_FLASH_SST +/* + * sst26 flash series has its own block protection implementation: + * 4x - 8 KByte blocks - read & write protection bits - upper addresses + * 1x - 32 KByte blocks - write protection bits + * rest - 64 KByte blocks - write protection bits + * 1x - 32 KByte blocks - write protection bits + * 4x - 8 KByte blocks - read & write protection bits - lower addresses + * + * We'll support only per 64k lock/unlock so lower and upper 64 KByte region + * will be treated as single block. + */ +#define SST26_BPR_8K_NUM 4 +#define SST26_MAX_BPR_REG_LEN (18 + 1) +#define SST26_BOUND_REG_SIZE ((32 + SST26_BPR_8K_NUM * 8) * SZ_1K) + +enum lock_ctl { + SST26_CTL_LOCK, + SST26_CTL_UNLOCK, + SST26_CTL_CHECK +}; + +static bool sst26_process_bpr(u32 bpr_size, u8 *cmd, u32 bit, enum lock_ctl ctl) +{ + switch (ctl) { + case SST26_CTL_LOCK: + cmd[bpr_size - (bit / 8) - 1] |= BIT(bit % 8); + break; + case SST26_CTL_UNLOCK: + cmd[bpr_size - (bit / 8) - 1] &= ~BIT(bit % 8); + break; + case SST26_CTL_CHECK: + return !!(cmd[bpr_size - (bit / 8) - 1] & BIT(bit % 8)); + } + + return false; +} + +/* + * Lock, unlock or check lock status of the flash region of the flash (depending + * on the lock_ctl value) + */ +static int sst26_lock_ctl(struct spi_nor *nor, loff_t ofs, uint64_t len, enum lock_ctl ctl) +{ + struct mtd_info *mtd = >mtd; + u32 i, bpr_ptr, rptr_64k, lptr_64k, bpr_size; + bool lower_64k = false, upper_64k = false; + u8 bpr_buff[SST26_MAX_BPR_REG_LEN] = {}; + int ret; + + /* Check length and offset for 64k alignment */ + if ((ofs & (SZ_64K - 1)) || (len & (SZ_64K - 1))) { + dev_err(nor->dev, "length or offset is not 64KiB allighned\n"); + return -EINVAL; + } + + if (ofs + len > mtd->size) { + dev_err(nor->dev, "range is more than device size: %#llx + %#llx > %#llx\n", + ofs, len, mtd->size); + return -EINVAL; + } + + /* SST26 family has only 16 Mbit, 32 Mbit and 64 Mbit IC */ + if (mtd->size != SZ_2M && + mtd->size != SZ_4M && + mtd->size != SZ_8M) + return -EINVAL; + + bpr_size = 2 + (mtd->size / SZ_64K / 8); + + ret = nor->read_reg(nor, SPINOR_OP_READ_BPR, bpr_buff, bpr_size); + if (ret < 0) { + dev_err(nor->dev, "fail to read block-protection register\n"); + return ret; + } + + rptr_64k = min_t(u32, ofs + len, mtd->size - SST26_BOUND_REG_SIZE); + lptr_64k = max_t(u32, ofs, SST26_BOUND_REG_SIZE); + + upper_64k = ((ofs + len) > (mtd->size - SST26_BOUND_REG_SIZE)); + lower_64k = (ofs < SST26_BOUND_REG_SIZE); + + /* Lower bits in block-protection register are about 64k region */ + bpr_ptr = lptr_64k / SZ_64K - 1; + + /* Process 64K blocks region */ + while (lptr_64k < rptr_64k) { + if (sst26_process_bpr(bpr_size, bpr_buff, bpr_ptr, ctl)) + return EACCES; + + bpr_ptr++; + lptr_64k += SZ_64K; + } + + /* 32K and 8K region bits in BPR are after 64k region bits */ + bpr_ptr = (mtd->size - 2 * SST26_BOUND_REG_SIZE) / SZ_64K; + + /* Process lower 32K block region