2019-07-31 09:13:09

by Tudor Ambarus

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

From: Tudor Ambarus <[email protected]>

Add post_sfdp() hook to tweak flash config.

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

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

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

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

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

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

This series opens doors for 5/.

Tested on sst26vf064b with atmel-quadspi SPIMEM driver.

Depends on:
https://patchwork.ozlabs.org/project/linux-mtd/list/?series=122420

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

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

drivers/mtd/spi-nor/spi-nor.c | 192 +++++++++++++++++-----------------
include/linux/mtd/spi-nor.h | 232 +++++++++++++++++++++++++++---------------
2 files changed, 245 insertions(+), 179 deletions(-)

--
2.9.5


2019-07-31 09:13:22

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH 3/6] mtd: spi-nor: Add a ->convert_addr() method

From: Boris Brezillon <[email protected]>

In order to separate manufacturer quirks from the core we need to get
rid of all the manufacturer specific flags, like the
SNOR_F_S3AN_ADDR_DEFAULT one.

This can easily be replaced by a ->convert_addr() hook, which when
implemented will provide the core with an easy way to convert an
absolute address into something the flash understands.

Right now the only user are the S3AN chips, but other manufacturers
can implement it if needed.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 30f9d524e72f..3b6810d76a79 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1045,10 +1045,9 @@ static void spi_nor_unlock_and_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
* Addr can safely be unsigned int, the biggest S3AN device is smaller than
* 4 MiB.
*/
-static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
+static u32 s3an_convert_addr(struct spi_nor *nor, u32 addr)
{
- unsigned int offset;
- unsigned int page;
+ u32 offset, page;

offset = addr % nor->page_size;
page = addr / nor->page_size;
@@ -1057,6 +1056,14 @@ static loff_t spi_nor_s3an_addr_convert(struct spi_nor *nor, unsigned int addr)
return page | offset;
}

+static u32 spi_nor_convert_addr(struct spi_nor *nor, loff_t addr)
+{
+ if (!nor->convert_addr)
+ return addr;
+
+ return nor->convert_addr(nor, addr);
+}
+
/*
* Initiate the erasure of a single sector
*/
@@ -1065,8 +1072,7 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 addr)
u8 buf[SPI_NOR_MAX_ADDR_WIDTH];
int i;

- if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
- addr = spi_nor_s3an_addr_convert(nor, addr);
+ addr = spi_nor_convert_addr(nor, addr);

if (nor->erase)
return nor->erase(nor, addr);
@@ -2675,8 +2681,7 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t from, size_t len,
while (len) {
loff_t addr = from;

- if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
- addr = spi_nor_s3an_addr_convert(nor, addr);
+ addr = spi_nor_convert_addr(nor, addr);

ret = spi_nor_read_data(nor, addr, len, buf);
if (ret == 0) {
@@ -2820,8 +2825,7 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
page_remain = min_t(size_t,
nor->page_size - page_offset, len - i);

- if (nor->flags & SNOR_F_S3AN_ADDR_DEFAULT)
- addr = spi_nor_s3an_addr_convert(nor, addr);
+ addr = spi_nor_convert_addr(nor, addr);

write_enable(nor);
ret = spi_nor_write_data(nor, addr, page_remain, buf + i);
@@ -2889,7 +2893,7 @@ static int s3an_nor_scan(struct spi_nor *nor)
nor->mtd.erasesize = 8 * nor->page_size;
} else {
/* Flash in Default addressing mode */
- nor->flags |= SNOR_F_S3AN_ADDR_DEFAULT;
+ nor->convert_addr = s3an_convert_addr;
}

return 0;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 185ca11bfb63..af04d3117188 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -238,12 +238,11 @@ enum spi_nor_option_flags {
SNOR_F_USE_FSR = BIT(0),
SNOR_F_HAS_SR_TB = BIT(1),
SNOR_F_NO_OP_CHIP_ERASE = BIT(2),
- SNOR_F_S3AN_ADDR_DEFAULT = BIT(3),
- SNOR_F_READY_XSR_RDY = BIT(4),
- SNOR_F_USE_CLSR = BIT(5),
- SNOR_F_BROKEN_RESET = BIT(6),
- SNOR_F_4B_OPCODES = BIT(7),
- SNOR_F_HAS_4BAIT = BIT(8),
+ SNOR_F_READY_XSR_RDY = BIT(3),
+ SNOR_F_USE_CLSR = BIT(4),
+ SNOR_F_BROKEN_RESET = BIT(5),
+ SNOR_F_4B_OPCODES = BIT(6),
+ SNOR_F_HAS_4BAIT = BIT(7),
};

/**
@@ -380,6 +379,9 @@ struct flash_info;
* @quad_enable: [FLASH-SPECIFIC] enables SPI NOR quad mode
* @set_4byte: [FLASH-SPECIFIC] puts the SPI NOR in 4 byte addressing
* mode
+ * @convert_addr: [FLASH-SPECIFIC] convert an absolute address into
+ * something the flash will understand. Particularly
+ * useful when pagesize is not a power-of-2
* @disable_write_protection: [FLASH-SPECIFIC] disable write protection during
* power-up
* completely locked
@@ -423,6 +425,7 @@ struct spi_nor {
int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
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 (*disable_write_protection)(struct spi_nor *nor);

const struct spi_nor_locking_ops *locking_ops;
--
2.9.5

2019-07-31 09:13:42

by Tudor Ambarus

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

From: Tudor Ambarus <[email protected]>

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

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 0ff474e5e4f5..5fea5d7ce2cb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2795,7 +2795,9 @@ 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_flash_parameter *params,
+ const struct spi_nor_hwcaps *hwcaps)
{
int ret;
u8 val;
@@ -4393,6 +4395,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->setup = s3an_nor_setup;
+}
+
static void
spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
struct spi_nor_flash_parameter *params)
@@ -4405,6 +4412,9 @@ spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
default:
break;
}
+
+ if (nor->info->flags & SPI_S3AN)
+ s3an_post_sfdp_fixups(nor);
}

static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
@@ -4582,9 +4592,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
return 0;
}

-static int spi_nor_setup(struct spi_nor *nor,
- const struct spi_nor_flash_parameter *params,
- const struct spi_nor_hwcaps *hwcaps)
+static int spi_nor_default_setup(struct spi_nor *nor,
+ const struct spi_nor_flash_parameter *params,
+ const struct spi_nor_hwcaps *hwcaps)
{
u32 ignored_mask, shared_mask;
int err;
@@ -4641,6 +4651,16 @@ static int spi_nor_setup(struct spi_nor *nor,
return err;
}

+static int spi_nor_setup(struct spi_nor *nor,
+ const struct spi_nor_flash_parameter *params,
+ const struct spi_nor_hwcaps *hwcaps)
+{
+ if (!nor->setup)
+ return 0;
+
+ return nor->setup(nor, params, hwcaps);
+}
+
static int spi_nor_disable_write_protection(struct spi_nor *nor)
{
if (!nor->disable_write_protection)
@@ -4804,6 +4824,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
/* Kept only for backward compatibility purpose. */
nor->quad_enable = spansion_quad_enable;
nor->set_4byte = spansion_set_4byte;
+ nor->setup = spi_nor_default_setup;

/* Default locking operations. */
if (info->flags & SPI_NOR_HAS_LOCK) {
@@ -4905,12 +4926,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-07-31 09:14:14

by Tudor Ambarus

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

From: Tudor Ambarus <[email protected]>

To be used by some manufacturers to configure the spi-nor flash
memory. Right now the only user will be the S3AN chips, but other
manufacturers can implement it if needed.

Move spi_nor_flash_parameter and spi_nor_hwcaps related code
to avoid forward declarations.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 3b6810d76a79..0ff474e5e4f5 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -40,69 +40,6 @@
#define SPI_NOR_MAX_ID_LEN 6
#define SPI_NOR_MAX_ADDR_WIDTH 4

-struct spi_nor_read_command {
- u8 num_mode_clocks;
- u8 num_wait_states;
- u8 opcode;
- enum spi_nor_protocol proto;
-};
-
-struct spi_nor_pp_command {
- u8 opcode;
- enum spi_nor_protocol proto;
-};
-
-enum spi_nor_read_command_index {
- SNOR_CMD_READ,
- SNOR_CMD_READ_FAST,
- SNOR_CMD_READ_1_1_1_DTR,
-
- /* Dual SPI */
- SNOR_CMD_READ_1_1_2,
- SNOR_CMD_READ_1_2_2,
- SNOR_CMD_READ_2_2_2,
- SNOR_CMD_READ_1_2_2_DTR,
-
- /* Quad SPI */
- SNOR_CMD_READ_1_1_4,
- SNOR_CMD_READ_1_4_4,
- SNOR_CMD_READ_4_4_4,
- SNOR_CMD_READ_1_4_4_DTR,
-
- /* Octal SPI */
- SNOR_CMD_READ_1_1_8,
- SNOR_CMD_READ_1_8_8,
- SNOR_CMD_READ_8_8_8,
- SNOR_CMD_READ_1_8_8_DTR,
-
- SNOR_CMD_READ_MAX
-};
-
-enum spi_nor_pp_command_index {
- SNOR_CMD_PP,
-
- /* Quad SPI */
- SNOR_CMD_PP_1_1_4,
- SNOR_CMD_PP_1_4_4,
- SNOR_CMD_PP_4_4_4,
-
- /* Octal SPI */
- SNOR_CMD_PP_1_1_8,
- SNOR_CMD_PP_1_8_8,
- SNOR_CMD_PP_8_8_8,
-
- SNOR_CMD_PP_MAX
-};
-
-struct spi_nor_flash_parameter {
- u64 size;
- u32 page_size;
-
- struct spi_nor_hwcaps hwcaps;
- struct spi_nor_read_command reads[SNOR_CMD_READ_MAX];
- struct spi_nor_pp_command page_programs[SNOR_CMD_PP_MAX];
-};
-
struct sfdp_parameter_header {
u8 id_lsb;
u8 minor;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index af04d3117188..c47f25d9c10f 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -197,6 +197,144 @@ enum spi_nor_protocol {
SNOR_PROTO_1_8_8_DTR = SNOR_PROTO_DTR(1, 8, 8),
};

+/**
+ * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
+ * supported by the SPI controller (bus master).
+ * @mask: the bitmask listing all the supported hw capabilies
+ */
+struct spi_nor_hwcaps {
+ u32 mask;
+};
+
+/*
+ *(Fast) Read capabilities.
+ * MUST be ordered by priority: the higher bit position, the higher priority.
+ * As a matter of performances, it is relevant to use Octal SPI protocols first,
+ * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
+ * (Slow) Read.
+ */
+#define SNOR_HWCAPS_READ_MASK GENMASK(14, 0)
+#define SNOR_HWCAPS_READ BIT(0)
+#define SNOR_HWCAPS_READ_FAST BIT(1)
+#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2)
+
+#define SNOR_HWCAPS_READ_DUAL GENMASK(6, 3)
+#define SNOR_HWCAPS_READ_1_1_2 BIT(3)
+#define SNOR_HWCAPS_READ_1_2_2 BIT(4)
+#define SNOR_HWCAPS_READ_2_2_2 BIT(5)
+#define SNOR_HWCAPS_READ_1_2_2_DTR BIT(6)
+
+#define SNOR_HWCAPS_READ_QUAD GENMASK(10, 7)
+#define SNOR_HWCAPS_READ_1_1_4 BIT(7)
+#define SNOR_HWCAPS_READ_1_4_4 BIT(8)
+#define SNOR_HWCAPS_READ_4_4_4 BIT(9)
+#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)
+
+#define SNOR_HWCAPS_READ_OCTAL GENMASK(14, 11)
+#define SNOR_HWCAPS_READ_1_1_8 BIT(11)
+#define SNOR_HWCAPS_READ_1_8_8 BIT(12)
+#define SNOR_HWCAPS_READ_8_8_8 BIT(13)
+#define SNOR_HWCAPS_READ_1_8_8_DTR BIT(14)
+
+/*
+ * Page Program capabilities.
+ * MUST be ordered by priority: the higher bit position, the higher priority.
+ * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
+ * legacy SPI 1-1-1 protocol.
+ * Note that Dual Page Programs are not supported because there is no existing
+ * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
+ * implements such commands.
+ */
+#define SNOR_HWCAPS_PP_MASK GENMASK(22, 16)
+#define SNOR_HWCAPS_PP BIT(16)
+
+#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
+#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
+#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
+#define SNOR_HWCAPS_PP_4_4_4 BIT(19)
+
+#define SNOR_HWCAPS_PP_OCTAL GENMASK(22, 20)
+#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
+#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
+#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
+
+#define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
+ SNOR_HWCAPS_READ_4_4_4 | \
+ SNOR_HWCAPS_READ_8_8_8 | \
+ SNOR_HWCAPS_PP_4_4_4 | \
+ SNOR_HWCAPS_PP_8_8_8)
+
+#define SNOR_HWCAPS_DTR (SNOR_HWCAPS_READ_1_1_1_DTR | \
+ SNOR_HWCAPS_READ_1_2_2_DTR | \
+ SNOR_HWCAPS_READ_1_4_4_DTR | \
+ SNOR_HWCAPS_READ_1_8_8_DTR)
+
+#define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
+ SNOR_HWCAPS_PP_MASK)
+
+struct spi_nor_read_command {
+ u8 num_mode_clocks;
+ u8 num_wait_states;
+ u8 opcode;
+ enum spi_nor_protocol proto;
+};
+
+struct spi_nor_pp_command {
+ u8 opcode;
+ enum spi_nor_protocol proto;
+};
+
+enum spi_nor_read_command_index {
+ SNOR_CMD_READ,
+ SNOR_CMD_READ_FAST,
+ SNOR_CMD_READ_1_1_1_DTR,
+
+ /* Dual SPI */
+ SNOR_CMD_READ_1_1_2,
+ SNOR_CMD_READ_1_2_2,
+ SNOR_CMD_READ_2_2_2,
+ SNOR_CMD_READ_1_2_2_DTR,
+
+ /* Quad SPI */
+ SNOR_CMD_READ_1_1_4,
+ SNOR_CMD_READ_1_4_4,
+ SNOR_CMD_READ_4_4_4,
+ SNOR_CMD_READ_1_4_4_DTR,
+
+ /* Octal SPI */
+ SNOR_CMD_READ_1_1_8,
+ SNOR_CMD_READ_1_8_8,
+ SNOR_CMD_READ_8_8_8,
+ SNOR_CMD_READ_1_8_8_DTR,
+
+ SNOR_CMD_READ_MAX
+};
+
+enum spi_nor_pp_command_index {
+ SNOR_CMD_PP,
+
+ /* Quad SPI */
+ SNOR_CMD_PP_1_1_4,
+ SNOR_CMD_PP_1_4_4,
+ SNOR_CMD_PP_4_4_4,
+
+ /* Octal SPI */
+ SNOR_CMD_PP_1_1_8,
+ SNOR_CMD_PP_1_8_8,
+ SNOR_CMD_PP_8_8_8,
+
+ SNOR_CMD_PP_MAX
+};
+
+struct spi_nor_flash_parameter {
+ u64 size;
+ u32 page_size;
+
+ struct spi_nor_hwcaps hwcaps;
+ struct spi_nor_read_command reads[SNOR_CMD_READ_MAX];
+ struct spi_nor_pp_command page_programs[SNOR_CMD_PP_MAX];
+};
+
static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
{
return !!(proto & SNOR_PROTO_IS_DTR);
@@ -384,6 +522,7 @@ struct flash_info;
* useful when pagesize is not a power-of-2
* @disable_write_protection: [FLASH-SPECIFIC] disable write protection during
* power-up
+ * @setup: [FLASH-SPECIFIC] configure the spi-nor memory
* completely locked
* @priv: the private data
*/
@@ -427,6 +566,9 @@ struct spi_nor {
int (*set_4byte)(struct spi_nor *nor, bool enable);
u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
int (*disable_write_protection)(struct spi_nor *nor);
+ int (*setup)(struct spi_nor *nor,
+ const struct spi_nor_flash_parameter *params,
+ const struct spi_nor_hwcaps *hwcaps);

const struct spi_nor_locking_ops *locking_ops;

@@ -486,81 +628,6 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
}

/**
- * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
- * supported by the SPI controller (bus master).
- * @mask: the bitmask listing all the supported hw capabilies
- */
-struct spi_nor_hwcaps {
- u32 mask;
-};
-
-/*
- *(Fast) Read capabilities.
- * MUST be ordered by priority: the higher bit position, the higher priority.
- * As a matter of performances, it is relevant to use Octal SPI protocols first,
- * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
- * (Slow) Read.
- */
-#define SNOR_HWCAPS_READ_MASK GENMASK(14, 0)
-#define SNOR_HWCAPS_READ BIT(0)
-#define SNOR_HWCAPS_READ_FAST BIT(1)
-#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2)
-
-#define SNOR_HWCAPS_READ_DUAL GENMASK(6, 3)
-#define SNOR_HWCAPS_READ_1_1_2 BIT(3)
-#define SNOR_HWCAPS_READ_1_2_2 BIT(4)
-#define SNOR_HWCAPS_READ_2_2_2 BIT(5)
-#define SNOR_HWCAPS_READ_1_2_2_DTR BIT(6)
-
-#define SNOR_HWCAPS_READ_QUAD GENMASK(10, 7)
-#define SNOR_HWCAPS_READ_1_1_4 BIT(7)
-#define SNOR_HWCAPS_READ_1_4_4 BIT(8)
-#define SNOR_HWCAPS_READ_4_4_4 BIT(9)
-#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)
-
-#define SNOR_HWCAPS_READ_OCTAL GENMASK(14, 11)
-#define SNOR_HWCAPS_READ_1_1_8 BIT(11)
-#define SNOR_HWCAPS_READ_1_8_8 BIT(12)
-#define SNOR_HWCAPS_READ_8_8_8 BIT(13)
-#define SNOR_HWCAPS_READ_1_8_8_DTR BIT(14)
-
-/*
- * Page Program capabilities.
- * MUST be ordered by priority: the higher bit position, the higher priority.
- * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
- * legacy SPI 1-1-1 protocol.
- * Note that Dual Page Programs are not supported because there is no existing
- * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
- * implements such commands.
- */
-#define SNOR_HWCAPS_PP_MASK GENMASK(22, 16)
-#define SNOR_HWCAPS_PP BIT(16)
-
-#define SNOR_HWCAPS_PP_QUAD GENMASK(19, 17)
-#define SNOR_HWCAPS_PP_1_1_4 BIT(17)
-#define SNOR_HWCAPS_PP_1_4_4 BIT(18)
-#define SNOR_HWCAPS_PP_4_4_4 BIT(19)
-
-#define SNOR_HWCAPS_PP_OCTAL GENMASK(22, 20)
-#define SNOR_HWCAPS_PP_1_1_8 BIT(20)
-#define SNOR_HWCAPS_PP_1_8_8 BIT(21)
-#define SNOR_HWCAPS_PP_8_8_8 BIT(22)
-
-#define SNOR_HWCAPS_X_X_X (SNOR_HWCAPS_READ_2_2_2 | \
- SNOR_HWCAPS_READ_4_4_4 | \
- SNOR_HWCAPS_READ_8_8_8 | \
- SNOR_HWCAPS_PP_4_4_4 | \
- SNOR_HWCAPS_PP_8_8_8)
-
-#define SNOR_HWCAPS_DTR (SNOR_HWCAPS_READ_1_1_1_DTR | \
- SNOR_HWCAPS_READ_1_2_2_DTR | \
- SNOR_HWCAPS_READ_1_4_4_DTR | \
- SNOR_HWCAPS_READ_1_8_8_DTR)
-
-#define SNOR_HWCAPS_ALL (SNOR_HWCAPS_READ_MASK | \
- SNOR_HWCAPS_PP_MASK)
-
-/**
* spi_nor_scan() - scan the SPI NOR
* @nor: the spi_nor structure
* @name: the chip type name
--
2.9.5

2019-07-31 09:14:22

by Tudor Ambarus

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

From: Boris Brezillon <[email protected]>

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

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

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

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

/* Kept only for backward compatibility purpose. */
--
2.9.5

2019-07-31 10:06:29

by Tudor Ambarus

[permalink] [raw]
Subject: [PATCH 2/6] mtd: spi-nor: Add spansion_post_sfdp_fixups()

From: Boris Brezillon <[email protected]>

Add a spansion_post_sfdp_fixups() function to fix the erase opcode,
erase sector size and set the SNOR_F_4B_OPCODES flag.
This way, all spansion related quirks are placed in the
spansion_post_sfdp_fixups() function.

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

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 2c2d13060427..30f9d524e72f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -737,18 +737,6 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)

static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
{
- /* Do some manufacturer fixups first */
- switch (JEDEC_MFR(nor->info)) {
- case SNOR_MFR_SPANSION:
- /* No small sector erase for 4-byte command set */
- nor->erase_opcode = SPINOR_OP_SE;
- nor->mtd.erasesize = nor->info->sector_size;
- break;
-
- default:
- break;
- }
-
nor->read_opcode = spi_nor_convert_3to4_read(nor->read_opcode);
nor->program_opcode = spi_nor_convert_3to4_program(nor->program_opcode);
nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
@@ -4451,9 +4439,38 @@ static void spi_nor_default_init_params(struct spi_nor *nor,
spi_nor_init_uniform_erase_map(map, erase_mask, params->size);
}

+static void spansion_post_sfdp_fixups(struct spi_nor *nor)
+{
+ struct mtd_info *mtd = &nor->mtd;
+
+ if (mtd->size <= SZ_16M)
+ return;
+
+ nor->flags |= SNOR_F_4B_OPCODES;
+ /* No small sector erase for 4-byte command set */
+ nor->erase_opcode = SPINOR_OP_SE;
+ nor->mtd.erasesize = nor->info->sector_size;
+}
+
+static void
+spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
+ struct spi_nor_flash_parameter *params)
+{
+ switch (JEDEC_MFR(nor->info)) {
+ case SNOR_MFR_SPANSION:
+ spansion_post_sfdp_fixups(nor);
+ break;
+
+ default:
+ break;
+ }
+}
+
static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
struct spi_nor_flash_parameter *params)
{
+ spi_nor_manufacturer_post_sfdp_fixups(nor, params);
+
if (nor->info->fixups && nor->info->fixups->post_sfdp)
return nor->info->fixups->post_sfdp(nor, params);
}
@@ -4934,8 +4951,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
nor->addr_width = 3;
}

- if (info->flags & SPI_NOR_4B_OPCODES ||
- (JEDEC_MFR(info) == SNOR_MFR_SPANSION && mtd->size > SZ_16M))
+ if (info->flags & SPI_NOR_4B_OPCODES)
nor->flags |= SNOR_F_4B_OPCODES;

if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
--
2.9.5

2019-07-31 12:47:26

by Boris Brezillon

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

On Wed, 31 Jul 2019 12:31:19 +0000
Naga Sureshkumar Relli <[email protected]> wrote:

> Hi Tudor,
>
> Thanks for the updates. With these kind of updates, we can add Vendor specific
> Code easily, like Xilinx Dual parallel and stacked modes.
> In these configurations we need to tweak the nor parameters like page_size, sectors etc.
> So with the help of these patches. we can easily update these parameters.

Absolutely not. This is just a solution to recover from broken SFDP
tables.

2019-07-31 13:10:06

by Tudor Ambarus

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

Hi, Naga,

On 07/31/2019 03:31 PM, Naga Sureshkumar Relli wrote:
>> + if (nor->info->flags & SPI_S3AN)
>> + s3an_post_sfdp_fixups(nor);
>> }
>>
> Instead of checking the flags, why can't we call directly the nor_fixups?
> like Boris implementation nor->info->fixups->post_sfdp()
> https://patchwork.ozlabs.org/patch/1009291/

This check will vanish and nor->info->fixups->post_sfdp() will be called
directly once I'll respin the manufacturer driver part. post_sfdp() will set
just flash parameters. Check Boris' patch at
https://patchwork.ozlabs.org/patch/1009295/

I'll try to respin the rest of Boris' patches sometime at the beginning of the
next week.

Cheers,
ta

2019-07-31 14:23:17

by Naga Sureshkumar Relli

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

Hi Tudor,

Thanks for the updates. With these kind of updates, we can add Vendor specific
Code easily, like Xilinx Dual parallel and stacked modes.
In these configurations we need to tweak the nor parameters like page_size, sectors etc.
So with the help of these patches. we can easily update these parameters.

> -----Original Message-----
> From: linux-mtd <[email protected]> On Behalf Of
> [email protected]
> Sent: Wednesday, July 31, 2019 2:42 PM
> To: [email protected]; [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]
> Subject: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
>
> From: Tudor Ambarus <[email protected]>
>
> s3an_nor_scan() was overriding the opcode selection done in spi_nor_default_setup(). Set nor-
> >setup() method in order to avoid unnecessary call to spi_nor_default_setup().
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
> 0ff474e5e4f5..5fea5d7ce2cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2795,7 +2795,9 @@ 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_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> int ret;
> u8 val;
> @@ -4393,6 +4395,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->setup = s3an_nor_setup;
> +}
> +
> static void
> spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
> struct spi_nor_flash_parameter *params) @@ -4405,6
> +4412,9 @@ spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
> default:
> break;
> }
> +
> + if (nor->info->flags & SPI_S3AN)
> + s3an_post_sfdp_fixups(nor);
> }
>
Instead of checking the flags, why can't we call directly the nor_fixups?
like Boris implementation nor->info->fixups->post_sfdp()
https://patchwork.ozlabs.org/patch/1009291/

Thanks,
Naga Sureshkumar Relli

> static void spi_nor_post_sfdp_fixups(struct spi_nor *nor, @@ -4582,9 +4592,9 @@ static
> int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
> return 0;
> }
>
> -static int spi_nor_setup(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> - const struct spi_nor_hwcaps *hwcaps)
> +static int spi_nor_default_setup(struct spi_nor *nor,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> u32 ignored_mask, shared_mask;
> int err;
> @@ -4641,6 +4651,16 @@ static int spi_nor_setup(struct spi_nor *nor,
> return err;
> }
>
> +static int spi_nor_setup(struct spi_nor *nor,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps) {
> + if (!nor->setup)
> + return 0;
> +
> + return nor->setup(nor, params, hwcaps); }
> +
> static int spi_nor_disable_write_protection(struct spi_nor *nor) {
> if (!nor->disable_write_protection)
> @@ -4804,6 +4824,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> /* Kept only for backward compatibility purpose. */
> nor->quad_enable = spansion_quad_enable;
> nor->set_4byte = spansion_set_4byte;
> + nor->setup = spi_nor_default_setup;
>
> /* Default locking operations. */
> if (info->flags & SPI_NOR_HAS_LOCK) {
> @@ -4905,12 +4926,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
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

2019-08-01 05:15:34

by Naga Sureshkumar Relli

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



> -----Original Message-----
> From: Boris Brezillon <[email protected]>
> Sent: Wednesday, July 31, 2019 6:08 PM
> To: Naga Sureshkumar Relli <[email protected]>
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
>
> On Wed, 31 Jul 2019 12:31:19 +0000
> Naga Sureshkumar Relli <[email protected]> wrote:
>
> > Hi Tudor,
> >
> > Thanks for the updates. With these kind of updates, we can add Vendor
> > specific Code easily, like Xilinx Dual parallel and stacked modes.
> > In these configurations we need to tweak the nor parameters like page_size, sectors etc.
> > So with the help of these patches. we can easily update these parameters.
>
> Absolutely not. This is just a solution to recover from broken SFDP tables.
Ok.

Thanks,
Naga Sureshkumar Relli

2019-08-01 05:17:48

by Naga Sureshkumar Relli

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

Hi Tudor,

> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Wednesday, July 31, 2019 6:37 PM
> To: Naga Sureshkumar Relli <[email protected]>; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
>
> Hi, Naga,
>
> On 07/31/2019 03:31 PM, Naga Sureshkumar Relli wrote:
> >> + if (nor->info->flags & SPI_S3AN)
> >> + s3an_post_sfdp_fixups(nor);
> >> }
> >>
> > Instead of checking the flags, why can't we call directly the nor_fixups?
> > like Boris implementation nor->info->fixups->post_sfdp()
> > https://patchwork.ozlabs.org/patch/1009291/
>
> This check will vanish and nor->info->fixups->post_sfdp() will be called directly once I'll
> respin the manufacturer driver part. post_sfdp() will set just flash parameters. Check Boris'
> patch at https://patchwork.ozlabs.org/patch/1009295/
>
> I'll try to respin the rest of Boris' patches sometime at the beginning of the next week.
Ok, Thanks.

Regards,
Naga Sureshkumar Relli
>
> Cheers,
> ta

2019-08-01 06:39:01

by Boris Brezillon

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

On Wed, 31 Jul 2019 09:12:14 +0000
<[email protected]> wrote:


> static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
> {
> return !!(proto & SNOR_PROTO_IS_DTR);
> @@ -384,6 +522,7 @@ struct flash_info;
> * useful when pagesize is not a power-of-2
> * @disable_write_protection: [FLASH-SPECIFIC] disable write protection during
> * power-up
> + * @setup: [FLASH-SPECIFIC] configure the spi-nor memory

Might be worth giving a example of the type of configuration that can
be done here.

The patch looks good otherwise.

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

> * completely locked

Looks like this 'completely locked' is a leftover from a previous move
(lock functions were move to a separate _ops struct IIRC). Can you fix
that?

> * @priv: the private data
> */
> @@ -427,6 +566,9 @@ struct spi_nor {
> int (*set_4byte)(struct spi_nor *nor, bool enable);
> u32 (*convert_addr)(struct spi_nor *nor, u32 addr);
> int (*disable_write_protection)(struct spi_nor *nor);
> + int (*setup)(struct spi_nor *nor,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps);
>
> const struct spi_nor_locking_ops *locking_ops;
>
> @@ -486,81 +628,6 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
> }
>

2019-08-01 07:30:10

by Boris Brezillon

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

On Wed, 31 Jul 2019 09:12:16 +0000
<[email protected]> wrote:

> From: Tudor Ambarus <[email protected]>
>
> s3an_nor_scan() was overriding the opcode selection done in
> spi_nor_default_setup(). Set nor->setup() method in order to
> avoid unnecessary call to spi_nor_default_setup().
>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++----------
> 1 file changed, 25 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0ff474e5e4f5..5fea5d7ce2cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2795,7 +2795,9 @@ 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_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> int ret;
> u8 val;
> @@ -4393,6 +4395,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->setup = s3an_nor_setup;
> +}
> +
> static void
> spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
> struct spi_nor_flash_parameter *params)
> @@ -4405,6 +4412,9 @@ spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
> default:
> break;
> }
> +
> + if (nor->info->flags & SPI_S3AN)
> + s3an_post_sfdp_fixups(nor);
> }
>
> static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
> @@ -4582,9 +4592,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
> return 0;
> }
>
> -static int spi_nor_setup(struct spi_nor *nor,
> - const struct spi_nor_flash_parameter *params,
> - const struct spi_nor_hwcaps *hwcaps)
> +static int spi_nor_default_setup(struct spi_nor *nor,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> {
> u32 ignored_mask, shared_mask;
> int err;
> @@ -4641,6 +4651,16 @@ static int spi_nor_setup(struct spi_nor *nor,
> return err;
> }
>
> +static int spi_nor_setup(struct spi_nor *nor,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> + if (!nor->setup)
> + return 0;
> +
> + return nor->setup(nor, params, hwcaps);
> +}
> +
> static int spi_nor_disable_write_protection(struct spi_nor *nor)
> {
> if (!nor->disable_write_protection)
> @@ -4804,6 +4824,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> /* Kept only for backward compatibility purpose. */
> nor->quad_enable = spansion_quad_enable;
> nor->set_4byte = spansion_set_4byte;
> + nor->setup = spi_nor_default_setup;
>
> /* Default locking operations. */
> if (info->flags & SPI_NOR_HAS_LOCK) {
> @@ -4905,12 +4926,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)

Almost all of this (except the s3an specific bits) should be done in
the previous patch. So I'll put a condition on the R-b I placed on patch
4: some of this code should be moved there.

2019-08-05 05:14:48

by Vignesh Raghavendra

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



On 31/07/19 2:42 PM, [email protected] wrote:
> From: Boris Brezillon <[email protected]>
>
> S3AN flashes use a specific opcode to read the status register.
> We currently use the SPI_S3AN flag to decide whether this specific
> SR read opcode should be used, but SPI_S3AN is about to disappear, so
> let's add a new flag.
>

I think you can drop SPI_S3AN right away either as separate patch in
this series or as part of this patch itself.

Regards
Vignesh

> Note that we use the same bit as SPI_S3AN implies SPI_NOR_XSR_RDY and
> vice versa.
>
> Signed-off-by: Boris Brezillon <[email protected]>
> Signed-off-by: Tudor Ambarus <[email protected]>
> ---
> drivers/mtd/spi-nor/spi-nor.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 5fea5d7ce2cb..01be6d49ce3b 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -213,6 +213,14 @@ struct flash_info {
> * bit. Must be used with
> * SPI_NOR_HAS_LOCK.
> */
> +#define SPI_NOR_XSR_RDY BIT(10) /*
> + * S3AN flashes have specific opcode to
> + * read the status register.
> + * Flags SPI_NOR_XSR_RDY and SPI_S3AN
> + * use the same bit as one implies the
> + * other, but we will get rid of
> + * SPI_S3AN soon.
> + */
> #define SPI_S3AN BIT(10) /*
> * Xilinx Spartan 3AN In-System Flash
> * (MFR cannot be used for probing
> @@ -4818,7 +4826,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> * spi_nor_wait_till_ready(). Xilinx S3AN share MFR
> * with Atmel spi-nor
> */
> - if (info->flags & SPI_S3AN)
> + if (info->flags & SPI_NOR_XSR_RDY)
> nor->flags |= SNOR_F_READY_XSR_RDY;
>
> /* Kept only for backward compatibility purpose. */
>

--
Regards
Vignesh

2019-08-05 06:38:38

by Tudor Ambarus

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



On 08/01/2019 09:36 AM, Boris Brezillon wrote:
> On Wed, 31 Jul 2019 09:12:14 +0000
> <[email protected]> wrote:
>
>
>> static inline bool spi_nor_protocol_is_dtr(enum spi_nor_protocol proto)
>> {
>> return !!(proto & SNOR_PROTO_IS_DTR);
>> @@ -384,6 +522,7 @@ struct flash_info;
>> * useful when pagesize is not a power-of-2
>> * @disable_write_protection: [FLASH-SPECIFIC] disable write protection during
>> * power-up
>> + * @setup: [FLASH-SPECIFIC] configure the spi-nor memory
>
> Might be worth giving a example of the type of configuration that can
> be done here.

will do

>
> The patch looks good otherwise.
>
> Reviewed-by: Boris Brezillon <[email protected]>
>
>> * completely locked
>
> Looks like this 'completely locked' is a leftover from a previous move
> (lock functions were move to a separate _ops struct IIRC). Can you fix
> that?

there's already a patch on ML for this, I'll apply it.

Thanks,
ta

2019-08-05 06:41:54

by Tudor Ambarus

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



On 08/01/2019 09:42 AM, Boris Brezillon wrote:
> Almost all of this (except the s3an specific bits) should be done in
> the previous patch. So I'll put a condition on the R-b I placed on patch
> 4: some of this code should be moved there.

You're right, will do.

Cheers,
ta

2019-08-05 07:33:46

by Tudor Ambarus

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



On 08/05/2019 08:14 AM, Vignesh Raghavendra wrote:
> External E-Mail
>
>
>
> On 31/07/19 2:42 PM, [email protected] wrote:
>> From: Boris Brezillon <[email protected]>
>>
>> S3AN flashes use a specific opcode to read the status register.
>> We currently use the SPI_S3AN flag to decide whether this specific
>> SR read opcode should be used, but SPI_S3AN is about to disappear, so
>> let's add a new flag.
>>
>
> I think you can drop SPI_S3AN right away either as separate patch in
> this series or as part of this patch itself.
>

SPI_NOR_XSR_RDY is more generic than SPI_S3AN, and lets other flashes use
SPINOR_OP_XRDSR SR read opcode if needed.

If I drop SPI_S3AN now, I'll have to select the s3an_nor_setup() method based on
SPI_NOR_XSR_RDY/SNOR_F_READY_XSR_RDY which might not be correct. There might be
flashes that use SPINOR_OP_XRDSR but have a different setup call.

Of course there are a lot of "might" here (because I couldn't find some other
NORs that use this opcode), and if you have a strong opinion I can change as you
suggested. I prefer to drop SPI_S3AN when moving the xillinx bits out of the
core, as in https://patchwork.ozlabs.org/patch/1009295/.

Cheers,
ta