| Message ID | 20230203044853.23513-1-Takahiro.Kuwano@infineon.com |
|---|---|
| State | Superseded |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | [v2] mtd: spi-nor: spansion: Add support for Infineon S25FS256T | expand |
Hi, Takahiro, On 03.02.2023 06:48, tkuw584924@gmail.com wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > > Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and > differences comparing to other Spansion/Cypress flash familes are: > - 4-byte address mode by factory default > - Quad mode is enabled by factory default > - OP_READ_FAST_4B(0Ch) is not supported > - Supports mixture of 128KB and 64KB sectors by OTP configuration > (this patch supports uniform 128KB only due to complexity of > non-uniform layout) > > Tested on Xilinx Zynq-7000 FPGA board. > > Link: https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?fileId=8ac78c8c80027ecd0180740c5a46707a > Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> > --- > Changes in v2: > - s/EOPNOTSUPP/ENODEV > - Drop READ_FAST support > - Amend comment in late_init() > - sector size and page size as 0 in sizeINFO6() > > --- > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname > s25fs256t > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id > 342b190f0890 > zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer > spansion > zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 53464450080101ff00000114000100ff84000102500100ffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff > ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff > ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1 > ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000 > 0000ffffffff710600fedcdcffff > zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > 13ecce2f195c4c71648e90d4a7e4a0df /sys/bus/spi/devices/spi0.0/spi-nor/sfdp > zynq> test_qspi.sh > random: crng init done > 6+0 records in > 6+0 records out > 6291456 bytes (6.0MB) copied, 2.025928 seconds, 3.0MB/s > Copied 6291456 bytes from qspi_test to address 0x00000000 in flash > Erased 6291456 bytes from address 0x00000000 in flash > Copied 6291456 bytes from address 0x00000000 in flash to qspi_read > 0000000 ffff ffff ffff ffff ffff ffff ffff ffff > * > 0600000 > Copied 6291456 bytes from qspi_test to address 0x00000000 in flash > Copied 6291456 bytes from address 0x00000000 in flash to qspi_read > 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_test > 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_read > --- > drivers/mtd/spi-nor/spansion.c | 60 ++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c > index 12a256c0ef4c..a499ce393454 100644 > --- a/drivers/mtd/spi-nor/spansion.c > +++ b/drivers/mtd/spi-nor/spansion.c > @@ -29,6 +29,7 @@ > SPINOR_REG_CYPRESS_CFR5_OPI) > #define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_DS SPINOR_REG_CYPRESS_CFR5_BIT6 > #define SPINOR_OP_CYPRESS_RD_FAST 0xee > +#define SPINOR_REG_CYPRESS_ARCFN 0x00000006 > > /* Cypress SPI NOR flash operations. */ > #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \ > @@ -218,6 +219,62 @@ static int cypress_nor_set_page_size(struct spi_nor *nor) > return 0; > } > > +static int > +s25fs256t_post_bfpt_fixup(struct spi_nor *nor, > + const struct sfdp_parameter_header *bfpt_header, > + const struct sfdp_bfpt *bfpt) > +{ > + struct spi_mem_op op; > + int ret; > + > + /* 4-byte address mode is enabled by default */ > + nor->params->addr_mode_nbytes = 4; you'll need to set params->addr_nbytes = 4; as well, even if it is set in parse_4bait(). From post_bfpt() to parse_4bait() you're uncovered. Also, 4bait is optional and if it fails, we just continue, so you'll end up with a wrong configuration. Please update. > + > + /* Read Architecture Configuration Register (ARCFN) */ > + op = (struct spi_mem_op) > + CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, > + SPINOR_REG_CYPRESS_ARCFN, > + nor->bouncebuf); > + op.dummy.nbytes = 1; From the datasheet I see that there are two flavors of the read any register command, one for volatile registers which does not require latency cycles and one for non volatile registers which needs latency cycles. Please rename CYPRESS_NOR_RD_ANY_REG_OP to CYPRESS_NOR_RD_ANY_VREG_OP and introduce a CYPRESS_NOR_RD_ANY_NVREG_OP which will handle the latency cycles. Also you should go with the maximum number of latency cycles if you want to be able to operate this flash at its maximum frequency, see Table 43 from the datasheet. BTW, I hate the dummy.nbytes nonsense, there's no such thing as dummy/latency number of bytes. This should have been defined as latency number of cycles. Sergiu Moga had a patch addressing this in MTD and SPI but I don't think he's going to send a new version. If there are no other volunteers I'll respin it myself. > + ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto); > + if (ret) > + return ret; > + > + /* ARCFN value must be 0 if uniform sector is selected */ > + if (nor->bouncebuf[0]) > + return -ENODEV; > + > + return cypress_nor_set_page_size(nor); > +} > + > +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor) > +{ > + struct spi_nor_flash_parameter *params = nor->params; > + > + /* PP_1_1_4_4B is supported but missing in SFDP. */ missing in the 4-Byte Address Table you mean, right? Overall the patch is good. I'd like to address the non volatile byte address mode if possible, but it's not a strong requirement. I'll read again the datasheet tomorrow. The problem is that even if the flash has a default 4 byte address mode (factory default), users can change the address mode to 3 by writing to the non-volatile configuration register and we'll end up with an unusable flash because as of now we assume the flash is in the 4 byte address mode state. I'd like to determine the address mode at runtime if possible. For this flash we can't use your CRC idea to determine the mode, as the flash does not have such support. I see in the datasheet that "RDARG_C_0 transaction can be used during embedded operations to read Status Register 1 (STR1V)." RDSR1_0_0 does not need any address, so an idea is to use RDARG_3_0, RDARG_4_0 and RDSR1_0_0, compare what we receive and determine the mode. If we're devilish, we can also do a WREN, change a bit in SR and reread all to double verify the result, then a WRDIS. Other idea is to force 4 byte address mode by default using EN4BA_0_0, this command does not need address bytes. Not the most elegant method though, but it should work. Again, I'd like to re-read the datasheet. If you have other idea, shoot. Cheers, ta
On 26.02.2023 21:34, Tudor Ambarus wrote: > Hi, Takahiro, > > On 03.02.2023 06:48, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> >> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and >> differences comparing to other Spansion/Cypress flash familes are: >> - 4-byte address mode by factory default >> - Quad mode is enabled by factory default >> - OP_READ_FAST_4B(0Ch) is not supported >> - Supports mixture of 128KB and 64KB sectors by OTP configuration >> (this patch supports uniform 128KB only due to complexity of >> non-uniform layout) >> >> Tested on Xilinx Zynq-7000 FPGA board. >> >> Link: >> https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?fileId=8ac78c8c80027ecd0180740c5a46707a >> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >> --- >> Changes in v2: >> - s/EOPNOTSUPP/ENODEV >> - Drop READ_FAST support >> - Amend comment in late_init() >> - sector size and page size as 0 in sizeINFO6() >> >> --- >> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname >> s25fs256t >> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id >> 342b190f0890 >> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer >> spansion >> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >> 53464450080101ff00000114000100ff84000102500100ffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >> ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff >> ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1 >> ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000 >> 0000ffffffff710600fedcdcffff >> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >> 13ecce2f195c4c71648e90d4a7e4a0df >> /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >> zynq> test_qspi.sh >> random: crng init done >> 6+0 records in >> 6+0 records out >> 6291456 bytes (6.0MB) copied, 2.025928 seconds, 3.0MB/s >> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash >> Erased 6291456 bytes from address 0x00000000 in flash >> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read >> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff >> * >> 0600000 >> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash >> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read >> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_test >> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_read >> --- >> drivers/mtd/spi-nor/spansion.c | 60 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 60 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/spansion.c >> b/drivers/mtd/spi-nor/spansion.c >> index 12a256c0ef4c..a499ce393454 100644 >> --- a/drivers/mtd/spi-nor/spansion.c >> +++ b/drivers/mtd/spi-nor/spansion.c >> @@ -29,6 +29,7 @@ >> SPINOR_REG_CYPRESS_CFR5_OPI) >> #define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_DS >> SPINOR_REG_CYPRESS_CFR5_BIT6 >> #define SPINOR_OP_CYPRESS_RD_FAST 0xee >> +#define SPINOR_REG_CYPRESS_ARCFN 0x00000006 >> /* Cypress SPI NOR flash operations. */ >> #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \ >> @@ -218,6 +219,62 @@ static int cypress_nor_set_page_size(struct >> spi_nor *nor) >> return 0; >> } >> +static int >> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor, >> + const struct sfdp_parameter_header *bfpt_header, >> + const struct sfdp_bfpt *bfpt) >> +{ >> + struct spi_mem_op op; >> + int ret; >> + >> + /* 4-byte address mode is enabled by default */ >> + nor->params->addr_mode_nbytes = 4; > > you'll need to set params->addr_nbytes = 4; as well, even if it is set > in parse_4bait(). From post_bfpt() to parse_4bait() you're uncovered. > Also, 4bait is optional and if it fails, we just continue, so you'll end > up with a wrong configuration. Please update. > >> + >> + /* Read Architecture Configuration Register (ARCFN) */ >> + op = (struct spi_mem_op) >> + CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, >> + SPINOR_REG_CYPRESS_ARCFN, >> + nor->bouncebuf); >> + op.dummy.nbytes = 1; > > From the datasheet I see that there are two flavors of the read any > register command, one for volatile registers which does not require > latency cycles and one for non volatile registers which needs latency > cycles. Please rename CYPRESS_NOR_RD_ANY_REG_OP to > CYPRESS_NOR_RD_ANY_VREG_OP and introduce a CYPRESS_NOR_RD_ANY_NVREG_OP > which will handle the latency cycles. > > Also you should go with the maximum number of latency cycles if you want > to be able to operate this flash at its maximum frequency, see Table 43 > from the datasheet. > > BTW, I hate the dummy.nbytes nonsense, there's no such thing as > dummy/latency number of bytes. This should have been defined as latency > number of cycles. Sergiu Moga had a patch addressing this in MTD and SPI > but I don't think he's going to send a new version. If there are no > other volunteers I'll respin it myself. > >> + ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto); >> + if (ret) >> + return ret; >> + >> + /* ARCFN value must be 0 if uniform sector is selected */ >> + if (nor->bouncebuf[0]) >> + return -ENODEV; >> + >> + return cypress_nor_set_page_size(nor); >> +} >> + >> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor) >> +{ >> + struct spi_nor_flash_parameter *params = nor->params; >> + >> + /* PP_1_1_4_4B is supported but missing in SFDP. */ > > missing in the 4-Byte Address Table you mean, right? > > Overall the patch is good. I'd like to address the non volatile byte > address mode if possible, but it's not a strong requirement. I'll read > again the datasheet tomorrow. The problem is that even if the flash has > a default 4 byte address mode (factory default), users can change the > address mode to 3 by writing to the non-volatile configuration register > and we'll end up with an unusable flash because as of now we assume the > flash is in the 4 byte address mode state. I'd like to determine the > address mode at runtime if possible. For this flash we can't use your > CRC idea to determine the mode, as the flash does not have such support. > I see in the datasheet that "RDARG_C_0 transaction can be used during > embedded operations to read Status Register 1 (STR1V)." RDSR1_0_0 does > not need any address, so an idea is to use RDARG_3_0, RDARG_4_0 and > RDSR1_0_0, compare what we receive and determine the mode. If we're > devilish, we can also do a WREN, change a bit in SR and reread all to > double verify the result, then a WRDIS. > Other idea is to force 4 byte address mode by default using EN4BA_0_0, > this command does not need address bytes. Not the most elegant method > though, but it should work. > Again, I'd like to re-read the datasheet. If you have other idea, shoot. > I've re-read the datasheet I couldn't find a better way to determine the address mode. In these conditions, I would do a hybrid approach of the above two methods: I would compare SR1 data by issuing RDSR1, RDARG_3_0, RDARG_4_0. If there's no match or all have the same value issue EN4BA_0_0 and return. If there's a match, do WREN and read all again and compare, then WRDIS. If there's no match or all are the same issue EN4BA_0_0 and return. Compare the address mode determined at the first iteration with the address mode determined at the second (with WREN). If there's no match, issue EN4BA_0_0 and return. If there is a match, save the determined address mode and return. No need to implement this right now, we can work in an incremental fashion. Please send a v3 addressing the other comments and then please try to solve this as well. With this you'll have a working flash even if the address mode was changed to 3, and we'll stop making assumptions on the address mode. Cheers, ta
Hi Tudor, On 2/28/2023 5:04 PM, Tudor Ambarus wrote: > > > On 26.02.2023 21:34, Tudor Ambarus wrote: >> Hi, Takahiro, >> >> On 03.02.2023 06:48, tkuw584924@gmail.com wrote: >>> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>> >>> Infineon S25FS256T is 256Mbit Quad SPI NOR flash. The key features and >>> differences comparing to other Spansion/Cypress flash familes are: >>> - 4-byte address mode by factory default >>> - Quad mode is enabled by factory default >>> - OP_READ_FAST_4B(0Ch) is not supported >>> - Supports mixture of 128KB and 64KB sectors by OTP configuration >>> (this patch supports uniform 128KB only due to complexity of >>> non-uniform layout) >>> >>> Tested on Xilinx Zynq-7000 FPGA board. >>> >>> Link: https://www.infineon.com/dgdlac/Infineon-S25FS256T_256Mb_SEMPER_Nano_Flash_Quad_SPI_1.8V-DataSheet-v12_00-EN.pdf?fileId=8ac78c8c80027ecd0180740c5a46707a >>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com> >>> --- >>> Changes in v2: >>> - s/EOPNOTSUPP/ENODEV >>> - Drop READ_FAST support >>> - Amend comment in late_init() >>> - sector size and page size as 0 in sizeINFO6() >>> >>> --- >>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/partname >>> s25fs256t >>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/jedec_id >>> 342b190f0890 >>> zynq> cat /sys/bus/spi/devices/spi0.0/spi-nor/manufacturer >>> spansion >>> zynq> xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >>> 53464450080101ff00000114000100ff84000102500100ffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff >>> ffffffffffffffffffffffffffffffffe7ffe2ffffffff0f48eb086bffff >>> ffffeeffffffffff00ffffff00ff11d810d800ff00ff321cfeff71e9ffe1 >>> ec031c607a757a75f766805c00d65dfff938c0a100000000000000000000 >>> 0000ffffffff710600fedcdcffff >>> zynq> md5sum /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >>> 13ecce2f195c4c71648e90d4a7e4a0df /sys/bus/spi/devices/spi0.0/spi-nor/sfdp >>> zynq> test_qspi.sh >>> random: crng init done >>> 6+0 records in >>> 6+0 records out >>> 6291456 bytes (6.0MB) copied, 2.025928 seconds, 3.0MB/s >>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash >>> Erased 6291456 bytes from address 0x00000000 in flash >>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read >>> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff >>> * >>> 0600000 >>> Copied 6291456 bytes from qspi_test to address 0x00000000 in flash >>> Copied 6291456 bytes from address 0x00000000 in flash to qspi_read >>> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_test >>> 09ed7922931fe10795adc0ba3aff4efd2127be02 qspi_read >>> --- >>> drivers/mtd/spi-nor/spansion.c | 60 ++++++++++++++++++++++++++++++++++ >>> 1 file changed, 60 insertions(+) >>> >>> diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c >>> index 12a256c0ef4c..a499ce393454 100644 >>> --- a/drivers/mtd/spi-nor/spansion.c >>> +++ b/drivers/mtd/spi-nor/spansion.c >>> @@ -29,6 +29,7 @@ >>> SPINOR_REG_CYPRESS_CFR5_OPI) >>> #define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_DS SPINOR_REG_CYPRESS_CFR5_BIT6 >>> #define SPINOR_OP_CYPRESS_RD_FAST 0xee >>> +#define SPINOR_REG_CYPRESS_ARCFN 0x00000006 >>> /* Cypress SPI NOR flash operations. */ >>> #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \ >>> @@ -218,6 +219,62 @@ static int cypress_nor_set_page_size(struct spi_nor *nor) >>> return 0; >>> } >>> +static int >>> +s25fs256t_post_bfpt_fixup(struct spi_nor *nor, >>> + const struct sfdp_parameter_header *bfpt_header, >>> + const struct sfdp_bfpt *bfpt) >>> +{ >>> + struct spi_mem_op op; >>> + int ret; >>> + >>> + /* 4-byte address mode is enabled by default */ >>> + nor->params->addr_mode_nbytes = 4; >> >> you'll need to set params->addr_nbytes = 4; as well, even if it is set >> in parse_4bait(). From post_bfpt() to parse_4bait() you're uncovered. >> Also, 4bait is optional and if it fails, we just continue, so you'll end >> up with a wrong configuration. Please update. >> >>> + >>> + /* Read Architecture Configuration Register (ARCFN) */ >>> + op = (struct spi_mem_op) >>> + CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, >>> + SPINOR_REG_CYPRESS_ARCFN, >>> + nor->bouncebuf); >>> + op.dummy.nbytes = 1; >> >> From the datasheet I see that there are two flavors of the read any >> register command, one for volatile registers which does not require >> latency cycles and one for non volatile registers which needs latency >> cycles. Please rename CYPRESS_NOR_RD_ANY_REG_OP to >> CYPRESS_NOR_RD_ANY_VREG_OP and introduce a CYPRESS_NOR_RD_ANY_NVREG_OP >> which will handle the latency cycles. >> >> Also you should go with the maximum number of latency cycles if you want >> to be able to operate this flash at its maximum frequency, see Table 43 >> from the datasheet. >> >> BTW, I hate the dummy.nbytes nonsense, there's no such thing as >> dummy/latency number of bytes. This should have been defined as latency >> number of cycles. Sergiu Moga had a patch addressing this in MTD and SPI >> but I don't think he's going to send a new version. If there are no >> other volunteers I'll respin it myself. >> >>> + ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto); >>> + if (ret) >>> + return ret; >>> + >>> + /* ARCFN value must be 0 if uniform sector is selected */ >>> + if (nor->bouncebuf[0]) >>> + return -ENODEV; >>> + >>> + return cypress_nor_set_page_size(nor); >>> +} >>> + >>> +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor) >>> +{ >>> + struct spi_nor_flash_parameter *params = nor->params; >>> + >>> + /* PP_1_1_4_4B is supported but missing in SFDP. */ >> >> missing in the 4-Byte Address Table you mean, right? >> >> Overall the patch is good. I'd like to address the non volatile byte >> address mode if possible, but it's not a strong requirement. I'll read >> again the datasheet tomorrow. The problem is that even if the flash has >> a default 4 byte address mode (factory default), users can change the >> address mode to 3 by writing to the non-volatile configuration register >> and we'll end up with an unusable flash because as of now we assume the >> flash is in the 4 byte address mode state. I'd like to determine the >> address mode at runtime if possible. For this flash we can't use your CRC idea to determine the mode, as the flash does not have such support. >> I see in the datasheet that "RDARG_C_0 transaction can be used during embedded operations to read Status Register 1 (STR1V)." RDSR1_0_0 does >> not need any address, so an idea is to use RDARG_3_0, RDARG_4_0 and >> RDSR1_0_0, compare what we receive and determine the mode. If we're >> devilish, we can also do a WREN, change a bit in SR and reread all to double verify the result, then a WRDIS. >> Other idea is to force 4 byte address mode by default using EN4BA_0_0, >> this command does not need address bytes. Not the most elegant method >> though, but it should work. >> Again, I'd like to re-read the datasheet. If you have other idea, shoot. >> > > I've re-read the datasheet I couldn't find a better way to determine the > address mode. In these conditions, I would do a hybrid approach of the > above two methods: > I would compare SR1 data by issuing RDSR1, RDARG_3_0, RDARG_4_0. If > there's no match or all have the same value issue EN4BA_0_0 and return. > If there's a match, do WREN and read all again and compare, then WRDIS. If there's no match or all are the same issue EN4BA_0_0 and return. > Compare the address mode determined at the first iteration with the > address mode determined at the second (with WREN). If there's no match, > issue EN4BA_0_0 and return. If there is a match, save the determined address mode and return. > I implemented and tested this. A problem was that all of RDSR1, RDARG_3_0, and RDARG_4_0 returned 00h in case the device is in 3-byte mode, resulting EN4BA_0_0. I don't think this is our expectation since the EN4BA should be the last resort. I modified the flow a little and submitted: https://patchwork.ozlabs.org/project/linux-mtd/patch/20230315034004.5535-1-Takahiro.Kuwano@infineon.com/ I tested it and confirmed S25Hx-T, S25FS256T, and S28Hx-T works. > No need to implement this right now, we can work in an incremental > fashion. Please send a v3 addressing the other comments and then please > try to solve this as well. With this you'll have a working flash even if > the address mode was changed to 3, and we'll stop making assumptions on > the address mode. > > Cheers, > ta
diff --git a/drivers/mtd/spi-nor/spansion.c b/drivers/mtd/spi-nor/spansion.c index 12a256c0ef4c..a499ce393454 100644 --- a/drivers/mtd/spi-nor/spansion.c +++ b/drivers/mtd/spi-nor/spansion.c @@ -29,6 +29,7 @@ SPINOR_REG_CYPRESS_CFR5_OPI) #define SPINOR_REG_CYPRESS_CFR5_OCT_DTR_DS SPINOR_REG_CYPRESS_CFR5_BIT6 #define SPINOR_OP_CYPRESS_RD_FAST 0xee +#define SPINOR_REG_CYPRESS_ARCFN 0x00000006 /* Cypress SPI NOR flash operations. */ #define CYPRESS_NOR_WR_ANY_REG_OP(naddr, addr, ndata, buf) \ @@ -218,6 +219,62 @@ static int cypress_nor_set_page_size(struct spi_nor *nor) return 0; } +static int +s25fs256t_post_bfpt_fixup(struct spi_nor *nor, + const struct sfdp_parameter_header *bfpt_header, + const struct sfdp_bfpt *bfpt) +{ + struct spi_mem_op op; + int ret; + + /* 4-byte address mode is enabled by default */ + nor->params->addr_mode_nbytes = 4; + + /* Read Architecture Configuration Register (ARCFN) */ + op = (struct spi_mem_op) + CYPRESS_NOR_RD_ANY_REG_OP(nor->params->addr_mode_nbytes, + SPINOR_REG_CYPRESS_ARCFN, + nor->bouncebuf); + op.dummy.nbytes = 1; + ret = spi_nor_read_any_reg(nor, &op, nor->reg_proto); + if (ret) + return ret; + + /* ARCFN value must be 0 if uniform sector is selected */ + if (nor->bouncebuf[0]) + return -ENODEV; + + return cypress_nor_set_page_size(nor); +} + +static void s25fs256t_post_sfdp_fixup(struct spi_nor *nor) +{ + struct spi_nor_flash_parameter *params = nor->params; + + /* PP_1_1_4_4B is supported but missing in SFDP. */ + params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_4; + spi_nor_set_pp_settings(¶ms->page_programs[SNOR_CMD_PP_1_1_4], + SPINOR_OP_PP_1_1_4_4B, + SNOR_PROTO_1_1_4); +} + +static void s25fs256t_late_init(struct spi_nor *nor) +{ + /* + * Programming is supported only in 16-byte ECC data unit granularity. + * Byte-programming, bit-walking, or multiple program operations to the + * same ECC data unit without an erase are not allowed. See chapter + * 5.3.1 and 5.6 in the datasheet. + */ + nor->params->writesize = 16; +} + +static struct spi_nor_fixups s25fs256t_fixups = { + .post_bfpt = s25fs256t_post_bfpt_fixup, + .post_sfdp = s25fs256t_post_sfdp_fixup, + .late_init = s25fs256t_late_init, +}; + static int s25hx_t_post_bfpt_fixup(struct spi_nor *nor, const struct sfdp_parameter_header *bfpt_header, @@ -446,6 +503,9 @@ static const struct flash_info spansion_nor_parts[] = { { "s25fl256l", INFO(0x016019, 0, 64 * 1024, 512) NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) FIXUP_FLAGS(SPI_NOR_4B_OPCODES) }, + { "s25fs256t", INFO6(0x342b19, 0x0f0890, 0, 0) + PARSE_SFDP + .fixups = &s25fs256t_fixups }, { "s25hl512t", INFO6(0x342a1a, 0x0f0390, 256 * 1024, 256) PARSE_SFDP MFR_FLAGS(USE_CLSR)