| Message ID | 20210401193133.18129-27-p.yadav@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: spi-nor-core: add xSPI Octal DTR support | expand |
On 4/2/2021 4:31 AM, Pratyush Yadav wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or > bottom, depending on the device configuration, while U-Boot supports > uniform sector layout only. > > The spansion_erase_non_uniform() erases overlaid 4KB sectors, > non-overlaid portion of normal sector, and remaining normal sectors, by > selecting correct erase command and size based on the address to erase > and size of overlaid portion in parameters. Since different Spansion > flashes can use different opcode for erasing the 4K sectors, the opcode > must pe passed in as a parameter based on the flash being used. > Typo: "must be passed" > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > [p.yadav@ti.com: Refactor the function to be compatible with nor->erase, > make 4K opcode customizable, call spi_nor_setup_op() before executing > the op.] > --- > Thank you for refactoring. I would like to take this for the next revision of my series. I think I need to remove spi_nor_setup_op() for my series. > Unfortunately there is a race between this and Takahiro's series [0]. > Some of his patches depend on this series and I am taking this patch > from his series. Not sure how to resolve this problem but I figure it is > worth pointing out. > > BTW, I changed the #ifdef to CONFIG_SPI_FLASH_S28HS512T instead of > CONFIG_SPI_FLASH_SPANSION otherwise there would be build warnings when > the S28 config is not enabled. Once Takahiro's series lands it should be > changed back to CONFIG_SPI_FLASH_SPANSION. > > [0] https://patchwork.ozlabs.org/project/uboot/list/?series=230084&archive=both&state=* > > drivers/mtd/spi/spi-nor-core.c | 61 ++++++++++++++++++++++++++++++++++ > 1 file changed, 61 insertions(+) > > -- > 2.30.0 > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > index 8934d65ce2..7637539087 100644 > --- a/drivers/mtd/spi/spi-nor-core.c > +++ b/drivers/mtd/spi/spi-nor-core.c > @@ -887,6 +887,67 @@ erase_err: > return ret; > } > > +#ifdef CONFIG_SPI_FLASH_S28HS512T > +/** > + * spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress > + * chips > + * @nor: pointer to a 'struct spi_nor' > + * @addr: address of the sector to erase > + * @opcode_4k: opcode for 4K sector erase > + * @ovlsz_top: size of overlaid portion at the top address > + * @ovlsz_btm: size of overlaid portion at the bottom address > + * > + * Erase an address range on the nor chip that can contain 4KB sectors overlaid > + * on top and/or bottom. The appropriate erase opcode and size are chosen by > + * address to erase and size of overlaid portion. > + * > + * Return: number of bytes erased on success, -errno otherwise. > + */ > +static int spansion_erase_non_uniform(struct spi_nor *nor, u32 addr, > + u8 opcode_4k, u32 ovlsz_top, > + u32 ovlsz_btm) > +{ > + struct spi_mem_op op = > + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 0), > + SPI_MEM_OP_ADDR(nor->addr_width, addr, 0), > + SPI_MEM_OP_NO_DUMMY, > + SPI_MEM_OP_NO_DATA); > + struct mtd_info *mtd = &nor->mtd; > + u32 erasesize; > + int ret; > + > + /* 4KB sectors */ > + if (op.addr.val < ovlsz_btm || > + op.addr.val >= mtd->size - ovlsz_top) { > + op.cmd.opcode = opcode_4k; > + erasesize = SZ_4K; > + > + /* Non-overlaid portion in the normal sector at the bottom */ > + } else if (op.addr.val == ovlsz_btm) { > + op.cmd.opcode = nor->erase_opcode; > + erasesize = mtd->erasesize - ovlsz_btm; > + > + /* Non-overlaid portion in the normal sector at the top */ > + } else if (op.addr.val == mtd->size - mtd->erasesize) { > + op.cmd.opcode = nor->erase_opcode; > + erasesize = mtd->erasesize - ovlsz_top; > + > + /* Normal sectors */ > + } else { > + op.cmd.opcode = nor->erase_opcode; > + erasesize = mtd->erasesize; > + } > + > + spi_nor_setup_op(nor, &op, nor->write_proto); > + > + ret = spi_mem_exec_op(nor->spi, &op); > + if (ret) > + return ret; > + > + return erasesize; > +} > +#endif > + > #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) > /* Write status register and ensure bits in mask match written values */ > static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) > Best Regards, Takahiro
On 06/04/21 10:48AM, Takahiro Kuwano wrote: > On 4/2/2021 4:31 AM, Pratyush Yadav wrote: > > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > > > Some of Spansion/Cypress chips have overlaid 4KB sectors at top and/or > > bottom, depending on the device configuration, while U-Boot supports > > uniform sector layout only. > > > > The spansion_erase_non_uniform() erases overlaid 4KB sectors, > > non-overlaid portion of normal sector, and remaining normal sectors, by > > selecting correct erase command and size based on the address to erase > > and size of overlaid portion in parameters. Since different Spansion > > flashes can use different opcode for erasing the 4K sectors, the opcode > > must pe passed in as a parameter based on the flash being used. > > > Typo: "must be passed" Ok. > > > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com> > > [p.yadav@ti.com: Refactor the function to be compatible with nor->erase, > > make 4K opcode customizable, call spi_nor_setup_op() before executing > > the op.] > > --- > > > Thank you for refactoring. I would like to take this for the next revision > of my series. I think I need to remove spi_nor_setup_op() for my series. Yes. It is unfortunate that our series have such interdependency between them. Jagan, which one do you think is more mature and more likely to go in first? That can help clear up the dependencies a bit. I also don't mind if you take in the "infrastructure" patches first (patches 9-14) to clear up the dependencies for Takahiro's series and then take the DTR patches later. Of course, if you feel like this series is good enough to go in then take them all at the same time ;-) > > > Unfortunately there is a race between this and Takahiro's series [0]. > > Some of his patches depend on this series and I am taking this patch > > from his series. Not sure how to resolve this problem but I figure it is > > worth pointing out. > > > > BTW, I changed the #ifdef to CONFIG_SPI_FLASH_S28HS512T instead of > > CONFIG_SPI_FLASH_SPANSION otherwise there would be build warnings when > > the S28 config is not enabled. Once Takahiro's series lands it should be > > changed back to CONFIG_SPI_FLASH_SPANSION. > > > > [0] https://patchwork.ozlabs.org/project/uboot/list/?series=230084&archive=both&state=* > > > > drivers/mtd/spi/spi-nor-core.c | 61 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > -- > > 2.30.0 > > > > diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c > > index 8934d65ce2..7637539087 100644 > > --- a/drivers/mtd/spi/spi-nor-core.c > > +++ b/drivers/mtd/spi/spi-nor-core.c > > @@ -887,6 +887,67 @@ erase_err: > > return ret; > > } > > > > +#ifdef CONFIG_SPI_FLASH_S28HS512T > > +/** > > + * spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress > > + * chips > > + * @nor: pointer to a 'struct spi_nor' > > + * @addr: address of the sector to erase > > + * @opcode_4k: opcode for 4K sector erase > > + * @ovlsz_top: size of overlaid portion at the top address > > + * @ovlsz_btm: size of overlaid portion at the bottom address > > + * > > + * Erase an address range on the nor chip that can contain 4KB sectors overlaid > > + * on top and/or bottom. The appropriate erase opcode and size are chosen by > > + * address to erase and size of overlaid portion. > > + * > > + * Return: number of bytes erased on success, -errno otherwise. > > + */ > > +static int spansion_erase_non_uniform(struct spi_nor *nor, u32 addr, > > + u8 opcode_4k, u32 ovlsz_top, > > + u32 ovlsz_btm) > > +{ > > + struct spi_mem_op op = > > + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 0), > > + SPI_MEM_OP_ADDR(nor->addr_width, addr, 0), > > + SPI_MEM_OP_NO_DUMMY, > > + SPI_MEM_OP_NO_DATA); > > + struct mtd_info *mtd = &nor->mtd; > > + u32 erasesize; > > + int ret; > > + > > + /* 4KB sectors */ > > + if (op.addr.val < ovlsz_btm || > > + op.addr.val >= mtd->size - ovlsz_top) { > > + op.cmd.opcode = opcode_4k; > > + erasesize = SZ_4K; > > + > > + /* Non-overlaid portion in the normal sector at the bottom */ > > + } else if (op.addr.val == ovlsz_btm) { > > + op.cmd.opcode = nor->erase_opcode; > > + erasesize = mtd->erasesize - ovlsz_btm; > > + > > + /* Non-overlaid portion in the normal sector at the top */ > > + } else if (op.addr.val == mtd->size - mtd->erasesize) { > > + op.cmd.opcode = nor->erase_opcode; > > + erasesize = mtd->erasesize - ovlsz_top; > > + > > + /* Normal sectors */ > > + } else { > > + op.cmd.opcode = nor->erase_opcode; > > + erasesize = mtd->erasesize; > > + } > > + > > + spi_nor_setup_op(nor, &op, nor->write_proto); > > + > > + ret = spi_mem_exec_op(nor->spi, &op); > > + if (ret) > > + return ret; > > + > > + return erasesize; > > +} > > +#endif > > + > > #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) > > /* Write status register and ensure bits in mask match written values */ > > static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask) > > > > Best Regards, > Takahiro
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c index 8934d65ce2..7637539087 100644 --- a/drivers/mtd/spi/spi-nor-core.c +++ b/drivers/mtd/spi/spi-nor-core.c @@ -887,6 +887,67 @@ erase_err: return ret; } +#ifdef CONFIG_SPI_FLASH_S28HS512T +/** + * spansion_erase_non_uniform() - erase non-uniform sectors for Spansion/Cypress + * chips + * @nor: pointer to a 'struct spi_nor' + * @addr: address of the sector to erase + * @opcode_4k: opcode for 4K sector erase + * @ovlsz_top: size of overlaid portion at the top address + * @ovlsz_btm: size of overlaid portion at the bottom address + * + * Erase an address range on the nor chip that can contain 4KB sectors overlaid + * on top and/or bottom. The appropriate erase opcode and size are chosen by + * address to erase and size of overlaid portion. + * + * Return: number of bytes erased on success, -errno otherwise. + */ +static int spansion_erase_non_uniform(struct spi_nor *nor, u32 addr, + u8 opcode_4k, u32 ovlsz_top, + u32 ovlsz_btm) +{ + struct spi_mem_op op = + SPI_MEM_OP(SPI_MEM_OP_CMD(nor->erase_opcode, 0), + SPI_MEM_OP_ADDR(nor->addr_width, addr, 0), + SPI_MEM_OP_NO_DUMMY, + SPI_MEM_OP_NO_DATA); + struct mtd_info *mtd = &nor->mtd; + u32 erasesize; + int ret; + + /* 4KB sectors */ + if (op.addr.val < ovlsz_btm || + op.addr.val >= mtd->size - ovlsz_top) { + op.cmd.opcode = opcode_4k; + erasesize = SZ_4K; + + /* Non-overlaid portion in the normal sector at the bottom */ + } else if (op.addr.val == ovlsz_btm) { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize - ovlsz_btm; + + /* Non-overlaid portion in the normal sector at the top */ + } else if (op.addr.val == mtd->size - mtd->erasesize) { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize - ovlsz_top; + + /* Normal sectors */ + } else { + op.cmd.opcode = nor->erase_opcode; + erasesize = mtd->erasesize; + } + + spi_nor_setup_op(nor, &op, nor->write_proto); + + ret = spi_mem_exec_op(nor->spi, &op); + if (ret) + return ret; + + return erasesize; +} +#endif + #if defined(CONFIG_SPI_FLASH_STMICRO) || defined(CONFIG_SPI_FLASH_SST) /* Write status register and ensure bits in mask match written values */ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)