[PATCH 2/2] mtd: spi-nor-core: Add fixups for Spansion S25FL256L

[Takahiro Kuwano]

On 9/18/2021 1:51 AM, Pratyush Yadav wrote:
> On 08/09/21 05:47PM, tkuw584924 at gmail.com wrote:
>> From: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>>
>> The S25FL256L is 32MB NOR Flash that does not support Bank Address
>> Register. This fixup is activated if CONFIG_SPI_FLASH_BAR is enabled and
>> returns ENOTSUPP in setup() hook to avoid further ops.
>>
>> Signed-off-by: Takahiro Kuwano <Takahiro.Kuwano at infineon.com>
>> ---
>>  drivers/mtd/spi/spi-nor-core.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
>> index d5d905fa5a..4940b35682 100644
>> --- a/drivers/mtd/spi/spi-nor-core.c
>> +++ b/drivers/mtd/spi/spi-nor-core.c
>> @@ -3222,6 +3222,23 @@ static struct spi_nor_fixups s25hx_t_fixups = {
>>  	.post_bfpt = s25hx_t_post_bfpt_fixup,
>>  	.post_sfdp = s25hx_t_post_sfdp_fixup,
>>  };
>> +
>> +#ifdef CONFIG_SPI_FLASH_BAR
>> +static int s25fl256l_setup(struct spi_nor *nor, const struct flash_info *info,
>> +			   const struct spi_nor_flash_parameter *params)
>> +{
>> +	return -ENOTSUPP; /* Bank Address Register is not supported */
> 
> I am not very familiar with BAR. Can you please explain what would 
> happen if we don't do this?
> 
The Bank Address Register (BAR) enables the 3-byte addressing opcodes to access
beyond 16MB memory space by switching 16MB banks. If we don't do this, the
spi-nor tries to use BAR with 3-byte addressing and the ops to the second half
of 32MB will be performed in the first half of 32MB. 

>> +}
>> +
>> +static void s25fl256l_default_init(struct spi_nor *nor)
>> +{
>> +	nor->setup = s25fl256l_setup;
>> +}
>> +
>> +static struct spi_nor_fixups s25fl256l_fixups = {
>> +	.default_init = s25fl256l_default_init,
>> +};
>> +#endif
>>  #endif
>>  
>>  #ifdef CONFIG_SPI_FLASH_S28HS512T
>> @@ -3644,6 +3661,10 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>>  			break;
>>  		}
>>  	}
>> +#ifdef CONFIG_SPI_FLASH_BAR
> 
> We should avoid using #ifdef as much as possible. Change this to
> 
>   if (CONFIG_IS_ENABLED(SPI_FLASH_BAR) && !strcmp())
> 
>> +	if (!strcmp(nor->info->name, "s25fl256l"))
>> +		nor->fixups = &s25fl256l_fixups;
>> +#endif
> 
> *sigh* we really need to find a better way to specify fixups. Let me see 
> if I can figure something out. In the meantime, this should be fine.
> 
>>  #endif
>>  
>>  #ifdef CONFIG_SPI_FLASH_S28HS512T
>> -- 
>> 2.25.1
>>
>