Messages in this thread | | | Date | Tue, 5 Sep 2023 15:59:24 +0100 | Subject | Re: [PATCH v2 09/41] mtd: spi-nor: push 4k SE handling into spi_nor_select_uniform_erase() | From | Tudor Ambarus <> |
| |
On 8/22/23 08:09, Michael Walle wrote: > 4k sector erase sizes are only a thing with uniform erase types. Push > the "we want 4k erase sizes" handling into spi_nor_select_uniform_erase(). > > One might wonder why the former sector_size isn't used anymore. It is > because we either search for the largest erase size or if selected > through kconfig, the 4k erase size. Now, why is that correct? For this, > we have to differentiate between (1) flashes with SFDP and (2) without > SFDP. For (1), we just set one (or two if SECT_4K is set) erase types > and wanted_size is exactly one of these. > > For (2) things are a bit more complicated. For flashes which we don't
:)
> have in our flash_info database, the generic driver is used and > sector_size was already 0, which in turn selected the largest erase > size. For flashes which had SFDP and an entry in flash_info, sector_size > was always the largest sector and thus the largest erase type. > > Signed-off-by: Michael Walle <mwalle@kernel.org> > --- > drivers/mtd/spi-nor/core.c | 27 +++++++++------------------ > 1 file changed, 9 insertions(+), 18 deletions(-) > > diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c > index 68baf6032639..c84be791341e 100644 > --- a/drivers/mtd/spi-nor/core.c > +++ b/drivers/mtd/spi-nor/core.c > @@ -2512,13 +2512,6 @@ static int spi_nor_select_pp(struct spi_nor *nor, > /** > * spi_nor_select_uniform_erase() - select optimum uniform erase type > * @map: the erase map of the SPI NOR > - * @wanted_size: the erase type size to search for. Contains the value of > - * info->sector_size, the "small sector" size in case > - * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined or 0 if > - * there is no information about the sector size. The > - * latter is the case if the flash parameters are parsed > - * solely by SFDP, then the largest supported erase type > - * is selected. > * > * Once the optimum uniform sector erase command is found, disable all the > * other. > @@ -2526,13 +2519,16 @@ static int spi_nor_select_pp(struct spi_nor *nor, > * Return: pointer to erase type on success, NULL otherwise. > */ > static const struct spi_nor_erase_type * > -spi_nor_select_uniform_erase(struct spi_nor_erase_map *map, > - const u32 wanted_size) > +spi_nor_select_uniform_erase(struct spi_nor_erase_map *map) > { > const struct spi_nor_erase_type *tested_erase, *erase = NULL; > int i; > u8 uniform_erase_type = map->uniform_erase_type; > > + /* > + * Search for the biggest erase size, except for when compiled > + * to use 4k erases. > + */ > for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) { > if (!(uniform_erase_type & BIT(i))) > continue; > @@ -2544,10 +2540,11 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map, > continue; > > /* > - * If the current erase size is the one, stop here: > + * If the current erase size is the 4k one, stop here, > * we have found the right uniform Sector Erase command. > */ > - if (tested_erase->size == wanted_size) { > + if (IS_ENABLED(CONFIG_MTD_SPI_NOR_USE_4K_SECTORS) &&
one difference is that the is enabled check may now be evaluated for each erase type, but looks cleaner and I can live with that as this is set at probe time anyway.
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
> + tested_erase->size == SZ_4K) { > erase = tested_erase; > break; > } > @@ -2575,7 +2572,6 @@ static int spi_nor_select_erase(struct spi_nor *nor) > struct spi_nor_erase_map *map = &nor->params->erase_map; > const struct spi_nor_erase_type *erase = NULL; > struct mtd_info *mtd = &nor->mtd; > - u32 wanted_size = nor->info->sector_size; > int i; > > /* > @@ -2586,13 +2582,8 @@ static int spi_nor_select_erase(struct spi_nor *nor) > * manage the SPI flash memory as uniform with a single erase sector > * size, when possible. > */ > -#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS > - /* prefer "small sector" erase if possible */ > - wanted_size = 4096u; > -#endif > - > if (spi_nor_has_uniform_erase(nor)) { > - erase = spi_nor_select_uniform_erase(map, wanted_size); > + erase = spi_nor_select_uniform_erase(map); > if (!erase) > return -EINVAL; > nor->erase_opcode = erase->opcode; >
| |