Re: [RESEND PATCH v3 14/20] mtd: spi_nor: Add a ->setup() method

Vignesh Raghavendra
2019-08-27T07:16:29Z

On 26/08/19 5:38 PM, Tudor.Ambarus@microchip.com wrote:
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>
> nor->params.setup() configures the SPI NOR memory. Useful for SPI NOR
> flashes that have peculiarities to the SPI NOR standard, e.g.
> different opcodes, specific address calculation, page size, etc.
> Right now the only user will be the S3AN chips, but other
> manufacturers can implement it if needed.
>
> Move spi_nor_setup() related code in order to avoid a forward
> declaration to spi_nor_default_setup().
>
> Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---

Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com>

Regards
Vignesh

> v3: collect R-b, rebase on previous commits
>
> drivers/mtd/spi-nor/spi-nor.c | 432 +++++++++++++++++++++---------------------
> include/linux/mtd/spi-nor.h | 5 +
> 2 files changed, 226 insertions(+), 211 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index b96a7066a36c..2aca56e07341 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4144,6 +4144,226 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
> return err;
> }
>
> +static int spi_nor_select_read(struct spi_nor *nor,
> + u32 shared_hwcaps)
> +{
> + int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
> + const struct spi_nor_read_command *read;
> +
> + if (best_match < 0)
> + return -EINVAL;
> +
> + cmd = spi_nor_hwcaps_read2cmd(BIT(best_match));
> + if (cmd < 0)
> + return -EINVAL;
> +
> + read = &nor->params.reads[cmd];
> + nor->read_opcode = read->opcode;
> + nor->read_proto = read->proto;
> +
> + /*
> + * In the spi-nor framework, we don't need to make the difference
> + * between mode clock cycles and wait state clock cycles.
> + * Indeed, the value of the mode clock cycles is used by a QSPI
> + * flash memory to know whether it should enter or leave its 0-4-4
> + * (Continuous Read / XIP) mode.
> + * eXecution In Place is out of the scope of the mtd sub-system.
> + * Hence we choose to merge both mode and wait state clock cycles
> + * into the so called dummy clock cycles.
> + */
> + nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
> + return 0;
> +}
> +
> +static int spi_nor_select_pp(struct spi_nor *nor,
> + u32 shared_hwcaps)
> +{
> + int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_PP_MASK) - 1;
> + const struct spi_nor_pp_command *pp;
> +
> + if (best_match < 0)
> + return -EINVAL;
> +
> + cmd = spi_nor_hwcaps_pp2cmd(BIT(best_match));
> + if (cmd < 0)
> + return -EINVAL;
> +
> + pp = &nor->params.page_programs[cmd];
> + nor->program_opcode = pp->opcode;
> + nor->write_proto = pp->proto;
> + return 0;
> +}
> +
> +/**
> + * 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 or of the "small sector" size in case
> + * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined.
> + *
> + * Once the optimum uniform sector erase command is found, disable all the
> + * other.
> + *
> + * 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)
> +{
> + const struct spi_nor_erase_type *tested_erase, *erase = NULL;
> + int i;
> + u8 uniform_erase_type = map->uniform_erase_type;
> +
> + for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> + if (!(uniform_erase_type & BIT(i)))
> + continue;
> +
> + tested_erase = &map->erase_type[i];
> +
> + /*
> + * If the current erase size is the one, stop here:
> + * we have found the right uniform Sector Erase command.
> + */
> + if (tested_erase->size == wanted_size) {
> + erase = tested_erase;
> + break;
> + }
> +
> + /*
> + * Otherwise, the current erase size is still a valid canditate.
> + * Select the biggest valid candidate.
> + */
> + if (!erase && tested_erase->size)
> + erase = tested_erase;
> + /* keep iterating to find the wanted_size */
> + }
> +
> + if (!erase)
> + return NULL;
> +
> + /* Disable all other Sector Erase commands. */
> + map->uniform_erase_type &= ~SNOR_ERASE_TYPE_MASK;
> + map->uniform_erase_type |= BIT(erase - map->erase_type);
> + return erase;
> +}
> +
> +static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
> +{
> + struct spi_nor_erase_map *map = &nor->params.erase_map;
> + const struct spi_nor_erase_type *erase = NULL;
> + struct mtd_info *mtd = &nor->mtd;
> + int i;
> +
> + /*
> + * The previous implementation handling Sector Erase commands assumed
> + * that the SPI flash memory has an uniform layout then used only one
> + * of the supported erase sizes for all Sector Erase commands.
> + * So to be backward compatible, the new implementation also tries to
> + * 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);
> + if (!erase)
> + return -EINVAL;
> + nor->erase_opcode = erase->opcode;
> + mtd->erasesize = erase->size;
> + return 0;
> + }
> +
> + /*
> + * For non-uniform SPI flash memory, set mtd->erasesize to the
> + * maximum erase sector size. No need to set nor->erase_opcode.
> + */
> + for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> + if (map->erase_type[i].size) {
> + erase = &map->erase_type[i];
> + break;
> + }
> + }
> +
> + if (!erase)
> + return -EINVAL;
> +
> + mtd->erasesize = erase->size;
> + return 0;
> +}
> +
> +static int spi_nor_default_setup(struct spi_nor *nor,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> + struct spi_nor_flash_parameter *params = &nor->params;
> + u32 ignored_mask, shared_mask;
> + int err;
> +
> + /*
> + * Keep only the hardware capabilities supported by both the SPI
> + * controller and the SPI flash memory.
> + */
> + shared_mask = hwcaps->mask & params->hwcaps.mask;
> +
> + if (nor->spimem) {
> + /*
> + * When called from spi_nor_probe(), all caps are set and we
> + * need to discard some of them based on what the SPI
> + * controller actually supports (using spi_mem_supports_op()).
> + */
> + spi_nor_spimem_adjust_hwcaps(nor, &shared_mask);
> + } else {
> + /*
> + * SPI n-n-n protocols are not supported when the SPI
> + * controller directly implements the spi_nor interface.
> + * Yet another reason to switch to spi-mem.
> + */
> + ignored_mask = SNOR_HWCAPS_X_X_X;
> + if (shared_mask & ignored_mask) {
> + dev_dbg(nor->dev,
> + "SPI n-n-n protocols are not supported.\n");
> + shared_mask &= ~ignored_mask;
> + }
> + }
> +
> + /* Select the (Fast) Read command. */
> + err = spi_nor_select_read(nor, shared_mask);
> + if (err) {
> + dev_err(nor->dev,
> + "can't select read settings supported by both the SPI controller and memory.\n");
> + return err;
> + }
> +
> + /* Select the Page Program command. */
> + err = spi_nor_select_pp(nor, shared_mask);
> + if (err) {
> + dev_err(nor->dev,
> + "can't select write settings supported by both the SPI controller and memory.\n");
> + return err;
> + }
> +
> + /* Select the Sector Erase command. */
> + err = spi_nor_select_erase(nor, nor->info->sector_size);
> + if (err) {
> + dev_err(nor->dev,
> + "can't select erase settings supported by both the SPI controller and memory.\n");
> + return err;
> + }
> +
> + return 0;
> +}
> +
> +static int spi_nor_setup(struct spi_nor *nor,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> + if (!nor->params.setup)
> + return 0;
> +
> + return nor->params.setup(nor, hwcaps);
> +}
> +
> static void macronix_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = macronix_quad_enable;
> @@ -4229,6 +4449,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> /* Initialize legacy flash parameters and settings. */
> params->quad_enable = spansion_quad_enable;
> params->set_4byte = spansion_set_4byte;
> + params->setup = spi_nor_default_setup;
>
> /* Set SPI NOR sizes. */
> params->size = (u64)info->sector_size * info->n_sectors;
> @@ -4403,217 +4624,6 @@ static void spi_nor_init_params(struct spi_nor *nor)
> spi_nor_late_init_params(nor);
> }
>
> -static int spi_nor_select_read(struct spi_nor *nor,
> - u32 shared_hwcaps)
> -{
> - int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
> - const struct spi_nor_read_command *read;
> -
> - if (best_match < 0)
> - return -EINVAL;
> -
> - cmd = spi_nor_hwcaps_read2cmd(BIT(best_match));
> - if (cmd < 0)
> - return -EINVAL;
> -
> - read = &nor->params.reads[cmd];
> - nor->read_opcode = read->opcode;
> - nor->read_proto = read->proto;
> -
> - /*
> - * In the spi-nor framework, we don't need to make the difference
> - * between mode clock cycles and wait state clock cycles.
> - * Indeed, the value of the mode clock cycles is used by a QSPI
> - * flash memory to know whether it should enter or leave its 0-4-4
> - * (Continuous Read / XIP) mode.
> - * eXecution In Place is out of the scope of the mtd sub-system.
> - * Hence we choose to merge both mode and wait state clock cycles
> - * into the so called dummy clock cycles.
> - */
> - nor->read_dummy = read->num_mode_clocks + read->num_wait_states;
> - return 0;
> -}
> -
> -static int spi_nor_select_pp(struct spi_nor *nor,
> - u32 shared_hwcaps)
> -{
> - int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_PP_MASK) - 1;
> - const struct spi_nor_pp_command *pp;
> -
> - if (best_match < 0)
> - return -EINVAL;
> -
> - cmd = spi_nor_hwcaps_pp2cmd(BIT(best_match));
> - if (cmd < 0)
> - return -EINVAL;
> -
> - pp = &nor->params.page_programs[cmd];
> - nor->program_opcode = pp->opcode;
> - nor->write_proto = pp->proto;
> - return 0;
> -}
> -
> -/**
> - * 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 or of the "small sector" size in case
> - * CONFIG_MTD_SPI_NOR_USE_4K_SECTORS is defined.
> - *
> - * Once the optimum uniform sector erase command is found, disable all the
> - * other.
> - *
> - * 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)
> -{
> - const struct spi_nor_erase_type *tested_erase, *erase = NULL;
> - int i;
> - u8 uniform_erase_type = map->uniform_erase_type;
> -
> - for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> - if (!(uniform_erase_type & BIT(i)))
> - continue;
> -
> - tested_erase = &map->erase_type[i];
> -
> - /*
> - * If the current erase size is the one, stop here:
> - * we have found the right uniform Sector Erase command.
> - */
> - if (tested_erase->size == wanted_size) {
> - erase = tested_erase;
> - break;
> - }
> -
> - /*
> - * Otherwise, the current erase size is still a valid canditate.
> - * Select the biggest valid candidate.
> - */
> - if (!erase && tested_erase->size)
> - erase = tested_erase;
> - /* keep iterating to find the wanted_size */
> - }
> -
> - if (!erase)
> - return NULL;
> -
> - /* Disable all other Sector Erase commands. */
> - map->uniform_erase_type &= ~SNOR_ERASE_TYPE_MASK;
> - map->uniform_erase_type |= BIT(erase - map->erase_type);
> - return erase;
> -}
> -
> -static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
> -{
> - struct spi_nor_erase_map *map = &nor->params.erase_map;
> - const struct spi_nor_erase_type *erase = NULL;
> - struct mtd_info *mtd = &nor->mtd;
> - int i;
> -
> - /*
> - * The previous implementation handling Sector Erase commands assumed
> - * that the SPI flash memory has an uniform layout then used only one
> - * of the supported erase sizes for all Sector Erase commands.
> - * So to be backward compatible, the new implementation also tries to
> - * 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);
> - if (!erase)
> - return -EINVAL;
> - nor->erase_opcode = erase->opcode;
> - mtd->erasesize = erase->size;
> - return 0;
> - }
> -
> - /*
> - * For non-uniform SPI flash memory, set mtd->erasesize to the
> - * maximum erase sector size. No need to set nor->erase_opcode.
> - */
> - for (i = SNOR_ERASE_TYPE_MAX - 1; i >= 0; i--) {
> - if (map->erase_type[i].size) {
> - erase = &map->erase_type[i];
> - break;
> - }
> - }
> -
> - if (!erase)
> - return -EINVAL;
> -
> - mtd->erasesize = erase->size;
> - return 0;
> -}
> -
> -static int spi_nor_setup(struct spi_nor *nor,
> - const struct spi_nor_hwcaps *hwcaps)
> -{
> - struct spi_nor_flash_parameter *params = &nor->params;
> - u32 ignored_mask, shared_mask;
> - int err;
> -
> - /*
> - * Keep only the hardware capabilities supported by both the SPI
> - * controller and the SPI flash memory.
> - */
> - shared_mask = hwcaps->mask & params->hwcaps.mask;
> -
> - if (nor->spimem) {
> - /*
> - * When called from spi_nor_probe(), all caps are set and we
> - * need to discard some of them based on what the SPI
> - * controller actually supports (using spi_mem_supports_op()).
> - */
> - spi_nor_spimem_adjust_hwcaps(nor, &shared_mask);
> - } else {
> - /*
> - * SPI n-n-n protocols are not supported when the SPI
> - * controller directly implements the spi_nor interface.
> - * Yet another reason to switch to spi-mem.
> - */
> - ignored_mask = SNOR_HWCAPS_X_X_X;
> - if (shared_mask & ignored_mask) {
> - dev_dbg(nor->dev,
> - "SPI n-n-n protocols are not supported.\n");
> - shared_mask &= ~ignored_mask;
> - }
> - }
> -
> - /* Select the (Fast) Read command. */
> - err = spi_nor_select_read(nor, shared_mask);
> - if (err) {
> - dev_err(nor->dev,
> - "can't select read settings supported by both the SPI controller and memory.\n");
> - return err;
> - }
> -
> - /* Select the Page Program command. */
> - err = spi_nor_select_pp(nor, shared_mask);
> - if (err) {
> - dev_err(nor->dev,
> - "can't select write settings supported by both the SPI controller and memory.\n");
> - return err;
> - }
> -
> - /* Select the Sector Erase command. */
> - err = spi_nor_select_erase(nor, nor->info->sector_size);
> - if (err) {
> - dev_err(nor->dev,
> - "can't select erase settings supported by both the SPI controller and memory.\n");
> - return err;
> - }
> -
> - return 0;
> -}
> -
> /**
> * spi_nor_quad_enable() - enable Quad I/O if needed.
> * @nor: pointer to a 'struct spi_nor'
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 35aad92a4ff8..fc0b4b19c900 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -498,6 +498,10 @@ struct spi_nor_locking_ops {
> * @convert_addr: converts an absolute address into something the flash
> * will understand. Particularly useful when pagesize is
> * not a power-of-2.
> + * @setup: configures the SPI NOR memory. Useful for SPI NOR
> + * flashes that have peculiarities to the SPI NOR standard
> + * e.g. different opcodes, specific address calculation,
> + * page size, etc.
> * @locking_ops: SPI NOR locking methods.
> */
> struct spi_nor_flash_parameter {
> @@ -513,6 +517,7 @@ struct spi_nor_flash_parameter {
> int (*quad_enable)(struct spi_nor *nor);
> int (*set_4byte)(struct spi_nor *nor, bool enable);
> u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
> + int (*setup)(struct spi_nor *nor, const struct spi_nor_hwcaps *hwcaps);
>
> const struct spi_nor_locking_ops *locking_ops;
> };
>

--
Regards
Vignesh