Re: [PATCH] mtd: spi-nor: fix erase_type array to indicate current map conf

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

On 11/22/2018 06:14 PM, Sverdlin, Alexander (Nokia - DE/Ulm) wrote:
> Hello Tudor,
> 
> On 22/11/2018 13:36, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
>> From: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>>
>> Bug reported for the out-of-tree S25FS128S flash memory.
>>
>> BFPT table advertises all the erase types supported by all the
>> possible map configurations. Update the erase_type array to indicate
>> which erase types are applicable to the current map configuration.
>>
>> Backward compatibility test done on sst26vf064b.
>>
>> Fixes: b038e8e3be72 ("mtd: spi-nor: parse SFDP Sector Map Parameter Table")
>> Reported-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 29 ++++++++++++++++++++++++++++-
>>  1 file changed, 28 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 93c9bc8931fc..a71adcd6ddfa 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -3000,7 +3000,8 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>>  	u64 offset;
>>  	u32 region_count;
>>  	int i, j;
>> -	u8 erase_type, uniform_erase_type;
>> +	u8 uniform_erase_type, save_uniform_erase_type;
>> +	u8 erase_type, regions_erase_type;
>>  
>>  	region_count = SMPT_MAP_REGION_COUNT(*smpt);
>>  	/*
>> @@ -3014,6 +3015,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>>  	map->regions = region;
>>  
>>  	uniform_erase_type = 0xff;
>> +	regions_erase_type = 0;
>>  	offset = 0;
>>  	/* Populate regions. */
>>  	for (i = 0; i < region_count; i++) {
>> @@ -3030,13 +3032,38 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>>  		 */
>>  		uniform_erase_type &= erase_type;
>>  
>> +		/*
>> +		 * regions_erase_type mask will indicate all the erase types
>> +		 * supported in this configuration map.
>> +		 */
>> +		regions_erase_type |= erase_type;
>> +
>>  		offset = (region[i].offset & ~SNOR_ERASE_FLAGS_MASK) +
>>  			 region[i].size;
>>  	}
>>  
>> +	save_uniform_erase_type = map->uniform_erase_type;
>>  	map->uniform_erase_type = 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;
>> +		return -EINVAL;
>> +	}
>> +
>> +	/*
>> +	 * BFPT table advertises all the erase types supported by all the
>> +	 * possible map configurations. Update the erase_type array to indicate
>> +	 * which erase types are applicable to the current map configuration.
>> +	 */
>> +	for (i = 0; i < SNOR_ERASE_TYPE_MAX; i++)
>> +		if (!(regions_erase_type & BIT(erase[i].idx)))
>> +			spi_nor_set_erase_type(&erase[i], 0, 0xFF);
> 
> have you noticed the following:
> 
> .../drivers/mtd/spi-nor/spi-nor.c: In function ‘spi_nor_init_non_uniform_erase_map’:
> .../drivers/mtd/spi-nor/spi-nor.c:2940:27: error: passing argument 1 of ‘spi_nor_set_erase_type’ discards ‘const’ qualifier
>  from pointer target type [-Werror=discarded-qualifiers]
>     spi_nor_set_erase_type(&erase[i], 0, 0xFF);
>                            ^
> .../drivers/mtd/spi-nor/spi-nor.c:2368:13: note: expected ‘struct spi_nor_erase_type *’ but argument is of type ‘const stru
> ct spi_nor_erase_type *’
>  static void spi_nor_set_erase_type(struct spi_nor_erase_type *erase,
>              ^~~~~~~~~~~~~~~~~~~~~~
> 
>> +


Oops, no, thanks! Removing the const qualifier will do the trick:

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a71adcd6ddfa..06086916ec3c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2995,7 +2995,7 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
                                              const u32 *smpt)
 {
        struct spi_nor_erase_map *map = &nor->erase_map;
-       const struct spi_nor_erase_type *erase = map->erase_type;
+       struct spi_nor_erase_type *erase = map->erase_type;
        struct spi_nor_erase_region *region;
        u64 offset;
        u32 region_count;

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