Re: [RESEND PATCH v2 1/3] mtd: spi-nor: add support to non-uniform SFDP SPI NOR flash memories

From: Marek Vasut
Date: Mon Sep 03 2018 - 13:40:52 EST


On 08/27/2018 12:26 PM, Tudor Ambarus wrote:
[...]

> +/* JEDEC JESD216B Standard imposes erase sizes to be power of 2. */
> +static inline u64
> +spi_nor_div_by_erase_size(const struct spi_nor_erase_type *erase,
> + u64 dividend, u32 *remainder)
> +{
> + *remainder = (u32)dividend & erase->size_mask;

Is the cast really needed ? btw I think there might be a macro doing
just this, div_by_ or something in include/ .

> + return dividend >> erase->size_shift;
> +}
> +
> +static const struct spi_nor_erase_type *
> +spi_nor_find_best_erase_type(const struct spi_nor_erase_map *map,
> + const struct spi_nor_erase_region *region,
> + u64 addr, u32 len)
> +{
> + 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 biggest erase type at
> + * index 0.
> + */
> + for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> + /* Does the erase region support the tested erase type? */
> + if (!(erase_mask & BIT(i)))
> + continue;
> +
> + erase = &map->erase_type[i];
> +
> + /* Don't erase more than what the user has asked for. */
> + if (erase->size > len)
> + continue;
> +
> + /* Alignment is not mandatory for overlaid regions */
> + if (region->offset & SNOR_OVERLAID_REGION)
> + return erase;
> +
> + spi_nor_div_by_erase_size(erase, addr, &rem);
> + if (rem)
> + continue;
> + else
> + return erase;
> + }
> +
> + return NULL;
> +}
> +
> +static struct spi_nor_erase_region *
> +spi_nor_region_next(struct spi_nor_erase_region *region)
> +{
> + if (spi_nor_region_is_last(region))
> + return NULL;
> + return ++region;

region++ ...

[...]

> +static int spi_nor_cmp_erase_type(const void *a, const void *b)
> +{
> + const struct spi_nor_erase_type *erase1 = a;
> + const struct spi_nor_erase_type *erase2 = b;
> +
> + return erase1->size - erase2->size;

What does this function do again ?

> +}
> +
> +static void spi_nor_regions_sort_erase_types(struct spi_nor_erase_map *map)
> +{
> + struct spi_nor_erase_region *region = map->regions;
> + struct spi_nor_erase_type *erase_type = map->erase_type;
> + int i;
> + u8 region_erase_mask, ordered_erase_mask;
> +
> + /*
> + * Sort each region's Erase Types in ascending order with the smallest
> + * Erase Type size starting at BIT(0).
> + */
> + while (region) {
> + region_erase_mask = region->offset & SNOR_ERASE_TYPE_MASK;
> +
> + /*
> + * The region's erase mask indicates which erase types are
> + * supported from the erase types defined in the map.
> + */
> + ordered_erase_mask = 0;
> + for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
> + if (erase_type[i].size &&
> + region_erase_mask & BIT(erase_type[i].idx))
> + ordered_erase_mask |= BIT(i);
> +
> + /* Overwrite erase mask. */
> + region->offset = (region->offset & ~SNOR_ERASE_TYPE_MASK) |
> + ordered_erase_mask;
> +
> + region = spi_nor_region_next(region);
> + }
> +}
> +
> +static inline void

Drop the inline

> +spi_nor_init_uniform_erase_map(struct spi_nor_erase_map *map,
> + u8 erase_mask, u64 flash_size)
> +{
> + map->uniform_region.offset = SNOR_ERASE_FLAGS_OFFSET(erase_mask, 1, 0,
> + 0);
> + map->uniform_region.size = flash_size;
> + map->regions = &map->uniform_region;
> + map->uniform_erase_type = erase_mask;
> +}
> +

[...]

> +#define spi_nor_region_is_last(region) (region->offset & SNOR_LAST_REGION)
> +
> +static inline u64 spi_nor_region_end(const struct spi_nor_erase_region *region)

Get rid of the inlines, really.

> +{
> + return (region->offset & ~SNOR_ERASE_FLAGS_MASK) + region->size;
> +}
> +
> +static inline bool spi_nor_has_uniform_erase(const struct spi_nor *nor)
> +{
> + return !!nor->erase_map.uniform_erase_type;
> +}
> +
> static inline void spi_nor_set_flash_node(struct spi_nor *nor,
> struct device_node *np)
> {
>

General question, what happens if the multi-block erase fails mid-way ,
is that handled or reported somehow to the user ?

--
Best regards,
Marek Vasut