| Message ID | 518a2f98449bc956e0e3d6b19ff60b18b08d973a.1631088494.git.Takahiro.Kuwano@infineon.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: spi-nor: Add support for Spansion S25FL256L | expand |
On 08/09/21 05:47PM, tkuw584924@gmail.com wrote: > From: Takahiro Kuwano <Takahiro.Kuwano@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@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? > +} > + > +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 >
On 9/18/2021 1:51 AM, Pratyush Yadav wrote: > On 08/09/21 05:47PM, tkuw584924@gmail.com wrote: >> From: Takahiro Kuwano <Takahiro.Kuwano@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@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 >> >
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 */ +} + +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 + if (!strcmp(nor->info->name, "s25fl256l")) + nor->fixups = &s25fl256l_fixups; +#endif #endif #ifdef CONFIG_SPI_FLASH_S28HS512T