* [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size @ 2018-11-14 12:56 Liu Xiang 2018-11-14 13:51 ` Boris Brezillon 0 siblings, 1 reply; 7+ messages in thread From: Liu Xiang @ 2018-11-14 12:56 UTC (permalink / raw) To: linux-mtd Cc: linux-kernel, boris.brezillon, marek.vasut, dwmw2, computersforpeace, richard, liuxiang_1999, Liu Xiang In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, means that 3-Byte only addressing. But the device size is larger than 16MB, nor->addr_width must be 4 to access the whole address. An error should be returned when nor->addr_width not match the device size in spi_nor_parse_sfdp(). Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> --- drivers/mtd/spi-nor/spi-nor.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 3eba13a..77eaf22 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, } params->size >>= 3; /* Convert to bytes. */ + /*if the device exceeds 16MiB, addr_width must be 4*/ + if ((params->size > 0x1000000) && (nor->addr_width == 3)) + return -EINVAL; + /* Fast Read settings. */ for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) { const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i]; -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-14 12:56 [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size Liu Xiang @ 2018-11-14 13:51 ` Boris Brezillon 2018-11-14 14:46 ` Liu Xiang 2018-11-15 10:54 ` Tudor.Ambarus 0 siblings, 2 replies; 7+ messages in thread From: Boris Brezillon @ 2018-11-14 13:51 UTC (permalink / raw) To: Liu Xiang Cc: linux-mtd, linux-kernel, marek.vasut, dwmw2, computersforpeace, richard, liuxiang_1999, Tudor Ambarus On Wed, 14 Nov 2018 20:56:05 +0800 Liu Xiang <liu.xiang6@zte.com.cn> wrote: > In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header > is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, > means that 3-Byte only addressing. According to your other patch this NOR supports 4B opcode, which means the SFDP table is wrong. > But the device size is larger > than 16MB, nor->addr_width must be 4 to access the whole address. > An error should be returned when nor->addr_width not match ^does not > the device size in spi_nor_parse_sfdp(). > > Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> > --- > drivers/mtd/spi-nor/spi-nor.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 3eba13a..77eaf22 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > } > params->size >>= 3; /* Convert to bytes. */ > > + /*if the device exceeds 16MiB, addr_width must be 4*/ Please add a white space after '/*' and before '*/': /* If the device exceeds 16MiB, ->addr_width must be 4. */ > + if ((params->size > 0x1000000) && (nor->addr_width == 3)) Parens are not needed around sub-conditions: if (params->size > 0x1000000 && nor->addr_width == 3) > + return -EINVAL; > + I'm not sure this is correct. Looks like some NORs only support 3B opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). Don't know what's reported by the BFPT section in this case though (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). Anyway, I think this check should be moved here [2] to cover the non-SFDP case. > /* Fast Read settings. */ > for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) { > const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i]; [1]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L278 [2]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L3758 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-14 13:51 ` Boris Brezillon @ 2018-11-14 14:46 ` Liu Xiang 2018-11-15 10:54 ` Tudor.Ambarus 1 sibling, 0 replies; 7+ messages in thread From: Liu Xiang @ 2018-11-14 14:46 UTC (permalink / raw) To: Boris Brezillon Cc: Liu Xiang, linux-mtd, linux-kernel, marek.vasut, dwmw2, computersforpeace, richard, Tudor Ambarus Hi, If the check is moved to [2]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L3758, the nor->addr_width selection logic may be needed to rework. I have sent an email to ISSI for the register value, but haven't got the reply. Do you think it is better to add this check until I get the right answer from ISSI? At 2018-11-14 21:51:29, "Boris Brezillon" <boris.brezillon@bootlin.com> wrote: >On Wed, 14 Nov 2018 20:56:05 +0800 >Liu Xiang <liu.xiang6@zte.com.cn> wrote: > >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, >> means that 3-Byte only addressing. > >According to your other patch this NOR supports 4B opcode, which means >the SFDP table is wrong. > >> But the device size is larger >> than 16MB, nor->addr_width must be 4 to access the whole address. >> An error should be returned when nor->addr_width not match > > ^does not > >> the device size in spi_nor_parse_sfdp(). >> >> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> >> --- >> drivers/mtd/spi-nor/spi-nor.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index 3eba13a..77eaf22 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> } >> params->size >>= 3; /* Convert to bytes. */ >> >> + /*if the device exceeds 16MiB, addr_width must be 4*/ > >Please add a white space after '/*' and before '*/': > > /* If the device exceeds 16MiB, ->addr_width must be 4. */ > >> + if ((params->size > 0x1000000) && (nor->addr_width == 3)) > >Parens are not needed around sub-conditions: > > if (params->size > 0x1000000 && nor->addr_width == 3) > >> + return -EINVAL; >> + > >I'm not sure this is correct. Looks like some NORs only support 3B >opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). >Don't know what's reported by the BFPT section in this case though >(BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). > >Anyway, I think this check should be moved here [2] to cover the >non-SFDP case. > >> /* Fast Read settings. */ >> for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) { >> const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i]; > >[1]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L278 >[2]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L3758 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-14 13:51 ` Boris Brezillon 2018-11-14 14:46 ` Liu Xiang @ 2018-11-15 10:54 ` Tudor.Ambarus 2018-11-15 11:02 ` Boris Brezillon 1 sibling, 1 reply; 7+ messages in thread From: Tudor.Ambarus @ 2018-11-15 10:54 UTC (permalink / raw) To: boris.brezillon, liu.xiang6, cyrille.pitchen Cc: linux-mtd, linux-kernel, marek.vasut, dwmw2, computersforpeace, richard, liuxiang_1999 Hi, Liu, Boris, Cyrille, On 11/14/2018 03:51 PM, Boris Brezillon wrote: > On Wed, 14 Nov 2018 20:56:05 +0800 > Liu Xiang <liu.xiang6@zte.com.cn> wrote: > >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, Liu, can you point us to a datasheet that has the JEDEC BFPT tables described? I couldn't find one ... >> means that 3-Byte only addressing. > > According to your other patch this NOR supports 4B opcode, which means > the SFDP table is wrong. > >> But the device size is larger >> than 16MB, nor->addr_width must be 4 to access the whole address. >> An error should be returned when nor->addr_width not match > > ^does not > >> the device size in spi_nor_parse_sfdp(). >> >> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> >> --- >> drivers/mtd/spi-nor/spi-nor.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> index 3eba13a..77eaf22 100644 >> --- a/drivers/mtd/spi-nor/spi-nor.c >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> } >> params->size >>= 3; /* Convert to bytes. */ >> >> + /*if the device exceeds 16MiB, addr_width must be 4*/ > > Please add a white space after '/*' and before '*/': > > /* If the device exceeds 16MiB, ->addr_width must be 4. */ > >> + if ((params->size > 0x1000000) && (nor->addr_width == 3)) > > Parens are not needed around sub-conditions: > > if (params->size > 0x1000000 && nor->addr_width == 3) > >> + return -EINVAL; >> + > > I'm not sure this is correct. Looks like some NORs only support 3B > opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). > Don't know what's reported by the BFPT section in this case though > (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). Boris, this is in close relation with your second patch: [PATCH v3 2/2] mtd: spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B. When looking again at this, I would say that for the flashes that have a "4-byte addressing" mode, but just 3B opcodes, I would expect the DWORD1[18:17] to be of value BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (enters 4-Byte mode on command - uses 3B opcodes). If BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 and 4B opcodes, then we can query BFPT DWORD16[31:24]: it should have value xx1x_xxxxb to indicate that 4B opcodes are supported. But which 4B opcodes are supported? Do all 3B opcodes have a 4B opcode correspondent if SFDP 4-byte table is not available? This might be a good assumption, but I can't see it anywhere in jesd216c. Cyrille, when looking at ba3ae6a1d4c ("mtd: spi-nor: add a stateless method to support memory size above 128Mib") I see that all flashes that declare SPI_NOR_4B_OPCODES will have the 3B opcodes converted to 4B opcodes, assuming that all 3B opcodes have a 4B correspondent. Is this assumption a general rule, or it's just for the Spansion flashes? Cheers, ta > > Anyway, I think this check should be moved here [2] to cover the > non-SFDP case. > >> /* Fast Read settings. */ >> for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) { >> const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i]; > > [1]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L278 > [2]https://elixir.bootlin.com/linux/v4.20-rc2/source/drivers/mtd/spi-nor/spi-nor.c#L3758 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-15 10:54 ` Tudor.Ambarus @ 2018-11-15 11:02 ` Boris Brezillon 2018-11-16 13:24 ` Liu Xiang 0 siblings, 1 reply; 7+ messages in thread From: Boris Brezillon @ 2018-11-15 11:02 UTC (permalink / raw) To: Tudor.Ambarus Cc: liu.xiang6, cyrille.pitchen, linux-mtd, linux-kernel, marek.vasut, dwmw2, computersforpeace, richard, liuxiang_1999 On Thu, 15 Nov 2018 10:54:39 +0000 <Tudor.Ambarus@microchip.com> wrote: > Hi, Liu, Boris, Cyrille, > > On 11/14/2018 03:51 PM, Boris Brezillon wrote: > > On Wed, 14 Nov 2018 20:56:05 +0800 > > Liu Xiang <liu.xiang6@zte.com.cn> wrote: > > > >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header > >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, > > Liu, can you point us to a datasheet that has the JEDEC BFPT tables described? I > couldn't find one ... > > >> means that 3-Byte only addressing. > > > > According to your other patch this NOR supports 4B opcode, which means > > the SFDP table is wrong. > > > >> But the device size is larger > >> than 16MB, nor->addr_width must be 4 to access the whole address. > >> An error should be returned when nor->addr_width not match > > > > ^does not > > > >> the device size in spi_nor_parse_sfdp(). > >> > >> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> > >> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> > >> --- > >> drivers/mtd/spi-nor/spi-nor.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > >> index 3eba13a..77eaf22 100644 > >> --- a/drivers/mtd/spi-nor/spi-nor.c > >> +++ b/drivers/mtd/spi-nor/spi-nor.c > >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, > >> } > >> params->size >>= 3; /* Convert to bytes. */ > >> > >> + /*if the device exceeds 16MiB, addr_width must be 4*/ > > > > Please add a white space after '/*' and before '*/': > > > > /* If the device exceeds 16MiB, ->addr_width must be 4. */ > > > >> + if ((params->size > 0x1000000) && (nor->addr_width == 3)) > > > > Parens are not needed around sub-conditions: > > > > if (params->size > 0x1000000 && nor->addr_width == 3) > > > >> + return -EINVAL; > >> + > > > > I'm not sure this is correct. Looks like some NORs only support 3B > > opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). > > Don't know what's reported by the BFPT section in this case though > > (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). > > Boris, this is in close relation with your second patch: [PATCH v3 2/2] mtd: > spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B. > > When looking again at this, I would say that for the flashes that have a "4-byte > addressing" mode, but just 3B opcodes, I would expect the DWORD1[18:17] to be of > value BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (enters 4-Byte mode on command - uses 3B > opcodes). The NOR we have and which is exposing BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 actually supports both 3B and 4B commands, so, in this particular case, BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 does not mean "3B opcode+4-byte addressing mode" > > If BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 and 4B opcodes, then we can query BFPT > DWORD16[31:24]: it should have value xx1x_xxxxb to indicate that 4B opcodes are > supported. But which 4B opcodes are supported? I hope all of them. Wouldn't make sense to have only some of them supported. > Do all 3B opcodes have a 4B > opcode correspondent if SFDP 4-byte table is not available? This might be a good > assumption, but I can't see it anywhere in jesd216c. I hope so... ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-15 11:02 ` Boris Brezillon @ 2018-11-16 13:24 ` Liu Xiang 2019-03-13 13:20 ` Liu Xiang 0 siblings, 1 reply; 7+ messages in thread From: Liu Xiang @ 2018-11-16 13:24 UTC (permalink / raw) To: Boris Brezillon Cc: Tudor.Ambarus, liu.xiang6, cyrille.pitchen, linux-mtd, linux-kernel, marek.vasut, dwmw2, computersforpeace, richard Hi Tudor, Boris, Cyrille, There is no JEDEC BFPT tables in the datasheet. In my test platform, I sent RDSFDP command to the flash and got the parameters back. My device type is IS25LP256D-JMLA, which is not in H/E/G/F series of flash that is described in chapter 11 of the datasheet. The reply from ISSI suggests only these certain devices can support SFDP table. I am confused why my device can support RDSFDP command and give parameters back. I will ask ISSI for more details. At 2018-11-15 19:02:56, "Boris Brezillon" <boris.brezillon@bootlin.com> wrote: >On Thu, 15 Nov 2018 10:54:39 +0000 ><Tudor.Ambarus@microchip.com> wrote: > >> Hi, Liu, Boris, Cyrille, >> >> On 11/14/2018 03:51 PM, Boris Brezillon wrote: >> > On Wed, 14 Nov 2018 20:56:05 +0800 >> > Liu Xiang <liu.xiang6@zte.com.cn> wrote: >> > >> >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header >> >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, >> >> Liu, can you point us to a datasheet that has the JEDEC BFPT tables described? I >> couldn't find one ... >> >> >> means that 3-Byte only addressing. >> > >> > According to your other patch this NOR supports 4B opcode, which means >> > the SFDP table is wrong. >> > >> >> But the device size is larger >> >> than 16MB, nor->addr_width must be 4 to access the whole address. >> >> An error should be returned when nor->addr_width not match >> > >> > ^does not >> > >> >> the device size in spi_nor_parse_sfdp(). >> >> >> >> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >> >> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> >> >> --- >> >> drivers/mtd/spi-nor/spi-nor.c | 4 ++++ >> >> 1 file changed, 4 insertions(+) >> >> >> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >> >> index 3eba13a..77eaf22 100644 >> >> --- a/drivers/mtd/spi-nor/spi-nor.c >> >> +++ b/drivers/mtd/spi-nor/spi-nor.c >> >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >> >> } >> >> params->size >>= 3; /* Convert to bytes. */ >> >> >> >> + /*if the device exceeds 16MiB, addr_width must be 4*/ >> > >> > Please add a white space after '/*' and before '*/': >> > >> > /* If the device exceeds 16MiB, ->addr_width must be 4. */ >> > >> >> + if ((params->size > 0x1000000) && (nor->addr_width == 3)) >> > >> > Parens are not needed around sub-conditions: >> > >> > if (params->size > 0x1000000 && nor->addr_width == 3) >> > >> >> + return -EINVAL; >> >> + >> > >> > I'm not sure this is correct. Looks like some NORs only support 3B >> > opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). >> > Don't know what's reported by the BFPT section in this case though >> > (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). >> >> Boris, this is in close relation with your second patch: [PATCH v3 2/2] mtd: >> spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B. >> >> When looking again at this, I would say that for the flashes that have a "4-byte >> addressing" mode, but just 3B opcodes, I would expect the DWORD1[18:17] to be of >> value BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (enters 4-Byte mode on command - uses 3B >> opcodes). > >The NOR we have and which is exposing BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 >actually supports both 3B and 4B commands, so, in this particular case, >BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 does not mean "3B opcode+4-byte >addressing mode" > >> >> If BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 and 4B opcodes, then we can query BFPT >> DWORD16[31:24]: it should have value xx1x_xxxxb to indicate that 4B opcodes are >> supported. But which 4B opcodes are supported? > >I hope all of them. Wouldn't make sense to have only some of them >supported. > >> Do all 3B opcodes have a 4B >> opcode correspondent if SFDP 4-byte table is not available? This might be a good >> assumption, but I can't see it anywhere in jesd216c. > >I hope so... ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re:Re: [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size 2018-11-16 13:24 ` Liu Xiang @ 2019-03-13 13:20 ` Liu Xiang 0 siblings, 0 replies; 7+ messages in thread From: Liu Xiang @ 2019-03-13 13:20 UTC (permalink / raw) To: Liu Xiang Cc: Tudor.Ambarus, richard, liu.xiang6, linux-kernel, Boris Brezillon, linux-mtd, cyrille.pitchen, computersforpeace, dwmw2, marek.vasut Hi, Boris I am sorry I have not got any further information from ISSI since last reply. As your suggest, when the Address Bytes we are reading from BFPT is wrong, an error is returned from spi_nor_parse_bfpt(). Then it can go back to use spi_nor_ids[] for setting addr_width. If we make sure the spi_nor_ids[] is right, it can work well. I will send a v2 patch. At 2018-11-16 21:24:10, "Liu Xiang" <liuxiang_1999@126.com> wrote: > >Hi Tudor, Boris, Cyrille, >There is no JEDEC BFPT tables in the datasheet. >In my test platform, I sent RDSFDP command to the flash and got the >parameters back. >My device type is IS25LP256D-JMLA, which is not in H/E/G/F series of flash >that is described in chapter 11 of the datasheet. The reply from ISSI suggests >only these certain devices can support SFDP table. >I am confused why my device can support RDSFDP command and give >parameters back. I will ask ISSI for more details. > > > >At 2018-11-15 19:02:56, "Boris Brezillon" <boris.brezillon@bootlin.com> wrote: >>On Thu, 15 Nov 2018 10:54:39 +0000 >><Tudor.Ambarus@microchip.com> wrote: >> >>> Hi, Liu, Boris, Cyrille, >>> >>> On 11/14/2018 03:51 PM, Boris Brezillon wrote: >>> > On Wed, 14 Nov 2018 20:56:05 +0800 >>> > Liu Xiang <liu.xiang6@zte.com.cn> wrote: >>> > >>> >> In is25lp256, the DWORD1 of JEDEC Basic Flash Parameter Header >>> >> is 0xfff920e5. So the DWORD1[18:17] Address Bytes bits are 0b00, >>> >>> Liu, can you point us to a datasheet that has the JEDEC BFPT tables described? I >>> couldn't find one ... >>> >>> >> means that 3-Byte only addressing. >>> > >>> > According to your other patch this NOR supports 4B opcode, which means >>> > the SFDP table is wrong. >>> > >>> >> But the device size is larger >>> >> than 16MB, nor->addr_width must be 4 to access the whole address. >>> >> An error should be returned when nor->addr_width not match >>> > >>> > ^does not >>> > >>> >> the device size in spi_nor_parse_sfdp(). >>> >> >>> >> Suggested-by: Boris Brezillon <boris.brezillon@bootlin.com> >>> >> Signed-off-by: Liu Xiang <liu.xiang6@zte.com.cn> >>> >> --- >>> >> drivers/mtd/spi-nor/spi-nor.c | 4 ++++ >>> >> 1 file changed, 4 insertions(+) >>> >> >>> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c >>> >> index 3eba13a..77eaf22 100644 >>> >> --- a/drivers/mtd/spi-nor/spi-nor.c >>> >> +++ b/drivers/mtd/spi-nor/spi-nor.c >>> >> @@ -2669,6 +2669,10 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor, >>> >> } >>> >> params->size >>= 3; /* Convert to bytes. */ >>> >> >>> >> + /*if the device exceeds 16MiB, addr_width must be 4*/ >>> > >>> > Please add a white space after '/*' and before '*/': >>> > >>> > /* If the device exceeds 16MiB, ->addr_width must be 4. */ >>> > >>> >> + if ((params->size > 0x1000000) && (nor->addr_width == 3)) >>> > >>> > Parens are not needed around sub-conditions: >>> > >>> > if (params->size > 0x1000000 && nor->addr_width == 3) >>> > >>> >> + return -EINVAL; >>> >> + >>> > >>> > I'm not sure this is correct. Looks like some NORs only support 3B >>> > opcodes but have a "4-byte addressing" mode (see set_4byte() [1]). >>> > Don't know what's reported by the BFPT section in this case though >>> > (BFPT_DWORD1_ADDRESS_BYTES_3_ONLY or BFPT_DWORD1_ADDRESS_BYTES_3_OR_4). >>> >>> Boris, this is in close relation with your second patch: [PATCH v3 2/2] mtd: >>> spi-nor: Use 4B opcodes when the NOR advertises both 3B and 4B. >>> >>> When looking again at this, I would say that for the flashes that have a "4-byte >>> addressing" mode, but just 3B opcodes, I would expect the DWORD1[18:17] to be of >>> value BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (enters 4-Byte mode on command - uses 3B >>> opcodes). >> >>The NOR we have and which is exposing BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 >>actually supports both 3B and 4B commands, so, in this particular case, >>BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 does not mean "3B opcode+4-byte >>addressing mode" >> >>> >>> If BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 and 4B opcodes, then we can query BFPT >>> DWORD16[31:24]: it should have value xx1x_xxxxb to indicate that 4B opcodes are >>> supported. But which 4B opcodes are supported? >> >>I hope all of them. Wouldn't make sense to have only some of them >>supported. >> >>> Do all 3B opcodes have a 4B >>> opcode correspondent if SFDP 4-byte table is not available? This might be a good >>> assumption, but I can't see it anywhere in jesd216c. >> >>I hope so... ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-03-13 13:21 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-11-14 12:56 [PATCH] mtd: spi-nor: Return error when nor->addr_width not match the device size Liu Xiang 2018-11-14 13:51 ` Boris Brezillon 2018-11-14 14:46 ` Liu Xiang 2018-11-15 10:54 ` Tudor.Ambarus 2018-11-15 11:02 ` Boris Brezillon 2018-11-16 13:24 ` Liu Xiang 2019-03-13 13:20 ` Liu Xiang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox