| Message ID | 1586939433-20861-1-git-send-email-yangyicong@hisilicon.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: Add support for s25fs128s | expand |
On Wednesday, April 15, 2020 11:30:33 AM EEST Yicong Yang wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > > Add support for Cypress s25fs128s flash. Previously the flash is > decoded as s25fl129p1 by mistake. > > Add it in the flash info list to correctly decode. No functional > changes. Further capability of the flash will be parsed from bfpt. > > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > --- > Change since v1: > - rebase the patch on v5.7 rc1 > > drivers/mtd/spi-nor/spansion.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c > index 6756202..01640c1 100644 > --- a/drivers/mtd/spi-nor/spansion.c > +++ b/drivers/mtd/spi-nor/spansion.c > @@ -29,6 +29,8 @@ static const struct flash_info spansion_parts[] = { > { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | > SPI_NOR_HAS_LOCK | USE_CLSR) }, > + { "s25fs128s", INFO6(0x012018, 0x4d0181, 64 * 1024, 256, > + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, The flash should be named "s25fs128s1" - this would be the flavor with uniform 64kB sectors. One should add a "s25fs128s0" entry for the flavor with uniform 256kB sectors. We usually don't add new flashes if they are not tested. Please specify in the commit message with which controller you tested the flash. The minimal test is a read, erase, read back, write and read back. This sequence should verify if read, erase and pp work fine. Cheers, ta
On 20.04.2020 13:18, Tudor.Ambarus@microchip.com wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the >> content is safe Hm? >> Add support for Cypress s25fs128s flash. Previously the flash is >> decoded as s25fl129p1 by mistake. >> >> Add it in the flash info list to correctly decode. No functional >> changes. Further capability of the flash will be parsed from bfpt. >> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> --- >> Change since v1: >> - rebase the patch on v5.7 rc1 >> >> drivers/mtd/spi-nor/spansion.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c >> index 6756202..01640c1 100644 >> --- a/drivers/mtd/spi-nor/spansion.c >> +++ b/drivers/mtd/spi-nor/spansion.c >> @@ -29,6 +29,8 @@ static const struct flash_info spansion_parts[] = { >> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >> SPI_NOR_HAS_LOCK | USE_CLSR) }, >> + { "s25fs128s", INFO6(0x012018, 0x4d0181, 64 * 1024, 256, >> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > > The flash should be named "s25fs128s1" - this would be the flavor with uniform > 64kB sectors. One should add a "s25fs128s0" entry for the flavor with uniform > 256kB sectors. > > We usually don't add new flashes if they are not tested. Please specify in the > commit message with which controller you tested the flash. The minimal test is > a read, erase, read back, write and read back. This sequence should verify if > read, erase and pp work fine. This chip (as part of the S25FS-S family) most probably requires a post-BFPT fixup to make the writes work. I was going to post the patch adding this fixup for S25FS512S chip. > Cheers, > ta MBR, Sergei
On Monday, April 20, 2020 2:05:29 PM EEST Sergei Shtylyov wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the > content is safe > On 20.04.2020 13:18, Tudor.Ambarus@microchip.com wrote: > >> EXTERNAL EMAIL: Do not click links or open attachments unless you know > >> the > >> content is safe > > Hm? > Please ignore. The mchp email server adds this message in the body of the messages received from people outside of the company. I see this so often that now I ignore it. > >> Add support for Cypress s25fs128s flash. Previously the flash is > >> decoded as s25fl129p1 by mistake. > >> > >> Add it in the flash info list to correctly decode. No functional > >> changes. Further capability of the flash will be parsed from bfpt. > >> > >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > >> --- > >> Change since v1: > >> - rebase the patch on v5.7 rc1 > >> > >> drivers/mtd/spi-nor/spansion.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/drivers/mtd/spi-nor/spansion.c > >> b/drivers/mtd/spi-nor/spansion.c index 6756202..01640c1 100644 > >> --- a/drivers/mtd/spi-nor/spansion.c > >> +++ b/drivers/mtd/spi-nor/spansion.c > >> @@ -29,6 +29,8 @@ static const struct flash_info spansion_parts[] = { > >> > >> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, > >> > >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | > >> SPI_NOR_HAS_LOCK | USE_CLSR) }, > >> > >> + { "s25fs128s", INFO6(0x012018, 0x4d0181, 64 * 1024, 256, > >> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > > > > The flash should be named "s25fs128s1" - this would be the flavor with > > uniform 64kB sectors. One should add a "s25fs128s0" entry for the flavor > > with uniform 256kB sectors. > > > > We usually don't add new flashes if they are not tested. Please specify in > > the commit message with which controller you tested the flash. The > > minimal test is a read, erase, read back, write and read back. This > > sequence should verify if read, erase and pp work fine. > > This chip (as part of the S25FS-S family) most probably requires a > post-BFPT fixup to make the writes work. I was going to post the patch > adding this fixup for S25FS512S chip. > Ok. Please add Yicong in cc or to. Cheers, ta
Hi Tudor, On 2020/4/20 18:18, Tudor.Ambarus@microchip.com wrote: > On Wednesday, April 15, 2020 11:30:33 AM EEST Yicong Yang wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the >> content is safe >> >> Add support for Cypress s25fs128s flash. Previously the flash is >> decoded as s25fl129p1 by mistake. >> >> Add it in the flash info list to correctly decode. No functional >> changes. Further capability of the flash will be parsed from bfpt. >> >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> --- >> Change since v1: >> - rebase the patch on v5.7 rc1 >> >> drivers/mtd/spi-nor/spansion.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c >> index 6756202..01640c1 100644 >> --- a/drivers/mtd/spi-nor/spansion.c >> +++ b/drivers/mtd/spi-nor/spansion.c >> @@ -29,6 +29,8 @@ static const struct flash_info spansion_parts[] = { >> { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, >> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | >> SPI_NOR_HAS_LOCK | USE_CLSR) }, >> + { "s25fs128s", INFO6(0x012018, 0x4d0181, 64 * 1024, 256, >> + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, > The flash should be named "s25fs128s1" - this would be the flavor with uniform > 64kB sectors. One should add a "s25fs128s0" entry for the flavor with uniform > 256kB sectors. Yes, it's right. I'll correct it in next version. > > We usually don't add new flashes if they are not tested. Please specify in the > commit message with which controller you tested the flash. The minimal test is > a read, erase, read back, write and read back. This sequence should verify if > read, erase and pp work fine. Sorry for that. I'll mentioned the tests I've done in next version. I've tested in SPI/Dual/Quad mode with our hisi-sfc-v3xx controller. All the operations work well, but something wrong with Quad read and I'm managing to figure it out. Regards, Yicong > > Cheers, > ta > >
Hi Sergei, On 2020/4/20 19:05, Sergei Shtylyov wrote: > On 20.04.2020 13:18, Tudor.Ambarus@microchip.com wrote: > >> The flash should be named "s25fs128s1" - this would be the flavor with uniform >> 64kB sectors. One should add a "s25fs128s0" entry for the flavor with uniform >> 256kB sectors. >> >> We usually don't add new flashes if they are not tested. Please specify in the >> commit message with which controller you tested the flash. The minimal test is >> a read, erase, read back, write and read back. This sequence should verify if >> read, erase and pp work fine. > > This chip (as part of the S25FS-S family) most probably requires a post-BFPT fixup to make the writes work. I was going to post the patch adding this fixup for S25FS512S chip. Thanks for the information, I'll test whether s25fs128s1 needs this fixup with your patch. Regards, Yicong > >> Cheers, >> ta > > MBR, Sergei > . >
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c index 6756202..01640c1 100644 --- a/drivers/mtd/spi-nor/spansion.c +++ b/drivers/mtd/spi-nor/spansion.c @@ -29,6 +29,8 @@ static const struct flash_info spansion_parts[] = { { "s25fl512s", INFO6(0x010220, 0x4d0080, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_HAS_LOCK | USE_CLSR) }, + { "s25fs128s", INFO6(0x012018, 0x4d0181, 64 * 1024, 256, + SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) }, { "s25fs512s", INFO6(0x010220, 0x4d0081, 256 * 1024, 256, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR) },
Add support for Cypress s25fs128s flash. Previously the flash is decoded as s25fl129p1 by mistake. Add it in the flash info list to correctly decode. No functional changes. Further capability of the flash will be parsed from bfpt. Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- Change since v1: - rebase the patch on v5.7 rc1 drivers/mtd/spi-nor/spansion.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.8.1