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

Reply via email to