[PATCH] mtd: spi-nor: core: Set mtd->eraseregions for non-uniform erase map

[Tudor Ambarus]

On 1/12/24 07:14, Takahiro Kuwano wrote:
> On 1/5/2024 9:23 PM, Michael Walle wrote:
>> Hi,
>>
>>> -static void spi_nor_set_mtd_info(struct spi_nor *nor)
>>> +static int spi_nor_set_mtd_eraseregions(struct spi_nor *nor)
>>> +{
>>> +    struct spi_nor_erase_map *map = &nor->params->erase_map;
>>> +    struct spi_nor_erase_region *region = map->regions;
>>> +    struct mtd_info *mtd = &nor->mtd;
>>> +    struct mtd_erase_region_info *mtd_region;
>>> +    u32 erase_size;
>>> +    u8 erase_mask;
>>> +    int n_regions, i, j;
>>> +
>>> +    for (i = 0; !spi_nor_region_is_last(&region[i]); i++)
>>> +        ;
>>
>> Please put that into a helper which returns the number of regions.
>>
> Yes, I will do it.
> 
>> FWIW, I really dislike the magic around encoding all sorts of stuff
>> into the offset. It makes the code just hard to read.
>>
>>
>>> +
>>> +    n_regions = i + 1;
>>> +    mtd_region = devm_kcalloc(nor->dev, n_regions, sizeof(*mtd_region),
>>> +                  GFP_KERNEL);
>>
>> Who's the owner? mtd->dev or nor->dev?
>>
> I think it should be nor->dev.
> The mtd device is not yet registered at this point.
> 
>>> +    if (!mtd_region)
>>> +        return -ENOMEM;
>>> +
>>> +    for (i = 0; i < n_regions; i++) {
>>> +        if (region[i].offset & SNOR_OVERLAID_REGION) {
>>
>> Btw. what is an overlaid region? I couldn't find any comment
>> about it.
>>
> It is the remaining part of regular sector that overlaid by 4KB sectors.
> In SEMPER case, regular sector is 256KB. If 32 x 4KB sectors are overlaid on
> bottom address, 128KB is overlaid region. The erase opcode for this region is
> same as 256KB sectors.
> 

In other words, the overlaid region is what remains from a sector that's
not covered by smaller erase types.

	_____

	128KB	<- 128KB overlaid region
	_____
	
	4KB
	_____
	.....	30 other 4KB sectors
	_____
	
	4KB
	_____

the overlaid region does not have a dedicated 128 KB erase command and
instead relies on the bigger erase type, 256KB. When the 256KB erase is
issued on the 128KB overlaid region, just the 128KB of the overlaid
region are erased.

Did I remember correctly? Maybe we can describe this somewhere ...

Cheers,
ta