diff mbox series

mtd: spi-nor: issi: Add in support for IS25LX256 chip, operating in 1S-1S-8S mode.

Message ID 20221128172455.159787-1-nathan.morrison@timesys.com
State New
Headers
Series mtd: spi-nor: issi: Add in support for IS25LX256 chip, operating in 1S-1S-8S mode. |

Commit Message

Nathan Barrett-Morrison Nov. 28, 2022, 5:24 p.m. UTC 
  Adds the is25lx256 entry to the nor_parts table along with the additional
fixup logic to operate in 1S-1S-8S mode while programming.

Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
---
 drivers/mtd/spi-nor/issi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Michael Walle Nov. 30, 2022, 8:45 a.m. UTC | #1 
Am 2022-11-28 18:24, schrieb Nathan Barrett-Morrison:
> Adds the is25lx256 entry to the nor_parts table along with the 
> additional
> fixup logic to operate in 1S-1S-8S mode while programming.
> 
> Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> ---
>  drivers/mtd/spi-nor/issi.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> index 89a66a19d754..e9b32b726bf3 100644
> --- a/drivers/mtd/spi-nor/issi.c
> +++ b/drivers/mtd/spi-nor/issi.c
> @@ -29,6 +29,21 @@ static const struct spi_nor_fixups is25lp256_fixups 
> = {
>  	.post_bfpt = is25lp256_post_bfpt_fixups,
>  };
> 
> +static void is25lx256_post_sfdp_fixup(struct spi_nor *nor)
> +{
> +	/* Fixup page program command to 1S-1S-8S */
> +	nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_8;
> +	spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_1_1_8],
> +				SPINOR_OP_PP_1_1_8, SNOR_PROTO_1_1_8);

Does this flash have SFDP data? If possible, this should be
derived from that. Could you dump the SFDP table and
post it here [1].

> +
> +	/* Disable quad_enable */
> +	nor->params->quad_enable = NULL;

why?

> +}
> +
> +static struct spi_nor_fixups is25lx256_fixups = {
> +	.post_sfdp = is25lx256_post_sfdp_fixup,
> +};
> +
>  static void pm25lv_nor_late_init(struct spi_nor *nor)
>  {
>  	struct spi_nor_erase_map *map = &nor->params->erase_map;
> @@ -74,6 +89,10 @@ static const struct flash_info issi_nor_parts[] = {
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>  		.fixups = &is25lp256_fixups },
> +	{ "is25lx256",  INFO(0x9d5a19, 0, 128 * 1024, 256)
> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ)
> +		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> +		.fixups = &is25lx256_fixups },
> 
>  	/* PMC */
>  	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2)

-michael

[1] 
https://lore.kernel.org/linux-mtd/4304e19f3399a0a6e856119d01ccabe0@walle.cc/
Nathan Barrett-Morrison Nov. 30, 2022, 3:34 p.m. UTC | #2 
Hi Michael,

> Does this flash have SFDP data? If possible, this should be
> derived from that. Could you dump the SFDP table and
> post it here [1].

# hexdump sfdp
0000000 4653 5044 0106 ff01 0600 1001 0030 ff00
0000010 0084 0201 0080 ff00 ffff ffff ffff ffff
0000020 ffff ffff ffff ffff ffff ffff ffff ffff
0000030 20e5 ff8a ffff 0fff 0000 0000 0000 0000
0000040 fffe ffff ffff ff00 ffff 0000 200c d811
0000050 520f ff00 2224 00a9 8e8b d103 01ac 3827
0000060 757a 757a bdfb 5cd5 0000 ff70 b081 2238
0000070 ffff ffff ffff ffff ffff ffff ffff ffff
0000080 0e43 ffff dc21 ff5c

Looking at the latest SFDP document from
https://www.jedec.org/standards-documents/docs/jesd216b, I see
1s-1s-8s would be in BFPT DWORD 17, which appears to be 0xffffffff if
I'm reading this hexdump correctly.

> why?

This was because ISSI's default_init was setting a quad_enable
function pointer which is not relevant to this part.  This probably
doesn't need to be done though, as SPI_NOR_QUAD_* isn't being set in
the flash_info table and therefore quad_enable will never be used?

Sincerely,
Nathan


On Wed, Nov 30, 2022 at 3:45 AM Michael Walle <michael@walle.cc> wrote:
>
> Am 2022-11-28 18:24, schrieb Nathan Barrett-Morrison:
> > Adds the is25lx256 entry to the nor_parts table along with the
> > additional
> > fixup logic to operate in 1S-1S-8S mode while programming.
> >
> > Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com>
> > ---
> >  drivers/mtd/spi-nor/issi.c | 19 +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
> > index 89a66a19d754..e9b32b726bf3 100644
> > --- a/drivers/mtd/spi-nor/issi.c
> > +++ b/drivers/mtd/spi-nor/issi.c
> > @@ -29,6 +29,21 @@ static const struct spi_nor_fixups is25lp256_fixups
> > = {
> >       .post_bfpt = is25lp256_post_bfpt_fixups,
> >  };
> >
> > +static void is25lx256_post_sfdp_fixup(struct spi_nor *nor)
> > +{
> > +     /* Fixup page program command to 1S-1S-8S */
> > +     nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_8;
> > +     spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_1_1_8],
> > +                             SPINOR_OP_PP_1_1_8, SNOR_PROTO_1_1_8);
>
> Does this flash have SFDP data? If possible, this should be
> derived from that. Could you dump the SFDP table and
> post it here [1].
>
> > +
> > +     /* Disable quad_enable */
> > +     nor->params->quad_enable = NULL;
>
> why?
>
> > +}
> > +
> > +static struct spi_nor_fixups is25lx256_fixups = {
> > +     .post_sfdp = is25lx256_post_sfdp_fixup,
> > +};
> > +
> >  static void pm25lv_nor_late_init(struct spi_nor *nor)
> >  {
> >       struct spi_nor_erase_map *map = &nor->params->erase_map;
> > @@ -74,6 +89,10 @@ static const struct flash_info issi_nor_parts[] = {
> >               NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
> >               FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> >               .fixups = &is25lp256_fixups },
> > +     { "is25lx256",  INFO(0x9d5a19, 0, 128 * 1024, 256)
> > +             NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ)
> > +             FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
> > +             .fixups = &is25lx256_fixups },
> >
> >       /* PMC */
> >       { "pm25lv512",   INFO(0,        0, 32 * 1024,    2)
>
> -michael
>
> [1]
> https://lore.kernel.org/linux-mtd/4304e19f3399a0a6e856119d01ccabe0@walle.cc/
Michael Walle Nov. 30, 2022, 10:32 p.m. UTC | #3 
Hi,

Am 2022-11-30 16:34, schrieb Nathan Barrett-Morrison:
>> Does this flash have SFDP data? If possible, this should be
>> derived from that. Could you dump the SFDP table and
>> post it here [1].
> 
> # hexdump sfdp
> 0000000 4653 5044 0106 ff01 0600 1001 0030 ff00
> 0000010 0084 0201 0080 ff00 ffff ffff ffff ffff
> 0000020 ffff ffff ffff ffff ffff ffff ffff ffff
> 0000030 20e5 ff8a ffff 0fff 0000 0000 0000 0000
> 0000040 fffe ffff ffff ff00 ffff 0000 200c d811
> 0000050 520f ff00 2224 00a9 8e8b d103 01ac 3827
> 0000060 757a 757a bdfb 5cd5 0000 ff70 b081 2238
> 0000070 ffff ffff ffff ffff ffff ffff ffff ffff
> 0000080 0e43 ffff dc21 ff5c
> 
> Looking at the latest SFDP document from
> https://www.jedec.org/standards-documents/docs/jesd216b, I see
> 1s-1s-8s would be in BFPT DWORD 17, which appears to be 0xffffffff if
> I'm reading this hexdump correctly.

There is no dword 17, the table is shorter than that. But
there is a 4BAIT table at the end, starting at offset 80h.
And from what I can parse with my sleepy eyes, it says
"support for 1s-1s-8s via 7Ch" and "support for 1s-8s-8s via
CCh", which is consistent with the datasheet.

So all you'd need to do is to extend the sfdp parser to parse
that modes in the 4bait table.

Btw there is a newer JESD216F (you can get it from JEDEC for
free, you just have to sign up there).

>> why?
> 
> This was because ISSI's default_init was setting a quad_enable
> function pointer which is not relevant to this part.  This probably
> doesn't need to be done though, as SPI_NOR_QUAD_* isn't being set in
> the flash_info table and therefore quad_enable will never be used?

Yes.

The SFDP specifies 111b as the enable method which is reserved
according to JESD216F. I still vote for setting the quad_enable
to NULL if there is SFDP which should know better. This should
be "fixed" (IOW unset) by the following patch (but it never
made it):
https://lore.kernel.org/linux-mtd/20220304185137.3376011-1-michael@walle.cc/

-michael
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/issi.c b/drivers/mtd/spi-nor/issi.c
index 89a66a19d754..e9b32b726bf3 100644
--- a/drivers/mtd/spi-nor/issi.c
+++ b/drivers/mtd/spi-nor/issi.c
@@ -29,6 +29,21 @@  static const struct spi_nor_fixups is25lp256_fixups = {
 	.post_bfpt = is25lp256_post_bfpt_fixups,
 };
 
+static void is25lx256_post_sfdp_fixup(struct spi_nor *nor)
+{
+	/* Fixup page program command to 1S-1S-8S */
+	nor->params->hwcaps.mask |= SNOR_HWCAPS_PP_1_1_8;
+	spi_nor_set_pp_settings(&nor->params->page_programs[SNOR_CMD_PP_1_1_8],
+				SPINOR_OP_PP_1_1_8, SNOR_PROTO_1_1_8);
+
+	/* Disable quad_enable */
+	nor->params->quad_enable = NULL;
+}
+
+static struct spi_nor_fixups is25lx256_fixups = {
+	.post_sfdp = is25lx256_post_sfdp_fixup,
+};
+
 static void pm25lv_nor_late_init(struct spi_nor *nor)
 {
 	struct spi_nor_erase_map *map = &nor->params->erase_map;
@@ -74,6 +89,10 @@  static const struct flash_info issi_nor_parts[] = {
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
 		.fixups = &is25lp256_fixups },
+	{ "is25lx256",  INFO(0x9d5a19, 0, 128 * 1024, 256)
+		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_OCTAL_READ)
+		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
+		.fixups = &is25lx256_fixups },
 
 	/* PMC */
 	{ "pm25lv512",   INFO(0,        0, 32 * 1024,    2)