2019-09-26 00:48:30

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 00/22] mtd: spi-nor: Quad Enable and (un)lock methods

From: Tudor Ambarus <[email protected]>

Patches 1 - 14 are just clean up patches for the Flash Register
Operations.

Patches 15 - 22 deal with the Quad Enable and the (un)lock methods.
Fixed the clearing of QE bit on (un)lock() operations. Reworked the
Quad Enable methods and the disabling of the block write protection
at power-up.

Again, this is just compile tested, I don't have (yet) a relevant
spansion-like flash memory to test the (un)lock() methods, so I'll need
your help for testing this patch set.

The patch set can be tested using mtd-utils:
1/ do a read-erase-write-read-back test immediately after boot, to check
the spi_nor_unlock_all() method. The focus is on the erase/write
methods, we want to see if the flash is unlocked at power-up.
mtd_debug read /dev/mtd-yours offset size read-file
hexdump read-file
mtd_debug erase /dev/mtd-yours offset size
dd if=/dev/urandom of=write-file bs=please-choose count=please-choose
mtd_debug write /dev/mtd-yours offset write-file-size write-file
mtd_debug read /dev/mtd-yours offset write-file-size read-file
sha1sum read-file write-file
2/ lock flash then try to erase/write it, to see if the lock works
flash_lock /dev/mtd-yours offset block-count
Do the read-erase-write-read-back test from 1/. The contents of
flash should not change in the erase and write steps.
3/ unlock flash and do the read-erase-write-read-back from 1/. The value of the
QEE should not change and you should be able to erase and write the flash.
Test 1/ should be successful.

v2:
- Introduce spi_nor_write_16bit_cr_and_check() as per Vignesh's suggestion. The
Configuration Register contains bits that can be updated in future: FREEZE,
CMP. Provide a generic method that allows updating all bits of the
Configuration Register.
- Fix SNOR_F_NO_READ_CR case in
"mtd: spi-nor: Rework the disabling of block write protection". When the flash
doesn't support the CR Read command, we make an assumption about the value of
the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can be sure
the QE bit has value one, because of the previous call to spi_nor_quad_enable().
- Fix if statement in spi_nor_write_sr_and_check():
if (nor->flags & SNOR_F_HAS_16BIT_SR)
- Fix documentation warnings.
- New patch: "mtd: spi-nor: Check all the bits written, not just the BP ones".
- Drop Global Unlock patches, will send them in a different patch set.

Tudor Ambarus (22):
mtd: spi-nor: hisi-sfc: Drop nor->erase NULL assignment
mtd: spi-nor: Introduce 'struct spi_nor_controller_ops'
mtd: spi-nor: cadence-quadspi: Fix cqspi_command_read() definition
mtd: spi-nor: Rename nor->params to nor->flash
mtd: spi-nor: Rework read_sr()
mtd: spi-nor: Rework read_fsr()
mtd: spi-nor: Rework read_cr()
mtd: spi-nor: Rework write_enable/disable()
mtd: spi-nor: Fix retlen handling in sst_write()
mtd: spi-nor: Rework write_sr()
mtd: spi-nor: Rework spi_nor_read/write_sr2()
mtd: spi-nor: Report error in spi_nor_xread_sr()
mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
mtd: spi-nor: Drop duplicated new line
mtd: spi-nor: Drop spansion_quad_enable()
mtd: spi-nor: Fix errno on quad_enable methods
mtd: spi-nor: Check all the bits written, not just the BP ones
mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
mtd: spi-nor: Rework macronix_quad_enable()
mtd: spi-nor: Rework spansion(_no)_read_cr_quad_enable()
mtd: spi-nor: Update sr2_bit7_quad_enable()
mtd: spi-nor: Rework the disabling of block write protection

drivers/mtd/spi-nor/aspeed-smc.c | 23 +-
drivers/mtd/spi-nor/cadence-quadspi.c | 54 +-
drivers/mtd/spi-nor/hisi-sfc.c | 23 +-
drivers/mtd/spi-nor/intel-spi.c | 24 +-
drivers/mtd/spi-nor/mtk-quadspi.c | 25 +-
drivers/mtd/spi-nor/nxp-spifi.c | 23 +-
drivers/mtd/spi-nor/spi-nor.c | 1716 ++++++++++++++++++---------------
include/linux/mtd/spi-nor.h | 74 +-
8 files changed, 1058 insertions(+), 904 deletions(-)

--
2.9.5


2019-09-26 00:48:47

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 04/22] mtd: spi-nor: Rename nor->params to nor->flash

From: Tudor Ambarus <[email protected]>

Rename nor->params to nor->flash for a clearer separation
between the controller and flash operations.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b8c7ded0f145..7d0c1b598250 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -172,7 +172,7 @@ struct spi_nor_fixups {
int (*post_bfpt)(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt,
- struct spi_nor_flash_parameter *params);
+ struct spi_nor_flash_parameter *flash);
void (*post_sfdp)(struct spi_nor *nor);
};

@@ -608,7 +608,7 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);

if (!spi_nor_has_uniform_erase(nor)) {
- struct spi_nor_erase_map *map = &nor->params.erase_map;
+ struct spi_nor_erase_map *map = &nor->flash.erase_map;
struct spi_nor_erase_type *erase;
int i;

@@ -927,10 +927,10 @@ static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)

static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr)
{
- if (!nor->params.convert_addr)
+ if (!nor->flash.convert_addr)
return addr;

- return nor->params.convert_addr(nor, addr);
+ return nor->flash.convert_addr(nor, addr);
}

/*
@@ -1140,7 +1140,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
struct list_head *erase_list,
u64 addr, u32 len)
{
- const struct spi_nor_erase_map *map = &nor->params.erase_map;
+ const struct spi_nor_erase_map *map = &nor->flash.erase_map;
const struct spi_nor_erase_type *erase, *prev_erase = NULL;
struct spi_nor_erase_region *region;
struct spi_nor_erase_command *cmd = NULL;
@@ -1628,7 +1628,7 @@ static int spi_nor_lock(struct mtd_info *mtd, loff_t ofs, uint64_t len)
if (ret)
return ret;

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

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

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

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

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

spi_nor_unlock_and_unprep(nor, SPI_NOR_OPS_LOCK);
return ret;
@@ -2093,7 +2093,7 @@ static int
is25lp256_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
/*
* IS25LP256 supports 4B opcodes, but the BFPT advertises a
@@ -2115,7 +2115,7 @@ static int
mx25l25635_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
/*
* MX25L25635F supports 4B opcodes but MX25L25635E does not.
@@ -2144,7 +2144,7 @@ static void gd25q256_default_init(struct spi_nor *nor)
* 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;
+ nor->flash.quad_enable = macronix_quad_enable;
}

static struct spi_nor_fixups gd25q256_fixups = {
@@ -2777,7 +2777,7 @@ static int s3an_nor_setup(struct spi_nor *nor,
nor->mtd.erasesize = 8 * nor->page_size;
} else {
/* Flash in Default addressing mode */
- nor->params.convert_addr = s3an_convert_addr;
+ nor->flash.convert_addr = s3an_convert_addr;
nor->mtd.erasesize = nor->info->sector_size;
}

@@ -3017,7 +3017,7 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
static void
spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
{
- struct spi_nor_flash_parameter *params = &nor->params;
+ struct spi_nor_flash_parameter *flash = &nor->flash;
unsigned int cap;

/* DTR modes are not supported yet, mask them all. */
@@ -3034,7 +3034,7 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)

rdidx = spi_nor_hwcaps_read2cmd(BIT(cap));
if (rdidx >= 0 &&
- spi_nor_spimem_check_readop(nor, &params->reads[rdidx]))
+ spi_nor_spimem_check_readop(nor, &flash->reads[rdidx]))
*hwcaps &= ~BIT(cap);

ppidx = spi_nor_hwcaps_pp2cmd(BIT(cap));
@@ -3042,7 +3042,7 @@ spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
continue;

if (spi_nor_spimem_check_pp(nor,
- &params->page_programs[ppidx]))
+ &flash->page_programs[ppidx]))
*hwcaps &= ~BIT(cap);
}
}
@@ -3091,7 +3091,7 @@ spi_nor_set_read_settings_from_bfpt(struct spi_nor_read_command *read,
}

struct sfdp_bfpt_read {
- /* The Fast Read x-y-z hardware capability in params->hwcaps.mask. */
+ /* The Fast Read x-y-z hardware capability in flash->hwcaps.mask. */
u32 hwcaps;

/*
@@ -3322,11 +3322,11 @@ static int
spi_nor_post_bfpt_fixups(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
const struct sfdp_bfpt *bfpt,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
if (nor->info->fixups && nor->info->fixups->post_bfpt)
return nor->info->fixups->post_bfpt(nor, bfpt_header, bfpt,
- params);
+ flash);

return 0;
}
@@ -3336,7 +3336,7 @@ spi_nor_post_bfpt_fixups(struct spi_nor *nor,
* @nor: pointer to a 'struct spi_nor'
* @bfpt_header: pointer to the 'struct sfdp_parameter_header' describing
* the Basic Flash Parameter Table length and version
- * @params: pointer to the 'struct spi_nor_flash_parameter' to be
+ * @flash: pointer to the 'struct spi_nor_flash_parameter' to be
* filled
*
* The Basic Flash Parameter Table is the main and only mandatory table as
@@ -3363,9 +3363,9 @@ spi_nor_post_bfpt_fixups(struct spi_nor *nor,
*/
static int spi_nor_parse_bfpt(struct spi_nor *nor,
const struct sfdp_parameter_header *bfpt_header,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
- struct spi_nor_erase_map *map = &params->erase_map;
+ struct spi_nor_erase_map *map = &flash->erase_map;
struct spi_nor_erase_type *erase_type = map->erase_type;
struct sfdp_bfpt bfpt;
size_t len;
@@ -3406,23 +3406,23 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
}

/* Flash Memory Density (in bits). */
- params->size = bfpt.dwords[BFPT_DWORD(2)];
- if (params->size & BIT(31)) {
- params->size &= ~BIT(31);
+ flash->size = bfpt.dwords[BFPT_DWORD(2)];
+ if (flash->size & BIT(31)) {
+ flash->size &= ~BIT(31);

/*
- * Prevent overflows on params->size. Anyway, a NOR of 2^64
+ * Prevent overflows on flash->size. Anyway, a NOR of 2^64
* bits is unlikely to exist so this error probably means
* the BFPT we are reading is corrupted/wrong.
*/
- if (params->size > 63)
+ if (flash->size > 63)
return -EINVAL;

- params->size = 1ULL << params->size;
+ flash->size = 1ULL << flash->size;
} else {
- params->size++;
+ flash->size++;
}
- params->size >>= 3; /* Convert to bytes. */
+ flash->size >>= 3; /* Convert to bytes. */

/* Fast Read settings. */
for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) {
@@ -3430,13 +3430,13 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
struct spi_nor_read_command *read;

if (!(bfpt.dwords[rd->supported_dword] & rd->supported_bit)) {
- params->hwcaps.mask &= ~rd->hwcaps;
+ flash->hwcaps.mask &= ~rd->hwcaps;
continue;
}

- params->hwcaps.mask |= rd->hwcaps;
+ flash->hwcaps.mask |= rd->hwcaps;
cmd = spi_nor_hwcaps_read2cmd(rd->hwcaps);
- read = &params->reads[cmd];
+ read = &flash->reads[cmd];
half = bfpt.dwords[rd->settings_dword] >> rd->settings_shift;
spi_nor_set_read_settings_from_bfpt(read, half, rd->proto);
}
@@ -3446,7 +3446,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
* Erase Types defined in the bfpt table.
*/
erase_mask = 0;
- memset(&params->erase_map, 0, sizeof(params->erase_map));
+ memset(&flash->erase_map, 0, sizeof(flash->erase_map));
for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_erases); i++) {
const struct sfdp_bfpt_erase *er = &sfdp_bfpt_erases[i];
u32 erasesize;
@@ -3465,7 +3465,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
spi_nor_set_erase_settings_from_bfpt(&erase_type[i], erasesize,
opcode, i);
}
- spi_nor_init_uniform_erase_map(map, erase_mask, params->size);
+ spi_nor_init_uniform_erase_map(map, erase_mask, flash->size);
/*
* Sort all the map's Erase Types in ascending order with the smallest
* erase size being the first member in the erase_type array.
@@ -3483,43 +3483,42 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,

/* Stop here if not JESD216 rev A or later. */
if (bfpt_header->length < BFPT_DWORD_MAX)
- return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt,
- params);
+ return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, flash);

/* Page size: this field specifies 'N' so the page size = 2^N bytes. */
- params->page_size = bfpt.dwords[BFPT_DWORD(11)];
- params->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
- params->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
- params->page_size = 1U << params->page_size;
+ flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
+ flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
+ flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
+ flash->page_size = 1U << flash->page_size;

/* Quad Enable Requirements. */
switch (bfpt.dwords[BFPT_DWORD(15)] & BFPT_DWORD15_QER_MASK) {
case BFPT_DWORD15_QER_NONE:
- params->quad_enable = NULL;
+ flash->quad_enable = NULL;
break;

case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
- params->quad_enable = spansion_no_read_cr_quad_enable;
+ flash->quad_enable = spansion_no_read_cr_quad_enable;
break;

case BFPT_DWORD15_QER_SR1_BIT6:
- params->quad_enable = macronix_quad_enable;
+ flash->quad_enable = macronix_quad_enable;
break;

case BFPT_DWORD15_QER_SR2_BIT7:
- params->quad_enable = sr2_bit7_quad_enable;
+ flash->quad_enable = sr2_bit7_quad_enable;
break;

case BFPT_DWORD15_QER_SR2_BIT1:
- params->quad_enable = spansion_read_cr_quad_enable;
+ flash->quad_enable = spansion_read_cr_quad_enable;
break;

default:
return -EINVAL;
}

- return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, params);
+ return spi_nor_post_bfpt_fixups(nor, bfpt_header, &bfpt, flash);
}

#define SMPT_CMD_ADDRESS_LEN_MASK GENMASK(23, 22)
@@ -3721,7 +3720,7 @@ spi_nor_region_check_overlay(struct spi_nor_erase_region *region,
/**
* spi_nor_init_non_uniform_erase_map() - initialize the non-uniform erase map
* @nor: pointer to a 'struct spi_nor'
- * @params: pointer to a duplicate 'struct spi_nor_flash_parameter' that is
+ * @flash: pointer to a duplicate 'struct spi_nor_flash_parameter' that is
* used for storing SFDP parsed data
* @smpt: pointer to the sector map parameter table
*
@@ -3729,10 +3728,10 @@ spi_nor_region_check_overlay(struct spi_nor_erase_region *region,
*/
static int
spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
- struct spi_nor_flash_parameter *params,
+ struct spi_nor_flash_parameter *flash,
const u32 *smpt)
{
- struct spi_nor_erase_map *map = &params->erase_map;
+ struct spi_nor_erase_map *map = &flash->erase_map;
struct spi_nor_erase_type *erase = map->erase_type;
struct spi_nor_erase_region *region;
u64 offset;
@@ -3811,7 +3810,7 @@ spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
* spi_nor_parse_smpt() - parse Sector Map Parameter Table
* @nor: pointer to a 'struct spi_nor'
* @smpt_header: sector map parameter table header
- * @params: pointer to a duplicate 'struct spi_nor_flash_parameter'
+ * @flash: pointer to a duplicate 'struct spi_nor_flash_parameter'
* that is used for storing SFDP parsed data
*
* This table is optional, but when available, we parse it to identify the
@@ -3822,7 +3821,7 @@ spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
*/
static int spi_nor_parse_smpt(struct spi_nor *nor,
const struct sfdp_parameter_header *smpt_header,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
const u32 *sector_map;
u32 *smpt;
@@ -3851,11 +3850,11 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
goto out;
}

- ret = spi_nor_init_non_uniform_erase_map(nor, params, sector_map);
+ ret = spi_nor_init_non_uniform_erase_map(nor, flash, sector_map);
if (ret)
goto out;

- spi_nor_regions_sort_erase_types(&params->erase_map);
+ spi_nor_regions_sort_erase_types(&flash->erase_map);
/* fall through */
out:
kfree(smpt);
@@ -3880,13 +3879,13 @@ struct sfdp_4bait {
* @nor: pointer to a 'struct spi_nor'.
* @param_header: pointer to the 'struct sfdp_parameter_header' describing
* the 4-Byte Address Instruction Table length and version.
- * @params: pointer to the 'struct spi_nor_flash_parameter' to be.
+ * @flash: pointer to the 'struct spi_nor_flash_parameter' to be.
*
* Return: 0 on success, -errno otherwise.
*/
static int spi_nor_parse_4bait(struct spi_nor *nor,
const struct sfdp_parameter_header *param_header,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
static const struct sfdp_4bait reads[] = {
{ SNOR_HWCAPS_READ, BIT(0) },
@@ -3910,8 +3909,8 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
{ 0u /* not used */, BIT(11) },
{ 0u /* not used */, BIT(12) },
};
- struct spi_nor_pp_command *params_pp = params->page_programs;
- struct spi_nor_erase_map *map = &params->erase_map;
+ struct spi_nor_pp_command *flash_pp = flash->page_programs;
+ struct spi_nor_erase_map *map = &flash->erase_map;
struct spi_nor_erase_type *erase_type = map->erase_type;
u32 *dwords;
size_t len;
@@ -3949,7 +3948,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
const struct sfdp_4bait *read = &reads[i];

discard_hwcaps |= read->hwcaps;
- if ((params->hwcaps.mask & read->hwcaps) &&
+ if ((flash->hwcaps.mask & read->hwcaps) &&
(dwords[0] & read->supported_bit))
read_hwcaps |= read->hwcaps;
}
@@ -3965,7 +3964,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
/*
* The 4 Byte Address Instruction (Optional) Table is the only
* SFDP table that indicates support for Page Program Commands.
- * Bypass the params->hwcaps.mask and consider 4BAIT the biggest
+ * Bypass the flash->hwcaps.mask and consider 4BAIT the biggest
* authority for specifying Page Program support.
*/
discard_hwcaps |= program->hwcaps;
@@ -4000,26 +3999,26 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
* Discard all operations from the 4-byte instruction set which are
* not supported by this memory.
*/
- params->hwcaps.mask &= ~discard_hwcaps;
- params->hwcaps.mask |= (read_hwcaps | pp_hwcaps);
+ flash->hwcaps.mask &= ~discard_hwcaps;
+ flash->hwcaps.mask |= (read_hwcaps | pp_hwcaps);

/* Use the 4-byte address instruction set. */
for (i = 0; i < SNOR_CMD_READ_MAX; i++) {
- struct spi_nor_read_command *read_cmd = &params->reads[i];
+ struct spi_nor_read_command *read_cmd = &flash->reads[i];

read_cmd->opcode = spi_nor_convert_3to4_read(read_cmd->opcode);
}

/* 4BAIT is the only SFDP table that indicates page program support. */
if (pp_hwcaps & SNOR_HWCAPS_PP)
- spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP],
+ spi_nor_set_pp_settings(&flash_pp[SNOR_CMD_PP],
SPINOR_OP_PP_4B, SNOR_PROTO_1_1_1);
if (pp_hwcaps & SNOR_HWCAPS_PP_1_1_4)
- spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP_1_1_4],
+ spi_nor_set_pp_settings(&flash_pp[SNOR_CMD_PP_1_1_4],
SPINOR_OP_PP_1_1_4_4B,
SNOR_PROTO_1_1_4);
if (pp_hwcaps & SNOR_HWCAPS_PP_1_4_4)
- spi_nor_set_pp_settings(&params_pp[SNOR_CMD_PP_1_4_4],
+ spi_nor_set_pp_settings(&flash_pp[SNOR_CMD_PP_1_4_4],
SPINOR_OP_PP_1_4_4_4B,
SNOR_PROTO_1_4_4);

@@ -4050,7 +4049,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
/**
* spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
* @nor: pointer to a 'struct spi_nor'
- * @params: pointer to the 'struct spi_nor_flash_parameter' to be
+ * @flash: pointer to the 'struct spi_nor_flash_parameter' to be
* filled
*
* The Serial Flash Discoverable Parameters are described by the JEDEC JESD216
@@ -4062,7 +4061,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
* Return: 0 on success, -errno otherwise.
*/
static int spi_nor_parse_sfdp(struct spi_nor *nor,
- struct spi_nor_flash_parameter *params)
+ struct spi_nor_flash_parameter *flash)
{
const struct sfdp_parameter_header *param_header, *bfpt_header;
struct sfdp_parameter_header *param_headers = NULL;
@@ -4131,7 +4130,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
bfpt_header = param_header;
}

- err = spi_nor_parse_bfpt(nor, bfpt_header, params);
+ err = spi_nor_parse_bfpt(nor, bfpt_header, flash);
if (err)
goto exit;

@@ -4141,11 +4140,11 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,

switch (SFDP_PARAM_HEADER_ID(param_header)) {
case SFDP_SECTOR_MAP_ID:
- err = spi_nor_parse_smpt(nor, param_header, params);
+ err = spi_nor_parse_smpt(nor, param_header, flash);
break;

case SFDP_4BAIT_ID:
- err = spi_nor_parse_4bait(nor, param_header, params);
+ err = spi_nor_parse_4bait(nor, param_header, flash);
break;

default:
@@ -4183,7 +4182,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
if (cmd < 0)
return -EINVAL;

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

@@ -4214,7 +4213,7 @@ static int spi_nor_select_pp(struct spi_nor *nor,
if (cmd < 0)
return -EINVAL;

- pp = &nor->params.page_programs[cmd];
+ pp = &nor->flash.page_programs[cmd];
nor->program_opcode = pp->opcode;
nor->write_proto = pp->proto;
return 0;
@@ -4275,7 +4274,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,

static int spi_nor_select_erase(struct spi_nor *nor)
{
- struct spi_nor_erase_map *map = &nor->params.erase_map;
+ struct spi_nor_erase_map *map = &nor->flash.erase_map;
const struct spi_nor_erase_type *erase = NULL;
struct mtd_info *mtd = &nor->mtd;
u32 wanted_size = nor->info->sector_size;
@@ -4324,7 +4323,7 @@ static int spi_nor_select_erase(struct spi_nor *nor)
static int spi_nor_default_setup(struct spi_nor *nor,
const struct spi_nor_hwcaps *hwcaps)
{
- struct spi_nor_flash_parameter *params = &nor->params;
+ struct spi_nor_flash_parameter *flash = &nor->flash;
u32 ignored_mask, shared_mask;
int err;

@@ -4332,7 +4331,7 @@ static int spi_nor_default_setup(struct spi_nor *nor,
* Keep only the hardware capabilities supported by both the SPI
* controller and the SPI flash memory.
*/
- shared_mask = hwcaps->mask & params->hwcaps.mask;
+ shared_mask = hwcaps->mask & flash->hwcaps.mask;

if (nor->spimem) {
/*
@@ -4385,36 +4384,36 @@ static int spi_nor_default_setup(struct spi_nor *nor,
static int spi_nor_setup(struct spi_nor *nor,
const struct spi_nor_hwcaps *hwcaps)
{
- if (!nor->params.setup)
+ if (!nor->flash.setup)
return 0;

- return nor->params.setup(nor, hwcaps);
+ return nor->flash.setup(nor, hwcaps);
}

static void macronix_set_default_init(struct spi_nor *nor)
{
- nor->params.quad_enable = macronix_quad_enable;
- nor->params.set_4byte = macronix_set_4byte;
+ nor->flash.quad_enable = macronix_quad_enable;
+ nor->flash.set_4byte = macronix_set_4byte;
}

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;
+ nor->flash.quad_enable = NULL;
+ nor->flash.set_4byte = st_micron_set_4byte;
}

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

/**
- * spi_nor_manufacturer_init_params() - Initialize the flash's parameters and
- * settings based on MFR register and ->default_init() hook.
+ * spi_nor_manufacturer_init_flash_params() - Initialize the flash's
+ * parameters and 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)
+static void spi_nor_manufacturer_init_flash_params(struct spi_nor *nor)
{
/* Init flash parameters based on MFR */
switch (JEDEC_MFR(nor->info)) {
@@ -4440,93 +4439,93 @@ static void spi_nor_manufacturer_init_params(struct spi_nor *nor)
}

/**
- * spi_nor_sfdp_init_params() - Initialize the flash's parameters and settings
- * based on JESD216 SFDP standard.
+ * spi_nor_sfdp_init_flash_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)
+static void spi_nor_sfdp_init_flash_params(struct spi_nor *nor)
{
- struct spi_nor_flash_parameter sfdp_params;
+ struct spi_nor_flash_parameter sfdp_flash;

- memcpy(&sfdp_params, &nor->params, sizeof(sfdp_params));
+ memcpy(&sfdp_flash, &nor->flash, sizeof(sfdp_flash));

- if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
+ if (spi_nor_parse_sfdp(nor, &sfdp_flash)) {
nor->addr_width = 0;
nor->flags &= ~SNOR_F_4B_OPCODES;
} else {
- memcpy(&nor->params, &sfdp_params, sizeof(nor->params));
+ memcpy(&nor->flash, &sfdp_flash, sizeof(nor->flash));
}
}

/**
- * spi_nor_info_init_params() - Initialize the flash's parameters and settings
- * based on nor->info data.
+ * spi_nor_info_init_flash_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)
+static void spi_nor_info_init_flash_params(struct spi_nor *nor)
{
- struct spi_nor_flash_parameter *params = &nor->params;
- struct spi_nor_erase_map *map = &params->erase_map;
+ struct spi_nor_flash_parameter *flash = &nor->flash;
+ struct spi_nor_erase_map *map = &flash->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. */
- params->quad_enable = spansion_quad_enable;
- params->set_4byte = spansion_set_4byte;
- params->setup = spi_nor_default_setup;
+ flash->quad_enable = spansion_quad_enable;
+ flash->set_4byte = spansion_set_4byte;
+ flash->setup = spi_nor_default_setup;

/* Set SPI NOR sizes. */
- params->size = (u64)info->sector_size * info->n_sectors;
- params->page_size = info->page_size;
+ flash->size = (u64)info->sector_size * info->n_sectors;
+ flash->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;
+ flash->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;
+ flash->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],
+ flash->hwcaps.mask |= SNOR_HWCAPS_READ;
+ spi_nor_set_read_settings(&flash->reads[SNOR_CMD_READ],
0, 0, SPINOR_OP_READ,
SNOR_PROTO_1_1_1);

- if (params->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
- spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_FAST],
+ if (flash->hwcaps.mask & SNOR_HWCAPS_READ_FAST)
+ spi_nor_set_read_settings(&flash->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;
- spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_2],
+ flash->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
+ spi_nor_set_read_settings(&flash->reads[SNOR_CMD_READ_1_1_2],
0, 8, SPINOR_OP_READ_1_1_2,
SNOR_PROTO_1_1_2);
}

if (info->flags & SPI_NOR_QUAD_READ) {
- params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
- spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_4],
+ flash->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
+ spi_nor_set_read_settings(&flash->reads[SNOR_CMD_READ_1_1_4],
0, 8, SPINOR_OP_READ_1_1_4,
SNOR_PROTO_1_1_4);
}

if (info->flags & SPI_NOR_OCTAL_READ) {
- params->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
- spi_nor_set_read_settings(&params->reads[SNOR_CMD_READ_1_1_8],
+ flash->hwcaps.mask |= SNOR_HWCAPS_READ_1_1_8;
+ spi_nor_set_read_settings(&flash->reads[SNOR_CMD_READ_1_1_8],
0, 8, SPINOR_OP_READ_1_1_8,
SNOR_PROTO_1_1_8);
}

/* Page Program settings. */
- params->hwcaps.mask |= SNOR_HWCAPS_PP;
- spi_nor_set_pp_settings(&params->page_programs[SNOR_CMD_PP],
+ flash->hwcaps.mask |= SNOR_HWCAPS_PP;
+ spi_nor_set_pp_settings(&flash->page_programs[SNOR_CMD_PP],
SPINOR_OP_PP, SNOR_PROTO_1_1_1);

/*
@@ -4549,7 +4548,7 @@ static void spi_nor_info_init_params(struct spi_nor *nor)
erase_mask |= BIT(i);
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_uniform_erase_map(map, erase_mask, flash->size);
}

static void spansion_post_sfdp_fixups(struct spi_nor *nor)
@@ -4567,7 +4566,7 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor)

static void s3an_post_sfdp_fixups(struct spi_nor *nor)
{
- nor->params.setup = s3an_nor_setup;
+ nor->flash.setup = s3an_nor_setup;
}

/**
@@ -4599,24 +4598,25 @@ static void spi_nor_post_sfdp_fixups(struct spi_nor *nor)
}

/**
- * spi_nor_late_init_params() - Late initialization of default flash parameters.
+ * spi_nor_late_init_flash_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)
+static void spi_nor_late_init_flash_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;
+ if (nor->flags & SNOR_F_HAS_LOCK && !nor->flash.locking_ops)
+ nor->flash.locking_ops = &stm_locking_ops;
}

/**
- * spi_nor_init_params() - Initialize the flash's parameters and settings.
+ * spi_nor_init_flash_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
@@ -4624,18 +4624,18 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
*
* 1/ Default flash parameters initialization. The initializations are done
* based on nor->info data:
- * spi_nor_info_init_params()
+ * spi_nor_info_init_flash_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()
+ * spi_nor_manufacturer_init_flash_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()
+ * spi_nor_sfdp_init_flash_params()
*
* Please note that there is a ->post_bfpt() fixup hook that can overwrite
* the flash parameters and settings immediately after parsing the Basic
@@ -4649,22 +4649,22 @@ static void spi_nor_late_init_params(struct spi_nor *nor)
* 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()
+ * ->default_init() hook or the SFDP parser do not set specific flash params.
+ * spi_nor_late_init_flash_params()
*/
-static void spi_nor_init_params(struct spi_nor *nor)
+static void spi_nor_init_flash_params(struct spi_nor *nor)
{
- spi_nor_info_init_params(nor);
+ spi_nor_info_init_flash_params(nor);

- spi_nor_manufacturer_init_params(nor);
+ spi_nor_manufacturer_init_flash_params(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_sfdp_init_flash_params(nor);

spi_nor_post_sfdp_fixups(nor);

- spi_nor_late_init_params(nor);
+ spi_nor_late_init_flash_params(nor);
}

/**
@@ -4675,14 +4675,14 @@ static void spi_nor_init_params(struct spi_nor *nor)
*/
static int spi_nor_quad_enable(struct spi_nor *nor)
{
- if (!nor->params.quad_enable)
+ if (!nor->flash.quad_enable)
return 0;

if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
spi_nor_get_protocol_width(nor->write_proto) == 4))
return 0;

- return nor->params.quad_enable(nor);
+ return nor->flash.quad_enable(nor);
}

static int spi_nor_init(struct spi_nor *nor)
@@ -4690,7 +4690,7 @@ static int spi_nor_init(struct spi_nor *nor)
int err;

if (nor->clear_sr_bp) {
- if (nor->params.quad_enable == spansion_quad_enable)
+ if (nor->flash.quad_enable == spansion_quad_enable)
nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;

err = nor->clear_sr_bp(nor);
@@ -4717,7 +4717,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");
- nor->params.set_4byte(nor, true);
+ nor->flash.set_4byte(nor, true);
}

return 0;
@@ -4741,7 +4741,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)
- nor->params.set_4byte(nor, false);
+ nor->flash.set_4byte(nor, false);
}
EXPORT_SYMBOL_GPL(spi_nor_restore);

@@ -4841,7 +4841,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
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;
+ struct spi_nor_flash_parameter *flash = &nor->flash;
int ret;
int i;

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

/* Init flash parameters based on flash_info struct and SFDP */
- spi_nor_init_params(nor);
+ spi_nor_init_flash_params(nor);

if (!mtd->name)
mtd->name = dev_name(dev);
@@ -4908,12 +4908,12 @@ 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 = flash->size;
mtd->_erase = spi_nor_erase;
mtd->_read = spi_nor_read;
mtd->_resume = spi_nor_resume;

- if (nor->params.locking_ops) {
+ if (nor->flash.locking_ops) {
mtd->_lock = spi_nor_lock;
mtd->_unlock = spi_nor_unlock;
mtd->_is_locked = spi_nor_is_locked;
@@ -4938,7 +4938,7 @@ 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 = flash->page_size;
mtd->writebufsize = nor->page_size;

if (of_property_read_bool(np, "broken-flash-reset"))
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index d1d736d3c8ab..12961b157743 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -580,10 +580,10 @@ struct flash_info;
* @controller_ops: SPI NOR controller driver specific operations.
* @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
- * hooks, or dynamically when parsing the SFDP tables.
+ * @flash: SPI-NOR flash parameters and settings. The structure
+ * includes default flash parameters and settings that can
+ * be overwritten by the spi_nor_fixups hooks, or
+ * dynamically when parsing the SFDP tables.
* @priv: the private data
*/
struct spi_nor {
@@ -609,7 +609,7 @@ struct spi_nor {
const struct spi_nor_controller_ops *controller_ops;

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

void *priv;
};
@@ -640,7 +640,7 @@ spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)

static bool __maybe_unused spi_nor_has_uniform_erase(const struct spi_nor *nor)
{
- return !!nor->params.erase_map.uniform_erase_type;
+ return !!nor->flash.erase_map.uniform_erase_type;
}

static inline void spi_nor_set_flash_node(struct spi_nor *nor,
--
2.9.5

2019-09-26 00:48:53

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 21/22] mtd: spi-nor: Update sr2_bit7_quad_enable()

From: Tudor Ambarus <[email protected]>

Rename the method to spi_nor_sr2_bit7_quad_enable(). Do the
read back test on all the eight bits of the Status Register,
not just the QE one.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8fd1c04f75d9..a53e2cdc564c 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2038,7 +2038,7 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
}

/**
- * sr2_bit7_quad_enable() - set QE bit in Status Register 2.
+ * spi_nor_sr2_bit7_quad_enable() - set QE bit in Status Register 2.
* @nor: pointer to a 'struct spi_nor'
*
* Set the Quad Enable (QE) bit in the Status Register 2.
@@ -2049,10 +2049,11 @@ static int spi_nor_sr2_bit1_quad_enable(struct spi_nor *nor)
*
* Return: 0 on success, -errno otherwise.
*/
-static int sr2_bit7_quad_enable(struct spi_nor *nor)
+static int spi_nor_sr2_bit7_quad_enable(struct spi_nor *nor)
{
u8 *sr2 = nor->bouncebuf;
int ret;
+ u8 sr2_written;

/* Check current Quad Enable bit value. */
ret = spi_nor_read_sr2(nor, sr2);
@@ -2069,13 +2070,15 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
if (ret)
return ret;

+ sr2_written = *sr2;
+
/* Read back and check it. */
ret = spi_nor_read_sr2(nor, sr2);
if (ret)
return ret;

- if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
- dev_err(nor->dev, "SR2 Quad bit not set\n");
+ if (*sr2 != sr2_written) {
+ dev_err(nor->dev, "Read back test failed\n");
return -EIO;
}

@@ -3649,7 +3652,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,

case BFPT_DWORD15_QER_SR2_BIT7:
nor->flags &= ~SNOR_F_HAS_16BIT_SR;
- flash->quad_enable = sr2_bit7_quad_enable;
+ flash->quad_enable = spi_nor_sr2_bit7_quad_enable;
break;

case BFPT_DWORD15_QER_SR2_BIT1:
--
2.9.5

2019-09-26 00:49:13

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 14/22] mtd: spi-nor: Drop duplicated new line

From: Tudor Ambarus <[email protected]>

Two new lines, remove one.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 191a76c3f7bb..d971f5a4b11f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -841,7 +841,6 @@ static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
return mtd->priv;
}

-
static u8 spi_nor_convert_opcode(u8 opcode, const u8 table[][2], size_t size)
{
size_t i;
--
2.9.5

2019-09-26 00:49:20

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 11/22] mtd: spi-nor: Rework spi_nor_read/write_sr2()

From: Tudor Ambarus <[email protected]>

Move the methods up in the file, where the other Register
operations reside.

The error is reported inside each SR2 function, to spare the callers
of duplicating code.

Constify sr2 in spi_nor_write_sr2(). Do the spi_nor_write_enable() and
spi_nor_wait_till_ready() inside spi_nor_write_sr2(), as the
spi_nor_write_sr() does.

While modyfing sr2_bit7_quad_enable(), add a new line for better code
readability.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 31a4622d1eb9..33130ee84164 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -731,6 +731,74 @@ static int spi_nor_write_sr(struct spi_nor *nor, const u8 *sr, size_t len)
return ret;
}

+/**
+ * spi_nor_write_sr2() - Write the Status Register 2 using the
+ * SPINOR_OP_WRSR2 (3eh) command.
+ * @nor: pointer to 'struct spi_nor'.
+ * @sr2: buffer to write to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr2(struct spi_nor *nor, const u8 *sr2)
+{
+ int ret;
+
+ ret = spi_nor_write_enable(nor);
+ if (ret)
+ return ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_OUT(1, sr2, 1));
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2,
+ sr2, 1);
+ }
+
+ if (ret)
+ dev_err(nor->dev, "error while writing Status Register 2\n");
+
+ ret = spi_nor_wait_till_ready(nor);
+
+ return ret;
+}
+
+/**
+ * spi_nor_read_sr2() - Read the Status Register 2 using the
+ * SPINOR_OP_RDSR2 (3fh) command.
+ * @nor: pointer to 'struct spi_nor'
+ * @sr2: buffer where the value of the Status Register will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
+{
+ int ret;
+
+ if (nor->spimem) {
+ struct spi_mem_op op =
+ SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
+ SPI_MEM_OP_NO_ADDR,
+ SPI_MEM_OP_NO_DUMMY,
+ SPI_MEM_OP_DATA_IN(1, sr2, 1));
+
+ ret = spi_mem_exec_op(nor->spimem, &op);
+ } else {
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2,
+ sr2, 1);
+ }
+
+ if (ret)
+ dev_err(nor->dev, "error while reading Status Register 2\n");
+
+ return ret;
+}
+
static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
@@ -1890,36 +1958,6 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
return 0;
}

-static int spi_nor_write_sr2(struct spi_nor *nor, u8 *sr2)
-{
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_WRSR2, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_OUT(1, sr2, 1));
-
- return spi_mem_exec_op(nor->spimem, &op);
- }
-
- return nor->controller_ops->write_reg(nor, SPINOR_OP_WRSR2, sr2, 1);
-}
-
-static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
-{
- if (nor->spimem) {
- struct spi_mem_op op =
- SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDSR2, 1),
- SPI_MEM_OP_NO_ADDR,
- SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(1, sr2, 1));
-
- return spi_mem_exec_op(nor->spimem, &op);
- }
-
- return nor->controller_ops->read_reg(nor, SPINOR_OP_RDSR2, sr2, 1);
-}
-
/**
* sr2_bit7_quad_enable() - set QE bit in Status Register 2.
* @nor: pointer to a 'struct spi_nor'
@@ -1941,31 +1979,23 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)
ret = spi_nor_read_sr2(nor, sr2);
if (ret)
return ret;
+
if (*sr2 & SR2_QUAD_EN_BIT7)
return 0;

/* Update the Quad Enable bit. */
*sr2 |= SR2_QUAD_EN_BIT7;

- ret = spi_nor_write_enable(nor);
- if (ret)
- return ret;
-
ret = spi_nor_write_sr2(nor, sr2);
- if (ret < 0) {
- dev_err(nor->dev, "error while writing status register 2\n");
- return -EINVAL;
- }
-
- ret = spi_nor_wait_till_ready(nor);
- if (ret < 0) {
- dev_err(nor->dev, "timeout while writing status register 2\n");
+ if (ret)
return ret;
- }

/* Read back and check it. */
ret = spi_nor_read_sr2(nor, sr2);
- if (!(ret > 0 && (*sr2 & SR2_QUAD_EN_BIT7))) {
+ if (ret)
+ return ret;
+
+ if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
dev_err(nor->dev, "SR2 Quad bit not set\n");
return -EINVAL;
}
--
2.9.5

2019-09-26 00:49:32

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 15/22] mtd: spi-nor: Drop spansion_quad_enable()

From: Tudor Ambarus <[email protected]>

Drop the default spansion_quad_enable() method and replace it with
spansion_read_cr_quad_enable().

The function was buggy, it didn't care about the previous values
of the Status and Configuration Registers. spansion_read_cr_quad_enable()
is a Read-Modify-Write-Check function that keeps track of what were
the previous values of the Status and Configuration Registers.

In terms of instruction types sent to the flash, the only difference
between the spansion_quad_enable() and spansion_read_cr_quad_enable()
is that the later calls spi_nor_read_sr(). We can safely assume that all
flashes support spi_nor_read_sr(), because all flashes call it in
spi_nor_sr_ready(). The transition from spansion_quad_enable() to
spansion_read_cr_quad_enable() will not affect anybody, drop the buggy
code.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d971f5a4b11f..668afa9a8c87 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1869,54 +1869,6 @@ static int macronix_quad_enable(struct spi_nor *nor)
}

/**
- * spansion_quad_enable() - set QE bit in Configuraiton Register.
- * @nor: pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function is kept for legacy purpose because it has been used for a
- * long time without anybody complaining but it should be considered as
- * deprecated and maybe buggy.
- * First, this function doesn't care about the previous values of the Status
- * and Configuration Registers when it sets the QE bit (bit 1) in the
- * Configuration Register: all other bits are cleared, which may have unwanted
- * side effects like removing some block protections.
- * Secondly, it uses the Read Configuration Register (35h) instruction though
- * some very old and few memories don't support this instruction. If a pull-up
- * resistor is present on the MISO/IO1 line, we might still be able to pass the
- * "read back" test because the QSPI memory doesn't recognize the command,
- * so leaves the MISO/IO1 line state unchanged, hence spi_nor_read_cr(nor, cr)
- * gets the 0xFF value.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_quad_enable(struct spi_nor *nor)
-{
- u8 *sr_cr = nor->bouncebuf;
- int ret;
-
- sr_cr[0] = 0;
- sr_cr[1] = CR_QUAD_EN_SPAN;
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- if (ret)
- return ret;
-
- /* read back and check it */
- ret = spi_nor_read_cr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
- dev_err(nor->dev, "Spansion Quad bit not set\n");
- return -EINVAL;
- }
-
- return 0;
-}
-
-/**
* spansion_no_read_cr_quad_enable() - set QE bit in Configuration Register.
* @nor: pointer to a 'struct spi_nor'
*
@@ -2071,9 +2023,9 @@ static int spi_nor_clear_sr_bp(struct spi_nor *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.
+ * coupled with the spansion_read_cr_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.
*/
@@ -4560,7 +4512,7 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
u8 i, erase_mask;

/* Initialize legacy flash parameters and settings. */
- flash->quad_enable = spansion_quad_enable;
+ flash->quad_enable = spansion_read_cr_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;

@@ -4776,7 +4728,7 @@ static int spi_nor_init(struct spi_nor *nor)
int err;

if (nor->clear_sr_bp) {
- if (nor->flash.quad_enable == spansion_quad_enable)
+ if (nor->flash.quad_enable == spansion_read_cr_quad_enable)
nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;

err = nor->clear_sr_bp(nor);
--
2.9.5

2019-09-26 00:49:41

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 18/22] mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()

From: Tudor Ambarus <[email protected]>

Make sure that when doing a lock() or an unlock() operation we don't clear
the QE bit from Status Register 2.

JESD216 revB or later offers information about the *default* Status
Register commands to use (see BFPT DWORDS[15], bits 22:20). In this
standard, Status Register 1 refers to the first data byte transferred on a
Read Status (05h) or Write Status (01h) command. Status register 2 refers
to the byte read using instruction 35h. Status register 2 is the second
byte transferred in a Write Status (01h) command.

Industry naming and definitions of these Status Registers may differ.
The definitions are described in JESD216B, BFPT DWORDS[15], bits 22:20.
There are cases in which writing only one byte to the Status Register 1
has the side-effect of clearing Status Register 2 and implicitly the Quad
Enable bit. This side-effect is hit just by the
BFPT_DWORD15_QER_SR2_BIT1_BUGGY and BFPT_DWORD15_QER_SR2_BIT1 cases.

Suggested-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 165 +++++++++++++++++++++++++++++++++++++-----
include/linux/mtd/spi-nor.h | 2 +
2 files changed, 147 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 48bcb2ee1be5..8ada2003f1c9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -836,6 +836,127 @@ static int spi_nor_read_sr2(struct spi_nor *nor, u8 *sr2)
return ret;
}

+/**
+ * spi_nor_write_sr1_and_check() - Write one byte to the Status Register 1 and
+ * ensure that the byte written match the received value.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr1_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+
+ nor->bouncebuf[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
+ if (ret)
+ return ret;
+
+ if (nor->bouncebuf[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_16bit_sr_and_check() - Write the Status Register 1 and the
+ * Status Register 2 in one shot. Ensure that the byte written in the Status
+ * Register 1 match the received value, and that the 16-bit Write did not
+ * affect what was already in the Status Register 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_16bit_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ int ret;
+ u8 *sr_cr = nor->bouncebuf;
+ u8 cr_written;
+
+ /* Make sure we don't overwrite the contents of Status Register 2. */
+ if (!(nor->flags & SNOR_F_NO_READ_CR)) {
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+ } else if (nor->flash.quad_enable) {
+ /*
+ * If the Status Register 2 Read command (35h) is not
+ * supported, we should at least be sure we don't
+ * change the value of the SR2 Quad Enable bit.
+ *
+ * We can safely assume that when the Quad Enable method is
+ * set, the value of the QE bit is one, as a consequence of the
+ * nor->flash.quad_enable() call.
+ *
+ * We can safely assume that the Quad Enable bit is present in
+ * the Status Register 2 at BIT(1). According to the JESD216
+ * revB standard, BFPT DWORDS[15], bits 22:20, the 16-bit
+ * Write Status (01h) command is available just for the cases
+ * in which the QE bit is described in SR2 at BIT(1).
+ */
+ sr_cr[1] = CR_QUAD_EN_SPAN;
+ } else {
+ sr_cr[1] = 0;
+ }
+
+ sr_cr[0] = sr1;
+
+ ret = spi_nor_write_sr(nor, sr_cr, 2);
+ if (ret)
+ return ret;
+
+ ret = spi_nor_read_sr(nor, &sr_cr[0]);
+ if (ret)
+ return ret;
+
+ if (sr_cr[0] != sr1) {
+ dev_err(nor->dev, "SR1: read back test failed\n");
+ return -EIO;
+ }
+
+ if (nor->flags & SNOR_F_NO_READ_CR)
+ return 0;
+
+ cr_written = sr_cr[1];
+
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (cr_written != sr_cr[1]) {
+ dev_err(nor->dev, "CR: read back test failed\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+/**
+ * spi_nor_write_sr_and_check() - Write the Status Register 1 and ensure that
+ * the byte written match the received value without affecting other bits in the
+ * Status Register 1 and 2.
+ * @nor: pointer to a 'struct spi_nor'.
+ * @sr1: byte value to be written to the Status Register 1.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_write_sr_and_check(struct spi_nor *nor, u8 sr1)
+{
+ if (nor->flags & SNOR_F_HAS_16BIT_SR)
+ return spi_nor_write_16bit_sr_and_check(nor, sr1);
+
+ return spi_nor_write_sr1_and_check(nor, sr1);
+}
+
static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
{
return mtd->priv;
@@ -1492,24 +1613,6 @@ 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)
-{
- int ret;
-
- nor->bouncebuf[0] = status_new;
-
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
- if (ret)
- return ret;
-
- ret = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- return (nor->bouncebuf[0] != status_new) ? -EIO : 0;
-}
-
static void stm_get_locked_range(struct spi_nor *nor, u8 sr, loff_t *ofs,
uint64_t *len)
{
@@ -1673,7 +1776,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);
+ return spi_nor_write_sr_and_check(nor, status_new);
}

/*
@@ -1758,7 +1861,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);
+ return spi_nor_write_sr_and_check(nor, status_new);
}

/*
@@ -3536,19 +3639,39 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
break;

case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
+ /*
+ * Writing only one byte to the Status Register has the
+ * side-effect of clearing Status Register 2.
+ */
+ /* fall through */
case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
+ /*
+ * Read Configuration Register (35h) instruction is not
+ * supported.
+ */
+ nor->flags |= SNOR_F_NO_READ_CR;
flash->quad_enable = spansion_no_read_cr_quad_enable;
break;

case BFPT_DWORD15_QER_SR1_BIT6:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = macronix_quad_enable;
break;

case BFPT_DWORD15_QER_SR2_BIT7:
+ nor->flags &= ~SNOR_F_HAS_16BIT_SR;
flash->quad_enable = sr2_bit7_quad_enable;
break;

case BFPT_DWORD15_QER_SR2_BIT1:
+ /*
+ * JESD216 rev B or later does not specify if writing only one
+ * byte to the Status Register clears or not the Status
+ * Register 2, so let's be cautious and keep the default
+ * assumption of a 16-bit Write Status (01h) command.
+ */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;
flash->quad_enable = spansion_read_cr_quad_enable;
break;

@@ -4515,6 +4638,8 @@ static void spi_nor_info_init_flash_params(struct spi_nor *nor)
flash->quad_enable = spansion_read_cr_quad_enable;
flash->set_4byte = spansion_set_4byte;
flash->setup = spi_nor_default_setup;
+ /* Default to 16-bit Write Status (01h) Command */
+ nor->flags |= SNOR_F_HAS_16BIT_SR;

/* Set SPI NOR sizes. */
flash->size = (u64)info->sector_size * info->n_sectors;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 12961b157743..fc3a8f5209f0 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -243,6 +243,8 @@ 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),
+ SNOR_F_NO_READ_CR = BIT(10),
};

/**
--
2.9.5

2019-09-26 00:49:54

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 22/22] 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.

Get rid of clear_sr_bp() and introduce spi_nor_unlock_all(), which is
just a call to spi_nor_unlock() for the entire flash memory array.
This fixes a bug that was present in spi_nor_spansion_clear_sr_bp().
When the QE bit was zero, we used the Write Status (01h) command with
one data byte, which might cleared the Status Register 2. We now always
use the Write Status (01h) command with two data bytes when
SNOR_F_HAS_16BIT_SR is set, to avoid clearing the Status Register 2.

The SNOR_F_NO_READ_CR case is treated as well. When the flash doesn't
support the CR Read command, we make an assumption about the value of
the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can
be sure the QE bit has value one, because of the previous call to
spi_nor_quad_enable().

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

Note that this changes a bit the logic for the SNOR_MFR_ATMEL,
SNOR_MFR_INTEL and SNOR_MFR_SST cases. Before this patch, the Atmel,
Intel and SST 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.

Suggested-by: Boris Brezillon <[email protected]>
Signed-off-by: Tudor Ambarus <[email protected]>
---
drivers/mtd/spi-nor/spi-nor.c | 146 +++++++++++++++---------------------------
include/linux/mtd/spi-nor.h | 3 -
2 files changed, 51 insertions(+), 98 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a53e2cdc564c..7b61edf284a1 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2085,79 +2085,6 @@ static int spi_nor_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 = spi_nor_read_sr(nor, &nor->bouncebuf[0]);
- if (ret)
- return ret;
-
- nor->bouncebuf[0] &= mask;
-
- ret = spi_nor_write_sr(nor, &nor->bouncebuf[0], 1);
-
- 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 spi_nor_sr2_bit1_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 = spi_nor_read_cr(nor, &sr_cr[1]);
- if (ret)
- 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 (sr_cr[1] & SR2_QUAD_EN_BIT1) {
- ret = spi_nor_read_sr(nor, &sr_cr[0]);
- if (ret)
- return ret;
-
- sr_cr[0] &= ~mask;
-
- ret = spi_nor_write_sr(nor, sr_cr, 2);
- 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 = { \
@@ -4542,6 +4469,16 @@ static int spi_nor_setup(struct spi_nor *nor,
return nor->flash.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->flash.quad_enable = spi_nor_sr1_bit6_quad_enable;
@@ -4555,6 +4492,11 @@ static void st_micron_set_default_init(struct spi_nor *nor)
nor->flash.set_4byte = st_micron_set_4byte;
}

+static void sst_set_default_init(struct spi_nor *nor)
+{
+ nor->flags |= SNOR_F_HAS_LOCK;
+}
+
static void winbond_set_default_init(struct spi_nor *nor)
{
nor->flash.set_4byte = winbond_set_4byte;
@@ -4569,6 +4511,14 @@ static void spi_nor_manufacturer_init_flash_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;
@@ -4578,6 +4528,10 @@ static void spi_nor_manufacturer_init_flash_params(struct spi_nor *nor)
st_micron_set_default_init(nor);
break;

+ case SNOR_MFR_SST:
+ sst_set_default_init(nor);
+ break;
+
case SNOR_MFR_WINBOND:
winbond_set_default_init(nor);
break;
@@ -4839,21 +4793,26 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
return nor->flash.quad_enable(nor);
}

-static int spi_nor_init(struct spi_nor *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)
{
- int err;
+ if (nor->flags & SNOR_F_HAS_LOCK)
+ return spi_nor_unlock(&nor->mtd, 0, nor->flash.size);

- if (nor->clear_sr_bp) {
- if (nor->flash.quad_enable == spi_nor_sr2_bit1_quad_enable)
- nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
+ return 0;
+}

- err = nor->clear_sr_bp(nor);
- if (err) {
- dev_err(nor->dev,
- "fail to clear block protection bits\n");
- return err;
- }
- }
+static int spi_nor_init(struct spi_nor *nor)
+{
+ int err;

err = spi_nor_quad_enable(nor);
if (err) {
@@ -4861,6 +4820,13 @@ static int spi_nor_init(struct spi_nor *nor)
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;
+ }
+
if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
/*
* If the RESET# pin isn't hooked up properly, or the system
@@ -5043,16 +5009,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_flash_params(nor);

diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 5590a36eb43e..ca650f895903 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -578,8 +578,6 @@ struct flash_info;
* @write_proto: the SPI protocol for write operations
* @reg_proto the SPI protocol for read_reg/write_reg/erase operations
* @controller_ops: SPI NOR controller driver specific operations.
- * @clear_sr_bp: [FLASH-SPECIFIC] clears the Block Protection Bits from
- * the SPI NOR Status Register.
* @flash: SPI-NOR flash parameters and settings. The structure
* includes default flash parameters and settings that can
* be overwritten by the spi_nor_fixups hooks, or
@@ -608,7 +606,6 @@ struct spi_nor {

const struct spi_nor_controller_ops *controller_ops;

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

void *priv;
--
2.9.5

2019-09-26 07:44:09

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 07/22] mtd: spi-nor: Rework read_cr()

From: Tudor Ambarus <[email protected]>

static int read_cr(struct spi_nor *nor)
becomes
static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)

The new function returns 0 on success and -errno otherwise.
We let the callers pass the pointer to the buffer where the
value of the Configuration Register will be written. This way
we avoid the casts between int and u8, which can be confusing.

Prepend spi_nor_ to the function name, all functions should begin
with that.

Vendors are using both the "Configuration Register" and the
"Status Register 2" terminology when referring to the second byte
of the Status Register. Indicate in the description of the function
that we use the SPINOR_OP_RDCR (35h) command to interrogate the
Configuration Register.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 8cd1cadcb8b1..0fb124bd2e77 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -448,12 +448,16 @@ static int spi_nor_read_fsr(struct spi_nor *nor, u8 *fsr)
return ret;
}

-/*
- * Read configuration register, returning its value in the
- * location. Return the configuration register value.
- * Returns negative if error occurred.
+/**
+ * spi_nor_read_cr() - Read the Configuration Register using the
+ * SPINOR_OP_RDCR (35h) command.
+ * @nor: pointer to 'struct spi_nor'
+ * @cr: buffer where the value of the Configuration Register
+ * will be written.
+ *
+ * Return: 0 on success, -errno otherwise.
*/
-static int read_cr(struct spi_nor *nor)
+static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
{
int ret;

@@ -462,20 +466,17 @@ static int read_cr(struct spi_nor *nor)
SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_RDCR, 1),
SPI_MEM_OP_NO_ADDR,
SPI_MEM_OP_NO_DUMMY,
- SPI_MEM_OP_DATA_IN(1, nor->bouncebuf, 1));
+ SPI_MEM_OP_DATA_IN(1, cr, 1));

ret = spi_mem_exec_op(nor->spimem, &op);
} else {
- ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDCR,
- nor->bouncebuf, 1);
+ ret = nor->controller_ops->read_reg(nor, SPINOR_OP_RDCR, cr, 1);
}

- if (ret < 0) {
+ if (ret)
dev_err(nor->dev, "error %d reading CR\n", ret);
- return ret;
- }

- return nor->bouncebuf[0];
+ return ret;
}

/*
@@ -1768,7 +1769,8 @@ static int macronix_quad_enable(struct spi_nor *nor)
* some very old and few memories don't support this instruction. If a pull-up
* resistor is present on the MISO/IO1 line, we might still be able to pass the
* "read back" test because the QSPI memory doesn't recognize the command,
- * so leaves the MISO/IO1 line state unchanged, hence read_cr() returns 0xFF.
+ * so leaves the MISO/IO1 line state unchanged, hence spi_nor_read_cr(nor, cr)
+ * gets the 0xFF value.
*
* bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
* memories.
@@ -1787,8 +1789,11 @@ static int spansion_quad_enable(struct spi_nor *nor)
return ret;

/* read back and check it */
- ret = read_cr(nor);
- if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
+ ret = spi_nor_read_cr(nor, &nor->bouncebuf[0]);
+ if (ret)
+ return ret;
+
+ if (!(nor->bouncebuf[0] & CR_QUAD_EN_SPAN)) {
dev_err(nor->dev, "Spansion Quad bit not set\n");
return -EINVAL;
}
@@ -1839,21 +1844,18 @@ static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
*/
static int spansion_read_cr_quad_enable(struct spi_nor *nor)
{
- struct device *dev = nor->dev;
u8 *sr_cr = nor->bouncebuf;
int ret;

/* Check current Quad Enable bit value. */
- ret = read_cr(nor);
- if (ret < 0) {
- dev_err(dev, "error while reading configuration register\n");
- return -EINVAL;
- }
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;

- if (ret & CR_QUAD_EN_SPAN)
+ if (sr_cr[1] & CR_QUAD_EN_SPAN)
return 0;

- sr_cr[1] = ret | CR_QUAD_EN_SPAN;
+ sr_cr[1] |= CR_QUAD_EN_SPAN;

/* Keep the current value of the Status Register. */
ret = spi_nor_read_sr(nor, &sr_cr[0]);
@@ -1865,8 +1867,11 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)
return ret;

/* Read back and check it. */
- ret = read_cr(nor);
- if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
+ return ret;
+
+ if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
dev_err(nor->dev, "Spansion Quad bit not set\n");
return -EINVAL;
}
@@ -2007,20 +2012,15 @@ static int spi_nor_spansion_clear_sr_bp(struct spi_nor *nor)
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");
+ ret = spi_nor_read_cr(nor, &sr_cr[1]);
+ if (ret)
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;
-
+ if (sr_cr[1] & CR_QUAD_EN_SPAN) {
ret = spi_nor_read_sr(nor, &sr_cr[0]);
if (ret)
return ret;
--
2.9.5

2019-09-26 07:58:39

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH v2 16/22] mtd: spi-nor: Fix errno on quad_enable methods

From: Tudor Ambarus <[email protected]>

When the Read-Modify-Write-Read-Back Quad Enable methods failed on
the Read-Back, they returned -EINVAL. Since this is an I/O error,
return -EIO.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 668afa9a8c87..6429c855547e 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1862,7 +1862,7 @@ static int macronix_quad_enable(struct spi_nor *nor)

if (!(nor->bouncebuf[0] & SR_QUAD_EN_MX)) {
dev_err(nor->dev, "Macronix Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}

return 0;
@@ -1940,7 +1940,7 @@ static int spansion_read_cr_quad_enable(struct spi_nor *nor)

if (!(sr_cr[1] & CR_QUAD_EN_SPAN)) {
dev_err(nor->dev, "Spansion Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}

return 0;
@@ -1985,7 +1985,7 @@ static int sr2_bit7_quad_enable(struct spi_nor *nor)

if (!(*sr2 & SR2_QUAD_EN_BIT7)) {
dev_err(nor->dev, "SR2 Quad bit not set\n");
- return -EINVAL;
+ return -EIO;
}

return 0;
--
2.9.5

2019-09-26 09:26:40

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 00/22] mtd: spi-nor: Quad Enable and (un)lock methods

Hi, Jonas,

s25fl512s is impacted by this patch set. Would you please do a little test to
see if everything is ok for your flash with these patches applied? I don't have
the flash, so I can't do the tests by myself. There is a possible test method
described below in the cover letter.

You can find the patches at
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=132270 or at
https://github.com/ambarus/linux-0day, branch spi-nor/quad-enable-rework-v2.

Thank you,
ta

On 09/24/2019 10:45 AM, Tudor Ambarus - M18064 wrote:
> From: Tudor Ambarus <[email protected]>
>
> Patches 1 - 14 are just clean up patches for the Flash Register
> Operations.
>
> Patches 15 - 22 deal with the Quad Enable and the (un)lock methods.
> Fixed the clearing of QE bit on (un)lock() operations. Reworked the
> Quad Enable methods and the disabling of the block write protection
> at power-up.
>
> Again, this is just compile tested, I don't have (yet) a relevant
> spansion-like flash memory to test the (un)lock() methods, so I'll need
> your help for testing this patch set.
>
> The patch set can be tested using mtd-utils:
> 1/ do a read-erase-write-read-back test immediately after boot, to check
> the spi_nor_unlock_all() method. The focus is on the erase/write
> methods, we want to see if the flash is unlocked at power-up.
> mtd_debug read /dev/mtd-yours offset size read-file
> hexdump read-file
> mtd_debug erase /dev/mtd-yours offset size
> dd if=/dev/urandom of=write-file bs=please-choose count=please-choose
> mtd_debug write /dev/mtd-yours offset write-file-size write-file
> mtd_debug read /dev/mtd-yours offset write-file-size read-file
> sha1sum read-file write-file
> 2/ lock flash then try to erase/write it, to see if the lock works
> flash_lock /dev/mtd-yours offset block-count
> Do the read-erase-write-read-back test from 1/. The contents of
> flash should not change in the erase and write steps.
> 3/ unlock flash and do the read-erase-write-read-back from 1/. The value of the
> QEE should not change and you should be able to erase and write the flash.
> Test 1/ should be successful.
>
> v2:
> - Introduce spi_nor_write_16bit_cr_and_check() as per Vignesh's suggestion. The
> Configuration Register contains bits that can be updated in future: FREEZE,
> CMP. Provide a generic method that allows updating all bits of the
> Configuration Register.
> - Fix SNOR_F_NO_READ_CR case in
> "mtd: spi-nor: Rework the disabling of block write protection". When the flash
> doesn't support the CR Read command, we make an assumption about the value of
> the QE bit. In spi_nor_init(), call spi_nor_quad_enable() first, then
> spi_nor_unlock_all(), so that at the spi_nor_unlock_all() time we can be sure
> the QE bit has value one, because of the previous call to spi_nor_quad_enable().
> - Fix if statement in spi_nor_write_sr_and_check():
> if (nor->flags & SNOR_F_HAS_16BIT_SR)
> - Fix documentation warnings.
> - New patch: "mtd: spi-nor: Check all the bits written, not just the BP ones".
> - Drop Global Unlock patches, will send them in a different patch set.
>
> Tudor Ambarus (22):
> mtd: spi-nor: hisi-sfc: Drop nor->erase NULL assignment
> mtd: spi-nor: Introduce 'struct spi_nor_controller_ops'
> mtd: spi-nor: cadence-quadspi: Fix cqspi_command_read() definition
> mtd: spi-nor: Rename nor->params to nor->flash
> mtd: spi-nor: Rework read_sr()
> mtd: spi-nor: Rework read_fsr()
> mtd: spi-nor: Rework read_cr()
> mtd: spi-nor: Rework write_enable/disable()
> mtd: spi-nor: Fix retlen handling in sst_write()
> mtd: spi-nor: Rework write_sr()
> mtd: spi-nor: Rework spi_nor_read/write_sr2()
> mtd: spi-nor: Report error in spi_nor_xread_sr()
> mtd: spi-nor: Void return type for spi_nor_clear_sr/fsr()
> mtd: spi-nor: Drop duplicated new line
> mtd: spi-nor: Drop spansion_quad_enable()
> mtd: spi-nor: Fix errno on quad_enable methods
> mtd: spi-nor: Check all the bits written, not just the BP ones
> mtd: spi-nor: Fix clearing of QE bit on lock()/unlock()
> mtd: spi-nor: Rework macronix_quad_enable()
> mtd: spi-nor: Rework spansion(_no)_read_cr_quad_enable()
> mtd: spi-nor: Update sr2_bit7_quad_enable()
> mtd: spi-nor: Rework the disabling of block write protection
>
> drivers/mtd/spi-nor/aspeed-smc.c | 23 +-
> drivers/mtd/spi-nor/cadence-quadspi.c | 54 +-
> drivers/mtd/spi-nor/hisi-sfc.c | 23 +-
> drivers/mtd/spi-nor/intel-spi.c | 24 +-
> drivers/mtd/spi-nor/mtk-quadspi.c | 25 +-
> drivers/mtd/spi-nor/nxp-spifi.c | 23 +-
> drivers/mtd/spi-nor/spi-nor.c | 1716 ++++++++++++++++++---------------
> include/linux/mtd/spi-nor.h | 74 +-
> 8 files changed, 1058 insertions(+), 904 deletions(-)
>

2019-10-10 07:06:59

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v2 04/22] mtd: spi-nor: Rename nor->params to nor->flash

On Tue, 24 Sep 2019 07:46:03 +0000
<[email protected]> wrote:

> From: Tudor Ambarus <[email protected]>
>
> Rename nor->params to nor->flash for a clearer separation
> between the controller and flash operations.

Hm, I'm not sure 'flash' is clearer than 'params', and the spi_nor
object is supposed to represent the NOR chip anyway, so it was pretty
clear to me that nor->params were the NOR flash parameters not the
NOR controller ones.
If I had anything to change it would be s/params/properties/ (and
s/spi_nor_flash_parameter/spi_nor_properties/) since those parameters
look like immutable information discovered during the NOR detection,
but I'm nitpicking here.

2019-10-10 07:22:38

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH v2 07/22] mtd: spi-nor: Rework read_cr()

On Tue, 24 Sep 2019 07:46:15 +0000
<[email protected]> wrote:

> From: Tudor Ambarus <[email protected]>
>
> static int read_cr(struct spi_nor *nor)
> becomes
> static int spi_nor_read_cr(struct spi_nor *nor, u8 *cr)
>
> The new function returns 0 on success and -errno otherwise.
> We let the callers pass the pointer to the buffer where the
> value of the Configuration Register will be written. This way
> we avoid the casts between int and u8, which can be confusing.
>
> Prepend spi_nor_ to the function name, all functions should begin
> with that.
>

Same as for patch 5, this should be split in several patches.

> Vendors are using both the "Configuration Register" and the
> "Status Register 2" terminology when referring to the second byte
> of the Status Register. Indicate in the description of the function
> that we use the SPINOR_OP_RDCR (35h) command to interrogate the

^query

> Configuration Register.
>

2019-10-24 10:15:30

by Tudor Ambarus

[permalink] [raw]
Subject: Re: [PATCH v2 04/22] mtd: spi-nor: Rename nor->params to nor->flash



On 10/10/2019 10:05 AM, Boris Brezillon wrote:
> External E-Mail
>
>
> On Tue, 24 Sep 2019 07:46:03 +0000
> <[email protected]> wrote:
>
>> From: Tudor Ambarus <[email protected]>
>>
>> Rename nor->params to nor->flash for a clearer separation
>> between the controller and flash operations.
>
> Hm, I'm not sure 'flash' is clearer than 'params', and the spi_nor
> object is supposed to represent the NOR chip anyway, so it was pretty
> clear to me that nor->params were the NOR flash parameters not the
> NOR controller ones.
> If I had anything to change it would be s/params/properties/ (and
> s/spi_nor_flash_parameter/spi_nor_properties/) since those parameters
> look like immutable information discovered during the NOR detection,
> but I'm nitpicking here.
>

I'll drop this for now.