On 9/18/2021 1:51 AM, Pratyush Yadav wrote:
> On 08/09/21 05:47PM, tkuw584...@gmail.com wrote:
>> From: Takahiro Kuwano <takahiro.kuw...@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.kuw...@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
>>
>
