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

[Michael Walle]

Hi,

> From: Takahiro Kuwano <Takahiro.Kuwano at 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 at infineon.com>
> ---
>  drivers/mtd/spi-nor/core.c | 59 ++++++++++++++++++++++++++++++++++++--
>  drivers/mtd/spi-nor/core.h |  2 ++
>  drivers/mtd/spi-nor/sfdp.c |  1 +
>  3 files changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 5b2f13d0888e..25702d604fc5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -2441,6 +2441,7 @@ void spi_nor_init_uniform_erase_map(struct 
> spi_nor_erase_map *map,
>  	map->uniform_region.erase_mask = erase_mask;
>  	map->uniform_region.flags = SNOR_LAST_REGION;
>  	map->regions = &map->uniform_region;
> +	map->n_regions = 1;

Ahh, here it is. So, this should go into the former patch
and the SNOR_LAST_REGION flag should be dropped. They are
redundant, right?

>  }
> 
>  int spi_nor_post_bfpt_fixups(struct spi_nor *nor,
> @@ -3403,7 +3404,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->flags & SNOR_OVERLAID_REGION)
> +		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;
> @@ -3434,6 +3482,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);

Btw I wonder if we can set it in any case?

Apart from the n_regions thing, this looks good.

-michael

> +
> +	return 0;
>  }
> 
>  static int spi_nor_hw_reset(struct spi_nor *nor)
> @@ -3526,7 +3579,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);
> diff --git a/drivers/mtd/spi-nor/core.h b/drivers/mtd/spi-nor/core.h
> index e002de22b18a..fefa990525ff 100644
> --- a/drivers/mtd/spi-nor/core.h
> +++ b/drivers/mtd/spi-nor/core.h
> @@ -264,11 +264,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;
>  };
> 
>  /**
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index c8d8b4e5b7e6..afe041f75f30 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -836,6 +836,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;