2019-08-26 12:10:59

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 00/20] mtd: spi-nor: move manuf out of the core

From: Tudor Ambarus <[email protected]>

v3:
- Drop patches:
"mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'"
"mtd: spi-nor: Rework the disabling of block write protection"
and replace them with the RFC patch:
"mtd: spi-nor: Rework the disabling of block write protection"
- rename spi_nor_legacy_init_params() to spi_nor_info_init_params()
- rebase patches and send them all in a single patch set.

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.

In order to test this, you'll have to merge v5.3-rc5 in spi-nor/next.
This patch set depends on
'commit 834de5c1aa76 ("mtd: spi-nor: Fix the disabling of write protection at init")

The scope of the "mtd: spi-nor: move manuf out of the core" batches,
is to move all manufacturer specific code out of the spi-nor core.

In the quest of removing the manufacturer specific code from the spi-nor
core, we want to impose a timeline/priority on how the flash parameters
are updated. As of now. the flash parameters initialization logic is as
following:

a/ default flash parameters init in spi_nor_init_params()
b/ manufacturer specific flash parameters updates, split across entire
spi-nor core code
c/ flash parameters updates based on SFDP tables
d/ post BFPT flash parameter updates

With the "mtd: spi-nor: move manuf out of the core" batches, 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.

Setting of flash parameters will no longer be spread interleaved across
the spi-nor core, there will be a clear separation on who and when will
update the flash parameters.

Tested on sst26vf064b with atmel-quadspi SPIMEM driver.

Boris Brezillon (7):
mtd: spi-nor: Add a default_init() fixup hook for gd25q256
mtd: spi-nor: Create a ->set_4byte() method
mtd: spi-nor: Rework the SPI NOR lock/unlock logic
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 (13):
mtd: spi-nor: Regroup flash parameter and settings
mtd: spi-nor: Use nor->params
mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'
mtd: spi-nor: Add default_init() hook to tweak flash parameters
mtd: spi_nor: Move manufacturer quad_enable() in ->default_init()
mtd: spi-nor: Split spi_nor_init_params()
mtd: spi_nor: Add a ->setup() method
mtd: spi-nor: Add s3an_post_sfdp_fixups()
mtd: spi-nor: Bring flash params init together
mtd: spi_nor: Introduce spi_nor_set_addr_width()
mtd: spi-nor: Introduce spi_nor_get_flash_info()
mtd: spi-nor: Rework the disabling of block write protection

drivers/mtd/spi-nor/spi-nor.c | 1304 +++++++++++++++++++++++------------------
include/linux/mtd/spi-nor.h | 298 +++++++---
2 files changed, 927 insertions(+), 675 deletions(-)

--
2.9.5


2019-08-26 12:11:11

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 11/20] mtd: spi-nor: Add post_sfdp() hook to tweak flash config

From: Boris Brezillon <[email protected]>

SFDP tables are sometimes wrong and we need a way to override the
config chosen by the SFDP parsing logic without discarding all of it.

Add a new hook called after the SFDP parsing has taken place to deal
with such problems.

Signed-off-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
v3: no changes, rebase on previous commits

drivers/mtd/spi-nor/spi-nor.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3f997797fa9d..b8caf5171ff5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -158,6 +158,11 @@ struct sfdp_bfpt {
* flash parameters when information provided by the flash_info
* table is incomplete or wrong.
* @post_bfpt: called after the BFPT table has been parsed
+ * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
+ * that do not support RDSFDP). Typically used to tweak various
+ * parameters that could not be extracted by other means (i.e.
+ * when information provided by the SFDP/flash_info tables are
+ * incomplete or wrong).
*
* Those hooks can be used to tweak the SPI NOR configuration when the SFDP
* table is broken or not available.
@@ -168,6 +173,7 @@ struct spi_nor_fixups {
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt,
struct spi_nor_flash_parameter *params);
+ void (*post_sfdp)(struct spi_nor *nor);
};

struct flash_info {
@@ -4299,6 +4305,22 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
}

/**
+ * 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
+ * support RDSFDP).
+ * @nor: pointer to a 'struct spi_nor'
+ *
+ * Typically used to tweak various parameters that could not be extracted by
+ * other means (i.e. when information provided by the SFDP/flash_info tables
+ * are incomplete or wrong).
+ */
+static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
+{
+ if (nor->info->fixups && nor->info->fixups->post_sfdp)
+ nor->info->fixups->post_sfdp(nor);
+}
+
+/**
* spi_nor_late_init_params() - Late initialization of default flash parameters.
* @nor: pointer to a 'struct spi_nor'
*
@@ -4341,7 +4363,14 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
* flash parameters and settings imediately after parsing the Basic Flash
* Parameter Table.
*
- * 4/ Late default flash parameters initialization, used when the
+ * which can be overwritten by:
+ * 4/ Post SFDP flash parameters initialization. Used to tweak various
+ * parameters that could not be extracted by other means (i.e. when
+ * information provided by the SFDP/flash_info tables are incomplete or
+ * wrong).
+ * spi_nor_post_sfdp_fixups()
+ *
+ * 5/ Late default flash parameters initialization, used when the
* ->default_init() hook or the SFDP parser do not set specific params.
* spi_nor_late_init_params()
*/
@@ -4355,6 +4384,8 @@ static void spi_nor_init_params(struct spi_nor *nor)
!(nor->info->flags & SPI_NOR_SKIP_SFDP))
spi_nor_sfdp_init_params(nor);

+ spi_nor_post_sfdp_fixups(nor);
+
spi_nor_late_init_params(nor);
}

--
2.9.5

2019-08-26 12:11:16

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 15/20] 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]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: no changes, rebase on previous commits

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 2aca56e07341..edf1c8badac9 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;

@@ -4530,6 +4531,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
@@ -4551,6 +4557,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);
}
@@ -4899,12 +4908,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-26 12:11:20

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 14/20] 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().

Reviewed-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
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;
};
--
2.9.5

2019-08-26 12:11:26

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 10/20] mtd: spi-nor: Rework the SPI NOR lock/unlock logic

From: Boris Brezillon <[email protected]>

Add the SNOR_F_HAS_LOCK flag and set it when SPI_NOR_HAS_LOCK is set
in the flash_info entry or when it's a Micron or ST flash.

Move the locking hooks in a separate struct so that we have just
one field to update when we change the locking implementation.

Signed-off-by: Boris Brezillon <[email protected]>
[[email protected]: use ->default_init() hook, introduce
spi_nor_late_init_params(), set ops in nor->params]
Signed-off-by: Tudor Ambarus <[email protected]>
---
v3: no changes, clear_sr_bp() is handled in the last patch of the
series.

drivers/mtd/spi-nor/spi-nor.c | 50 ++++++++++++++++++++++++++++++++-----------
include/linux/mtd/spi-nor.h | 23 ++++++++++++++------
2 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 235e82a121a1..3f997797fa9d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1598,6 +1598,12 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
return stm_is_locked_sr(nor, ofs, len, status);
}

+static const struct spi_nor_locking_ops stm_locking_ops = {
+ .lock = stm_lock,
+ .unlock = stm_unlock,
+ .is_locked = stm_is_locked,
+};
+
static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
{
struct spi_nor *nor = mtd_to_spi_nor(mtd);
@@ -1607,7 +1613,7 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
if (ret)
return ret;

- ret = nor->flash_lock(nor, ofs, len);
+ ret = nor->params.locking_ops->lock(nor, ofs, len);

spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK);
return ret;
@@ -1622,7 +1628,7 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
if (ret)
return ret;

- ret = nor->flash_unlock(nor, ofs, len);
+ ret = nor->params.locking_ops->unlock(nor, ofs, len);

spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
return ret;
@@ -1637,7 +1643,7 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
if (ret)
return ret;

- ret = nor->flash_is_locked(nor, ofs, len);
+ ret = nor->params.locking_ops->is_locked(nor, ofs, len);

spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
return ret;
@@ -4148,6 +4154,7 @@ static void macronix_set_default_init(struct spi_nor *nor)

static void st_micron_set_default_init(struct spi_nor *nor)
{
+ nor->flags = SNOR_F_HAS_LOCK;
nor->params.quad_enable = NULL;
nor->params.set_4byte = st_micron_set_4byte;
}
@@ -4292,6 +4299,23 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
}

/**
+ * spi_nor_late_init_params() - Late initialization of default flash parameters.
+ * @nor: pointer to a 'struct spi_nor'
+ *
+ * Used to set default flash parameters and settings when the ->default_init()
+ * hook or the SFDP parser let voids.
+ */
+static void spi_nor_late_init_params(struct spi_nor *nor)
+{
+ /*
+ * NOR protection support. When locking_ops are not provided, we pick
+ * the default ones.
+ */
+ if (nor->flags & SNOR_F_HAS_LOCK && !nor->params.locking_ops)
+ nor->params.locking_ops = &stm_locking_ops;
+}
+
+/**
* spi_nor_init_params() - Initialize the flash's parameters and settings.
* @nor: pointer to a 'struct spi-nor'.
*
@@ -4316,6 +4340,10 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
* Please not that there is a ->post_bfpt() fixup hook that can overwrite the
* flash parameters and settings imediately after parsing the Basic Flash
* Parameter Table.
+ *
+ * 4/ Late default flash parameters initialization, used when the
+ * ->default_init() hook or the SFDP parser do not set specific params.
+ * spi_nor_late_init_params()
*/
static void spi_nor_init_params(struct spi_nor *nor)
{
@@ -4326,6 +4354,8 @@ static void spi_nor_init_params(struct spi_nor *nor)
if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
!(nor->info->flags & SPI_NOR_SKIP_SFDP))
spi_nor_sfdp_init_params(nor);
+
+ spi_nor_late_init_params(nor);
}

static int spi_nor_select_read(struct spi_nor *nor,
@@ -4707,6 +4737,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (info->flags & SPI_S3AN)
nor->flags |= SNOR_F_READY_XSR_RDY;

+ if (info->flags & SPI_NOR_HAS_LOCK)
+ nor->flags |= SNOR_F_HAS_LOCK;
+
/*
* Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
* with the software protection bits set.
@@ -4731,16 +4764,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
mtd->_read = spi_nor_read;
mtd->_resume = spi_nor_resume;

- /* NOR protection support for STmicro/Micron chips and similar */
- if (JEDEC_MFR(info) == SNOR_MFR_ST ||
- JEDEC_MFR(info) == SNOR_MFR_MICRON ||
- info->flags & SPI_NOR_HAS_LOCK) {
- nor->flash_lock = stm_lock;
- nor->flash_unlock = stm_unlock;
- nor->flash_is_locked = stm_is_locked;
- }
-
- if (nor->flash_lock && nor->flash_unlock && nor->flash_is_locked) {
+ if (nor->params.locking_ops) {
mtd->_lock = spi_nor_lock;
mtd->_unlock = spi_nor_unlock;
mtd->_is_locked = spi_nor_is_locked;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 7da89dd483cb..ea3bcac54dc2 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,7 @@ enum spi_nor_option_flags {
SNOR_F_BROKEN_RESET = BIT(6),
SNOR_F_4B_OPCODES = BIT(7),
SNOR_F_HAS_4BAIT = BIT(8),
+ SNOR_F_HAS_LOCK = BIT(9),
};

/**
@@ -466,6 +467,18 @@ enum spi_nor_pp_command_index {
struct spi_nor;

/**
+ * struct spi_nor_locking_ops - SPI NOR locking methods
+ * @lock: lock a region of the SPI NOR.
+ * @unlock: unlock a region of the SPI NOR.
+ * @is_locked: check if a region of the SPI NOR is completely locked
+ */
+struct spi_nor_locking_ops {
+ int (*lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
+ int (*unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
+ int (*is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
+};
+
+/**
* struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
* Includes legacy flash parameters and settings that can be overwritten
* by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
@@ -483,6 +496,7 @@ struct spi_nor;
* Table.
* @quad_enable: enables SPI NOR quad mode.
* @set_4byte: puts the SPI NOR in 4 byte addressing mode.
+ * @locking_ops: SPI NOR locking methods.
*/
struct spi_nor_flash_parameter {
u64 size;
@@ -496,6 +510,8 @@ struct spi_nor_flash_parameter {

int (*quad_enable)(struct spi_nor *nor);
int (*set_4byte)(struct spi_nor *nor, bool enable);
+
+ const struct spi_nor_locking_ops *locking_ops;
};

/**
@@ -536,10 +552,6 @@ struct flash_info;
* @erase: [DRIVER-SPECIFIC] erase a sector of the SPI NOR
* at the offset @offs; if not provided by the driver,
* spi-nor will send the erase opcode via write_reg()
- * @flash_lock: [FLASH-SPECIFIC] lock a region of the SPI NOR
- * @flash_unlock: [FLASH-SPECIFIC] unlock a region of the SPI NOR
- * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
- * completely locked
* @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
* the SPI NOR Status Register.
* @params: [FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
@@ -579,9 +591,6 @@ struct spi_nor {
size_t len, const u_char *write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);

- int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
- int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
- int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
int (*clear_sr_bp)(struct spi_nor *nor);
struct spi_nor_flash_parameter params;

--
2.9.5

2019-08-26 12:11:30

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 08/20] mtd: spi-nor: Split spi_nor_init_params()

From: Tudor Ambarus <[email protected]>

Add functions to delimit what the chunks of code do:

static void spi_nor_init_params()
{
spi_nor_info_init_params()
spi_nor_manufacturer_init_params()
spi_nor_sfdp_init_params()
}

Add descriptions to all methods.

spi_nor_init_params() becomes of type void, as all its children
return void.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: rename spi_nor_legacy_init_params() to spi_nor_info_init_params()

drivers/mtd/spi-nor/spi-nor.c | 83 ++++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 2a239531704a..1e7f8dc3457d 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4186,7 +4186,34 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
nor->info->fixups->default_init(nor);
}

-static int spi_nor_init_params(struct spi_nor *nor)
+/**
+ * spi_nor_sfdp_init_params() - Initialize the flash's parameters and settings
+ * based on JESD216 SFDP standard.
+ * @nor: pointer to a 'struct spi-nor'.
+ *
+ * The method has a roll-back mechanism: in case the SFDP parsing fails, the
+ * legacy flash parameters and settings will be restored.
+ */
+static void spi_nor_sfdp_init_params(struct spi_nor *nor)
+{
+ struct spi_nor_flash_parameter sfdp_params;
+
+ memcpy(&sfdp_params, &nor->params, sizeof(sfdp_params));
+
+ if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
+ nor->addr_width = 0;
+ nor->flags &= ~SNOR_F_4B_OPCODES;
+ } else {
+ memcpy(&nor->params, &sfdp_params, sizeof(nor->params));
+ }
+}
+
+/**
+ * spi_nor_info_init_params() - Initialize the flash's parameters and settings
+ * based on nor->info data.
+ * @nor: pointer to a 'struct spi-nor'.
+ */
+static void spi_nor_info_init_params(struct spi_nor *nor)
{
struct spi_nor_flash_parameter *params = &nor->params;
struct spi_nor_erase_map *map = &params->erase_map;
@@ -4260,25 +4287,43 @@ static int spi_nor_init_params(struct spi_nor *nor)
spi_nor_set_erase_type(&map->erase_type[i], info->sector_size,
SPINOR_OP_SE);
spi_nor_init_uniform_erase_map(map, erase_mask, params->size);
+}

+/**
+ * spi_nor_init_params() - Initialize the flash's parameters and settings.
+ * @nor: pointer to a 'struct spi-nor'.
+ *
+ * The flash parameters and settings are initialized based on a sequence of
+ * calls that are ordered by priority:
+ *
+ * 1/ Default flash parameters initialization. The initializations are done
+ * based on nor->info data:
+ * spi_nor_info_init_params()
+ *
+ * which can be overwritten by:
+ * 2/ Manufacturer flash parameters initialization. The initializations are
+ * done based on MFR register, or when the decisions can not be done solely
+ * based on MFR, by using specific flash_info tweeks, ->default_init():
+ * spi_nor_manufacturer_init_params()
+ *
+ * which can be overwritten by:
+ * 3/ SFDP flash parameters initialization. JESD216 SFDP is a standard and
+ * should be more accurate that the above.
+ * spi_nor_sfdp_init_params()
+ *
+ * Please not that there is a ->post_bfpt() fixup hook that can overwrite the
+ * flash parameters and settings imediately after parsing the Basic Flash
+ * Parameter Table.
+ */
+static void spi_nor_init_params(struct spi_nor *nor)
+{
+ spi_nor_info_init_params(nor);

spi_nor_manufacturer_init_params(nor);

- if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
- !(info->flags & SPI_NOR_SKIP_SFDP)) {
- struct spi_nor_flash_parameter sfdp_params;
-
- memcpy(&sfdp_params, params, sizeof(sfdp_params));
-
- if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
- nor->addr_width = 0;
- nor->flags &= ~SNOR_F_4B_OPCODES;
- } else {
- memcpy(params, &sfdp_params, sizeof(*params));
- }
- }
-
- return 0;
+ if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
+ !(nor->info->flags & SPI_NOR_SKIP_SFDP))
+ spi_nor_sfdp_init_params(nor);
}

static int spi_nor_select_read(struct spi_nor *nor,
@@ -4670,10 +4715,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->info->flags & SPI_NOR_HAS_LOCK)
nor->clear_sr_bp = spi_nor_clear_sr_bp;

- /* Parse the Serial Flash Discoverable Parameters table. */
- ret = spi_nor_init_params(nor);
- if (ret)
- return ret;
+ /* Init flash parameters based on flash_info struct and SFDP */
+ spi_nor_init_params(nor);

if (!mtd->name)
mtd->name = dev_name(dev);
--
2.9.5

2019-08-26 12:11:46

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 09/20] mtd: spi-nor: Create a ->set_4byte() method

From: Boris Brezillon <[email protected]>

The procedure used to enable 4 byte addressing mode depends on the NOR
device, so let's provide a hook so that manufacturer specific handling
can be implemented in a sane way.

Signed-off-by: Boris Brezillon <[email protected]>
[[email protected]: use nor->params.set_4byte() instead of
nor->set_4byte()]
Signed-off-by: Tudor Ambarus <[email protected]>
---
v3: no changes

drivers/mtd/spi-nor/spi-nor.c | 76 ++++++++++++++++++++++---------------------
include/linux/mtd/spi-nor.h | 2 ++
2 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 1e7f8dc3457d..235e82a121a1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -633,6 +633,17 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
NULL, 0);
}

+static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
+{
+ int ret;
+
+ write_enable(nor);
+ ret = macronix_set_4byte(nor, enable);
+ write_disable(nor);
+
+ return ret;
+}
+
static int spansion_set_4byte(struct spi_nor *nor, bool enable)
{
nor->bouncebuf[0] = enable << 7;
@@ -667,45 +678,24 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
return nor->write_reg(nor, SPINOR_OP_WREAR, nor->bouncebuf, 1);
}

-/* Enable/disable 4-byte addressing mode. */
-static int set_4byte(struct spi_nor *nor, bool enable)
+static int winbond_set_4byte(struct spi_nor *nor, bool enable)
{
- int status;
- bool need_wren = false;
-
- switch (JEDEC_MFR(nor->info)) {
- case SNOR_MFR_ST:
- case SNOR_MFR_MICRON:
- /* Some Micron need WREN command; all will accept it */
- need_wren = true;
- /* fall through */
- case SNOR_MFR_MACRONIX:
- case SNOR_MFR_WINBOND:
- if (need_wren)
- write_enable(nor);
+ int ret;

- status = macronix_set_4byte(nor, enable);
- if (need_wren)
- write_disable(nor);
+ ret = macronix_set_4byte(nor, enable);
+ if (ret || enable)
+ return ret;

- if (!status && !enable &&
- JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
- /*
- * On Winbond W25Q256FV, leaving 4byte mode causes
- * the Extended Address Register to be set to 1, so all
- * 3-byte-address reads come from the second 16M.
- * We must clear the register to enable normal behavior.
- */
- write_enable(nor);
- spi_nor_write_ear(nor, 0);
- write_disable(nor);
- }
+ /*
+ * On Winbond W25Q256FV, leaving 4byte mode causes the Extended Address
+ * Register to be set to 1, so all 3-byte-address reads come from the
+ * second 16M. We must clear the register to enable normal behavior.
+ */
+ write_enable(nor);
+ ret = spi_nor_write_ear(nor, 0);
+ write_disable(nor);

- return status;
- default:
- /* Spansion style */
- return spansion_set_4byte(nor, enable);
- }
+ return ret;
}

static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
@@ -4153,11 +4143,18 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
static void macronix_set_default_init(struct spi_nor *nor)
{
nor->params.quad_enable = macronix_quad_enable;
+ nor->params.set_4byte = macronix_set_4byte;
}

static void st_micron_set_default_init(struct spi_nor *nor)
{
nor->params.quad_enable = NULL;
+ nor->params.set_4byte = st_micron_set_4byte;
+}
+
+static void winbond_set_default_init(struct spi_nor *nor)
+{
+ nor->params.set_4byte = winbond_set_4byte;
}

/**
@@ -4178,6 +4175,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
st_micron_set_default_init(nor);
break;

+ case SNOR_MFR_WINBOND:
+ winbond_set_default_init(nor);
+ break;
+
default:
break;
}
@@ -4222,6 +4223,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;

/* Set SPI NOR sizes. */
params->size = (u64)info->sector_size * info->n_sectors;
@@ -4587,7 +4589,7 @@ static int spi_nor_init(struct spi_nor *nor)
*/
WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
"enabling reset hack; may not recover from unexpected reboots\n");
- set_4byte(nor, true);
+ nor->params.set_4byte(nor, true);
}

return 0;
@@ -4611,7 +4613,7 @@ void spi_nor_restore(struct spi_nor *nor)
/* restore the addressing mode */
if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
nor->flags & SNOR_F_BROKEN_RESET)
- set_4byte(nor, false);
+ nor->params.set_4byte(nor, false);
}
EXPORT_SYMBOL_GPL(spi_nor_restore);

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index a86c0d9fb01d..7da89dd483cb 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -482,6 +482,7 @@ struct spi_nor;
* @erase_map: the erase map parsed from the SFDP Sector Map Parameter
* Table.
* @quad_enable: enables SPI NOR quad mode.
+ * @set_4byte: puts the SPI NOR in 4 byte addressing mode.
*/
struct spi_nor_flash_parameter {
u64 size;
@@ -494,6 +495,7 @@ struct spi_nor_flash_parameter {
struct spi_nor_erase_map erase_map;

int (*quad_enable)(struct spi_nor *nor);
+ int (*set_4byte)(struct spi_nor *nor, bool enable);
};

/**
--
2.9.5

2019-08-26 12:12:15

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 18/20] mtd: spi_nor: Introduce spi_nor_set_addr_width()

From: Tudor Ambarus <[email protected]>

Parsing of flash parameters were interleaved with setting of the
nor addr width. Dedicate a function for setting nor addr width.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: no changes

drivers/mtd/spi-nor/spi-nor.c | 50 ++++++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index dcda96a20f6c..d13317d1f372 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4739,6 +4739,33 @@ static const struct flash_info *spi_nor_match_id(const char *name)
return NULL;
}

+static int spi_nor_set_addr_width(struct spi_nor *nor)
+{
+ if (nor->addr_width) {
+ /* already configured from SFDP */
+ } else if (nor->info->addr_width) {
+ nor->addr_width = nor->info->addr_width;
+ } else if (nor->mtd.size > 0x1000000) {
+ /* enable 4-byte addressing if the device exceeds 16MiB */
+ nor->addr_width = 4;
+ } else {
+ nor->addr_width = 3;
+ }
+
+ if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
+ dev_err(nor->dev, "address width is too large: %u\n",
+ nor->addr_width);
+ return -EINVAL;
+ }
+
+ /* Set 4byte opcodes when possible. */
+ if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
+ !(nor->flags & SNOR_F_HAS_4BAIT))
+ spi_nor_set_4byte_opcodes(nor);
+
+ return 0;
+}
+
int spi_nor_scan(struct spi_nor *nor, const char *name,
const struct spi_nor_hwcaps *hwcaps)
{
@@ -4885,29 +4912,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (ret)
return ret;

- if (nor->addr_width) {
- /* already configured from SFDP */
- } else if (info->addr_width) {
- nor->addr_width = info->addr_width;
- } else if (mtd->size > 0x1000000) {
- /* enable 4-byte addressing if the device exceeds 16MiB */
- nor->addr_width = 4;
- } else {
- nor->addr_width = 3;
- }
-
if (info->flags & SPI_NOR_4B_OPCODES)
nor->flags |= SNOR_F_4B_OPCODES;

- if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
- !(nor->flags & SNOR_F_HAS_4BAIT))
- spi_nor_set_4byte_opcodes(nor);
-
- if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
- dev_err(dev, "address width is too large: %u\n",
- nor->addr_width);
- return -EINVAL;
- }
+ ret = spi_nor_set_addr_width(nor);
+ if (ret)
+ return ret;

/* Send all the required SPI flash commands to initialize device */
ret = spi_nor_init(nor);
--
2.9.5

2019-08-26 12:12:25

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 17/20] mtd: spi-nor: Bring flash params init together

From: Tudor Ambarus <[email protected]>

Bring all flash parameters default initialization in
spi_nor_legacy_params_init().

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: collect R-b

drivers/mtd/spi-nor/spi-nor.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 2699e999d21a..dcda96a20f6c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4453,6 +4453,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
struct spi_nor_flash_parameter *params = &nor->params;
struct spi_nor_erase_map *map = &params->erase_map;
const struct flash_info *info = nor->info;
+ struct device_node *np = spi_nor_get_flash_node(nor);
u8 i, erase_mask;

/* Initialize legacy flash parameters and settings. */
@@ -4464,18 +4465,25 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
params->size = (u64)info->sector_size * info->n_sectors;
params->page_size = info->page_size;

+ if (!(info->flags & SPI_NOR_NO_FR)) {
+ /* Default to Fast Read for DT and non-DT platform devices. */
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+
+ /* Mask out Fast Read if not requested at DT instantiation. */
+ if (np && !of_property_read_bool(np, "m25p,fast-read"))
+ params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+ }
+
/* (Fast) Read settings. */
params->hwcaps.mask |= SNOR_HWCAPS_READ;
spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
0, 0, SPINOR_OP_READ,
SNOR_PROTO_1_1_1);

- if (!(info->flags & SPI_NOR_NO_FR)) {
- params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+ if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
0, 8, SPINOR_OP_READ_FAST,
SNOR_PROTO_1_1_1);
- }

if (info->flags & SPI_NOR_DUAL_READ) {
params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
@@ -4864,24 +4872,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->page_size = params->page_size;
mtd->writebufsize = nor->page_size;

- if (np) {
- /* If we were instantiated by DT, use it */
- if (of_property_read_bool(np, "m25p,fast-read"))
- params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
- else
- params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
- } else {
- /* If we weren't instantiated by DT, default to fast-read */
- params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
- }
-
if (of_property_read_bool(np, "broken-flash-reset"))
nor->flags |= SNOR_F_BROKEN_RESET;

- /* Some devices cannot do fast-read, no matter what DT tells us */
- if (info->flags & SPI_NOR_NO_FR)
- params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
-
/*
* Configure the SPI memory:
* - select op codes for (Fast) Read, Page Program and Sector Erase.
--
2.9.5

2019-08-26 12:12:49

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 02/20] mtd: spi-nor: Use nor->params

From: Tudor Ambarus <[email protected]>

The Flash parameters and settings are now stored in 'struct spi_nor'.
Use this instead of the stack allocated params.

Few functions stop passing pointer to params, as they can get it from
'struct spi_nor'. spi_nor_parse_sfdp() and children will keep passing
pointer to params because of the roll-back mechanism: in case the
parsing of SFDP fails, the legacy flash parameter and settings will be
restored.

Zeroing params is no longer needed because all SPI NOR users kzalloc
'struct spi_nor'.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: collect R-b

drivers/mtd/spi-nor/spi-nor.c | 46 ++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d35dc6a97521..e9b9cd70a999 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2974,16 +2974,13 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
* spi_nor_spimem_adjust_hwcaps - Find optimal Read/Write protocol
* based on SPI controller capabilities
* @nor: pointer to a 'struct spi_nor'
- * @params: pointer to the 'struct spi_nor_flash_parameter'
- * representing SPI NOR flash capabilities
* @hwcaps: pointer to resulting capabilities after adjusting
* according to controller and flash's capability
*/
static void
-spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor,
- const struct spi_nor_flash_parameter *params,
- u32 *hwcaps)
+spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
{
+ struct spi_nor_flash_parameter *params = &nor->params;
unsigned int cap;

/* DTR modes are not supported yet, mask them all. */
@@ -4129,16 +4126,13 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
return err;
}

-static int spi_nor_init_params(struct spi_nor *nor,
- struct spi_nor_flash_parameter *params)
+static int spi_nor_init_params(struct spi_nor *nor)
{
+ struct spi_nor_flash_parameter *params = &nor->params;
struct spi_nor_erase_map *map = &nor->erase_map;
const struct flash_info *info = nor->info;
u8 i, erase_mask;

- /* Set legacy flash parameters as default. */
- memset(params, 0, sizeof(*params));
-
/* Set SPI NOR sizes. */
params->size = (u64)info->sector_size * info->n_sectors;
params->page_size = info->page_size;
@@ -4255,7 +4249,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
}

static int spi_nor_select_read(struct spi_nor *nor,
- const struct spi_nor_flash_parameter *params,
u32 shared_hwcaps)
{
int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
@@ -4268,7 +4261,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
if (cmd < 0)
return -EINVAL;

- read = &params->reads[cmd];
+ read = &nor->params.reads[cmd];
nor->read_opcode = read->opcode;
nor->read_proto = read->proto;

@@ -4287,7 +4280,6 @@ static int spi_nor_select_read(struct spi_nor *nor,
}

static int spi_nor_select_pp(struct spi_nor *nor,
- const struct spi_nor_flash_parameter *params,
u32 shared_hwcaps)
{
int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_PP_MASK) - 1;
@@ -4300,7 +4292,7 @@ static int spi_nor_select_pp(struct spi_nor *nor,
if (cmd < 0)
return -EINVAL;

- pp = &params->page_programs[cmd];
+ pp = &nor->params.page_programs[cmd];
nor->program_opcode = pp->opcode;
nor->write_proto = pp->proto;
return 0;
@@ -4407,9 +4399,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
}

static int spi_nor_setup(struct spi_nor *nor,
- const struct spi_nor_flash_parameter *params,
const struct spi_nor_hwcaps *hwcaps)
{
+ struct spi_nor_flash_parameter *params = &nor->params;
u32 ignored_mask, shared_mask;
bool enable_quad_io;
int err;
@@ -4426,7 +4418,7 @@ static int spi_nor_setup(struct spi_nor *nor,
* 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, params, &shared_mask);
+ spi_nor_spimem_adjust_hwcaps(nor, &shared_mask);
} else {
/*
* SPI n-n-n protocols are not supported when the SPI
@@ -4442,7 +4434,7 @@ static int spi_nor_setup(struct spi_nor *nor,
}

/* Select the (Fast) Read command. */
- err = spi_nor_select_read(nor, params, shared_mask);
+ 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");
@@ -4450,7 +4442,7 @@ static int spi_nor_setup(struct spi_nor *nor,
}

/* Select the Page Program command. */
- err = spi_nor_select_pp(nor, params, shared_mask);
+ 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");
@@ -4553,11 +4545,11 @@ static const struct flash_info *spi_nor_match_id(const char *name)
int spi_nor_scan(struct spi_nor *nor, const char *name,
const struct spi_nor_hwcaps *hwcaps)
{
- struct spi_nor_flash_parameter params;
const struct flash_info *info = NULL;
struct device *dev = nor->dev;
struct mtd_info *mtd = &nor->mtd;
struct device_node *np = spi_nor_get_flash_node(nor);
+ struct spi_nor_flash_parameter *params = &nor->params;
int ret;
int i;

@@ -4639,7 +4631,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->clear_sr_bp = spi_nor_clear_sr_bp;

/* Parse the Serial Flash Discoverable Parameters table. */
- ret = spi_nor_init_params(nor, &params);
+ ret = spi_nor_init_params(nor);
if (ret)
return ret;

@@ -4649,7 +4641,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
mtd->type = MTD_NORFLASH;
mtd->writesize = 1;
mtd->flags = MTD_CAP_NORFLASH;
- mtd->size = params.size;
+ mtd->size = params->size;
mtd->_erase = spi_nor_erase;
mtd->_read = spi_nor_read;
mtd->_resume = spi_nor_resume;
@@ -4688,18 +4680,18 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
mtd->flags |= MTD_NO_ERASE;

mtd->dev.parent = dev;
- nor->page_size = params.page_size;
+ nor->page_size = params->page_size;
mtd->writebufsize = nor->page_size;

if (np) {
/* If we were instantiated by DT, use it */
if (of_property_read_bool(np, "m25p,fast-read"))
- params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
else
- params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+ params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
} else {
/* If we weren't instantiated by DT, default to fast-read */
- params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+ params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
}

if (of_property_read_bool(np, "broken-flash-reset"))
@@ -4707,7 +4699,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,

/* Some devices cannot do fast-read, no matter what DT tells us */
if (info->flags & SPI_NOR_NO_FR)
- params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+ params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;

/*
* Configure the SPI memory:
@@ -4716,7 +4708,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
* - set the SPI protocols for register and memory accesses.
* - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
*/
- ret = spi_nor_setup(nor, &params, hwcaps);
+ ret = spi_nor_setup(nor, hwcaps);
if (ret)
return ret;

--
2.9.5

2019-08-26 12:13:06

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 07/20] mtd: spi_nor: Move manufacturer quad_enable() in ->default_init()

From: Tudor Ambarus <[email protected]>

The goal is to move the quad_enable manufacturer specific init in the
nor->manufacturer->fixups->default_init()

The legacy quad_enable() implementation is spansion_quad_enable(),
select this method by default.

Set specific manufacturer fixups->default_init() hooks to overwrite
the default quad_enable() implementation when needed.

Signed-off-by: Tudor Ambarus <[email protected]>
Reviewed-by: Boris Brezillon <[email protected]>
---
v3: collect R-b

drivers/mtd/spi-nor/spi-nor.c | 48 ++++++++++++++++++++++++++-----------------
1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3dbbfe34d1d2..2a239531704a 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4150,13 +4150,38 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
return err;
}

+static void macronix_set_default_init(struct spi_nor *nor)
+{
+ nor->params.quad_enable = macronix_quad_enable;
+}
+
+static void st_micron_set_default_init(struct spi_nor *nor)
+{
+ nor->params.quad_enable = NULL;
+}
+
/**
* spi_nor_manufacturer_init_params() - Initialize the flash's parameters and
- * settings based on ->default_init() hook.
+ * settings based on MFR register and ->default_init() hook.
* @nor: pointer to a 'struct spi-nor'.
*/
static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
{
+ /* Init flash parameters based on MFR */
+ switch (JEDEC_MFR(nor->info)) {
+ case SNOR_MFR_MACRONIX:
+ macronix_set_default_init(nor);
+ break;
+
+ case SNOR_MFR_ST:
+ case SNOR_MFR_MICRON:
+ st_micron_set_default_init(nor);
+ break;
+
+ default:
+ break;
+ }
+
if (nor->info->fixups && nor->info->fixups->default_init)
nor->info->fixups->default_init(nor);
}
@@ -4168,6 +4193,9 @@ static int spi_nor_init_params(struct spi_nor *nor)
const struct flash_info *info = nor->info;
u8 i, erase_mask;

+ /* Initialize legacy flash parameters and settings. */
+ params->quad_enable = spansion_quad_enable;
+
/* Set SPI NOR sizes. */
params->size = (u64)info->sector_size * info->n_sectors;
params->page_size = info->page_size;
@@ -4233,24 +4261,6 @@ static int spi_nor_init_params(struct spi_nor *nor)
SPINOR_OP_SE);
spi_nor_init_uniform_erase_map(map, erase_mask, params->size);

- /* Select the procedure to set the Quad Enable bit. */
- if (params->hwcaps.mask & (SNOR_HWCAPS_READ_QUAD |
- SNOR_HWCAPS_PP_QUAD)) {
- switch (JEDEC_MFR(info)) {
- case SNOR_MFR_MACRONIX:
- params->quad_enable = macronix_quad_enable;
- break;
-
- case SNOR_MFR_ST:
- case SNOR_MFR_MICRON:
- break;
-
- default:
- /* Kept only for backward compatibility purpose. */
- params->quad_enable = spansion_quad_enable;
- break;
- }
- }

spi_nor_manufacturer_init_params(nor);

--
2.9.5

2019-08-26 13:39:54

by Frieder Schrempf

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

On 26.08.19 14:40, Boris Brezillon wrote:
> On Mon, 26 Aug 2019 12:08:58 +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().
>>
>> Reviewed-by: Boris Brezillon <[email protected]>
>
> Nitpick: R-bs should normally be placed after your SoB.

Just a question unrelated to the patch content:

I learned to add R-b tags after my SoB when submitting MTD patches, but
recently I submitted a patch to the serial subsystem and was told to put
my SoB last. Is there an "official" rule for this? And if so where to
find it?

>
>> Signed-off-by: Tudor Ambarus <[email protected]>
>> ---
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>

2019-08-26 14:24:17

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND PATCH v3 06/20] mtd: spi-nor: Add a default_init() fixup hook for gd25q256

From: Boris Brezillon <[email protected]>

gd25q256 needs to tweak the ->quad_enable() implementation and the
->default_init() fixup hook is the perfect place to do that. This way,
if we ever need to tweak more things for this flash, we won't have to
add new fields in flash_info.

We can get rid of the flash_info->quad_enable field as gd25q256 was
the only user.

Signed-off-by: Boris Brezillon <[email protected]>
[[email protected]: use ->default_init() hook instead of
->post_sfdp()]
Signed-off-by: Tudor Ambarus <[email protected]>
---
v3: no changes

drivers/mtd/spi-nor/spi-nor.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8fd60e1eebd2..3dbbfe34d1d2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -222,8 +222,6 @@ struct flash_info {

/* Part specific fixup hooks. */
const struct spi_nor_fixups *fixups;
-
- int (*quad_enable)(struct spi_nor *nor);
};

#define JEDEC_MFR(info) ((info)->id[0])
@@ -2126,6 +2124,21 @@ static struct spi_nor_fixups mx25l25635_fixups = {
.post_bfpt = mx25l25635_post_bfpt_fixups,
};

+static void gd25q256_default_init(struct spi_nor *nor)
+{
+ /*
+ * Some manufacturer like GigaDevice may use different
+ * bit to set QE on different memories, so the MFR can't
+ * indicate the quad_enable method for this case, we need
+ * to set it in the default_init fixup hook.
+ */
+ nor->params.quad_enable = macronix_quad_enable;
+}
+
+static struct spi_nor_fixups gd25q256_fixups = {
+ .default_init = gd25q256_default_init,
+};
+
/* NOTE: double check command sets and memory organization when you add
* more nor chips. This current list focusses on newer chips, which
* have been converging on command sets which including JEDEC ID.
@@ -2218,7 +2231,7 @@ static const struct flash_info spi_nor_ids[] = {
"gd25q256", INFO(0xc84019, 0, 64 * 1024, 512,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
- .quad_enable = macronix_quad_enable,
+ .fixups = &gd25q256_fixups,
},

/* Intel/Numonyx -- xxxs33b */
@@ -4237,15 +4250,6 @@ static int spi_nor_init_params(struct spi_nor *nor)
params->quad_enable = spansion_quad_enable;
break;
}
-
- /*
- * Some manufacturer like GigaDevice may use different
- * bit to set QE on different memories, so the MFR can't
- * indicate the quad_enable method for this case, we need
- * set it in flash info list.
- */
- if (info->quad_enable)
- params->quad_enable = info->quad_enable;
}

spi_nor_manufacturer_init_params(nor);
--
2.9.5

2019-08-26 14:32:53

by Tudor Ambarus

[permalink] [raw]
Subject: [RESEND RFC PATCH v3 20/20] mtd: spi-nor: Rework the disabling of block write protection

From: Tudor Ambarus <[email protected]>

spi_nor_unlock() unlocks blocks of memory or the entire flash memory
array, if requested. clear_sr_bp() unlocks the entire flash memory
array at boot time. This calls for some unification, clear_sr_bp() is
just an optimization for the case when the unlock request covers the
entire flash size.

Merge the clear_sr_bp() and stm_lock/unlock logic and introduce
spi_nor_unlock_all(), which makes an unlock request that covers the
entire flash size.

Get rid of the MFR handling and implement specific manufacturer
default_init() fixup hooks.

Move write_sr_cr() to avoid to add a forward declaration. Prefix
new function with 'spi_nor_'.

Note that this changes a bit the logic for the SNOR_MFR_ATMEL and
SNOR_MFR_INTEL cases. Before this patch, the Atmel and Intel chips
did not set the locking ops, but unlocked the entire flash at boot
time, while now they are setting the locking ops to stm_locking_ops.
This should work, since the the disable of the block protection at the
boot time used the same Status Register bits to unlock the flash, as
in the stm_locking_ops case.

In future, we should probably add new hooks to
'struct spi_nor_flash_parameter' to describe how to interact with the
Status and Configuration Registers in the form of:
nor->params.ops->read_sr
nor->params.ops->write_sr
nor->params.ops->read_cr
nor->params.ops->write_sr
We can retrieve this info starting with JESD216 revB, by checking the
15th DWORD of Basic Flash Parameter Table, or with later revisions of
the standard, by parsing the "Status, Control and Configuration Register
Map for SPI Memory Devices".

Suggested-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
v3: new patch

drivers/mtd/spi-nor/spi-nor.c | 337 +++++++++++++++++++++++-------------------
include/linux/mtd/spi-nor.h | 4 +-
2 files changed, 185 insertions(+), 156 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index ec70b58294ec..01d4051510b6 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1320,8 +1320,63 @@ static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
return ret;
}

-/* Write status register and ensure bits in mask match written values */
-static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
+/*
+ * write_sr_cr() - Write the Status Register and Configuration Register.
+ * @nor: pointer to a 'struct spi_nor'.
+ * status_new: byte value to be written to the Status Register.
+ * mask: mask with which to check the written values.
+ *
+ * Write Status Register and Configuration Register with 2 bytes. The first
+ * byte will be written to the status register, while the second byte will be
+ * written to the configuration register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
+{
+ int ret;
+
+ write_enable(nor);
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(2, sr_cr, 1));
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
+ }
+
+ if (ret < 0) {
+ dev_err(nor->dev,
+ "error while writing configuration register\n");
+ return -EINVAL;
+ }
+
+ ret = spi_nor_wait_till_ready(nor);
+ if (ret) {
+ dev_err(nor->dev,
+ "timeout while writing configuration register\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register and
+ * ensure the bits in the mask match the written values.
+ * @nor: pointer to a 'struct spi_nor'.
+ * status_new: byte value to be written to the Status Register.
+ * mask: mask with which to check the written values.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 status_new,
+ u8 mask)
{
int ret;

@@ -1341,6 +1396,88 @@ static int write_sr_and_check(struct spi_nor *nor, u8 status_new, u8 mask)
return ((ret & mask) != (status_new & mask)) ? -EIO : 0;
}

+/**
+ * spi_nor_write_sr_cr_and_check() - Write the Status Register and
+ * Configuration Register and ensure the bits in the mask match the written
+ * values.
+ * @nor: pointer to a 'struct spi_nor'.
+ * status_new: byte value to be written to the Status Register.
+ * mask: mask with which to check the written values.
+ *
+ * The Configuration Register is written only when it has the Quad Enable (QE)
+ * bit set to one. When QE bit is set to one, only the Write Status (01h)
+ * command with two data bytes may be used, so both the Status and
+ * Configuration registers will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_cr_and_check(struct spi_nor *nor, u8 status_new,
+ u8 mask)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+
+ /* Check current Quad Enable bit value. */
+ ret = read_cr(nor);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * If the Quad Enable bit is zero, use the Write Status (01h) command
+ * with one data byte.
+ */
+ if (!(ret & CR_QUAD_EN_SPAN))
+ return spi_nor_write_sr1_and_check(nor, status_new, mask);
+
+ /*
+ * When the configuration register Quad Enable bit is one, only the
+ * Write Status (01h) command with two data bytes may be used.
+ *
+ */
+ sr_cr[0] = status_new;
+ sr_cr[1] = ret;
+
+ ret = write_sr_cr(nor, sr_cr);
+ if (ret) {
+ dev_err(nor->dev, "16-bit write register failed\n");
+ return ret;
+ }
+
+ ret = read_sr(nor);
+ if (ret < 0)
+ return ret;
+
+ if ((ret & mask) != (status_new & mask))
+ return -EIO;
+
+ ret = read_cr(nor);
+ if (ret < 0)
+ return ret;
+
+ if (ret != sr_cr[1])
+ return -EIO;
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_sr_and_check() - Write the Status Register and ensure the bits
+ * in the mask match the written values.
+ * @nor: pointer to a 'struct spi_nor'.
+ * status_new: byte value to be written to the Status Register.
+ * mask: mask with which to check the written values.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 status_new,
+ u8 mask)
+{
+ if (nor->flags == SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_sr_cr_and_check(nor, status_new, mask);
+
+ return spi_nor_write_sr1_and_check(nor, status_new, mask);
+}
+
static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
uint64_t *len)
{
@@ -1502,7 +1639,7 @@ static int stm_lock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) < (status_old & mask))
return -EINVAL;

- return write_sr_and_check(nor, status_new, mask);
+ return spi_nor_write_sr_and_check(nor, status_new, mask);
}

/*
@@ -1585,7 +1722,7 @@ static int stm_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
if ((status_new & mask) > (status_old & mask))
return -EINVAL;

- return write_sr_and_check(nor, status_new, mask);
+ return spi_nor_write_sr_and_check(nor, status_new, mask);
}

/*
@@ -1657,46 +1794,6 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
return ret;
}

-/*
- * Write status Register and configuration register with 2 bytes
- * The first byte will be written to the status register, while the
- * second byte will be written to the configuration register.
- * Return negative if error occurred.
- */
-static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
-{
- int ret;
-
- write_enable(nor);
-
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(2, sr_cr, 1));
-
- ret = spi_mem_exec_op(nor->spimem, &op);
- } else {
- ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
- }
-
- if (ret < 0) {
- dev_err(nor->dev,
- "error while writing configuration register\n");
- return -EINVAL;
- }
-
- ret = spi_nor_wait_till_ready(nor);
- if (ret) {
- dev_err(nor->dev,
- "timeout while writing configuration register\n");
- return ret;
- }
-
- return 0;
-}
-
/**
* macronix_quad_enable() - set QE bit in Status Register.
* @nor: pointer to a 'struct spi_nor'
@@ -1942,95 +2039,6 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
return 0;
}

-/**
- * spi_nor_clear_sr_bp() - clear the Status Register Block Protection bits.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_clear_sr_bp(struct spi_nor *nor)
-{
- int ret;
- u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
-
- ret = read_sr(nor);
- if (ret < 0) {
- dev_err(nor->dev, "error while reading status register\n");
- return ret;
- }
-
- write_enable(nor);
-
- ret = write_sr(nor, ret & ~mask);
- if (ret) {
- dev_err(nor->dev, "write to status register failed\n");
- return ret;
- }
-
- ret = spi_nor_wait_till_ready(nor);
- if (ret)
- dev_err(nor->dev, "timeout while writing status register\n");
- return ret;
-}
-
-/**
- * spi_nor_spansion_clear_sr_bp() - clear the Status Register Block Protection
- * bits on spansion flashes.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Read-modify-write function that clears the Block Protection bits from the
- * Status Register without affecting other bits. The function is tightly
- * coupled with the spansion_quad_enable() function. Both assume that the Write
- * Register with 16 bits, together with the Read Configuration Register (35h)
- * instructions are supported.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
-{
- int ret;
- u8 mask = SR_BP2 | SR_BP1 | SR_BP0;
- u8 *sr_cr = nor->bouncebuf;
-
- /* Check current Quad Enable bit value. */
- ret = read_cr(nor);
- if (ret < 0) {
- dev_err(nor->dev,
- "error while reading configuration register\n");
- return ret;
- }
-
- /*
- * When the configuration register Quad Enable bit is one, only the
- * Write Status (01h) command with two data bytes may be used.
- */
- if (ret & CR_QUAD_EN_SPAN) {
- sr_cr[1] = ret;
-
- ret = read_sr(nor);
- if (ret < 0) {
- dev_err(nor->dev,
- "error while reading status register\n");
- return ret;
- }
- sr_cr[0] = ret & ~mask;
-
- ret = write_sr_cr(nor, sr_cr);
- if (ret)
- dev_err(nor->dev, "16-bit write register failed\n");
- return ret;
- }
-
- /*
- * If the Quad Enable bit is zero, use the Write Status (01h) command
- * with one data byte.
- */
- return spi_nor_clear_sr_bp(nor);
-}
-
/* Used when the "_ext_id" is two bytes at most */
#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \
.id = { \
@@ -4373,6 +4381,16 @@ static int spi_nor_setup(struct spi_nor *nor,
return nor->params.setup(nor, hwcaps);
}

+static void atmel_set_default_init(struct spi_nor *nor)
+{
+ nor->flags = SNOR_F_HAS_LOCK;
+}
+
+static void intel_set_default_init(struct spi_nor *nor)
+{
+ nor->flags = SNOR_F_HAS_LOCK;
+}
+
static void macronix_set_default_init(struct spi_nor *nor)
{
nor->params.quad_enable = macronix_quad_enable;
@@ -4400,6 +4418,14 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
{
/* Init flash parameters based on MFR */
switch (JEDEC_MFR(nor->info)) {
+ case SNOR_MFR_ATMEL:
+ atmel_set_default_init(nor);
+ break;
+
+ case SNOR_MFR_INTEL:
+ intel_set_default_init(nor);
+ break;
+
case SNOR_MFR_MACRONIX:
macronix_set_default_init(nor);
break;
@@ -4595,6 +4621,9 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
*/
if (nor->flags & SNOR_F_HAS_LOCK && !nor->params.locking_ops)
nor->params.locking_ops = &stm_locking_ops;
+
+ if (nor->params.quad_enable == spansion_quad_enable)
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
}

/**
@@ -4667,20 +4696,32 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
return nor->params.quad_enable(nor);
}

+/**
+ * spi_nor_unlock_all() - Unlocks the entire flash memory array.
+ * @nor: pointer to a 'struct spi_nor'
+ *
+ * Some SPI NOR flashes are write protected by default after a power-on reset
+ * cycle, in order to avoid inadvertent writes during power-up. Backward
+ * compatibility imposes to unlock the entire flash memory array at power-up
+ * by default.
+ */
+static int spi_nor_unlock_all(struct spi_nor *nor)
+{
+ if (nor->flags & SNOR_F_HAS_LOCK)
+ return spi_nor_unlock(&nor->mtd, 0, nor->params.size);
+
+ return 0;
+}
+
static int spi_nor_init(struct spi_nor *nor)
{
int err;

- if (nor->clear_sr_bp) {
- if (nor->params.quad_enable == spansion_quad_enable)
- nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
-
- err = nor->clear_sr_bp(nor);
- if (err) {
- dev_err(nor->dev,
- "fail to clear block protection bits\n");
- return err;
- }
+ err = spi_nor_unlock_all(nor);
+ if (err) {
+ dev_err(nor->dev,
+ "Failed to unlock the entire flash memory array\n");
+ return err;
}

err = spi_nor_quad_enable(nor);
@@ -4859,16 +4900,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
if (info->flags & SPI_NOR_HAS_LOCK)
nor->flags |= SNOR_F_HAS_LOCK;

- /*
- * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
- * with the software protection bits set.
- */
- if (JEDEC_MFR(nor->info) == SNOR_MFR_ATMEL ||
- JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
- JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
- nor->info->flags & SPI_NOR_HAS_LOCK)
- nor->clear_sr_bp = spi_nor_clear_sr_bp;
-
/* Init flash parameters based on flash_info struct and SFDP */
spi_nor_init_params(nor);

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index fc0b4b19c900..d7506dccb7da 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,7 @@ enum spi_nor_option_flags {
SNOR_F_4B_OPCODES = BIT(6),
SNOR_F_HAS_4BAIT = BIT(7),
SNOR_F_HAS_LOCK = BIT(8),
+ SNOR_F_HAS_16BIT_SR = BIT(9),
};

/**
@@ -560,8 +561,6 @@ struct flash_info;
* @erase: [DRIVER-SPECIFIC] erase a sector of the SPI NOR
* at the offset @offs; if not provided by the driver,
* spi-nor will send the erase opcode via write_reg()
- * @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
- * the SPI NOR Status Register.
* @params: [FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
* The structure includes legacy flash parameters and
* settings that can be overwritten by the spi_nor_fixups
@@ -599,7 +598,6 @@ struct spi_nor {
size_t len, const u_char *write_buf);
int (*erase)(struct spi_nor *nor, loff_t offs);

- int (*clear_sr_bp)(struct spi_nor *nor);
struct spi_nor_flash_parameter params;

void *priv;
--
2.9.5

2019-08-26 14:59:59

by Boris Brezillon

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

On Mon, 26 Aug 2019 12:08:58 +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().
>
> Reviewed-by: Boris Brezillon <[email protected]>

Nitpick: R-bs should normally be placed after your SoB.

> Signed-off-by: Tudor Ambarus <[email protected]>
> ---

2019-08-26 15:33:11

by Frieder Schrempf

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

On 26.08.19 16:02, Boris Brezillon wrote:
> On Mon, 26 Aug 2019 13:38:48 +0000
> Schrempf Frieder <[email protected]> wrote:
>
>> On 26.08.19 14:40, Boris Brezillon wrote:
>>> On Mon, 26 Aug 2019 12:08:58 +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().
>>>>
>>>> Reviewed-by: Boris Brezillon <[email protected]>
>>>
>>> Nitpick: R-bs should normally be placed after your SoB.
>>
>> Just a question unrelated to the patch content:
>>
>> I learned to add R-b tags after my SoB when submitting MTD patches, but
>> recently I submitted a patch to the serial subsystem and was told to put
>> my SoB last. Is there an "official" rule for this? And if so where to
>> find it?
>
> Should match the order of addition: if you picked an existing patch that
> had already received R-b/A-b tags and applied it to your tree you
> should add your SoB at the end. But if you are the author, your SoB
> should come first. At least that's the rule I follow :-).

Ok, thanks for clarifying. I was the author of said patch and was adding
an R-b just like in this case here, so the people in the serial
subsystem seem to apply different rules ;)

2019-08-26 16:30:36

by Boris Brezillon

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

On Mon, 26 Aug 2019 13:38:48 +0000
Schrempf Frieder <[email protected]> wrote:

> On 26.08.19 14:40, Boris Brezillon wrote:
> > On Mon, 26 Aug 2019 12:08:58 +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().
> >>
> >> Reviewed-by: Boris Brezillon <[email protected]>
> >
> > Nitpick: R-bs should normally be placed after your SoB.
>
> Just a question unrelated to the patch content:
>
> I learned to add R-b tags after my SoB when submitting MTD patches, but
> recently I submitted a patch to the serial subsystem and was told to put
> my SoB last. Is there an "official" rule for this? And if so where to
> find it?

Should match the order of addition: if you picked an existing patch that
had already received R-b/A-b tags and applied it to your tree you
should add your SoB at the end. But if you are the author, your SoB
should come first. At least that's the rule I follow :-).

2019-08-27 04:33:53

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 02/20] mtd: spi-nor: Use nor->params



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
>
> The Flash parameters and settings are now stored in 'struct spi_nor'.
> Use this instead of the stack allocated params.
>
> Few functions stop passing pointer to params, as they can get it from
> 'struct spi_nor'. spi_nor_parse_sfdp() and children will keep passing
> pointer to params because of the roll-back mechanism: in case the
> parsing of SFDP fails, the legacy flash parameter and settings will be
> restored.
>
> Zeroing params is no longer needed because all SPI NOR users kzalloc
> 'struct spi_nor'.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: collect R-b
>
> drivers/mtd/spi-nor/spi-nor.c | 46 ++++++++++++++++++-------------------------
> 1 file changed, 19 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d35dc6a97521..e9b9cd70a999 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2974,16 +2974,13 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
> * spi_nor_spimem_adjust_hwcaps - Find optimal Read/Write protocol
> * based on SPI controller capabilities
> * @nor: pointer to a 'struct spi_nor'
> - * @params: pointer to the 'struct spi_nor_flash_parameter'
> - * representing SPI NOR flash capabilities
> * @hwcaps: pointer to resulting capabilities after adjusting
> * according to controller and flash's capability
> */
> static void
> -spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> - u32 *hwcaps)
> +spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
> {
> + struct spi_nor_flash_parameter *params = &nor->params;
> unsigned int cap;
>
> /* DTR modes are not supported yet, mask them all. */
> @@ -4129,16 +4126,13 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
> return err;
> }
>
> -static int spi_nor_init_params(struct spi_nor *nor,
> - struct spi_nor_flash_parameter *params)
> +static int spi_nor_init_params(struct spi_nor *nor)
> {
> + struct spi_nor_flash_parameter *params = &nor->params;
> struct spi_nor_erase_map *map = &nor->erase_map;
> const struct flash_info *info = nor->info;
> u8 i, erase_mask;
>
> - /* Set legacy flash parameters as default. */
> - memset(params, 0, sizeof(*params));
> -
> /* Set SPI NOR sizes. */
> params->size = (u64)info->sector_size * info->n_sectors;
> params->page_size = info->page_size;
> @@ -4255,7 +4249,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
> }
>
> static int spi_nor_select_read(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> u32 shared_hwcaps)
> {
> int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
> @@ -4268,7 +4261,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
> if (cmd < 0)
> return -EINVAL;
>
> - read = &params->reads[cmd];
> + read = &nor->params.reads[cmd];
> nor->read_opcode = read->opcode;
> nor->read_proto = read->proto;
>
> @@ -4287,7 +4280,6 @@ static int spi_nor_select_read(struct spi_nor *nor,
> }
>
> static int spi_nor_select_pp(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> u32 shared_hwcaps)
> {
> int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_PP_MASK) - 1;
> @@ -4300,7 +4292,7 @@ static int spi_nor_select_pp(struct spi_nor *nor,
> if (cmd < 0)
> return -EINVAL;
>
> - pp = &params->page_programs[cmd];
> + pp = &nor->params.page_programs[cmd];
> nor->program_opcode = pp->opcode;
> nor->write_proto = pp->proto;
> return 0;
> @@ -4407,9 +4399,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
> }
>
> static int spi_nor_setup(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> const struct spi_nor_hwcaps *hwcaps)
> {
> + struct spi_nor_flash_parameter *params = &nor->params;
> u32 ignored_mask, shared_mask;
> bool enable_quad_io;
> int err;
> @@ -4426,7 +4418,7 @@ static int spi_nor_setup(struct spi_nor *nor,
> * 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, params, &shared_mask);
> + spi_nor_spimem_adjust_hwcaps(nor, &shared_mask);
> } else {
> /*
> * SPI n-n-n protocols are not supported when the SPI
> @@ -4442,7 +4434,7 @@ static int spi_nor_setup(struct spi_nor *nor,
> }
>
> /* Select the (Fast) Read command. */
> - err = spi_nor_select_read(nor, params, shared_mask);
> + 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");
> @@ -4450,7 +4442,7 @@ static int spi_nor_setup(struct spi_nor *nor,
> }
>
> /* Select the Page Program command. */
> - err = spi_nor_select_pp(nor, params, shared_mask);
> + 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");
> @@ -4553,11 +4545,11 @@ static const struct flash_info *spi_nor_match_id(const char *name)
> int spi_nor_scan(struct spi_nor *nor, const char *name,
> const struct spi_nor_hwcaps *hwcaps)
> {
> - struct spi_nor_flash_parameter params;
> const struct flash_info *info = NULL;
> struct device *dev = nor->dev;
> struct mtd_info *mtd = &nor->mtd;
> struct device_node *np = spi_nor_get_flash_node(nor);
> + struct spi_nor_flash_parameter *params = &nor->params;
> int ret;
> int i;
>
> @@ -4639,7 +4631,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> nor->clear_sr_bp = spi_nor_clear_sr_bp;
>
> /* Parse the Serial Flash Discoverable Parameters table. */
> - ret = spi_nor_init_params(nor, &params);
> + ret = spi_nor_init_params(nor);
> if (ret)
> return ret;
>
> @@ -4649,7 +4641,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> mtd->type = MTD_NORFLASH;
> mtd->writesize = 1;
> mtd->flags = MTD_CAP_NORFLASH;
> - mtd->size = params.size;
> + mtd->size = params->size;
> mtd->_erase = spi_nor_erase;
> mtd->_read = spi_nor_read;
> mtd->_resume = spi_nor_resume;
> @@ -4688,18 +4680,18 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> mtd->flags |= MTD_NO_ERASE;
>
> mtd->dev.parent = dev;
> - nor->page_size = params.page_size;
> + nor->page_size = params->page_size;
> mtd->writebufsize = nor->page_size;
>
> if (np) {
> /* If we were instantiated by DT, use it */
> if (of_property_read_bool(np, "m25p,fast-read"))
> - params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> else
> - params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> } else {
> /* If we weren't instantiated by DT, default to fast-read */
> - params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> }
>
> if (of_property_read_bool(np, "broken-flash-reset"))
> @@ -4707,7 +4699,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>
> /* Some devices cannot do fast-read, no matter what DT tells us */
> if (info->flags & SPI_NOR_NO_FR)
> - params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
>
> /*
> * Configure the SPI memory:
> @@ -4716,7 +4708,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> * - set the SPI protocols for register and memory accesses.
> * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
> */
> - ret = spi_nor_setup(nor, &params, hwcaps);
> + ret = spi_nor_setup(nor, hwcaps);
> if (ret)
> return ret;
>
>

--
Regards
Vignesh

2019-08-27 05:34:13

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 00/20] mtd: spi-nor: move manuf out of the core



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
[...]
>
> Tested on sst26vf064b with atmel-quadspi SPIMEM driver.
>

Tested s25fl256s, mx66l51235l with ti-qspi and s25fl512s with
cadence-quadspi. n25q128a13 with legacy 1 bit SPI controller.

> Boris Brezillon (7):
> mtd: spi-nor: Add a default_init() fixup hook for gd25q256
> mtd: spi-nor: Create a ->set_4byte() method
> mtd: spi-nor: Rework the SPI NOR lock/unlock logic
> 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 (13):
> mtd: spi-nor: Regroup flash parameter and settings
> mtd: spi-nor: Use nor->params
> mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
> mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'
> mtd: spi-nor: Add default_init() hook to tweak flash parameters
> mtd: spi_nor: Move manufacturer quad_enable() in ->default_init()
> mtd: spi-nor: Split spi_nor_init_params()
> mtd: spi_nor: Add a ->setup() method
> mtd: spi-nor: Add s3an_post_sfdp_fixups()
> mtd: spi-nor: Bring flash params init together
> mtd: spi_nor: Introduce spi_nor_set_addr_width()
> mtd: spi-nor: Introduce spi_nor_get_flash_info()
> mtd: spi-nor: Rework the disabling of block write protection
>
> drivers/mtd/spi-nor/spi-nor.c | 1304 +++++++++++++++++++++++------------------
> include/linux/mtd/spi-nor.h | 298 +++++++---
> 2 files changed, 927 insertions(+), 675 deletions(-)
>

--
Regards
Vignesh

2019-08-27 05:50:05

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 06/20] mtd: spi-nor: Add a default_init() fixup hook for gd25q256



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Boris Brezillon <[email protected]>
>
> gd25q256 needs to tweak the ->quad_enable() implementation and the
> ->default_init() fixup hook is the perfect place to do that. This way,
> if we ever need to tweak more things for this flash, we won't have to
> add new fields in flash_info.
>
> We can get rid of the flash_info->quad_enable field as gd25q256 was
> the only user.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> [[email protected]: use ->default_init() hook instead of
> ->post_sfdp()]
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---


Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: no changes
> > drivers/mtd/spi-nor/spi-nor.c | 28 ++++++++++++++++------------
> 1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 8fd60e1eebd2..3dbbfe34d1d2 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -222,8 +222,6 @@ struct flash_info {
>
> /* Part specific fixup hooks. */
> const struct spi_nor_fixups *fixups;
> -
> - int (*quad_enable)(struct spi_nor *nor);
> };
>
> #define JEDEC_MFR(info) ((info)->id[0])
> @@ -2126,6 +2124,21 @@ static struct spi_nor_fixups mx25l25635_fixups = {
> .post_bfpt = mx25l25635_post_bfpt_fixups,
> };
>
> +static void gd25q256_default_init(struct spi_nor *nor)
> +{
> + /*
> + * Some manufacturer like GigaDevice may use different
> + * bit to set QE on different memories, so the MFR can't
> + * indicate the quad_enable method for this case, we need
> + * to set it in the default_init fixup hook.
> + */
> + nor->params.quad_enable = macronix_quad_enable;
> +}
> +
> +static struct spi_nor_fixups gd25q256_fixups = {
> + .default_init = gd25q256_default_init,
> +};
> +
> /* NOTE: double check command sets and memory organization when you add
> * more nor chips. This current list focusses on newer chips, which
> * have been converging on command sets which including JEDEC ID.
> @@ -2218,7 +2231,7 @@ static const struct flash_info spi_nor_ids[] = {
> "gd25q256", INFO(0xc84019, 0, 64 * 1024, 512,
> SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> SPI_NOR_4B_OPCODES | SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> - .quad_enable = macronix_quad_enable,
> + .fixups = &gd25q256_fixups,
> },
>
> /* Intel/Numonyx -- xxxs33b */
> @@ -4237,15 +4250,6 @@ static int spi_nor_init_params(struct spi_nor *nor)
> params->quad_enable = spansion_quad_enable;
> break;
> }
> -
> - /*
> - * Some manufacturer like GigaDevice may use different
> - * bit to set QE on different memories, so the MFR can't
> - * indicate the quad_enable method for this case, we need
> - * set it in flash info list.
> - */
> - if (info->quad_enable)
> - params->quad_enable = info->quad_enable;
> }
>
> spi_nor_manufacturer_init_params(nor);
>

--
Regards
Vignesh

2019-08-27 05:55:40

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 07/20] mtd: spi_nor: Move manufacturer quad_enable() in ->default_init()



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
>
> The goal is to move the quad_enable manufacturer specific init in the
> nor->manufacturer->fixups->default_init()
>
> The legacy quad_enable() implementation is spansion_quad_enable(),
> select this method by default.
>
> Set specific manufacturer fixups->default_init() hooks to overwrite
> the default quad_enable() implementation when needed.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---


Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh


> v3: collect R-b
>
> drivers/mtd/spi-nor/spi-nor.c | 48 ++++++++++++++++++++++++++-----------------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3dbbfe34d1d2..2a239531704a 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4150,13 +4150,38 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
> return err;
> }
>
> +static void macronix_set_default_init(struct spi_nor *nor)
> +{
> + nor->params.quad_enable = macronix_quad_enable;
> +}
> +
> +static void st_micron_set_default_init(struct spi_nor *nor)
> +{
> + nor->params.quad_enable = NULL;
> +}
> +
> /**
> * spi_nor_manufacturer_init_params() - Initialize the flash's parameters and
> - * settings based on ->default_init() hook.
> + * settings based on MFR register and ->default_init() hook.
> * @nor: pointer to a 'struct spi-nor'.
> */
> static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
> {
> + /* Init flash parameters based on MFR */
> + switch (JEDEC_MFR(nor->info)) {
> + case SNOR_MFR_MACRONIX:
> + macronix_set_default_init(nor);
> + break;
> +
> + case SNOR_MFR_ST:
> + case SNOR_MFR_MICRON:
> + st_micron_set_default_init(nor);
> + break;
> +
> + default:
> + break;
> + }
> +
> if (nor->info->fixups && nor->info->fixups->default_init)
> nor->info->fixups->default_init(nor);
> }
> @@ -4168,6 +4193,9 @@ static int spi_nor_init_params(struct spi_nor *nor)
> const struct flash_info *info = nor->info;
> u8 i, erase_mask;
>
> + /* Initialize legacy flash parameters and settings. */
> + params->quad_enable = spansion_quad_enable;
> +
> /* Set SPI NOR sizes. */
> params->size = (u64)info->sector_size * info->n_sectors;
> params->page_size = info->page_size;
> @@ -4233,24 +4261,6 @@ static int spi_nor_init_params(struct spi_nor *nor)
> SPINOR_OP_SE);
> spi_nor_init_uniform_erase_map(map, erase_mask, params->size);
>
> - /* Select the procedure to set the Quad Enable bit. */
> - if (params->hwcaps.mask & (SNOR_HWCAPS_READ_QUAD |
> - SNOR_HWCAPS_PP_QUAD)) {
> - switch (JEDEC_MFR(info)) {
> - case SNOR_MFR_MACRONIX:
> - params->quad_enable = macronix_quad_enable;
> - break;
> -
> - case SNOR_MFR_ST:
> - case SNOR_MFR_MICRON:
> - break;
> -
> - default:
> - /* Kept only for backward compatibility purpose. */
> - params->quad_enable = spansion_quad_enable;
> - break;
> - }
> - }
>
> spi_nor_manufacturer_init_params(nor);
>
>

--
Regards
Vignesh

2019-08-27 06:00:44

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 08/20] mtd: spi-nor: Split spi_nor_init_params()



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
>
> Add functions to delimit what the chunks of code do:
>
> static void spi_nor_init_params()
> {
> spi_nor_info_init_params()
> spi_nor_manufacturer_init_params()
> spi_nor_sfdp_init_params()
> }
>
> Add descriptions to all methods.
>
> spi_nor_init_params() becomes of type void, as all its children
> return void.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---
> v3: rename spi_nor_legacy_init_params() to spi_nor_info_init_params()


Reviewed-by: Vignesh Raghavendra <[email protected]>

Minor nits below...

[...]
>
> +/**
> + * spi_nor_init_params() - Initialize the flash's parameters and settings.
> + * @nor: pointer to a 'struct spi-nor'.
> + *
> + * The flash parameters and settings are initialized based on a sequence of
> + * calls that are ordered by priority:
> + *
> + * 1/ Default flash parameters initialization. The initializations are done
> + * based on nor->info data:
> + * spi_nor_info_init_params()
> + *
> + * which can be overwritten by:
> + * 2/ Manufacturer flash parameters initialization. The initializations are
> + * done based on MFR register, or when the decisions can not be done solely
> + * based on MFR, by using specific flash_info tweeks, ->default_init():
> + * spi_nor_manufacturer_init_params()
> + *
> + * which can be overwritten by:
> + * 3/ SFDP flash parameters initialization. JESD216 SFDP is a standard and
> + * should be more accurate that the above.
> + * spi_nor_sfdp_init_params()
> + *
> + * Please not that there is a ->post_bfpt() fixup hook that can overwrite the

s/not/note

> + * flash parameters and settings imediately after parsing the Basic Flash

s/imediately/immediately

> + * Parameter Table.
> + */
> +static void spi_nor_init_params(struct spi_nor *nor)
> +{
> + spi_nor_info_init_params(nor);
>
> spi_nor_manufacturer_init_params(nor);
>
> - if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
> - !(info->flags & SPI_NOR_SKIP_SFDP)) {
> - struct spi_nor_flash_parameter sfdp_params;
> -
> - memcpy(&sfdp_params, params, sizeof(sfdp_params));
> -
> - if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
> - nor->addr_width = 0;
> - nor->flags &= ~SNOR_F_4B_OPCODES;
> - } else {
> - memcpy(params, &sfdp_params, sizeof(*params));
> - }
> - }
> -
> - return 0;
> + if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
> + !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> + spi_nor_sfdp_init_params(nor);
> }
>
> static int spi_nor_select_read(struct spi_nor *nor,
> @@ -4670,10 +4715,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> nor->info->flags & SPI_NOR_HAS_LOCK)
> nor->clear_sr_bp = spi_nor_clear_sr_bp;
>
> - /* Parse the Serial Flash Discoverable Parameters table. */
> - ret = spi_nor_init_params(nor);
> - if (ret)
> - return ret;
> + /* Init flash parameters based on flash_info struct and SFDP */
> + spi_nor_init_params(nor);
>
> if (!mtd->name)
> mtd->name = dev_name(dev);
>

--
Regards
Vignesh

2019-08-27 06:08:18

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 09/20] mtd: spi-nor: Create a ->set_4byte() method



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Boris Brezillon <[email protected]>
>
> The procedure used to enable 4 byte addressing mode depends on the NOR
> device, so let's provide a hook so that manufacturer specific handling
> can be implemented in a sane way.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> [[email protected]: use nor->params.set_4byte() instead of
> nor->set_4byte()]
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: no changes>
> drivers/mtd/spi-nor/spi-nor.c | 76 ++++++++++++++++++++++---------------------
> include/linux/mtd/spi-nor.h | 2 ++
> 2 files changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1e7f8dc3457d..235e82a121a1 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -633,6 +633,17 @@ static int macronix_set_4byte(struct spi_nor *nor, bool enable)
> NULL, 0);
> }
>
> +static int st_micron_set_4byte(struct spi_nor *nor, bool enable)
> +{
> + int ret;
> +
> + write_enable(nor);
> + ret = macronix_set_4byte(nor, enable);
> + write_disable(nor);
> +
> + return ret;
> +}
> +
> static int spansion_set_4byte(struct spi_nor *nor, bool enable)
> {
> nor->bouncebuf[0] = enable << 7;
> @@ -667,45 +678,24 @@ static int spi_nor_write_ear(struct spi_nor *nor, u8 ear)
> return nor->write_reg(nor, SPINOR_OP_WREAR, nor->bouncebuf, 1);
> }
>
> -/* Enable/disable 4-byte addressing mode. */
> -static int set_4byte(struct spi_nor *nor, bool enable)
> +static int winbond_set_4byte(struct spi_nor *nor, bool enable)
> {
> - int status;
> - bool need_wren = false;
> -
> - switch (JEDEC_MFR(nor->info)) {
> - case SNOR_MFR_ST:
> - case SNOR_MFR_MICRON:
> - /* Some Micron need WREN command; all will accept it */
> - need_wren = true;
> - /* fall through */
> - case SNOR_MFR_MACRONIX:
> - case SNOR_MFR_WINBOND:
> - if (need_wren)
> - write_enable(nor);
> + int ret;
>
> - status = macronix_set_4byte(nor, enable);
> - if (need_wren)
> - write_disable(nor);
> + ret = macronix_set_4byte(nor, enable);
> + if (ret || enable)
> + return ret;
>
> - if (!status && !enable &&
> - JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
> - /*
> - * On Winbond W25Q256FV, leaving 4byte mode causes
> - * the Extended Address Register to be set to 1, so all
> - * 3-byte-address reads come from the second 16M.
> - * We must clear the register to enable normal behavior.
> - */
> - write_enable(nor);
> - spi_nor_write_ear(nor, 0);
> - write_disable(nor);
> - }
> + /*
> + * On Winbond W25Q256FV, leaving 4byte mode causes the Extended Address
> + * Register to be set to 1, so all 3-byte-address reads come from the
> + * second 16M. We must clear the register to enable normal behavior.
> + */
> + write_enable(nor);
> + ret = spi_nor_write_ear(nor, 0);
> + write_disable(nor);
>
> - return status;
> - default:
> - /* Spansion style */
> - return spansion_set_4byte(nor, enable);
> - }
> + return ret;
> }
>
> static int spi_nor_xread_sr(struct spi_nor *nor, u8 *sr)
> @@ -4153,11 +4143,18 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
> static void macronix_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = macronix_quad_enable;
> + nor->params.set_4byte = macronix_set_4byte;
> }
>
> static void st_micron_set_default_init(struct spi_nor *nor)
> {
> nor->params.quad_enable = NULL;
> + nor->params.set_4byte = st_micron_set_4byte;
> +}
> +
> +static void winbond_set_default_init(struct spi_nor *nor)
> +{
> + nor->params.set_4byte = winbond_set_4byte;
> }
>
> /**
> @@ -4178,6 +4175,10 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
> st_micron_set_default_init(nor);
> break;
>
> + case SNOR_MFR_WINBOND:
> + winbond_set_default_init(nor);
> + break;
> +
> default:
> break;
> }
> @@ -4222,6 +4223,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;
>
> /* Set SPI NOR sizes. */
> params->size = (u64)info->sector_size * info->n_sectors;
> @@ -4587,7 +4589,7 @@ static int spi_nor_init(struct spi_nor *nor)
> */
> WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
> "enabling reset hack; may not recover from unexpected reboots\n");
> - set_4byte(nor, true);
> + nor->params.set_4byte(nor, true);
> }
>
> return 0;
> @@ -4611,7 +4613,7 @@ void spi_nor_restore(struct spi_nor *nor)
> /* restore the addressing mode */
> if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
> nor->flags & SNOR_F_BROKEN_RESET)
> - set_4byte(nor, false);
> + nor->params.set_4byte(nor, false);
> }
> EXPORT_SYMBOL_GPL(spi_nor_restore);
>
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index a86c0d9fb01d..7da89dd483cb 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -482,6 +482,7 @@ struct spi_nor;
> * @erase_map: the erase map parsed from the SFDP Sector Map Parameter
> * Table.
> * @quad_enable: enables SPI NOR quad mode.
> + * @set_4byte: puts the SPI NOR in 4 byte addressing mode.
> */
> struct spi_nor_flash_parameter {
> u64 size;
> @@ -494,6 +495,7 @@ struct spi_nor_flash_parameter {
> struct spi_nor_erase_map erase_map;
>
> int (*quad_enable)(struct spi_nor *nor);
> + int (*set_4byte)(struct spi_nor *nor, bool enable);
> };
>
> /**
>

--
Regards
Vignesh

2019-08-27 06:39:05

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 10/20] mtd: spi-nor: Rework the SPI NOR lock/unlock logic



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Boris Brezillon <[email protected]>
>
> Add the SNOR_F_HAS_LOCK flag and set it when SPI_NOR_HAS_LOCK is set
> in the flash_info entry or when it's a Micron or ST flash.
>
> Move the locking hooks in a separate struct so that we have just
> one field to update when we change the locking implementation.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> [[email protected]: use ->default_init() hook, introduce
> spi_nor_late_init_params(), set ops in nor->params]
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> v3: no changes, clear_sr_bp() is handled in the last patch of the
> series.
>

Reviewed-by: Vignesh Raghavendra <[email protected]>

One suggestion below:


> drivers/mtd/spi-nor/spi-nor.c | 50 ++++++++++++++++++++++++++++++++-----------
> include/linux/mtd/spi-nor.h | 23 ++++++++++++++------
> 2 files changed, 53 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 235e82a121a1..3f997797fa9d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1598,6 +1598,12 @@ static int stm_is_locked(struct spi_nor *nor, loff_t ofs, uint64_t len)
> return stm_is_locked_sr(nor, ofs, len, status);
> }
>
> +static const struct spi_nor_locking_ops stm_locking_ops = {
> + .lock = stm_lock,
> + .unlock = stm_unlock,
> + .is_locked = stm_is_locked,
> +};
> +
> static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> {
> struct spi_nor *nor = mtd_to_spi_nor(mtd);
> @@ -1607,7 +1613,7 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> if (ret)
> return ret;
>
> - ret = nor->flash_lock(nor, ofs, len);
> + ret = nor->params.locking_ops->lock(nor, ofs, len);
>
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_UNLOCK);
> return ret;
> @@ -1622,7 +1628,7 @@ static int spi_nor_unlock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> if (ret)
> return ret;
>
> - ret = nor->flash_unlock(nor, ofs, len);
> + ret = nor->params.locking_ops->unlock(nor, ofs, len);
>
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
> return ret;
> @@ -1637,7 +1643,7 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
> if (ret)
> return ret;
>
> - ret = nor->flash_is_locked(nor, ofs, len);
> + ret = nor->params.locking_ops->is_locked(nor, ofs, len);
>
> spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
> return ret;
> @@ -4148,6 +4154,7 @@ static void macronix_set_default_init(struct spi_nor *nor)
>
> static void st_micron_set_default_init(struct spi_nor *nor)
> {
> + nor->flags = SNOR_F_HAS_LOCK;

This is okay for now. But Perhaps its safer to do:

nor->flags |= SNOR_F_HAS_LOCK;

so that we don't override flags if set earlier than
spi_nor_manufacturer_init_params(). I see that patch 20/20 does indeed
present one such case wrt atmel/Xilinix flashes IIUC

Regards
Vignesh

> nor->params.quad_enable = NULL;
> nor->params.set_4byte = st_micron_set_4byte;
> }
> @@ -4292,6 +4299,23 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> }
>
> /**
> + * spi_nor_late_init_params() - Late initialization of default flash parameters.
> + * @nor: pointer to a 'struct spi_nor'
> + *
> + * Used to set default flash parameters and settings when the ->default_init()
> + * hook or the SFDP parser let voids.
> + */
> +static void spi_nor_late_init_params(struct spi_nor *nor)
> +{
> + /*
> + * NOR protection support. When locking_ops are not provided, we pick
> + * the default ones.
> + */
> + if (nor->flags & SNOR_F_HAS_LOCK && !nor->params.locking_ops)
> + nor->params.locking_ops = &stm_locking_ops;
> +}
> +
> +/**
> * spi_nor_init_params() - Initialize the flash's parameters and settings.
> * @nor: pointer to a 'struct spi-nor'.
> *
> @@ -4316,6 +4340,10 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> * Please not that there is a ->post_bfpt() fixup hook that can overwrite the
> * flash parameters and settings imediately after parsing the Basic Flash
> * Parameter Table.
> + *
> + * 4/ Late default flash parameters initialization, used when the
> + * ->default_init() hook or the SFDP parser do not set specific params.
> + * spi_nor_late_init_params()
> */
> static void spi_nor_init_params(struct spi_nor *nor)
> {
> @@ -4326,6 +4354,8 @@ static void spi_nor_init_params(struct spi_nor *nor)
> if ((nor->info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
> !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> spi_nor_sfdp_init_params(nor);
> +
> + spi_nor_late_init_params(nor);
> }
>
> static int spi_nor_select_read(struct spi_nor *nor,
> @@ -4707,6 +4737,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (info->flags & SPI_S3AN)
> nor->flags |= SNOR_F_READY_XSR_RDY;
>
> + if (info->flags & SPI_NOR_HAS_LOCK)
> + nor->flags |= SNOR_F_HAS_LOCK;
> +
> /*
> * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
> * with the software protection bits set.
> @@ -4731,16 +4764,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> mtd->_read = spi_nor_read;
> mtd->_resume = spi_nor_resume;
>
> - /* NOR protection support for STmicro/Micron chips and similar */
> - if (JEDEC_MFR(info) == SNOR_MFR_ST ||
> - JEDEC_MFR(info) == SNOR_MFR_MICRON ||
> - info->flags & SPI_NOR_HAS_LOCK) {
> - nor->flash_lock = stm_lock;
> - nor->flash_unlock = stm_unlock;
> - nor->flash_is_locked = stm_is_locked;
> - }
> -
> - if (nor->flash_lock && nor->flash_unlock && nor->flash_is_locked) {
> + if (nor->params.locking_ops) {
> mtd->_lock = spi_nor_lock;
> mtd->_unlock = spi_nor_unlock;
> mtd->_is_locked = spi_nor_is_locked;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 7da89dd483cb..ea3bcac54dc2 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -243,6 +243,7 @@ enum spi_nor_option_flags {
> SNOR_F_BROKEN_RESET = BIT(6),
> SNOR_F_4B_OPCODES = BIT(7),
> SNOR_F_HAS_4BAIT = BIT(8),
> + SNOR_F_HAS_LOCK = BIT(9),
> };
>
> /**
> @@ -466,6 +467,18 @@ enum spi_nor_pp_command_index {
> struct spi_nor;
>
> /**
> + * struct spi_nor_locking_ops - SPI NOR locking methods
> + * @lock: lock a region of the SPI NOR.
> + * @unlock: unlock a region of the SPI NOR.
> + * @is_locked: check if a region of the SPI NOR is completely locked
> + */
> +struct spi_nor_locking_ops {
> + int (*lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> + int (*unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> + int (*is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> +};
> +
> +/**
> * struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
> * Includes legacy flash parameters and settings that can be overwritten
> * by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
> @@ -483,6 +496,7 @@ struct spi_nor;
> * Table.
> * @quad_enable: enables SPI NOR quad mode.
> * @set_4byte: puts the SPI NOR in 4 byte addressing mode.
> + * @locking_ops: SPI NOR locking methods.
> */
> struct spi_nor_flash_parameter {
> u64 size;
> @@ -496,6 +510,8 @@ struct spi_nor_flash_parameter {
>
> int (*quad_enable)(struct spi_nor *nor);
> int (*set_4byte)(struct spi_nor *nor, bool enable);
> +
> + const struct spi_nor_locking_ops *locking_ops;
> };
>
> /**
> @@ -536,10 +552,6 @@ struct flash_info;
> * @erase: [DRIVER-SPECIFIC] erase a sector of the SPI NOR
> * at the offset @offs; if not provided by the driver,
> * spi-nor will send the erase opcode via write_reg()
> - * @flash_lock: [FLASH-SPECIFIC] lock a region of the SPI NOR
> - * @flash_unlock: [FLASH-SPECIFIC] unlock a region of the SPI NOR
> - * @flash_is_locked: [FLASH-SPECIFIC] check if a region of the SPI NOR is
> - * completely locked
> * @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
> * the SPI NOR Status Register.
> * @params: [FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
> @@ -579,9 +591,6 @@ struct spi_nor {
> size_t len, const u_char *write_buf);
> int (*erase)(struct spi_nor *nor, loff_t offs);
>
> - int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> - int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> - int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> int (*clear_sr_bp)(struct spi_nor *nor);
> struct spi_nor_flash_parameter params;
>
>

--
Regards
Vignesh

2019-08-27 07:00:04

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 10/20] mtd: spi-nor: Rework the SPI NOR lock/unlock logic



On 08/27/2019 09:36 AM, Vignesh Raghavendra wrote:
>> + nor->flags = SNOR_F_HAS_LOCK;
> This is okay for now. But Perhaps its safer to do:
>
> nor->flags |= SNOR_F_HAS_LOCK;
>
> so that we don't override flags if set earlier than
> spi_nor_manufacturer_init_params(). I see that patch 20/20 does indeed
> present one such case wrt atmel/Xilinix flashes IIUC

yep, you're right, I'll update as you suggested.

Thanks!

2019-08-27 07:02:57

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 08/20] mtd: spi-nor: Split spi_nor_init_params()



On 08/27/2019 09:00 AM, Vignesh Raghavendra wrote:
>> + * Please not that there is a ->post_bfpt() fixup hook that can overwrite the
> s/not/note
>
>> + * flash parameters and settings imediately after parsing the Basic Flash
> s/imediately/immediately
>

will update, thanks!

2019-08-27 07:09:24

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 11/20] mtd: spi-nor: Add post_sfdp() hook to tweak flash config



On 26/08/19 5:38 PM, [email protected] wrote:
> From: Boris Brezillon <[email protected]>
>
> SFDP tables are sometimes wrong and we need a way to override the
> config chosen by the SFDP parsing logic without discarding all of it.
>
> Add a new hook called after the SFDP parsing has taken place to deal
> with such problems.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: no changes, rebase on previous commits
>
> drivers/mtd/spi-nor/spi-nor.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 3f997797fa9d..b8caf5171ff5 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -158,6 +158,11 @@ struct sfdp_bfpt {
> * flash parameters when information provided by the flash_info
> * table is incomplete or wrong.
> * @post_bfpt: called after the BFPT table has been parsed
> + * @post_sfdp: called after SFDP has been parsed (is also called for SPI NORs
> + * that do not support RDSFDP). Typically used to tweak various
> + * parameters that could not be extracted by other means (i.e.
> + * when information provided by the SFDP/flash_info tables are
> + * incomplete or wrong).
> *
> * Those hooks can be used to tweak the SPI NOR configuration when the SFDP
> * table is broken or not available.
> @@ -168,6 +173,7 @@ struct spi_nor_fixups {
> const struct sfdp_parameter_header *bfpt_header,
> const struct sfdp_bfpt *bfpt,
> struct spi_nor_flash_parameter *params);
> + void (*post_sfdp)(struct spi_nor *nor);
> };
>
> struct flash_info {
> @@ -4299,6 +4305,22 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> }
>
> /**
> + * 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
> + * support RDSFDP).
> + * @nor: pointer to a 'struct spi_nor'
> + *
> + * Typically used to tweak various parameters that could not be extracted by
> + * other means (i.e. when information provided by the SFDP/flash_info tables
> + * are incomplete or wrong).
> + */
> +static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
> +{
> + if (nor->info->fixups && nor->info->fixups->post_sfdp)
> + nor->info->fixups->post_sfdp(nor);
> +}
> +
> +/**
> * spi_nor_late_init_params() - Late initialization of default flash parameters.
> * @nor: pointer to a 'struct spi_nor'
> *
> @@ -4341,7 +4363,14 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
> * flash parameters and settings imediately after parsing the Basic Flash
> * Parameter Table.
> *
> - * 4/ Late default flash parameters initialization, used when the
> + * which can be overwritten by:
> + * 4/ Post SFDP flash parameters initialization. Used to tweak various
> + * parameters that could not be extracted by other means (i.e. when
> + * information provided by the SFDP/flash_info tables are incomplete or
> + * wrong).
> + * spi_nor_post_sfdp_fixups()
> + *
> + * 5/ Late default flash parameters initialization, used when the
> * ->default_init() hook or the SFDP parser do not set specific params.
> * spi_nor_late_init_params()
> */
> @@ -4355,6 +4384,8 @@ static void spi_nor_init_params(struct spi_nor *nor)
> !(nor->info->flags & SPI_NOR_SKIP_SFDP))
> spi_nor_sfdp_init_params(nor);
>
> + spi_nor_post_sfdp_fixups(nor);
> +
> spi_nor_late_init_params(nor);
> }
>
>

--
Regards
Vignesh

2019-08-27 07:19:00

by Vignesh Raghavendra

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



On 26/08/19 5:38 PM, [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().
>
> Reviewed-by: Boris Brezillon <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

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

2019-08-27 07:20:10

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 15/20] mtd: spi-nor: Add s3an_post_sfdp_fixups()



On 26/08/19 5:39 PM, [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]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: no changes, rebase on previous commits
>
> 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 2aca56e07341..edf1c8badac9 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;
>
> @@ -4530,6 +4531,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
> @@ -4551,6 +4557,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);
> }
> @@ -4899,12 +4908,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)
>

--
Regards
Vignesh

2019-08-27 07:36:37

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 17/20] mtd: spi-nor: Bring flash params init together



On 26/08/19 5:39 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
>
> Bring all flash parameters default initialization in
> spi_nor_legacy_params_init().
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh


> v3: collect R-b
>
> drivers/mtd/spi-nor/spi-nor.c | 29 +++++++++++------------------
> 1 file changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 2699e999d21a..dcda96a20f6c 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4453,6 +4453,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> struct spi_nor_flash_parameter *params = &nor->params;
> struct spi_nor_erase_map *map = &params->erase_map;
> const struct flash_info *info = nor->info;
> + struct device_node *np = spi_nor_get_flash_node(nor);
> u8 i, erase_mask;
>
> /* Initialize legacy flash parameters and settings. */
> @@ -4464,18 +4465,25 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
> params->size = (u64)info->sector_size * info->n_sectors;
> params->page_size = info->page_size;
>
> + if (!(info->flags & SPI_NOR_NO_FR)) {
> + /* Default to Fast Read for DT and non-DT platform devices. */
> + params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> +
> + /* Mask out Fast Read if not requested at DT instantiation. */
> + if (np && !of_property_read_bool(np, "m25p,fast-read"))
> + params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> + }
> +
> /* (Fast) Read settings. */
> params->hwcaps.mask |= SNOR_HWCAPS_READ;
> spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ],
> 0, 0, SPINOR_OP_READ,
> SNOR_PROTO_1_1_1);
>
> - if (!(info->flags & SPI_NOR_NO_FR)) {
> - params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> + if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
> spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
> 0, 8, SPINOR_OP_READ_FAST,
> SNOR_PROTO_1_1_1);
> - }
>
> if (info->flags & SPI_NOR_DUAL_READ) {
> params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
> @@ -4864,24 +4872,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> nor->page_size = params->page_size;
> mtd->writebufsize = nor->page_size;
>
> - if (np) {
> - /* If we were instantiated by DT, use it */
> - if (of_property_read_bool(np, "m25p,fast-read"))
> - params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> - else
> - params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> - } else {
> - /* If we weren't instantiated by DT, default to fast-read */
> - params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
> - }
> -
> if (of_property_read_bool(np, "broken-flash-reset"))
> nor->flags |= SNOR_F_BROKEN_RESET;
>
> - /* Some devices cannot do fast-read, no matter what DT tells us */
> - if (info->flags & SPI_NOR_NO_FR)
> - params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
> -
> /*
> * Configure the SPI memory:
> * - select op codes for (Fast) Read, Page Program and Sector Erase.
>

--
Regards
Vignesh

2019-08-27 07:39:16

by Vignesh Raghavendra

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 18/20] mtd: spi_nor: Introduce spi_nor_set_addr_width()



On 26/08/19 5:39 PM, [email protected] wrote:
> From: Tudor Ambarus <[email protected]>
>
> Parsing of flash parameters were interleaved with setting of the
> nor addr width. Dedicate a function for setting nor addr width.
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> Reviewed-by: Boris Brezillon <[email protected]>
> ---

Reviewed-by: Vignesh Raghavendra <[email protected]>

Regards
Vignesh

> v3: no changes
>
> drivers/mtd/spi-nor/spi-nor.c | 50 ++++++++++++++++++++++++++-----------------
> 1 file changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index dcda96a20f6c..d13317d1f372 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4739,6 +4739,33 @@ static const struct flash_info *spi_nor_match_id(const char *name)
> return NULL;
> }
>
> +static int spi_nor_set_addr_width(struct spi_nor *nor)
> +{
> + if (nor->addr_width) {
> + /* already configured from SFDP */
> + } else if (nor->info->addr_width) {
> + nor->addr_width = nor->info->addr_width;
> + } else if (nor->mtd.size > 0x1000000) {
> + /* enable 4-byte addressing if the device exceeds 16MiB */
> + nor->addr_width = 4;
> + } else {
> + nor->addr_width = 3;
> + }
> +
> + if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
> + dev_err(nor->dev, "address width is too large: %u\n",
> + nor->addr_width);
> + return -EINVAL;
> + }
> +
> + /* Set 4byte opcodes when possible. */
> + if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
> + !(nor->flags & SNOR_F_HAS_4BAIT))
> + spi_nor_set_4byte_opcodes(nor);
> +
> + return 0;
> +}
> +
> int spi_nor_scan(struct spi_nor *nor, const char *name,
> const struct spi_nor_hwcaps *hwcaps)
> {
> @@ -4885,29 +4912,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (ret)
> return ret;
>
> - if (nor->addr_width) {
> - /* already configured from SFDP */
> - } else if (info->addr_width) {
> - nor->addr_width = info->addr_width;
> - } else if (mtd->size > 0x1000000) {
> - /* enable 4-byte addressing if the device exceeds 16MiB */
> - nor->addr_width = 4;
> - } else {
> - nor->addr_width = 3;
> - }
> -
> if (info->flags & SPI_NOR_4B_OPCODES)
> nor->flags |= SNOR_F_4B_OPCODES;
>
> - if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
> - !(nor->flags & SNOR_F_HAS_4BAIT))
> - spi_nor_set_4byte_opcodes(nor);
> -
> - if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
> - dev_err(dev, "address width is too large: %u\n",
> - nor->addr_width);
> - return -EINVAL;
> - }
> + ret = spi_nor_set_addr_width(nor);
> + if (ret)
> + return ret;
>
> /* Send all the required SPI flash commands to initialize device */
> ret = spi_nor_init(nor);
>

--
Regards
Vignesh

2019-08-28 10:21:37

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [RESEND PATCH v3 00/20] mtd: spi-nor: move manuf out of the core



On 08/26/2019 03:08 PM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <[email protected]>
>
> v3:
> - Drop patches:
> "mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'"
> "mtd: spi-nor: Rework the disabling of block write protection"
> and replace them with the RFC patch:
> "mtd: spi-nor: Rework the disabling of block write protection"
> - rename spi_nor_legacy_init_params() to spi_nor_info_init_params()
> - rebase patches and send them all in a single patch set.
>
> 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.
>
> In order to test this, you'll have to merge v5.3-rc5 in spi-nor/next.
> This patch set depends on
> 'commit 834de5c1aa76 ("mtd: spi-nor: Fix the disabling of write protection at init")
>
> The scope of the "mtd: spi-nor: move manuf out of the core" batches,
> is to move all manufacturer specific code out of the spi-nor core.
>
> In the quest of removing the manufacturer specific code from the spi-nor
> core, we want to impose a timeline/priority on how the flash parameters
> are updated. As of now. the flash parameters initialization logic is as
> following:
>
> a/ default flash parameters init in spi_nor_init_params()
> b/ manufacturer specific flash parameters updates, split across entire
> spi-nor core code
> c/ flash parameters updates based on SFDP tables
> d/ post BFPT flash parameter updates
>
> With the "mtd: spi-nor: move manuf out of the core" batches, 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.
>
> Setting of flash parameters will no longer be spread interleaved across
> the spi-nor core, there will be a clear separation on who and when will
> update the flash parameters.
>
> Tested on sst26vf064b with atmel-quadspi SPIMEM driver.
>
> Boris Brezillon (7):
> mtd: spi-nor: Add a default_init() fixup hook for gd25q256
> mtd: spi-nor: Create a ->set_4byte() method
> mtd: spi-nor: Rework the SPI NOR lock/unlock logic
> 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 (13):
> mtd: spi-nor: Regroup flash parameter and settings
> mtd: spi-nor: Use nor->params
> mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
> mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'
> mtd: spi-nor: Add default_init() hook to tweak flash parameters
> mtd: spi_nor: Move manufacturer quad_enable() in ->default_init()
> mtd: spi-nor: Split spi_nor_init_params()
> mtd: spi_nor: Add a ->setup() method
> mtd: spi-nor: Add s3an_post_sfdp_fixups()
> mtd: spi-nor: Bring flash params init together
> mtd: spi_nor: Introduce spi_nor_set_addr_width()
> mtd: spi-nor: Introduce spi_nor_get_flash_info()
> mtd: spi-nor: Rework the disabling of block write protection
>
> drivers/mtd/spi-nor/spi-nor.c | 1304 +++++++++++++++++++++++------------------
> include/linux/mtd/spi-nor.h | 298 +++++++---
> 2 files changed, 927 insertions(+), 675 deletions(-)
>

Addressed Boris's and Vingnesh's latest small updates.

All but last applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git,
spi-nor/next branch.

Thanks,
ta