linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf
@ 2018-11-26 12:45 Tudor.Ambarus
  2018-11-26 13:04 ` Sverdlin, Alexander (Nokia - DE/Ulm)
  2018-11-30  9:37 ` [v2] " Boris Brezillon
  0 siblings, 2 replies; 4+ messages in thread
From: Tudor.Ambarus @ 2018-11-26 12:45 UTC (permalink / raw)
  To: marek.vasut, dwmw2, computersforpeace, boris.brezillon, richard
  Cc: linux-mtd, linux-kernel, alexander.sverdlin, Tudor.Ambarus

From: Tudor Ambarus <tudor.ambarus@microchip.com>

BFPT advertises all the erase types supported by all the possible
map configurations. Mask out the erase types that are not supported
by the current map configuration.

Backward compatibility test done on sst26vf064b.

Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
v2:
- drop const qualifier
- update commit message and comment

 drivers/mtd/spi-nor/spi-nor.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 93c9bc8931fc..1fdd2834fbcb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2995,12 +2995,13 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 					      const u32 *smpt)
 {
 	struct spi_nor_erase_map *map = &nor->erase_map;
-	const struct spi_nor_erase_type *erase = map->erase_type;
+	struct spi_nor_erase_type *erase = map->erase_type;
 	struct spi_nor_erase_region *region;
 	u64 offset;
 	u32 region_count;
 	int i, j;
-	u8 erase_type, uniform_erase_type;
+	u8 uniform_erase_type, save_uniform_erase_type;
+	u8 erase_type, regions_erase_type;
 
 	region_count = SMPT_MAP_REGION_COUNT(*smpt);
 	/*
@@ -3014,6 +3015,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 	map->regions = region;
 
 	uniform_erase_type = 0xff;
+	regions_erase_type = 0;
 	offset = 0;
 	/* Populate regions. */
 	for (i = 0; i < region_count; i++) {
@@ -3030,13 +3032,38 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 		 */
 		uniform_erase_type &= erase_type;
 
+		/*
+		 * regions_erase_type mask will indicate all the erase types
+		 * supported in this configuration map.
+		 */
+		regions_erase_type |= erase_type;
+
 		offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
 			 region[i].size;
 	}
 
+	save_uniform_erase_type = map->uniform_erase_type;
 	map->uniform_erase_type = spi_nor_sort_erase_mask(map,
 							  uniform_erase_type);
 
+	if (!regions_erase_type) {
+		/*
+		 * Roll back to the previous uniform_erase_type mask, SMPT is
+		 * broken.
+		 */
+		map->uniform_erase_type = save_uniform_erase_type;
+		return -EINVAL;
+	}
+
+	/*
+	 * BFPT advertises all the erase types supported by all the possible
+	 * map configurations. Mask out the erase types that are not supported
+	 * by the current map configuration.
+	 */
+	for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
+		if (!(regions_erase_type & BIT(erase[i].idx)))
+			spi_nor_set_erase_type(&erase[i], 0, 0xFF);
+
 	spi_nor_region_mark_end(&region[i - 1]);
 
 	return 0;
-- 
2.9.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf
  2018-11-26 12:45 [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf Tudor.Ambarus
@ 2018-11-26 13:04 ` Sverdlin, Alexander (Nokia - DE/Ulm)
  2018-11-28 14:17   ` Boris Brezillon
  2018-11-30  9:37 ` [v2] " Boris Brezillon
  1 sibling, 1 reply; 4+ messages in thread
From: Sverdlin, Alexander (Nokia - DE/Ulm) @ 2018-11-26 13:04 UTC (permalink / raw)
  To: Tudor.Ambarus, marek.vasut, dwmw2, computersforpeace,
	boris.brezillon, richard
  Cc: linux-mtd, linux-kernel

Hello Tudor and all,

On 26/11/2018 13:45, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> BFPT advertises all the erase types supported by all the possible
> map configurations. Mask out the erase types that are not supported
> by the current map configuration.
> 
> Backward compatibility test done on sst26vf064b.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

I've verified the erasesize, partitioning and erase function of the S25FS128S
and all the above is at least back to pre-SFDP state.

So the patch works as intended and it's
Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

There is still a regression with write when S25FS128S is used with SFDP
because SFDP of the S25FS128S is just broken. It advertises 512 bytes
of write page size but factory delivery configuration wraps the address
on 256 bytes boundary. I found no way to fetch this configuration in a
vendor-neutral way (or JEDEC-conform way). Which means, S25FS128S
could be the first user of SPI_NOR_SKIP_SFDP flag :\

> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
> v2:
> - drop const qualifier
> - update commit message and comment
> 
>  drivers/mtd/spi-nor/spi-nor.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 93c9bc8931fc..1fdd2834fbcb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2995,12 +2995,13 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  					      const u32 *smpt)
>  {
>  	struct spi_nor_erase_map *map = &nor->erase_map;
> -	const struct spi_nor_erase_type *erase = map->erase_type;
> +	struct spi_nor_erase_type *erase = map->erase_type;
>  	struct spi_nor_erase_region *region;
>  	u64 offset;
>  	u32 region_count;
>  	int i, j;
> -	u8 erase_type, uniform_erase_type;
> +	u8 uniform_erase_type, save_uniform_erase_type;
> +	u8 erase_type, regions_erase_type;
>  
>  	region_count = SMPT_MAP_REGION_COUNT(*smpt);
>  	/*
> @@ -3014,6 +3015,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  	map->regions = region;
>  
>  	uniform_erase_type = 0xff;
> +	regions_erase_type = 0;
>  	offset = 0;
>  	/* Populate regions. */
>  	for (i = 0; i < region_count; i++) {
> @@ -3030,13 +3032,38 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  		 */
>  		uniform_erase_type &= erase_type;
>  
> +		/*
> +		 * regions_erase_type mask will indicate all the erase types
> +		 * supported in this configuration map.
> +		 */
> +		regions_erase_type |= erase_type;
> +
>  		offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
>  			 region[i].size;
>  	}
>  
> +	save_uniform_erase_type = map->uniform_erase_type;
>  	map->uniform_erase_type = spi_nor_sort_erase_mask(map,
>  							  uniform_erase_type);
>  
> +	if (!regions_erase_type) {
> +		/*
> +		 * Roll back to the previous uniform_erase_type mask, SMPT is
> +		 * broken.
> +		 */
> +		map->uniform_erase_type = save_uniform_erase_type;
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * BFPT advertises all the erase types supported by all the possible
> +	 * map configurations. Mask out the erase types that are not supported
> +	 * by the current map configuration.
> +	 */
> +	for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
> +		if (!(regions_erase_type & BIT(erase[i].idx)))
> +			spi_nor_set_erase_type(&erase[i], 0, 0xFF);
> +
>  	spi_nor_region_mark_end(&region[i - 1]);
>  
>  	return 0;

-- 
Best regards,
Alexander Sverdlin.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf
  2018-11-26 13:04 ` Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-11-28 14:17   ` Boris Brezillon
  0 siblings, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-11-28 14:17 UTC (permalink / raw)
  To: Sverdlin, Alexander (Nokia - DE/Ulm)
  Cc: Tudor.Ambarus, marek.vasut, dwmw2, computersforpeace, richard,
	linux-mtd, linux-kernel

Hi Alexander,

On Mon, 26 Nov 2018 13:04:45 +0000
"Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@nokia.com>
wrote:

> Hello Tudor and all,
> 
> On 26/11/2018 13:45, Tudor.Ambarus@microchip.com wrote:
> > From: Tudor Ambarus <tudor.ambarus@microchip.com>
> > 
> > BFPT advertises all the erase types supported by all the possible
> > map configurations. Mask out the erase types that are not supported
> > by the current map configuration.
> > 
> > Backward compatibility test done on sst26vf064b.
> > 
> > Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> > Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>  
> 
> I've verified the erasesize, partitioning and erase function of the S25FS128S
> and all the above is at least back to pre-SFDP state.
> 
> So the patch works as intended and it's
> Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> 
> There is still a regression with write when S25FS128S is used with SFDP
> because SFDP of the S25FS128S is just broken. It advertises 512 bytes
> of write page size but factory delivery configuration wraps the address
> on 256 bytes boundary. I found no way to fetch this configuration in a
> vendor-neutral way (or JEDEC-conform way). Which means, S25FS128S
> could be the first user of SPI_NOR_SKIP_SFDP flag :\.

You might be interested in my work around SPI NOR fixups[1]. I added a
few fixup hooks that can be implemented by manufacturer drivers (a new
concept) or on a per-chip basis. Looks like you'd need to implement a
->post_bfpt() fixup hook for this particular chip (see what's been done
for this Macronix chip [2]).

Regards,

Boris

[1]https://github.com/bbrezillon/linux-0day/commits/spi-nor/manuf-drv
[2]https://github.com/bbrezillon/linux-0day/commit/b29625fdc55fa8ccfd4299904d727b44f8382c18

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [v2] mtd: spi-nor: fix erase_type array to indicate current map conf
  2018-11-26 12:45 [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf Tudor.Ambarus
  2018-11-26 13:04 ` Sverdlin, Alexander (Nokia - DE/Ulm)
@ 2018-11-30  9:37 ` Boris Brezillon
  1 sibling, 0 replies; 4+ messages in thread
From: Boris Brezillon @ 2018-11-30  9:37 UTC (permalink / raw)
  To: Tudor.Ambarus, marek.vasut, dwmw2, computersforpeace,
	boris.brezillon, richard
  Cc: alexander.sverdlin, linux-mtd, linux-kernel

On Mon, 2018-11-26 at 12:45:44 UTC,  wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> BFPT advertises all the erase types supported by all the possible
> map configurations. Mask out the erase types that are not supported
> by the current map configuration.
> 
> Backward compatibility test done on sst26vf064b.
> 
> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
> Reported-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> Tested-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

Applied to http://git.infradead.org/linux-mtd.git master, thanks.

Boris

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-11-30  9:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26 12:45 [PATCH v2] mtd: spi-nor: fix erase_type array to indicate current map conf Tudor.Ambarus
2018-11-26 13:04 ` Sverdlin, Alexander (Nokia - DE/Ulm)
2018-11-28 14:17   ` Boris Brezillon
2018-11-30  9:37 ` [v2] " Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).