2019-08-24 12:08:48

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 0/6] mtd: spi-nor: move manuf out of the core - batch 2

From: Tudor Ambarus <[email protected]>

Depends on 'mtd: spi-nor: move manuf out of the core - batch 1' series:
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=127121
which depends on:

Depends on 'mtd: spi-nor: move manuf out of the core - batch 0' series:
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=127030

v2:
- addressed all the comments
- all flash parameters and settings are now set in 'struct
spi_nor_flash_parameter', for a clearer separation between the SPI NOR
layer and the flash params.

Add post_sfdp() hook to tweak flash config. This series opens doors for 5/
from below.

In the quest of moving the manufacturer code out of the spi-nor core,
we want to impose the following sequence of calls:

1/ spi-nor core legacy flash parameters init:
spi_nor_default_init_params()

2/ MFR-based manufacturer flash parameters init:
nor->manufacturer->fixups->default_init()

3/ specific flash_info tweeks done when decisions can not be done just on
MFR:
nor->info->fixups->default_init()

4/ SFDP tables flash parameters init - SFDP knows better:
spi_nor_sfdp_init_params()

5/ post SFDP tables flash parameters updates - in case manufacturers get
the serial flash tables wrong or incomplete.
nor->info->fixups->post_sfdp()
The later can be extended to nor->manufacturer->fixups->post_sfdp() if
needed.

Tested on sst26vf064b with atmel-quadspi SPIMEM driver.

Boris Brezillon (4):
mtd: spi-nor: Add post_sfdp() hook to tweak flash config
mtd: spi-nor: Add spansion_post_sfdp_fixups()
mtd: spi-nor: Add a ->convert_addr() method
mtd: spi-nor: Add the SPI_NOR_XSR_RDY flag

Tudor Ambarus (2):
mtd: spi_nor: Add a ->setup() method
mtd: spi-nor: Add s3an_post_sfdp_fixups()

drivers/mtd/spi-nor/spi-nor.c | 549 +++++++++++++++++++++++-------------------
include/linux/mtd/spi-nor.h | 22 +-
2 files changed, 322 insertions(+), 249 deletions(-)

--
2.9.5


2019-08-24 12:09:30

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 4/6] mtd: spi_nor: Add a ->setup() method

From: Tudor Ambarus <[email protected]>

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().

Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 428 +++++++++++++++++++++---------------------
include/linux/mtd/spi-nor.h | 5 +
2 files changed, 224 insertions(+), 209 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 0dc6a683719e..17e6c96f9f9a 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4144,6 +4144,224 @@ 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;
+}
+
+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 atmel_set_default_init(struct spi_nor *nor)
{
nor->params.disable_block_protection = spi_nor_clear_sr_bp;
@@ -4247,6 +4465,7 @@ static void spi_nor_legacy_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;
@@ -4421,215 +4640,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;
-}
-
/**
* 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 f9f1947f7aeb..4752d08e9a3e 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.
* @disable_block_protection: disables block protection during power-up.
* @locking_ops: SPI NOR locking methods.
*/
@@ -514,6 +518,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);
int (*disable_block_protection)(struct spi_nor *nor);

const struct spi_nor_locking_ops *locking_ops;
--
2.9.5

2019-08-24 12:09:50

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()

From: Tudor Ambarus <[email protected]>

s3an_nor_scan() was overriding the opcode selection done in
spi_nor_default_setup(). Set nor->setup() method in order to
avoid the unnecessary call to spi_nor_default_setup().

Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 17e6c96f9f9a..5e16f293a83b 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2718,7 +2718,8 @@ static int spi_nor_check(struct spi_nor *nor)
return 0;
}

-static int s3an_nor_scan(struct spi_nor *nor)
+static int s3an_nor_setup(struct spi_nor *nor,
+ const struct spi_nor_hwcaps *hwcaps)
{
int ret;

@@ -4546,6 +4547,11 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor)
nor->mtd.erasesize = nor->info->sector_size;
}

+static void s3an_post_sfdp_fixups(struct spi_nor *nor)
+{
+ nor->params.setup = s3an_nor_setup;
+}
+
/**
* spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
* after SFDP has been parsed (is also called for SPI NORs that do not
@@ -4567,6 +4573,9 @@ static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
break;
}

+ if (nor->info->flags & SPI_S3AN)
+ s3an_post_sfdp_fixups(nor);
+
if (nor->info->fixups && nor->info->fixups->post_sfdp)
nor->info->fixups->post_sfdp(nor);
}
@@ -4932,12 +4941,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
return -EINVAL;
}

- if (info->flags & SPI_S3AN) {
- ret = s3an_nor_scan(nor);
- if (ret)
- return ret;
- }
-
/* Send all the required SPI flash commands to initialize device */
ret = spi_nor_init(nor);
if (ret)
--
2.9.5

2019-08-24 12:09:50

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 6/6] mtd: spi-nor: Add the SPI_NOR_XSR_RDY flag

From: Boris Brezillon <[email protected]>

S3AN flashes use a specific opcode to read the status register.
We currently use the SPI_S3AN flag to decide whether this specific
SR read opcode should be used, but SPI_S3AN is about to disappear, so
let's add a new flag.

Note that we use the same bit as SPI_S3AN implies SPI_NOR_XSR_RDY and
vice versa.

Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 5e16f293a83b..e76c23d1c54a 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -211,6 +211,14 @@ struct flash_info {
* bit. Must be used with
* SPI_NOR_HAS_LOCK.
*/
+#define SPI_NOR_XSR_RDY BIT(10) /*
+ * S3AN flashes have specific opcode to
+ * read the status register.
+ * Flags SPI_NOR_XSR_RDY and SPI_S3AN
+ * use the same bit as one implies the
+ * other, but we will get rid of
+ * SPI_S3AN soon.
+ */
#define SPI_S3AN BIT(10) /*
* Xilinx Spartan 3AN In-System Flash
* (MFR cannot be used for probing
@@ -4839,7 +4847,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
* spi_nor_wait_till_ready(). Xilinx S3AN share MFR
* with Atmel spi-nor
*/
- if (info->flags & SPI_S3AN)
+ if (info->flags & SPI_NOR_XSR_RDY)
nor->flags |= SNOR_F_READY_XSR_RDY;

if (info->flags & SPI_NOR_HAS_LOCK) {
--
2.9.5

2019-08-25 12:35:35

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] mtd: spi_nor: Add a ->setup() method

On Sat, 24 Aug 2019 12:07:14 +0000
<[email protected]> wrote:

> From: Tudor Ambarus <[email protected]>
>
> 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().
>
> Signed-off-by: Tudor Ambarus <[email protected]>

Reviewed-by: Boris Brezillon <[email protected]>

> ---
> drivers/mtd/spi-nor/spi-nor.c | 428 +++++++++++++++++++++---------------------
> include/linux/mtd/spi-nor.h | 5 +
> 2 files changed, 224 insertions(+), 209 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0dc6a683719e..17e6c96f9f9a 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4144,6 +4144,224 @@ 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;
> +}
> +
> +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 atmel_set_default_init(struct spi_nor *nor)
> {
> nor->params.disable_block_protection = spi_nor_clear_sr_bp;
> @@ -4247,6 +4465,7 @@ static void spi_nor_legacy_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;
> @@ -4421,215 +4640,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;
> -}
> -
> /**
> * 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 f9f1947f7aeb..4752d08e9a3e 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.
> * @disable_block_protection: disables block protection during power-up.
> * @locking_ops: SPI NOR locking methods.
> */
> @@ -514,6 +518,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);
> int (*disable_block_protection)(struct spi_nor *nor);
>
> const struct spi_nor_locking_ops *locking_ops;

2019-08-25 12:39:01

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()

On Sat, 24 Aug 2019 12:07:16 +0000
<[email protected]> wrote:

> From: Tudor Ambarus <[email protected]>
>
> s3an_nor_scan() was overriding the opcode selection done in
> spi_nor_default_setup(). Set nor->setup() method in order to
> avoid the unnecessary call to spi_nor_default_setup().
>
> Signed-off-by: Tudor Ambarus <[email protected]>

I guess you checked that nothing in the S3AN init was relying on things
done in the default_setup() implementation.

Reviewed-by: Boris Brezillon <[email protected]>

> ---
> drivers/mtd/spi-nor/spi-nor.c | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 17e6c96f9f9a..5e16f293a83b 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2718,7 +2718,8 @@ static int spi_nor_check(struct spi_nor *nor)
> return 0;
> }
>
> -static int s3an_nor_scan(struct spi_nor *nor)
> +static int s3an_nor_setup(struct spi_nor *nor,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> int ret;
>
> @@ -4546,6 +4547,11 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor)
> nor->mtd.erasesize = nor->info->sector_size;
> }
>
> +static void s3an_post_sfdp_fixups(struct spi_nor *nor)
> +{
> + nor->params.setup = s3an_nor_setup;
> +}
> +
> /**
> * spi_nor_post_sfdp_fixups() - Updates the flash's parameters and settings
> * after SFDP has been parsed (is also called for SPI NORs that do not
> @@ -4567,6 +4573,9 @@ static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
> break;
> }
>
> + if (nor->info->flags & SPI_S3AN)
> + s3an_post_sfdp_fixups(nor);
> +
> if (nor->info->fixups && nor->info->fixups->post_sfdp)
> nor->info->fixups->post_sfdp(nor);
> }
> @@ -4932,12 +4941,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> return -EINVAL;
> }
>
> - if (info->flags & SPI_S3AN) {
> - ret = s3an_nor_scan(nor);
> - if (ret)
> - return ret;
> - }
> -
> /* Send all the required SPI flash commands to initialize device */
> ret = spi_nor_init(nor);
> if (ret)