[PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

In v4, split two flags removement into separated patches. Reworked
spi_nor_init_erase_cmd() to use n_regions in outer loop.

In v3, reworked to remove LAST_REGION and OVERLAID_REGION flags. Updated
and tested debugfs.

In v2, reworked spi_nor_erase_region to stop bitmask encoding in offset.
And added n_regions member to spi_nor_erase_map.

Takahiro Kuwano (4):
  mtd: spi-nor: core: rework struct spi_nor_erase_region
  mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag
  mtd: spi-nor: core: get rid of SNOR_OVERLAID_REGION flag
  mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

 drivers/mtd/spi-nor/core.c    | 187 +++++++++++++++++-----------------
 drivers/mtd/spi-nor/core.h    |  30 ++----
 drivers/mtd/spi-nor/debugfs.c |  26 +++--
 drivers/mtd/spi-nor/sfdp.c    |  47 +++------
 4 files changed, 128 insertions(+), 162 deletions(-)

-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 1/4] mtd: spi-nor: core: rework struct spi_nor_erase_region

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Encoding bitmask flags into offset worsen the code readability. The
erase type mask and flags should be stored in dedicated members. Also,
erase_map.uniform_erase_type can be removed as it is redundant.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Suggested-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/core.c    | 35 +++++++++++++++--------------------
 drivers/mtd/spi-nor/core.h    | 28 ++++++++++------------------
 drivers/mtd/spi-nor/debugfs.c | 13 +++++++------
 drivers/mtd/spi-nor/sfdp.c    | 30 +++++++++++++-----------------
 4 files changed, 45 insertions(+), 61 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1b0c6770c14e..5b2f13d0888e 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1150,7 +1150,7 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)
 
 static bool spi_nor_has_uniform_erase(const struct spi_nor *nor)
 {
-	return !!nor->params->erase_map.uniform_erase_type;
+	return !!nor->params->erase_map.uniform_region.erase_mask;
 }
 
 static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
@@ -1534,7 +1534,6 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 	const struct spi_nor_erase_type *erase;
 	u32 rem;
 	int i;
-	u8 erase_mask = region->offset & SNOR_ERASE_TYPE_MASK;
 
 	/*
 	 * Erase types are ordered by size, with the smallest erase type at
@@ -1542,7 +1541,7 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 	 */
 	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
 		/* Does the erase region support the tested erase type? */
-		if (!(erase_mask & BIT(i)))
+		if (!(region->erase_mask & BIT(i)))
 			continue;
 
 		erase = &map->erase_type[i];
@@ -1550,7 +1549,7 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 			continue;
 
 		/* Alignment is not mandatory for overlaid regions */
-		if (region->offset & SNOR_OVERLAID_REGION &&
+		if (region->flags & SNOR_OVERLAID_REGION &&
 		    region->size <= len)
 			return erase;
 
@@ -1568,12 +1567,12 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 
 static u64 spi_nor_region_is_last(const struct spi_nor_erase_region *region)
 {
-	return region->offset & SNOR_LAST_REGION;
+	return region->flags & SNOR_LAST_REGION;
 }
 
 static u64 spi_nor_region_end(const struct spi_nor_erase_region *region)
 {
-	return (region->offset & ~SNOR_ERASE_FLAGS_MASK) + region->size;
+	return region->offset + region->size;
 }
 
 /**
@@ -1604,16 +1603,14 @@ static struct spi_nor_erase_region *
 spi_nor_find_erase_region(const struct spi_nor_erase_map *map, u64 addr)
 {
 	struct spi_nor_erase_region *region = map->regions;
-	u64 region_start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
-	u64 region_end = region_start + region->size;
+	u64 region_end = spi_nor_region_end(region);
 
-	while (addr < region_start || addr >= region_end) {
+	while (addr < region->offset || addr >= region_end) {
 		region = spi_nor_region_next(region);
 		if (!region)
 			return ERR_PTR(-EINVAL);
 
-		region_start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
-		region_end = region_start + region->size;
+		region_end = spi_nor_region_end(region);
 	}
 
 	return region;
@@ -1641,7 +1638,7 @@ spi_nor_init_erase_cmd(const struct spi_nor_erase_region *region,
 	cmd->opcode = erase->opcode;
 	cmd->count = 1;
 
-	if (region->offset & SNOR_OVERLAID_REGION)
+	if (region->flags & SNOR_OVERLAID_REGION)
 		cmd->size = region->size;
 	else
 		cmd->size = erase->size;
@@ -1700,7 +1697,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
 
 		if (prev_erase != erase ||
 		    erase->size != cmd->size ||
-		    region->offset & SNOR_OVERLAID_REGION) {
+		    region->flags & SNOR_OVERLAID_REGION) {
 			cmd = spi_nor_init_erase_cmd(region, erase);
 			if (IS_ERR(cmd)) {
 				ret = PTR_ERR(cmd);
@@ -2439,12 +2436,11 @@ void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase)
 void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
 				    u8 erase_mask, u64 flash_size)
 {
-	/* Offset 0 with erase_mask and SNOR_LAST_REGION bit set */
-	map->uniform_region.offset = (erase_mask & SNOR_ERASE_TYPE_MASK) |
-				     SNOR_LAST_REGION;
+	map->uniform_region.offset = 0;
 	map->uniform_region.size = flash_size;
+	map->uniform_region.erase_mask = erase_mask;
+	map->uniform_region.flags = SNOR_LAST_REGION;
 	map->regions = &map->uniform_region;
-	map->uniform_erase_type = erase_mask;
 }
 
 int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
@@ -2539,7 +2535,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
 {
 	const struct spi_nor_erase_type *tested_erase, *erase = NULL;
 	int i;
-	u8 uniform_erase_type = map->uniform_erase_type;
+	u8 uniform_erase_type = map->uniform_region.erase_mask;
 
 	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
 		if (!(uniform_erase_type & BIT(i)))
@@ -2573,8 +2569,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
 		return NULL;
 
 	/* Disable all other Sector Erase commands. */
-	map->uniform_erase_type &= ~SNOR_ERASE_TYPE_MASK;
-	map->uniform_erase_type |= BIT(erase - map->erase_type);
+	map->uniform_region.erase_mask = BIT(erase - map->erase_type);
 	return erase;
 }
 
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 9217379b9cfe..e002de22b18a 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -233,27 +233,25 @@ struct spi_nor_erase_command {
 /**
  * struct spi_nor_erase_region - Structure to describe a SPI NOR erase region
  * @offset:		the offset in the data array of erase region start.
- *			LSB bits are used as a bitmask encoding flags to
- *			determine if this region is overlaid, if this region is
- *			the last in the SPI NOR flash memory and to indicate
- *			all the supported erase commands inside this region.
- *			The erase types are sorted in ascending order with the
- *			smallest Erase Type size being at BIT(0).
  * @size:		the size of the region in bytes.
+ * @erase_mask:		bitmask to indicate all the supported erase commands
+ *			inside this region. The erase types are sorted in
+ *			ascending order with the smallest Erase Type size being
+ *			at BIT(0).
+ * @flags:		flags to determine if this region is overlaid, if this
+ *			region is the last in the SPI NOR flash memory
  */
 struct spi_nor_erase_region {
 	u64		offset;
 	u64		size;
+	u8		erase_mask;
+	u8		flags;
 };
 
 #define SNOR_ERASE_TYPE_MAX	4
-#define SNOR_ERASE_TYPE_MASK	GENMASK_ULL(SNOR_ERASE_TYPE_MAX - 1, 0)
 
-#define SNOR_LAST_REGION	BIT(4)
-#define SNOR_OVERLAID_REGION	BIT(5)
-
-#define SNOR_ERASE_FLAGS_MAX	6
-#define SNOR_ERASE_FLAGS_MASK	GENMASK_ULL(SNOR_ERASE_FLAGS_MAX - 1, 0)
+#define SNOR_LAST_REGION	BIT(0)
+#define SNOR_OVERLAID_REGION	BIT(1)
 
 /**
  * struct spi_nor_erase_map - Structure to describe the SPI NOR erase map
@@ -266,17 +264,11 @@ struct spi_nor_erase_region {
  *			The erase types are sorted in ascending order, with the
  *			smallest Erase Type size being the first member in the
  *			erase_type array.
- * @uniform_erase_type:	bitmask encoding erase types that can erase the
- *			entire memory. This member is completed at init by
- *			uniform and non-uniform SPI NOR flash memories if they
- *			support at least one erase type that can erase the
- *			entire memory.
  */
 struct spi_nor_erase_map {
 	struct spi_nor_erase_region	*regions;
 	struct spi_nor_erase_region	uniform_region;
 	struct spi_nor_erase_type	erase_type[SNOR_ERASE_TYPE_MAX];
-	u8				uniform_erase_type;
 };
 
 /**
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 6e163cb5b478..16b30f1a3066 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -147,16 +147,17 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 	for (region = erase_map->regions;
 	     region;
 	     region = spi_nor_region_next(region)) {
-		u64 start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
-		u64 flags = region->offset & SNOR_ERASE_FLAGS_MASK;
+		u64 start = region->offset;
 		u64 end = start + region->size - 1;
+		u8 erase_mask = region->erase_mask;
+		u8 flags = region->flags;
 
 		seq_printf(s, " %08llx-%08llx |     [%c%c%c%c] | %s\n",
 			   start, end,
-			   flags & BIT(0) ? '0' : ' ',
-			   flags & BIT(1) ? '1' : ' ',
-			   flags & BIT(2) ? '2' : ' ',
-			   flags & BIT(3) ? '3' : ' ',
+			   erase_mask & BIT(0) ? '0' : ' ',
+			   erase_mask & BIT(1) ? '1' : ' ',
+			   erase_mask & BIT(2) ? '2' : ' ',
+			   erase_mask & BIT(3) ? '3' : ' ',
 			   flags & SNOR_OVERLAID_REGION ? "overlaid" : "");
 	}
 
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index b3b11dfed789..c8d8b4e5b7e6 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -389,17 +389,14 @@ static u8 spi_nor_sort_erase_mask(struct spi_nor_erase_map *map, u8 erase_mask)
 static void spi_nor_regions_sort_erase_types(struct spi_nor_erase_map *map)
 {
 	struct spi_nor_erase_region *region = map->regions;
-	u8 region_erase_mask, sorted_erase_mask;
+	u8 sorted_erase_mask;
 
 	while (region) {
-		region_erase_mask = region->offset & SNOR_ERASE_TYPE_MASK;
-
 		sorted_erase_mask = spi_nor_sort_erase_mask(map,
-							    region_erase_mask);
+							    region->erase_mask);
 
 		/* Overwrite erase mask. */
-		region->offset = (region->offset & ~SNOR_ERASE_TYPE_MASK) |
-				 sorted_erase_mask;
+		region->erase_mask = sorted_erase_mask;
 
 		region = spi_nor_region_next(region);
 	}
@@ -553,8 +550,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	 * selecting the uniform erase.
 	 */
 	spi_nor_regions_sort_erase_types(map);
-	map->uniform_erase_type = map->uniform_region.offset &
-				  SNOR_ERASE_TYPE_MASK;
 
 	/* Stop here if not JESD216 rev A or later. */
 	if (bfpt_header->length == BFPT_DWORD_MAX_JESD216)
@@ -781,12 +776,12 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 
 static void spi_nor_region_mark_end(struct spi_nor_erase_region *region)
 {
-	region->offset |= SNOR_LAST_REGION;
+	region->flags |= SNOR_LAST_REGION;
 }
 
 static void spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
 {
-	region->offset |= SNOR_OVERLAID_REGION;
+	region->flags |= SNOR_OVERLAID_REGION;
 }
 
 /**
@@ -848,9 +843,10 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 	/* Populate regions. */
 	for (i = 0; i < region_count; i++) {
 		j = i + 1; /* index for the region dword */
+		region[i].offset = offset;
 		region[i].size = SMPT_MAP_REGION_SIZE(smpt[j]);
 		erase_type = SMPT_MAP_REGION_ERASE_TYPE(smpt[j]);
-		region[i].offset = offset | erase_type;
+		region[i].erase_mask = erase_type;
 
 		spi_nor_region_check_overlay(&region[i], erase, erase_type);
 
@@ -866,21 +862,21 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 		 */
 		regions_erase_type |= erase_type;
 
-		offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
-			 region[i].size;
+		offset = region[i].offset + region[i].size;
 	}
 	spi_nor_region_mark_end(&region[i - 1]);
 
-	save_uniform_erase_type = map->uniform_erase_type;
-	map->uniform_erase_type = spi_nor_sort_erase_mask(map,
-							  uniform_erase_type);
+	save_uniform_erase_type = map->uniform_region.erase_mask;
+	map->uniform_region.erase_mask =
+				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;
+		map->uniform_region.erase_mask = save_uniform_erase_type;
 		return -EINVAL;
 	}
 
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/4] mtd: spi-nor: core: rework struct spi_nor_erase_region

[Michael Walle]

On Tue Feb 20, 2024 at 9:34 AM CET, tkuw584924 wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>
> Encoding bitmask flags into offset worsen the code readability. The
> erase type mask and flags should be stored in dedicated members. Also,
> erase_map.uniform_erase_type can be removed as it is redundant.
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> Suggested-by: Michael Walle <mwalle@kernel.org>
> ---
>  drivers/mtd/spi-nor/core.c    | 35 +++++++++++++++--------------------
>  drivers/mtd/spi-nor/core.h    | 28 ++++++++++------------------
>  drivers/mtd/spi-nor/debugfs.c | 13 +++++++------
>  drivers/mtd/spi-nor/sfdp.c    | 30 +++++++++++++-----------------
>  4 files changed, 45 insertions(+), 61 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 1b0c6770c14e..5b2f13d0888e 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1150,7 +1150,7 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)
>  
>  static bool spi_nor_has_uniform_erase(const struct spi_nor *nor)
>  {
> -	return !!nor->params->erase_map.uniform_erase_type;
> +	return !!nor->params->erase_map.uniform_region.erase_mask;
>  }
>  
>  static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
> @@ -1534,7 +1534,6 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
>  	const struct spi_nor_erase_type *erase;
>  	u32 rem;
>  	int i;
> -	u8 erase_mask = region->offset & SNOR_ERASE_TYPE_MASK;
>  
>  	/*
>  	 * Erase types are ordered by size, with the smallest erase type at
> @@ -1542,7 +1541,7 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
>  	 */
>  	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
>  		/* Does the erase region support the tested erase type? */
> -		if (!(erase_mask & BIT(i)))
> +		if (!(region->erase_mask & BIT(i)))
>  			continue;
>  
>  		erase = &map->erase_type[i];
> @@ -1550,7 +1549,7 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
>  			continue;
>  
>  		/* Alignment is not mandatory for overlaid regions */
> -		if (region->offset & SNOR_OVERLAID_REGION &&
> +		if (region->flags & SNOR_OVERLAID_REGION &&
>  		    region->size <= len)
>  			return erase;
>  
> @@ -1568,12 +1567,12 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
>  
>  static u64 spi_nor_region_is_last(const struct spi_nor_erase_region *region)
>  {
> -	return region->offset & SNOR_LAST_REGION;
> +	return region->flags & SNOR_LAST_REGION;
>  }
>  
>  static u64 spi_nor_region_end(const struct spi_nor_erase_region *region)

I'd drop this helper too. It doesn't add much value anymore.

>  {
> -	return (region->offset & ~SNOR_ERASE_FLAGS_MASK) + region->size;
> +	return region->offset + region->size;
>  }
>  
>  /**
> @@ -1604,16 +1603,14 @@ static struct spi_nor_erase_region *
>  spi_nor_find_erase_region(const struct spi_nor_erase_map *map, u64 addr)
>  {
>  	struct spi_nor_erase_region *region = map->regions;
> -	u64 region_start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
> -	u64 region_end = region_start + region->size;
> +	u64 region_end = spi_nor_region_end(region);
>  
> -	while (addr < region_start || addr >= region_end) {
> +	while (addr < region->offset || addr >= region_end) {

while (addr < region->offset || addr >= region->offset + region->size) {

>  		region = spi_nor_region_next(region);
>  		if (!region)
>  			return ERR_PTR(-EINVAL);
>  
> -		region_start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
> -		region_end = region_start + region->size;
> +		region_end = spi_nor_region_end(region);

Then get rid of the spi_nor_region_end() helper, which I don't see
as useful as it was anymore. Also debugfs.c and sfdp.c don't use
it neither.

>  	}
>  
>  	return region;
> @@ -1641,7 +1638,7 @@ spi_nor_init_erase_cmd(const struct spi_nor_erase_region *region,
>  	cmd->opcode = erase->opcode;
>  	cmd->count = 1;
>  
> -	if (region->offset & SNOR_OVERLAID_REGION)
> +	if (region->flags & SNOR_OVERLAID_REGION)
>  		cmd->size = region->size;
>  	else
>  		cmd->size = erase->size;
> @@ -1700,7 +1697,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
>  
>  		if (prev_erase != erase ||
>  		    erase->size != cmd->size ||
> -		    region->offset & SNOR_OVERLAID_REGION) {
> +		    region->flags & SNOR_OVERLAID_REGION) {
>  			cmd = spi_nor_init_erase_cmd(region, erase);
>  			if (IS_ERR(cmd)) {
>  				ret = PTR_ERR(cmd);
> @@ -2439,12 +2436,11 @@ void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase)
>  void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
>  				    u8 erase_mask, u64 flash_size)
>  {
> -	/* Offset 0 with erase_mask and SNOR_LAST_REGION bit set */
> -	map->uniform_region.offset = (erase_mask & SNOR_ERASE_TYPE_MASK) |
> -				     SNOR_LAST_REGION;
> +	map->uniform_region.offset = 0;
>  	map->uniform_region.size = flash_size;
> +	map->uniform_region.erase_mask = erase_mask;
> +	map->uniform_region.flags = SNOR_LAST_REGION;
>  	map->regions = &map->uniform_region;
> -	map->uniform_erase_type = erase_mask;
>  }
>  
>  int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> @@ -2539,7 +2535,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
>  {
>  	const struct spi_nor_erase_type *tested_erase, *erase = NULL;
>  	int i;
> -	u8 uniform_erase_type = map->uniform_erase_type;
> +	u8 uniform_erase_type = map->uniform_region.erase_mask;
>  
>  	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
>  		if (!(uniform_erase_type & BIT(i)))
> @@ -2573,8 +2569,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
>  		return NULL;
>  
>  	/* Disable all other Sector Erase commands. */
> -	map->uniform_erase_type &= ~SNOR_ERASE_TYPE_MASK;
> -	map->uniform_erase_type |= BIT(erase - map->erase_type);
> +	map->uniform_region.erase_mask = BIT(erase - map->erase_type);
>  	return erase;
>  }
>  
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index 9217379b9cfe..e002de22b18a 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -233,27 +233,25 @@ struct spi_nor_erase_command {
>  /**
>   * struct spi_nor_erase_region - Structure to describe a SPI NOR erase region
>   * @offset:		the offset in the data array of erase region start.
> - *			LSB bits are used as a bitmask encoding flags to
> - *			determine if this region is overlaid, if this region is
> - *			the last in the SPI NOR flash memory and to indicate
> - *			all the supported erase commands inside this region.
> - *			The erase types are sorted in ascending order with the
> - *			smallest Erase Type size being at BIT(0).
>   * @size:		the size of the region in bytes.
> + * @erase_mask:		bitmask to indicate all the supported erase commands
> + *			inside this region. The erase types are sorted in
> + *			ascending order with the smallest Erase Type size being
> + *			at BIT(0).
> + * @flags:		flags to determine if this region is overlaid, if this
> + *			region is the last in the SPI NOR flash memory
>   */
>  struct spi_nor_erase_region {
>  	u64		offset;
>  	u64		size;
> +	u8		erase_mask;
> +	u8		flags;
>  };
>  
>  #define SNOR_ERASE_TYPE_MAX	4
> -#define SNOR_ERASE_TYPE_MASK	GENMASK_ULL(SNOR_ERASE_TYPE_MAX - 1, 0)
>  
> -#define SNOR_LAST_REGION	BIT(4)
> -#define SNOR_OVERLAID_REGION	BIT(5)
> -
> -#define SNOR_ERASE_FLAGS_MAX	6
> -#define SNOR_ERASE_FLAGS_MASK	GENMASK_ULL(SNOR_ERASE_FLAGS_MAX - 1, 0)
> +#define SNOR_LAST_REGION	BIT(0)
> +#define SNOR_OVERLAID_REGION	BIT(1)
>  
>  /**
>   * struct spi_nor_erase_map - Structure to describe the SPI NOR erase map
> @@ -266,17 +264,11 @@ struct spi_nor_erase_region {
>   *			The erase types are sorted in ascending order, with the
>   *			smallest Erase Type size being the first member in the
>   *			erase_type array.
> - * @uniform_erase_type:	bitmask encoding erase types that can erase the
> - *			entire memory. This member is completed at init by
> - *			uniform and non-uniform SPI NOR flash memories if they
> - *			support at least one erase type that can erase the
> - *			entire memory.
>   */
>  struct spi_nor_erase_map {
>  	struct spi_nor_erase_region	*regions;
>  	struct spi_nor_erase_region	uniform_region;
>  	struct spi_nor_erase_type	erase_type[SNOR_ERASE_TYPE_MAX];
> -	u8				uniform_erase_type;
>  };
>  
>  /**
> diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
> index 6e163cb5b478..16b30f1a3066 100644
> --- a/drivers/mtd/spi-nor/debugfs.c
> +++ b/drivers/mtd/spi-nor/debugfs.c
> @@ -147,16 +147,17 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
>  	for (region = erase_map->regions;
>  	     region;
>  	     region = spi_nor_region_next(region)) {
> -		u64 start = region->offset & ~SNOR_ERASE_FLAGS_MASK;
> -		u64 flags = region->offset & SNOR_ERASE_FLAGS_MASK;
> +		u64 start = region->offset;
>  		u64 end = start + region->size - 1;
> +		u8 erase_mask = region->erase_mask;
> +		u8 flags = region->flags;
>  
>  		seq_printf(s, " %08llx-%08llx |     [%c%c%c%c] | %s\n",
>  			   start, end,
> -			   flags & BIT(0) ? '0' : ' ',
> -			   flags & BIT(1) ? '1' : ' ',
> -			   flags & BIT(2) ? '2' : ' ',
> -			   flags & BIT(3) ? '3' : ' ',
> +			   erase_mask & BIT(0) ? '0' : ' ',
> +			   erase_mask & BIT(1) ? '1' : ' ',
> +			   erase_mask & BIT(2) ? '2' : ' ',
> +			   erase_mask & BIT(3) ? '3' : ' ',
>  			   flags & SNOR_OVERLAID_REGION ? "overlaid" : "");
>  	}
>  
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index b3b11dfed789..c8d8b4e5b7e6 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -389,17 +389,14 @@ static u8 spi_nor_sort_erase_mask(struct spi_nor_erase_map *map, u8 erase_mask)
>  static void spi_nor_regions_sort_erase_types(struct spi_nor_erase_map *map)
>  {
>  	struct spi_nor_erase_region *region = map->regions;
> -	u8 region_erase_mask, sorted_erase_mask;
> +	u8 sorted_erase_mask;
>  
>  	while (region) {
> -		region_erase_mask = region->offset & SNOR_ERASE_TYPE_MASK;
> -
>  		sorted_erase_mask = spi_nor_sort_erase_mask(map,
> -							    region_erase_mask);
> +							    region->erase_mask);
>  
>  		/* Overwrite erase mask. */

This comment could also be dropped now. The assignment is pretty
obvious now.

With above remarks fixed:
Reviewed-by: Michael Walle <mwalle@kernel.org>

Maybe Tudor can fix while applying.

-michael

> -		region->offset = (region->offset & ~SNOR_ERASE_TYPE_MASK) |
> -				 sorted_erase_mask;
> +		region->erase_mask = sorted_erase_mask;
>  
>  		region = spi_nor_region_next(region);
>  	}
> @@ -553,8 +550,6 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	 * selecting the uniform erase.
>  	 */
>  	spi_nor_regions_sort_erase_types(map);
> -	map->uniform_erase_type = map->uniform_region.offset &
> -				  SNOR_ERASE_TYPE_MASK;
>  
>  	/* Stop here if not JESD216 rev A or later. */
>  	if (bfpt_header->length == BFPT_DWORD_MAX_JESD216)
> @@ -781,12 +776,12 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  
>  static void spi_nor_region_mark_end(struct spi_nor_erase_region *region)
>  {
> -	region->offset |= SNOR_LAST_REGION;
> +	region->flags |= SNOR_LAST_REGION;
>  }
>  
>  static void spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
>  {
> -	region->offset |= SNOR_OVERLAID_REGION;
> +	region->flags |= SNOR_OVERLAID_REGION;
>  }
>  
>  /**
> @@ -848,9 +843,10 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  	/* Populate regions. */
>  	for (i = 0; i < region_count; i++) {
>  		j = i + 1; /* index for the region dword */
> +		region[i].offset = offset;
>  		region[i].size = SMPT_MAP_REGION_SIZE(smpt[j]);
>  		erase_type = SMPT_MAP_REGION_ERASE_TYPE(smpt[j]);
> -		region[i].offset = offset | erase_type;
> +		region[i].erase_mask = erase_type;
>  
>  		spi_nor_region_check_overlay(&region[i], erase, erase_type);
>  
> @@ -866,21 +862,21 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  		 */
>  		regions_erase_type |= erase_type;
>  
> -		offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
> -			 region[i].size;
> +		offset = region[i].offset + region[i].size;
>  	}
>  	spi_nor_region_mark_end(&region[i - 1]);
>  
> -	save_uniform_erase_type = map->uniform_erase_type;
> -	map->uniform_erase_type = spi_nor_sort_erase_mask(map,
> -							  uniform_erase_type);
> +	save_uniform_erase_type = map->uniform_region.erase_mask;
> +	map->uniform_region.erase_mask =
> +				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;
> +		map->uniform_region.erase_mask = save_uniform_erase_type;
>  		return -EINVAL;
>  	}
>  


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 1/4] mtd: spi-nor: core: rework struct spi_nor_erase_region

[Tudor Ambarus]

On 2/23/24 10:14, Michael Walle wrote:
> With above remarks fixed:
> Reviewed-by: Michael Walle <mwalle@kernel.org>
> 
> Maybe Tudor can fix while applying.


sure, I can do it. Thanks for reviewing the series. I'll take a look at
it later today.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 2/4] mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Introduce n_regions in spi_nor_erase_map structure and remove
SNOR_LAST_REGION flag. Loop logics that depend on the flag are also
reworked to use n_regions as loop condition.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Suggested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Suggested-by: Michael Walle <mwalle@kernel.org>
---
 drivers/mtd/spi-nor/core.c    | 105 +++++++++-------------------------
 drivers/mtd/spi-nor/core.h    |  10 ++--
 drivers/mtd/spi-nor/debugfs.c |  16 +++---
 drivers/mtd/spi-nor/sfdp.c    |  18 ++----
 4 files changed, 43 insertions(+), 106 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 5b2f13d0888e..d9a0c0e31950 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1565,57 +1565,11 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 	return NULL;
 }
 
-static u64 spi_nor_region_is_last(const struct spi_nor_erase_region *region)
-{
-	return region->flags & SNOR_LAST_REGION;
-}
-
 static u64 spi_nor_region_end(const struct spi_nor_erase_region *region)
 {
 	return region->offset + region->size;
 }
 
-/**
- * spi_nor_region_next() - get the next spi nor region
- * @region:	pointer to a structure that describes a SPI NOR erase region
- *
- * Return: the next spi nor region or NULL if last region.
- */
-struct spi_nor_erase_region *
-spi_nor_region_next(struct spi_nor_erase_region *region)
-{
-	if (spi_nor_region_is_last(region))
-		return NULL;
-	region++;
-	return region;
-}
-
-/**
- * spi_nor_find_erase_region() - find the region of the serial flash memory in
- *				 which the offset fits
- * @map:	the erase map of the SPI NOR
- * @addr:	offset in the serial flash memory
- *
- * Return: a pointer to the spi_nor_erase_region struct, ERR_PTR(-errno)
- *	   otherwise.
- */
-static struct spi_nor_erase_region *
-spi_nor_find_erase_region(const struct spi_nor_erase_map *map, u64 addr)
-{
-	struct spi_nor_erase_region *region = map->regions;
-	u64 region_end = spi_nor_region_end(region);
-
-	while (addr < region->offset || addr >= region_end) {
-		region = spi_nor_region_next(region);
-		if (!region)
-			return ERR_PTR(-EINVAL);
-
-		region_end = spi_nor_region_end(region);
-	}
-
-	return region;
-}
-
 /**
  * spi_nor_init_erase_cmd() - initialize an erase command
  * @region:	pointer to a structure that describes a SPI NOR erase region
@@ -1682,48 +1636,41 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
 	struct spi_nor_erase_region *region;
 	struct spi_nor_erase_command *cmd = NULL;
 	u64 region_end;
+	unsigned int i;
 	int ret = -EINVAL;
 
-	region = spi_nor_find_erase_region(map, addr);
-	if (IS_ERR(region))
-		return PTR_ERR(region);
-
-	region_end = spi_nor_region_end(region);
+	for (i = 0; i < map->n_regions; i++) {
+		region = &map->regions[i];
+		region_end = spi_nor_region_end(region);
 
-	while (len) {
-		erase = spi_nor_find_best_erase_type(map, region, addr, len);
-		if (!erase)
-			goto destroy_erase_cmd_list;
-
-		if (prev_erase != erase ||
-		    erase->size != cmd->size ||
-		    region->flags & SNOR_OVERLAID_REGION) {
-			cmd = spi_nor_init_erase_cmd(region, erase);
-			if (IS_ERR(cmd)) {
-				ret = PTR_ERR(cmd);
+		while (len && addr >= region->offset && addr < region_end) {
+			erase = spi_nor_find_best_erase_type(map, region, addr,
+							     len);
+			if (!erase)
 				goto destroy_erase_cmd_list;
-			}
 
-			list_add_tail(&cmd->list, erase_list);
-		} else {
-			cmd->count++;
-		}
-
-		addr += cmd->size;
-		len -= cmd->size;
+			if (prev_erase != erase || erase->size != cmd->size ||
+			    region->flags & SNOR_OVERLAID_REGION) {
+				cmd = spi_nor_init_erase_cmd(region, erase);
+				if (IS_ERR(cmd)) {
+					ret = PTR_ERR(cmd);
+					goto destroy_erase_cmd_list;
+				}
+
+				list_add_tail(&cmd->list, erase_list);
+			} else {
+				cmd->count++;
+			}
 
-		if (len && addr >= region_end) {
-			region = spi_nor_region_next(region);
-			if (!region)
-				goto destroy_erase_cmd_list;
-			region_end = spi_nor_region_end(region);
+			len -= cmd->size;
+			addr += cmd->size;
+			prev_erase = erase;
 		}
 
-		prev_erase = erase;
+		if (!len)
+			return 0;
 	}
 
-	return 0;
-
 destroy_erase_cmd_list:
 	spi_nor_destroy_erase_cmd_list(erase_list);
 	return ret;
@@ -2439,8 +2386,8 @@ void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
 	map->uniform_region.offset = 0;
 	map->uniform_region.size = flash_size;
 	map->uniform_region.erase_mask = erase_mask;
-	map->uniform_region.flags = SNOR_LAST_REGION;
 	map->regions = &map->uniform_region;
+	map->n_regions = 1;
 }
 
 int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index e002de22b18a..1668d79f55bc 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -238,8 +238,7 @@ struct spi_nor_erase_command {
  *			inside this region. The erase types are sorted in
  *			ascending order with the smallest Erase Type size being
  *			at BIT(0).
- * @flags:		flags to determine if this region is overlaid, if this
- *			region is the last in the SPI NOR flash memory
+ * @flags:		flags to determine if this region is overlaid.
  */
 struct spi_nor_erase_region {
 	u64		offset;
@@ -250,8 +249,7 @@ struct spi_nor_erase_region {
 
 #define SNOR_ERASE_TYPE_MAX	4
 
-#define SNOR_LAST_REGION	BIT(0)
-#define SNOR_OVERLAID_REGION	BIT(1)
+#define SNOR_OVERLAID_REGION	BIT(0)
 
 /**
  * struct spi_nor_erase_map - Structure to describe the SPI NOR erase map
@@ -264,11 +262,13 @@ struct spi_nor_erase_region {
  *			The erase types are sorted in ascending order, with the
  *			smallest Erase Type size being the first member in the
  *			erase_type array.
+ * @n_regions:		number of erase regions.
  */
 struct spi_nor_erase_map {
 	struct spi_nor_erase_region	*regions;
 	struct spi_nor_erase_region	uniform_region;
 	struct spi_nor_erase_type	erase_type[SNOR_ERASE_TYPE_MAX];
+	unsigned int			n_regions;
 };
 
 /**
@@ -699,8 +699,6 @@ void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
 void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size,
 			    u8 opcode);
 void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase);
-struct spi_nor_erase_region *
-spi_nor_region_next(struct spi_nor_erase_region *region);
 void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
 				    u8 erase_mask, u64 flash_size);
 
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 16b30f1a3066..0832b5893d6b 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -78,10 +78,10 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 	struct spi_nor *nor = s->private;
 	struct spi_nor_flash_parameter *params = nor->params;
 	struct spi_nor_erase_map *erase_map = &params->erase_map;
-	struct spi_nor_erase_region *region;
+	struct spi_nor_erase_region *region = erase_map->regions;
 	const struct flash_info *info = nor->info;
 	char buf[16], *str;
-	int i;
+	unsigned int i;
 
 	seq_printf(s, "name\t\t%s\n", info->name);
 	seq_printf(s, "id\t\t%*ph\n", SPI_NOR_MAX_ID_LEN, nor->id);
@@ -144,13 +144,11 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 	seq_puts(s, "\nsector map\n");
 	seq_puts(s, " region (in hex)   | erase mask | flags\n");
 	seq_puts(s, " ------------------+------------+----------\n");
-	for (region = erase_map->regions;
-	     region;
-	     region = spi_nor_region_next(region)) {
-		u64 start = region->offset;
-		u64 end = start + region->size - 1;
-		u8 erase_mask = region->erase_mask;
-		u8 flags = region->flags;
+	for (i = 0; i < erase_map->n_regions; i++) {
+		u64 start = region[i].offset;
+		u64 end = start + region[i].size - 1;
+		u8 erase_mask = region[i].erase_mask;
+		u8 flags = region[i].flags;
 
 		seq_printf(s, " %08llx-%08llx |     [%c%c%c%c] | %s\n",
 			   start, end,
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index c8d8b4e5b7e6..c4721d7dc97a 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -390,15 +390,14 @@ static void spi_nor_regions_sort_erase_types(struct spi_nor_erase_map *map)
 {
 	struct spi_nor_erase_region *region = map->regions;
 	u8 sorted_erase_mask;
+	unsigned int i;
 
-	while (region) {
-		sorted_erase_mask = spi_nor_sort_erase_mask(map,
-							    region->erase_mask);
+	for (i = 0; i < map->n_regions; i++) {
+		sorted_erase_mask =
+			spi_nor_sort_erase_mask(map, region[i].erase_mask);
 
 		/* Overwrite erase mask. */
-		region->erase_mask = sorted_erase_mask;
-
-		region = spi_nor_region_next(region);
+		region[i].erase_mask = sorted_erase_mask;
 	}
 }
 
@@ -774,11 +773,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 	return ret;
 }
 
-static void spi_nor_region_mark_end(struct spi_nor_erase_region *region)
-{
-	region->flags |= SNOR_LAST_REGION;
-}
-
 static void spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
 {
 	region->flags |= SNOR_OVERLAID_REGION;
@@ -836,6 +830,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 	if (!region)
 		return -ENOMEM;
 	map->regions = region;
+	map->n_regions = region_count;
 
 	uniform_erase_type = 0xff;
 	regions_erase_type = 0;
@@ -864,7 +859,6 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
 
 		offset = region[i].offset + region[i].size;
 	}
-	spi_nor_region_mark_end(&region[i - 1]);
 
 	save_uniform_erase_type = map->uniform_region.erase_mask;
 	map->uniform_region.erase_mask =
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 2/4] mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag

[Michael Walle]

On Tue Feb 20, 2024 at 9:34 AM CET, tkuw584924 wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>
> Introduce n_regions in spi_nor_erase_map structure and remove
> SNOR_LAST_REGION flag. Loop logics that depend on the flag are also
> reworked to use n_regions as loop condition.
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> Suggested-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> Suggested-by: Michael Walle <mwalle@kernel.org>
> ---
>  drivers/mtd/spi-nor/core.c    | 105 +++++++++-------------------------
>  drivers/mtd/spi-nor/core.h    |  10 ++--
>  drivers/mtd/spi-nor/debugfs.c |  16 +++---
>  drivers/mtd/spi-nor/sfdp.c    |  18 ++----
>  4 files changed, 43 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5b2f13d0888e..d9a0c0e31950 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1565,57 +1565,11 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
>  	return NULL;
>  }
>  
> -static u64 spi_nor_region_is_last(const struct spi_nor_erase_region *region)
> -{
> -	return region->flags & SNOR_LAST_REGION;
> -}
> -
>  static u64 spi_nor_region_end(const struct spi_nor_erase_region *region)
>  {
>  	return region->offset + region->size;
>  }
>  
> -/**
> - * spi_nor_region_next() - get the next spi nor region
> - * @region:	pointer to a structure that describes a SPI NOR erase region
> - *
> - * Return: the next spi nor region or NULL if last region.
> - */
> -struct spi_nor_erase_region *
> -spi_nor_region_next(struct spi_nor_erase_region *region)
> -{
> -	if (spi_nor_region_is_last(region))
> -		return NULL;
> -	region++;
> -	return region;
> -}
> -
> -/**
> - * spi_nor_find_erase_region() - find the region of the serial flash memory in
> - *				 which the offset fits
> - * @map:	the erase map of the SPI NOR
> - * @addr:	offset in the serial flash memory
> - *
> - * Return: a pointer to the spi_nor_erase_region struct, ERR_PTR(-errno)
> - *	   otherwise.
> - */
> -static struct spi_nor_erase_region *
> -spi_nor_find_erase_region(const struct spi_nor_erase_map *map, u64 addr)
> -{
> -	struct spi_nor_erase_region *region = map->regions;
> -	u64 region_end = spi_nor_region_end(region);
> -
> -	while (addr < region->offset || addr >= region_end) {
> -		region = spi_nor_region_next(region);
> -		if (!region)
> -			return ERR_PTR(-EINVAL);
> -
> -		region_end = spi_nor_region_end(region);
> -	}
> -
> -	return region;
> -}
> -
>  /**
>   * spi_nor_init_erase_cmd() - initialize an erase command
>   * @region:	pointer to a structure that describes a SPI NOR erase region
> @@ -1682,48 +1636,41 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
>  	struct spi_nor_erase_region *region;
>  	struct spi_nor_erase_command *cmd = NULL;
>  	u64 region_end;
> +	unsigned int i;
>  	int ret = -EINVAL;
>  
> -	region = spi_nor_find_erase_region(map, addr);
> -	if (IS_ERR(region))
> -		return PTR_ERR(region);
> -
> -	region_end = spi_nor_region_end(region);
> +	for (i = 0; i < map->n_regions; i++) {
> +		region = &map->regions[i];
> +		region_end = spi_nor_region_end(region);

Likewise,

region_end = return region->offset + region->size;

>  
> -	while (len) {
> -		erase = spi_nor_find_best_erase_type(map, region, addr, len);
> -		if (!erase)
> -			goto destroy_erase_cmd_list;
> -
> -		if (prev_erase != erase ||
> -		    erase->size != cmd->size ||
> -		    region->flags & SNOR_OVERLAID_REGION) {
> -			cmd = spi_nor_init_erase_cmd(region, erase);
> -			if (IS_ERR(cmd)) {
> -				ret = PTR_ERR(cmd);
> +		while (len && addr >= region->offset && addr < region_end) {
> +			erase = spi_nor_find_best_erase_type(map, region, addr,
> +							     len);
> +			if (!erase)
>  				goto destroy_erase_cmd_list;
> -			}
>  
> -			list_add_tail(&cmd->list, erase_list);
> -		} else {
> -			cmd->count++;
> -		}
> -
> -		addr += cmd->size;
> -		len -= cmd->size;
> +			if (prev_erase != erase || erase->size != cmd->size ||
> +			    region->flags & SNOR_OVERLAID_REGION) {
> +				cmd = spi_nor_init_erase_cmd(region, erase);
> +				if (IS_ERR(cmd)) {
> +					ret = PTR_ERR(cmd);
> +					goto destroy_erase_cmd_list;
> +				}
> +
> +				list_add_tail(&cmd->list, erase_list);
> +			} else {
> +				cmd->count++;
> +			}
>  
> -		if (len && addr >= region_end) {
> -			region = spi_nor_region_next(region);
> -			if (!region)
> -				goto destroy_erase_cmd_list;
> -			region_end = spi_nor_region_end(region);
> +			len -= cmd->size;
> +			addr += cmd->size;
> +			prev_erase = erase;
>  		}
>  
> -		prev_erase = erase;
> +		if (!len)
> +			return 0;
>  	}

From what I can tell, this looks ok,

Reviewed-by: Michael Walle <mwalle@kernel.org>

-michael

>  
> -	return 0;
> -
>  destroy_erase_cmd_list:
>  	spi_nor_destroy_erase_cmd_list(erase_list);
>  	return ret;
> @@ -2439,8 +2386,8 @@ void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
>  	map->uniform_region.offset = 0;
>  	map->uniform_region.size = flash_size;
>  	map->uniform_region.erase_mask = erase_mask;
> -	map->uniform_region.flags = SNOR_LAST_REGION;
>  	map->regions = &map->uniform_region;
> +	map->n_regions = 1;
>  }
>  
>  int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index e002de22b18a..1668d79f55bc 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -238,8 +238,7 @@ struct spi_nor_erase_command {
>   *			inside this region. The erase types are sorted in
>   *			ascending order with the smallest Erase Type size being
>   *			at BIT(0).
> - * @flags:		flags to determine if this region is overlaid, if this
> - *			region is the last in the SPI NOR flash memory
> + * @flags:		flags to determine if this region is overlaid.
>   */
>  struct spi_nor_erase_region {
>  	u64		offset;
> @@ -250,8 +249,7 @@ struct spi_nor_erase_region {
>  
>  #define SNOR_ERASE_TYPE_MAX	4
>  
> -#define SNOR_LAST_REGION	BIT(0)
> -#define SNOR_OVERLAID_REGION	BIT(1)
> +#define SNOR_OVERLAID_REGION	BIT(0)
>  
>  /**
>   * struct spi_nor_erase_map - Structure to describe the SPI NOR erase map
> @@ -264,11 +262,13 @@ struct spi_nor_erase_region {
>   *			The erase types are sorted in ascending order, with the
>   *			smallest Erase Type size being the first member in the
>   *			erase_type array.
> + * @n_regions:		number of erase regions.
>   */
>  struct spi_nor_erase_map {
>  	struct spi_nor_erase_region	*regions;
>  	struct spi_nor_erase_region	uniform_region;
>  	struct spi_nor_erase_type	erase_type[SNOR_ERASE_TYPE_MAX];
> +	unsigned int			n_regions;
>  };
>  
>  /**
> @@ -699,8 +699,6 @@ void spi_nor_set_pp_settings(struct spi_nor_pp_command *pp, u8 opcode,
>  void spi_nor_set_erase_type(struct spi_nor_erase_type *erase, u32 size,
>  			    u8 opcode);
>  void spi_nor_mask_erase_type(struct spi_nor_erase_type *erase);
> -struct spi_nor_erase_region *
> -spi_nor_region_next(struct spi_nor_erase_region *region);
>  void spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
>  				    u8 erase_mask, u64 flash_size);
>  
> diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
> index 16b30f1a3066..0832b5893d6b 100644
> --- a/drivers/mtd/spi-nor/debugfs.c
> +++ b/drivers/mtd/spi-nor/debugfs.c
> @@ -78,10 +78,10 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
>  	struct spi_nor *nor = s->private;
>  	struct spi_nor_flash_parameter *params = nor->params;
>  	struct spi_nor_erase_map *erase_map = &params->erase_map;
> -	struct spi_nor_erase_region *region;
> +	struct spi_nor_erase_region *region = erase_map->regions;
>  	const struct flash_info *info = nor->info;
>  	char buf[16], *str;
> -	int i;
> +	unsigned int i;
>  
>  	seq_printf(s, "name\t\t%s\n", info->name);
>  	seq_printf(s, "id\t\t%*ph\n", SPI_NOR_MAX_ID_LEN, nor->id);
> @@ -144,13 +144,11 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
>  	seq_puts(s, "\nsector map\n");
>  	seq_puts(s, " region (in hex)   | erase mask | flags\n");
>  	seq_puts(s, " ------------------+------------+----------\n");
> -	for (region = erase_map->regions;
> -	     region;
> -	     region = spi_nor_region_next(region)) {
> -		u64 start = region->offset;
> -		u64 end = start + region->size - 1;
> -		u8 erase_mask = region->erase_mask;
> -		u8 flags = region->flags;
> +	for (i = 0; i < erase_map->n_regions; i++) {
> +		u64 start = region[i].offset;
> +		u64 end = start + region[i].size - 1;
> +		u8 erase_mask = region[i].erase_mask;
> +		u8 flags = region[i].flags;
>  
>  		seq_printf(s, " %08llx-%08llx |     [%c%c%c%c] | %s\n",
>  			   start, end,
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index c8d8b4e5b7e6..c4721d7dc97a 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -390,15 +390,14 @@ static void spi_nor_regions_sort_erase_types(struct spi_nor_erase_map *map)
>  {
>  	struct spi_nor_erase_region *region = map->regions;
>  	u8 sorted_erase_mask;
> +	unsigned int i;
>  
> -	while (region) {
> -		sorted_erase_mask = spi_nor_sort_erase_mask(map,
> -							    region->erase_mask);
> +	for (i = 0; i < map->n_regions; i++) {
> +		sorted_erase_mask =
> +			spi_nor_sort_erase_mask(map, region[i].erase_mask);
>  
>  		/* Overwrite erase mask. */
> -		region->erase_mask = sorted_erase_mask;
> -
> -		region = spi_nor_region_next(region);
> +		region[i].erase_mask = sorted_erase_mask;
>  	}
>  }
>  
> @@ -774,11 +773,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
>  	return ret;
>  }
>  
> -static void spi_nor_region_mark_end(struct spi_nor_erase_region *region)
> -{
> -	region->flags |= SNOR_LAST_REGION;
> -}
> -
>  static void spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
>  {
>  	region->flags |= SNOR_OVERLAID_REGION;
> @@ -836,6 +830,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  	if (!region)
>  		return -ENOMEM;
>  	map->regions = region;
> +	map->n_regions = region_count;
>  
>  	uniform_erase_type = 0xff;
>  	regions_erase_type = 0;
> @@ -864,7 +859,6 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>  
>  		offset = region[i].offset + region[i].size;
>  	}
> -	spi_nor_region_mark_end(&region[i - 1]);
>  
>  	save_uniform_erase_type = map->uniform_region.erase_mask;
>  	map->uniform_region.erase_mask =


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 2/4] mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag

[Tudor Ambarus]

On 20.02.2024 10:34, tkuw584924@gmail.com wrote:
> @@ -1682,48 +1636,41 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
>  	struct spi_nor_erase_region *region;
>  	struct spi_nor_erase_command *cmd = NULL;
>  	u64 region_end;
> +	unsigned int i;
>  	int ret = -EINVAL;
>  
> -	region = spi_nor_find_erase_region(map, addr);
> -	if (IS_ERR(region))
> -		return PTR_ERR(region);
> -
> -	region_end = spi_nor_region_end(region);
> +	for (i = 0; i < map->n_regions; i++) {
> +		region = &map->regions[i];
> +		region_end = spi_nor_region_end(region);
>  
> -	while (len) {
> -		erase = spi_nor_find_best_erase_type(map, region, addr, len);
> -		if (!erase)
> -			goto destroy_erase_cmd_list;
> -
> -		if (prev_erase != erase ||
> -		    erase->size != cmd->size ||
> -		    region->flags & SNOR_OVERLAID_REGION) {
> -			cmd = spi_nor_init_erase_cmd(region, erase);
> -			if (IS_ERR(cmd)) {
> -				ret = PTR_ERR(cmd);
> +		while (len && addr >= region->offset && addr < region_end) {
> +			erase = spi_nor_find_best_erase_type(map, region, addr,
> +							     len);
> +			if (!erase)
>  				goto destroy_erase_cmd_list;
> -			}
>  
> -			list_add_tail(&cmd->list, erase_list);
> -		} else {
> -			cmd->count++;
> -		}
> -
> -		addr += cmd->size;
> -		len -= cmd->size;
> +			if (prev_erase != erase || erase->size != cmd->size ||
> +			    region->flags & SNOR_OVERLAID_REGION) {
> +				cmd = spi_nor_init_erase_cmd(region, erase);
> +				if (IS_ERR(cmd)) {
> +					ret = PTR_ERR(cmd);
> +					goto destroy_erase_cmd_list;
> +				}
> +
> +				list_add_tail(&cmd->list, erase_list);
> +			} else {
> +				cmd->count++;
> +			}
>  
> -		if (len && addr >= region_end) {
> -			region = spi_nor_region_next(region);
> -			if (!region)
> -				goto destroy_erase_cmd_list;
> -			region_end = spi_nor_region_end(region);
> +			len -= cmd->size;
> +			addr += cmd->size;
> +			prev_erase = erase;
>  		}
>  
> -		prev_erase = erase;
> +		if (!len)
> +			return 0;

breaking the for loop here is odd and confusing.
I'll update this patch with the following:

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 44427d58c082..eda49c30958d 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1688,7 +1688,7 @@ static int spi_nor_init_erase_cmd_list(struct
spi_nor *nor,
        unsigned int i;
        int ret = -EINVAL;

-       for (i = 0; i < map->n_regions; i++) {
+       for (i = 0; i < map->n_regions && len; i++) {
                region = &map->regions[i];
                region_end = region->offset + region->size;

@@ -1715,11 +1715,10 @@ static int spi_nor_init_erase_cmd_list(struct
spi_nor *nor,
                        addr += cmd->size;
                        prev_erase = erase;
                }
-
-               if (!len)
-                       return 0;
        }

+       return 0;
+
 destroy_erase_cmd_list:
        spi_nor_destroy_erase_cmd_list(erase_list);
        return ret;


>  	}
>  
> -	return 0;
> -
>  destroy_erase_cmd_list:
>  	spi_nor_destroy_erase_cmd_list(erase_list);
>  	return ret;

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 3/4] mtd: spi-nor: core: get rid of SNOR_OVERLAID_REGION flag

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Only SNOR_OVERLAID_REGION is defined for flags in the spi_nor_erase_region
structure. It can be replaced by a boolean parameter.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c    | 7 +++----
 drivers/mtd/spi-nor/core.h    | 6 ++----
 drivers/mtd/spi-nor/debugfs.c | 5 ++---
 drivers/mtd/spi-nor/sfdp.c    | 7 +------
 4 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index d9a0c0e31950..21775d5eccd5 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1549,8 +1549,7 @@ spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
 			continue;
 
 		/* Alignment is not mandatory for overlaid regions */
-		if (region->flags & SNOR_OVERLAID_REGION &&
-		    region->size <= len)
+		if (region->overlaid && region->size <= len)
 			return erase;
 
 		/* Don't erase more than what the user has asked for. */
@@ -1592,7 +1591,7 @@ spi_nor_init_erase_cmd(const struct spi_nor_erase_region *region,
 	cmd->opcode = erase->opcode;
 	cmd->count = 1;
 
-	if (region->flags & SNOR_OVERLAID_REGION)
+	if (region->overlaid)
 		cmd->size = region->size;
 	else
 		cmd->size = erase->size;
@@ -1650,7 +1649,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
 				goto destroy_erase_cmd_list;
 
 			if (prev_erase != erase || erase->size != cmd->size ||
-			    region->flags & SNOR_OVERLAID_REGION) {
+			    region->overlaid) {
 				cmd = spi_nor_init_erase_cmd(region, erase);
 				if (IS_ERR(cmd)) {
 					ret = PTR_ERR(cmd);
diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
index 1668d79f55bc..77ae2edf513f 100644
--- a/drivers/mtd/spi-nor/core.h
+++ b/drivers/mtd/spi-nor/core.h
@@ -238,19 +238,17 @@ struct spi_nor_erase_command {
  *			inside this region. The erase types are sorted in
  *			ascending order with the smallest Erase Type size being
  *			at BIT(0).
- * @flags:		flags to determine if this region is overlaid.
+ * @overlaid:		determine if this region is overlaid.
  */
 struct spi_nor_erase_region {
 	u64		offset;
 	u64		size;
 	u8		erase_mask;
-	u8		flags;
+	bool		overlaid;
 };
 
 #define SNOR_ERASE_TYPE_MAX	4
 
-#define SNOR_OVERLAID_REGION	BIT(0)
-
 /**
  * struct spi_nor_erase_map - Structure to describe the SPI NOR erase map
  * @regions:		array of erase regions. The regions are consecutive in
diff --git a/drivers/mtd/spi-nor/debugfs.c b/drivers/mtd/spi-nor/debugfs.c
index 0832b5893d6b..c9a344e0f96f 100644
--- a/drivers/mtd/spi-nor/debugfs.c
+++ b/drivers/mtd/spi-nor/debugfs.c
@@ -142,13 +142,12 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 	}
 
 	seq_puts(s, "\nsector map\n");
-	seq_puts(s, " region (in hex)   | erase mask | flags\n");
+	seq_puts(s, " region (in hex)   | erase mask | overlaid\n");
 	seq_puts(s, " ------------------+------------+----------\n");
 	for (i = 0; i < erase_map->n_regions; i++) {
 		u64 start = region[i].offset;
 		u64 end = start + region[i].size - 1;
 		u8 erase_mask = region[i].erase_mask;
-		u8 flags = region[i].flags;
 
 		seq_printf(s, " %08llx-%08llx |     [%c%c%c%c] | %s\n",
 			   start, end,
@@ -156,7 +155,7 @@ static int spi_nor_params_show(struct seq_file *s, void *data)
 			   erase_mask & BIT(1) ? '1' : ' ',
 			   erase_mask & BIT(2) ? '2' : ' ',
 			   erase_mask & BIT(3) ? '3' : ' ',
-			   flags & SNOR_OVERLAID_REGION ? "overlaid" : "");
+			   region[i].overlaid ? "yes" : "no");
 	}
 
 	return 0;
diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index c4721d7dc97a..248e1e071fed 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -773,11 +773,6 @@ static const u32 *spi_nor_get_map_in_use(struct spi_nor *nor, const u32 *smpt,
 	return ret;
 }
 
-static void spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
-{
-	region->flags |= SNOR_OVERLAID_REGION;
-}
-
 /**
  * spi_nor_region_check_overlay() - set overlay bit when the region is overlaid
  * @region:	pointer to a structure that describes a SPI NOR erase region
@@ -795,7 +790,7 @@ spi_nor_region_check_overlay(struct spi_nor_erase_region *region,
 		if (!(erase[i].size && erase_type & BIT(erase[i].idx)))
 			continue;
 		if (region->size & erase[i].size_mask) {
-			spi_nor_region_mark_overlay(region);
+			region->overlaid = true;
 			return;
 		}
 	}
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 3/4] mtd: spi-nor: core: get rid of SNOR_OVERLAID_REGION flag

[Michael Walle]

On Tue Feb 20, 2024 at 9:34 AM CET, tkuw584924 wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>
> Only SNOR_OVERLAID_REGION is defined for flags in the spi_nor_erase_region
> structure. It can be replaced by a boolean parameter.
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
Reviewed-by: Michael Walle <mwalle@kernel.org>

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH v4 4/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[tkuw584924]

From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>

Some of Infineon SPI NOR flash devices support hybrid sector layout that
overlays 4KB sectors on a 256KB sector and SPI NOR framework recognizes
that by parsing SMPT and construct params->erase_map. The hybrid sector
layout is similar to CFI flash devices that have small sectors on top
and/or bottom address. In case of CFI flash devices, the erase map
information is parsed through CFI table and populated into
mtd->eraseregions so that users can create MTD partitions that aligned
with small sector boundaries. This patch provides the same capability to
SPI NOR flash devices that have non-uniform erase map.

Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
---
 drivers/mtd/spi-nor/core.c | 58 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 21775d5eccd5..5ba570248cb9 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -3349,7 +3349,54 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
 	return info;
 }
 
-static void spi_nor_set_mtd_info(struct spi_nor *nor)
+static u32
+spi_nor_get_region_erasesize(const struct spi_nor_erase_region *region,
+			     const struct spi_nor_erase_type *erase_type)
+{
+	u8 i;
+
+	if (region->overlaid)
+		return region->size;
+
+	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
+		if (region->erase_mask & BIT(i))
+			return erase_type[i].size;
+	}
+
+	return 0;
+}
+
+static int spi_nor_set_mtd_eraseregions(struct spi_nor *nor)
+{
+	const struct spi_nor_erase_map *map = &nor->params->erase_map;
+	const struct spi_nor_erase_region *region = map->regions;
+	struct mtd_erase_region_info *mtd_region;
+	struct mtd_info *mtd = &nor->mtd;
+	u32 erasesize, i;
+
+	mtd_region = devm_kcalloc(nor->dev, map->n_regions, sizeof(*mtd_region),
+				  GFP_KERNEL);
+	if (!mtd_region)
+		return -ENOMEM;
+
+	for (i = 0; i < map->n_regions; i++) {
+		erasesize = spi_nor_get_region_erasesize(&region[i],
+							 map->erase_type);
+		if (!erasesize)
+			return -EINVAL;
+
+		mtd_region[i].erasesize = erasesize;
+		mtd_region[i].numblocks = div64_ul(region[i].size, erasesize);
+		mtd_region[i].offset = region[i].offset;
+	}
+
+	mtd->numeraseregions = map->n_regions;
+	mtd->eraseregions = mtd_region;
+
+	return 0;
+}
+
+static int spi_nor_set_mtd_info(struct spi_nor *nor)
 {
 	struct mtd_info *mtd = &nor->mtd;
 	struct device *dev = nor->dev;
@@ -3380,6 +3427,11 @@ static void spi_nor_set_mtd_info(struct spi_nor *nor)
 	mtd->_resume = spi_nor_resume;
 	mtd->_get_device = spi_nor_get_device;
 	mtd->_put_device = spi_nor_put_device;
+
+	if (!spi_nor_has_uniform_erase(nor))
+		return spi_nor_set_mtd_eraseregions(nor);
+
+	return 0;
 }
 
 static int spi_nor_hw_reset(struct spi_nor *nor)
@@ -3472,7 +3524,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		return ret;
 
 	/* No mtd_info fields should be used up to this point. */
-	spi_nor_set_mtd_info(nor);
+	ret = spi_nor_set_mtd_info(nor);
+	if (ret)
+		return ret;
 
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 4/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[Michael Walle]

On Tue Feb 20, 2024 at 9:34 AM CET, tkuw584924 wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
>
> Some of Infineon SPI NOR flash devices support hybrid sector layout that
> overlays 4KB sectors on a 256KB sector and SPI NOR framework recognizes
> that by parsing SMPT and construct params->erase_map. The hybrid sector
> layout is similar to CFI flash devices that have small sectors on top
> and/or bottom address. In case of CFI flash devices, the erase map
> information is parsed through CFI table and populated into
> mtd->eraseregions so that users can create MTD partitions that aligned
> with small sector boundaries. This patch provides the same capability to
> SPI NOR flash devices that have non-uniform erase map.
>
> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> ---
>  drivers/mtd/spi-nor/core.c | 58 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 56 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 21775d5eccd5..5ba570248cb9 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3349,7 +3349,54 @@ static const struct flash_info *spi_nor_get_flash_info(struct spi_nor *nor,
>  	return info;
>  }
>  
> -static void spi_nor_set_mtd_info(struct spi_nor *nor)
> +static u32
> +spi_nor_get_region_erasesize(const struct spi_nor_erase_region *region,
> +			     const struct spi_nor_erase_type *erase_type)
> +{
> +	u8 i;
> +
> +	if (region->overlaid)
> +		return region->size;
> +
> +	for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> +		if (region->erase_mask & BIT(i))
> +			return erase_type[i].size;
> +	}

nit: You could drop the braces.

Reviewed-by: Michael Walle <mwalle@kernel.org>

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[Tudor Ambarus]

Hi, Takahiro,

I'll try to review this today or tomorrow and queue it if I'll find it
fine. Please let us know if you tested the series and on which flash.

Cheers,
ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[Takahiro Kuwano]

Hi Tudor,

I tested with S25HS01GT(hybrid config) and S25HL01GT(uniform config).

Thanks,
Takahiro

On 2/22/2024 6:16 PM, Tudor Ambarus wrote:
> Hi, Takahiro,
> 
> I'll try to review this today or tomorrow and queue it if I'll find it
> fine. Please let us know if you tested the series and on which flash.
> 
> Cheers,
> ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[Tudor Ambarus]

Takahiro, which base did you use for the patch set? It does not apply
cleanly on spi_nor/next (v6.8-rc1), neither on spi-nor/for-6.8.

Please rebase and resubmit.

On 20.02.2024 10:34, tkuw584924@gmail.com wrote:
> From: Takahiro Kuwano <Takahiro.Kuwano@infineon.com>
> 
> In v4, split two flags removement into separated patches. Reworked
> spi_nor_init_erase_cmd() to use n_regions in outer loop.
> 
> In v3, reworked to remove LAST_REGION and OVERLAID_REGION flags. Updated
> and tested debugfs.
> 
> In v2, reworked spi_nor_erase_region to stop bitmask encoding in offset.
> And added n_regions member to spi_nor_erase_map.
> 
> Takahiro Kuwano (4):
>   mtd: spi-nor: core: rework struct spi_nor_erase_region
>   mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag
>   mtd: spi-nor: core: get rid of SNOR_OVERLAID_REGION flag
>   mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map
> 
>  drivers/mtd/spi-nor/core.c    | 187 +++++++++++++++++-----------------
>  drivers/mtd/spi-nor/core.h    |  30 ++----
>  drivers/mtd/spi-nor/debugfs.c |  26 +++--
>  drivers/mtd/spi-nor/sfdp.c    |  47 +++------
>  4 files changed, 128 insertions(+), 162 deletions(-)
> 

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v4 0/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map

[Tudor Ambarus]

On Tue, 20 Feb 2024 17:34:05 +0900, tkuw584924@gmail.com wrote:
> In v4, split two flags removement into separated patches. Reworked
> spi_nor_init_erase_cmd() to use n_regions in outer loop.
> 
> In v3, reworked to remove LAST_REGION and OVERLAID_REGION flags. Updated
> and tested debugfs.
> 
> In v2, reworked spi_nor_erase_region to stop bitmask encoding in offset.
> And added n_regions member to spi_nor_erase_map.
> 
> [...]

Updated patch 1 and 2, see commit message.
Applied to git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git,
spi-nor/next branch. Thanks!

[1/4] mtd: spi-nor: core: rework struct spi_nor_erase_region
      https://git.kernel.org/mtd/c/0e164238bb07
[2/4] mtd: spi-nor: core: get rid of SNOR_LAST_REGION flag
      https://git.kernel.org/mtd/c/df6e36edac23
[3/4] mtd: spi-nor: core: get rid of SNOR_OVERLAID_REGION flag
      https://git.kernel.org/mtd/c/2865ed0e2c71
[4/4] mtd: spi-nor: core: set mtd->eraseregions for non-uniform erase map
      https://git.kernel.org/mtd/c/6a9eda34418f

Cheers,
-- 
Tudor Ambarus <tudor.ambarus@linaro.org>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/