mtd: spi-nor: spansion: fix writes on S25FS512S

Spansion S25FS-S family has an issue in the Basic Flash Parameter Table
(BFPT): Dword-11 bits 7:4 specify a page size of 512 bytes.  Actually
this is configurable in the vendor unique register (CR3V) and even the
factory default setting is to "wrap at 256 bytes", so blindly relying
on BFPT breaks the page writes on these chips. Add the post-BFPT fixup
which restores the default page size of 256 bytes -- to properly read
CR3V this early is quite intrusive and should better be done as a new
feature; Alexander Sverdlin had the patch doing that:

https://patchwork.ozlabs.org/project/linux-mtd/patch/20200227123657.26030-1-alexander.sverdlin@nokia.com/

Fixes: dfd2b74530e ("mtd: spi-nor: add Spansion S25FS512S ID")
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
This patch is against the 'mtd/fixes' branch of the MTD 'linux.git' repo.

 drivers/mtd/spi-nor/spansion.c |   25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

Hi!

On 20/04/2020 21:13, Sergei Shtylyov wrote:
> Spansion S25FS-S family has an issue in the Basic Flash Parameter Table
> (BFPT): Dword-11 bits 7:4 specify a page size of 512 bytes.  Actually
> this is configurable in the vendor unique register (CR3V) and even the
> factory default setting is to "wrap at 256 bytes", so blindly relying
> on BFPT breaks the page writes on these chips. Add the post-BFPT fixup
> which restores the default page size of 256 bytes -- to properly read
> CR3V this early is quite intrusive and should better be done as a new
> feature; Alexander Sverdlin had the patch doing that:
> 
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20200227123657.26030-1-alexander.sverdlin@nokia.com/

Right, you patch has a performance impact, but at least unf*cks the chip.

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

> Fixes: dfd2b74530e ("mtd: spi-nor: add Spansion S25FS512S ID")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---
> This patch is against the 'mtd/fixes' branch of the MTD 'linux.git' repo.
> 
>  drivers/mtd/spi-nor/spansion.c |   25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> Index: linux/drivers/mtd/spi-nor/spansion.c
> ===================================================================
> --- linux.orig/drivers/mtd/spi-nor/spansion.c
> +++ linux/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,27 @@
>  
>  #include "core.h"
>  
> +static int
> +s25fs_s_post_bfpt_fixups(struct spi_nor *nor,
> +			 const struct sfdp_parameter_header *bfpt_header,
> +			 const struct sfdp_bfpt *bfpt,
> +			 struct spi_nor_flash_parameter *params)
> +{
> +	/*
> +	 * The S25FS-S chip family reports 512-byte pages in BFPT but
> +	 * in reality the write buffer still wraps at the safe default
> +	 * of 256 bytes.  Overwrite the page size advertised by BFPT
> +	 * to get the writes working.
> +	 */
> +	params->page_size = 256;
> +
> +	return 0;
> +}
> +
> +static struct spi_nor_fixups s25fs_s_fixups = {
> +	.post_bfpt = s25fs_s_post_bfpt_fixups,
> +};
> +
>  static const struct flash_info spansion_parts[] = {
>  	/* Spansion/Cypress -- single (large) sector size only, at least
>  	 * for the chips listed here (without boot sectors).
> @@ -30,8 +51,8 @@ static const struct flash_info spansion_
>  			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>  			      SPI_NOR_HAS_LOCK | USE_CLSR) },
>  	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256,
> -			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -			      USE_CLSR) },
> +			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
> +	  .fixups = &s25fs_s_fixups, },
>  	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>  	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
>  	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
>

Hi Sergei,

I tested the flash and check the datasheet, s25fs128s0 may also need this fixup. So I
will add it in the next version of my patch.

Thanks,
Yicong

On 2020/4/21 3:13, Sergei Shtylyov wrote:
> Spansion S25FS-S family has an issue in the Basic Flash Parameter Table
> (BFPT): Dword-11 bits 7:4 specify a page size of 512 bytes.  Actually
> this is configurable in the vendor unique register (CR3V) and even the
> factory default setting is to "wrap at 256 bytes", so blindly relying
> on BFPT breaks the page writes on these chips. Add the post-BFPT fixup
> which restores the default page size of 256 bytes -- to properly read
> CR3V this early is quite intrusive and should better be done as a new
> feature; Alexander Sverdlin had the patch doing that:
>
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20200227123657.26030-1-alexander.sverdlin@nokia.com/
>
> Fixes: dfd2b74530e ("mtd: spi-nor: add Spansion S25FS512S ID")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> This patch is against the 'mtd/fixes' branch of the MTD 'linux.git' repo.
>
>  drivers/mtd/spi-nor/spansion.c |   25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
>
> Index: linux/drivers/mtd/spi-nor/spansion.c
> ===================================================================
> --- linux.orig/drivers/mtd/spi-nor/spansion.c
> +++ linux/drivers/mtd/spi-nor/spansion.c
> @@ -8,6 +8,27 @@
>  
>  #include "core.h"
>  
> +static int
> +s25fs_s_post_bfpt_fixups(struct spi_nor *nor,
> +			 const struct sfdp_parameter_header *bfpt_header,
> +			 const struct sfdp_bfpt *bfpt,
> +			 struct spi_nor_flash_parameter *params)
> +{
> +	/*
> +	 * The S25FS-S chip family reports 512-byte pages in BFPT but
> +	 * in reality the write buffer still wraps at the safe default
> +	 * of 256 bytes.  Overwrite the page size advertised by BFPT
> +	 * to get the writes working.
> +	 */
> +	params->page_size = 256;
> +
> +	return 0;
> +}
> +
> +static struct spi_nor_fixups s25fs_s_fixups = {
> +	.post_bfpt = s25fs_s_post_bfpt_fixups,
> +};
> +
>  static const struct flash_info spansion_parts[] = {
>  	/* Spansion/Cypress -- single (large) sector size only, at least
>  	 * for the chips listed here (without boot sectors).
> @@ -30,8 +51,8 @@ static const struct flash_info spansion_
>  			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>  			      SPI_NOR_HAS_LOCK | USE_CLSR) },
>  	{ "s25fs512s",  INFO6(0x010220, 0x4d0081, 256 * 1024, 256,
> -			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> -			      USE_CLSR) },
> +			      SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | USE_CLSR)
> +	  .fixups = &s25fs_s_fixups, },
>  	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
>  	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
>  	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
> .
>

> -----Original Message-----
> From: linux-mtd <linux-mtd-bounces@lists.infradead.org> On Behalf Of
> Sergei Shtylyov
> Sent: Tuesday, April 21, 2020 12:44 AM
> To: Tudor Ambarus <tudor.ambarus@microchip.com>; linux-
> mtd@lists.infradead.org
> Cc: Richard Weinberger <richard@nod.at>; Yicong Yang
> <yangyicong@hisilicon.com>; Alexander Sverdlin
> <alexander.sverdlin@nokia.com>; Vignesh Raghavendra <vigneshr@ti.com>;
> Miquel Raynal <miquel.raynal@bootlin.com>
> Subject: [EXT] [PATCH] mtd: spi-nor: spansion: fix writes on S25FS512S
> 
> Caution: EXT Email
> 
> Spansion S25FS-S family has an issue in the Basic Flash Parameter Table
> (BFPT): Dword-11 bits 7:4 specify a page size of 512 bytes.  Actually this is
> configurable in the vendor unique register (CR3V) and even the factory
> default setting is to "wrap at 256 bytes", so blindly relying on BFPT breaks
> the page writes on these chips. Add the post-BFPT fixup which restores the
> default page size of 256 bytes -- to properly read CR3V this early is quite
> intrusive and should better be done as a new feature; Alexander Sverdlin
> had the patch doing that:
> 
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fproject%2Flinux-
> mtd%2Fpatch%2F20200227123657.26030-1-
> alexander.sverdlin%40nokia.com%2F&amp;data=02%7C01%7Ckuldeep.sing
> h%40nxp.com%7Cb99b2075b3cc44c5f06108d7e55f0342%7C686ea1d3bc2b4
> c6fa92cd99c5c301635%7C0%7C0%7C637230068559206386&amp;sdata=BN
> ebniG68OFN7uJr1%2Fpm7wiQq6EPASNPrwEusHfc6zc%3D&amp;reserved=0
> 
> Fixes: dfd2b74530e ("mtd: spi-nor: add Spansion S25FS512S ID")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Tested-by: Kuldeep Singh <kuldeep.singh@nxp.com>
This page size fixup is required for various NXP platforms to run quad mode successfully.

-Kuldeep

On Monday, April 20, 2020 10:13:58 PM EEST Sergei Shtylyov wrote:
> Spansion S25FS-S family has an issue in the Basic Flash Parameter Table
> (BFPT): Dword-11 bits 7:4 specify a page size of 512 bytes.  Actually
> this is configurable in the vendor unique register (CR3V) and even the
> factory default setting is to "wrap at 256 bytes", so blindly relying
> on BFPT breaks the page writes on these chips. Add the post-BFPT fixup
> which restores the default page size of 256 bytes -- to properly read
> CR3V this early is quite intrusive and should better be done as a new
> feature; Alexander Sverdlin had the patch doing that:
> 
> https://patchwork.ozlabs.org/project/linux-mtd/patch/20200227123657.26030-1-> alexander.sverdlin@nokia.com/
> 
> Fixes: dfd2b74530e ("mtd: spi-nor: add Spansion S25FS512S ID")
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> 
> ---

Applied, thanks.