[PATCH] mtd: spi-nor: Add support for is25lp01g

[PATCH] mtd: spi-nor: Add support for is25lp01g

[Clement Leger]

Update the issi_parts table for is25lp01g (128MB) device from ISSI.
Tested on Kalray K200 board.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 drivers/mtd/spi-nor/issi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index ffcb60e54a80..c3c3438e3d08 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -49,6 +49,8 @@ static const struct flash_info issi_parts[] = {
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
 			     SPI_NOR_4B_OPCODES)
 		.fixups = &is25lp256_fixups },
+	{ "is25lp01g",  INFO(0x9d601b, 0, 64 * 1024, 2048,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp032",  INFO(0x9d7016, 0, 64 * 1024,  64,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ "is25wp064",  INFO(0x9d7017, 0, 64 * 1024, 128,
-- 
2.17.1

Re: [PATCH] mtd: spi-nor: Add support for is25lp01g

[Tudor.Ambarus]

Hi, Clement,

On Friday, April 17, 2020 7:08:39 PM EEST Clement Leger wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Update the issi_parts table for is25lp01g (128MB) device from ISSI.
> Tested on Kalray K200 board.
> 
> Signed-off-by: Clement Leger <cleger@kalray.eu>
> ---
>  drivers/mtd/spi-nor/issi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index ffcb60e54a80..c3c3438e3d08 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -49,6 +49,8 @@ static const struct flash_info issi_parts[] = {
>                              SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
> | SPI_NOR_4B_OPCODES)
>                 .fixups = &is25lp256_fixups },
> +       { "is25lp01g",  INFO(0x9d601b, 0, 64 * 1024, 2048,

There is a "K" flavor of this flash which has 512 Byte Page size with 256 KB
Block size. While the page size can be determined by parsing SFDP, I think we 
will have some problems with sector_size because as of now, the sector_size is 
always set to 64KB. An incorrect sector_size will affect erases and locking.

Cheers,
ta

Re: [PATCH] mtd: spi-nor: Add support for is25lp01g

[Clément Leger]

Hi Tudor,

----- On 20 Apr, 2020, at 14:14, Tudor Ambarus Tudor.Ambarus@microchip.com wrote:

> Hi, Clement,
> 
> On Friday, April 17, 2020 7:08:39 PM EEST Clement Leger wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>> 
>> Update the issi_parts table for is25lp01g (128MB) device from ISSI.
>> Tested on Kalray K200 board.
>> 
>> Signed-off-by: Clement Leger <cleger@kalray.eu>
>> ---
>>  drivers/mtd/spi-nor/issi.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>> index ffcb60e54a80..c3c3438e3d08 100644
>> --- a/drivers/mtd/spi-nor/issi.c
>> +++ b/drivers/mtd/spi-nor/issi.c
>> @@ -49,6 +49,8 @@ static const struct flash_info issi_parts[] = {
>>                              SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ
>> | SPI_NOR_4B_OPCODES)
>>                 .fixups = &is25lp256_fixups },
>> +       { "is25lp01g",  INFO(0x9d601b, 0, 64 * 1024, 2048,
> 
> There is a "K" flavor of this flash which has 512 Byte Page size with 256 KB
> Block size. While the page size can be determined by parsing SFDP, I think we
> will have some problems with sector_size because as of now, the sector_size is
> always set to 64KB. An incorrect sector_size will affect erases and locking.

Thanks, I did not noticed that ! If I understand, this will require to modify
the core to handle sector size the same way as page_size and probably add a
fixup to detect the "K" options from SFDP ?
This is probably more changes than I can handle, and you can probably drop this
patch since not really functional for "K" type flash.

But thanks for noticing it !

Clément

> 
> Cheers,
> ta

Re: [PATCH] mtd: spi-nor: Add support for is25lp01g

[Tudor.Ambarus]

On Monday, April 20, 2020 5:50:02 PM EEST Clément Leger wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
> content is safe
> 
> Hi Tudor,

Hi, Clement,

> 
> ----- On 20 Apr, 2020, at 14:14, Tudor Ambarus Tudor.Ambarus@microchip.com 
wrote:
> > Hi, Clement,
> > 
> > On Friday, April 17, 2020 7:08:39 PM EEST Clement Leger wrote:
> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> >> the
> >> content is safe
> >> 
> >> Update the issi_parts table for is25lp01g (128MB) device from ISSI.
> >> Tested on Kalray K200 board.
> >> 
> >> Signed-off-by: Clement Leger <cleger@kalray.eu>
> >> ---
> >> 
> >>  drivers/mtd/spi-nor/issi.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> >> index ffcb60e54a80..c3c3438e3d08 100644
> >> --- a/drivers/mtd/spi-nor/issi.c
> >> +++ b/drivers/mtd/spi-nor/issi.c
> >> @@ -49,6 +49,8 @@ static const struct flash_info issi_parts[] = {
> >> 
> >>                              SECT_4K | SPI_NOR_DUAL_READ |
> >>                              SPI_NOR_QUAD_READ
> >> | 
> >> | SPI_NOR_4B_OPCODES)
> >> | 
> >>                 .fixups = &is25lp256_fixups },
> >> 
> >> +       { "is25lp01g",  INFO(0x9d601b, 0, 64 * 1024, 2048,
> > 
> > There is a "K" flavor of this flash which has 512 Byte Page size with 256
> > KB Block size. While the page size can be determined by parsing SFDP, I
> > think we will have some problems with sector_size because as of now, the
> > sector_size is always set to 64KB. An incorrect sector_size will affect
> > erases and locking.
> Thanks, I did not noticed that ! If I understand, this will require to
> modify the core to handle sector size the same way as page_size and
> probably add a fixup to detect the "K" options from SFDP ?

Right. You can add a post_bfpt fixup hook for this flash. You can 
differentiate between the "K" version and the rest by the page size. Since the 
page size is tightly coupled with the sector size, you can amend both in the 
post_bfpt hook.

> This is probably more changes than I can handle, and you can probably drop
> this patch since not really functional for "K" type flash.

I dropped it. You should try to fix it, I can guide you if needed. Or I can do 
it myself, but I'll need some help from you at testing.

Cheers,
ta

Re: [PATCH] mtd: spi-nor: Add support for is25lp01g

[Clément Leger]

Hi Tudor,

----- On 21 Apr, 2020, at 06:40, Tudor Ambarus Tudor.Ambarus@microchip.com wrote:

> On Monday, April 20, 2020 5:50:02 PM EEST Clément Leger wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the
>> content is safe
>> 
>> Hi Tudor,
> 
> Hi, Clement,
> 
>> 
>> ----- On 20 Apr, 2020, at 14:14, Tudor Ambarus Tudor.Ambarus@microchip.com
> wrote:
>> > Hi, Clement,
>> > 
>> > On Friday, April 17, 2020 7:08:39 PM EEST Clement Leger wrote:
>> >> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> >> the
>> >> content is safe
>> >> 
>> >> Update the issi_parts table for is25lp01g (128MB) device from ISSI.
>> >> Tested on Kalray K200 board.
>> >> 
>> >> Signed-off-by: Clement Leger <cleger@kalray.eu>
>> >> ---
>> >> 
>> >>  drivers/mtd/spi-nor/issi.c | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >> 
>> >> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
>> >> index ffcb60e54a80..c3c3438e3d08 100644
>> >> --- a/drivers/mtd/spi-nor/issi.c
>> >> +++ b/drivers/mtd/spi-nor/issi.c
>> >> @@ -49,6 +49,8 @@ static const struct flash_info issi_parts[] = {
>> >> 
>> >>                              SECT_4K | SPI_NOR_DUAL_READ |
>> >>                              SPI_NOR_QUAD_READ
>> >> | 
>> >> | SPI_NOR_4B_OPCODES)
>> >> | 
>> >>                 .fixups = &is25lp256_fixups },
>> >> 
>> >> +       { "is25lp01g",  INFO(0x9d601b, 0, 64 * 1024, 2048,
>> > 
>> > There is a "K" flavor of this flash which has 512 Byte Page size with 256
>> > KB Block size. While the page size can be determined by parsing SFDP, I
>> > think we will have some problems with sector_size because as of now, the
>> > sector_size is always set to 64KB. An incorrect sector_size will affect
>> > erases and locking.
>> Thanks, I did not noticed that ! If I understand, this will require to
>> modify the core to handle sector size the same way as page_size and
>> probably add a fixup to detect the "K" options from SFDP ?
> 
> Right. You can add a post_bfpt fixup hook for this flash. You can
> differentiate between the "K" version and the rest by the page size. Since the
> page size is tightly coupled with the sector size, you can amend both in the
> post_bfpt hook.

Ok, this seems clear ! I'll give it a try. By looking quickly at the code I
think that n_sectors will also have to be updated after discovering the
sector_size from BFPT (for flash size computation). Since some parameters
of the nor are initialized early in spi_nor_info_init_params using
sector_size, should I move the call making use of sector_size later in the
init (in spi_nor_late_init_params for instance) ?

> 
>> This is probably more changes than I can handle, and you can probably drop
>> this patch since not really functional for "K" type flash.
> 
> I dropped it. You should try to fix it, I can guide you if needed. Or I can do
> it myself, but I'll need some help from you at testing.

I will try to do it but I will probably only be able to test the patches in a 
couple of weeks due to our architecture not being rebased on 5.7 yet.

Thanks,

Clément

> 
> Cheers,
> ta