[PATCH v3 0/5] mtd: spi-nor: Random cleanups

[PATCH v3 0/5] mtd: spi-nor: Random cleanups

[Boris Brezillon]

Hi,

The patch series is a collection of minor cleanups I have accumulated
while working on the "SPI NOR SFDP fixups" infrastructure.

Regards,

Boris

Changes in v3:
- Merged patch 6 into patch 3
- Add an SPDX tag to spi-nor.h

Changes in v2:
- Drop
  "mtd: spi-nor: Set SPI_NOR_4B_OPCODES on all >16MB Spansion NORs",
  "mtd: spi-nor: Create a ->set_4byte() method",
  "mtd: spi-nor: Explicitly flag all Micron NORs as supporting locking"
  "mtd: spi-nor: Move Spansion erase related fixup to a dedicated function"
- Add a patch adding the SPDX tag
- Add a patch moving nor->info assignment earlier in the spi_nor_scan()
  function


*** BLURB HERE ***

Boris Brezillon (5):
  mtd: spi-nor: Drop inline on all internal helpers
  mtd: spi-nor: Avoid forward declaration of internal functions
  mtd: spi-nor: Stop passing flash_info around
  mtd: spi-nor: Make the enable argument passed to set_byte() a bool
  mtd: spi-nor: Add an SPDX tag to spi-nor.{c,h}

 drivers/mtd/spi-nor/spi-nor.c | 679 +++++++++++++++++-----------------
 include/linux/mtd/spi-nor.h   |   6 +-
 2 files changed, 336 insertions(+), 349 deletions(-)

-- 
2.17.1

[PATCH v3 1/5] mtd: spi-nor: Drop inline on all internal helpers

[Boris Brezillon]

gcc should be smart enough to decide when inlining a function makes
sense. Drop all inline specifiers.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
Changes in v3:
- Add Tudor's R-b

Changes in v2:
- None
---
 drivers/mtd/spi-nor/spi-nor.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index fdff18cb1196..d099aee075e9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -352,7 +352,7 @@ static int read_cr(struct spi_nor *nor)
  * Write status register 1 byte
  * Returns negative if error occurred.
  */
-static inline int write_sr(struct spi_nor *nor, u8 val)
+static int write_sr(struct spi_nor *nor, u8 val)
 {
 	nor->cmd_buf[0] = val;
 	return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
@@ -362,7 +362,7 @@ static inline int write_sr(struct spi_nor *nor, u8 val)
  * Set write enable latch with Write Enable command.
  * Returns negative if error occurred.
  */
-static inline int write_enable(struct spi_nor *nor)
+static int write_enable(struct spi_nor *nor)
 {
 	return nor->write_reg(nor, SPINOR_OP_WREN, NULL, 0);
 }
@@ -370,12 +370,12 @@ static inline int write_enable(struct spi_nor *nor)
 /*
  * Send write disable instruction to the chip.
  */
-static inline int write_disable(struct spi_nor *nor)
+static int write_disable(struct spi_nor *nor)
 {
 	return nor->write_reg(nor, SPINOR_OP_WRDI, NULL, 0);
 }
 
-static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
+static struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 {
 	return mtd->priv;
 }
@@ -393,7 +393,7 @@ static u8 spi_nor_convert_opcode(u8 opcode, const u8 table[][2], size_t size)
 	return opcode;
 }
 
-static inline u8 spi_nor_convert_3to4_read(u8 opcode)
+static u8 spi_nor_convert_3to4_read(u8 opcode)
 {
 	static const u8 spi_nor_3to4_read[][2] = {
 		{ SPINOR_OP_READ,	SPINOR_OP_READ_4B },
@@ -412,7 +412,7 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
 				      ARRAY_SIZE(spi_nor_3to4_read));
 }
 
-static inline u8 spi_nor_convert_3to4_program(u8 opcode)
+static u8 spi_nor_convert_3to4_program(u8 opcode)
 {
 	static const u8 spi_nor_3to4_program[][2] = {
 		{ SPINOR_OP_PP,		SPINOR_OP_PP_4B },
@@ -424,7 +424,7 @@ static inline u8 spi_nor_convert_3to4_program(u8 opcode)
 				      ARRAY_SIZE(spi_nor_3to4_program));
 }
 
-static inline u8 spi_nor_convert_3to4_erase(u8 opcode)
+static u8 spi_nor_convert_3to4_erase(u8 opcode)
 {
 	static const u8 spi_nor_3to4_erase[][2] = {
 		{ SPINOR_OP_BE_4K,	SPINOR_OP_BE_4K_4B },
@@ -469,8 +469,8 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
 }
 
 /* Enable/disable 4-byte addressing mode. */
-static inline int set_4byte(struct spi_nor *nor, const struct flash_info *info,
-			    int enable)
+static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
+		     int enable)
 {
 	int status;
 	bool need_wren = false;
@@ -528,7 +528,7 @@ static int s3an_sr_ready(struct spi_nor *nor)
 	return !!(val & XSR_RDY);
 }
 
-static inline int spi_nor_sr_ready(struct spi_nor *nor)
+static int spi_nor_sr_ready(struct spi_nor *nor)
 {
 	int sr = read_sr(nor);
 	if (sr < 0)
@@ -547,7 +547,7 @@ static inline int spi_nor_sr_ready(struct spi_nor *nor)
 	return !(sr & SR_WIP);
 }
 
-static inline int spi_nor_fsr_ready(struct spi_nor *nor)
+static int spi_nor_fsr_ready(struct spi_nor *nor)
 {
 	int fsr = read_fsr(nor);
 	if (fsr < 0)
@@ -2420,7 +2420,7 @@ static int spi_nor_read_sfdp_dma_unsafe(struct spi_nor *nor, u32 addr,
 
 /* Fast Read settings. */
 
-static inline void
+static void
 spi_nor_set_read_settings_from_bfpt(struct spi_nor_read_command *read,
 				    u16 half,
 				    enum spi_nor_protocol proto)
-- 
2.17.1

Re: [v3,1/5] mtd: spi-nor: Drop inline on all internal helpers

[Boris Brezillon]

On Thu, 2018-12-06 at 10:41:16 UTC, Boris Brezillon wrote:
> gcc should be smart enough to decide when inlining a function makes
> sense. Drop all inline specifiers.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next.

Boris

[PATCH v3 2/5] mtd: spi-nor: Avoid forward declaration of internal functions

[Boris Brezillon]

Reorganize the code to kill forward declarations of spi_nor_match_id()
macronix_quad_enable() and spi_nor_hwcaps_read2cmd().

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- Group all _quad_enable() implems
- Move spi_nor_hwcaps_xx2cmd() just after spi_nor_set_pp_settings()

Changes in v2:
- None
---
 drivers/mtd/spi-nor/spi-nor.c | 614 +++++++++++++++++-----------------
 1 file changed, 304 insertions(+), 310 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d099aee075e9..b440f29637ab 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -289,8 +289,6 @@ struct flash_info {
 
 #define JEDEC_MFR(info)	((info)->id[0])
 
-static const struct flash_info *spi_nor_match_id(const char *name);
-
 /*
  * Read the status register, returning its value in the location
  * Return the status register value.
@@ -1395,7 +1393,247 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
 	return ret;
 }
 
-static int macronix_quad_enable(struct spi_nor *nor);
+/*
+ * Write status Register and configuration register with 2 bytes
+ * The first byte will be written to the status register, while the
+ * second byte will be written to the configuration register.
+ * Return negative if error occurred.
+ */
+static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
+{
+	int ret;
+
+	write_enable(nor);
+
+	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
+	if (ret < 0) {
+		dev_err(nor->dev,
+			"error while writing configuration register\n");
+		return -EINVAL;
+	}
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret) {
+		dev_err(nor->dev,
+			"timeout while writing configuration register\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * macronix_quad_enable() - set QE bit in Status Register.
+ * @nor:	pointer to a 'struct spi_nor'
+ *
+ * Set the Quad Enable (QE) bit in the Status Register.
+ *
+ * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int macronix_quad_enable(struct spi_nor *nor)
+{
+	int ret, val;
+
+	val = read_sr(nor);
+	if (val < 0)
+		return val;
+	if (val & SR_QUAD_EN_MX)
+		return 0;
+
+	write_enable(nor);
+
+	write_sr(nor, val | SR_QUAD_EN_MX);
+
+	ret = spi_nor_wait_till_ready(nor);
+	if (ret)
+		return ret;
+
+	ret = read_sr(nor);
+	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
+		dev_err(nor->dev, "Macronix Quad bit not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * 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 read_cr() returns 0xFF.
+ *
+ * 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[2] = {0, CR_QUAD_EN_SPAN};
+	int ret;
+
+	ret = write_sr_cr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	/* read back and check it */
+	ret = read_cr(nor);
+	if (!(ret > 0 && (ret & 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'
+ *
+ * Set the Quad Enable (QE) bit in the Configuration Register.
+ * This function should be used with QSPI memories not supporting the Read
+ * Configuration Register (35h) instruction.
+ *
+ * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
+ * memories.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
+{
+	u8 sr_cr[2];
+	int ret;
+
+	/* Keep the current value of the Status Register. */
+	ret = read_sr(nor);
+	if (ret < 0) {
+		dev_err(nor->dev, "error while reading status register\n");
+		return -EINVAL;
+	}
+	sr_cr[0] = ret;
+	sr_cr[1] = CR_QUAD_EN_SPAN;
+
+	return write_sr_cr(nor, sr_cr);
+}
+
+/**
+ * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
+ * @nor:	pointer to a 'struct spi_nor'
+ *
+ * Set the Quad Enable (QE) bit in the Configuration Register.
+ * This function should be used with QSPI memories supporting the Read
+ * Configuration Register (35h) instruction.
+ *
+ * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
+ * memories.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spansion_read_cr_quad_enable(struct spi_nor *nor)
+{
+	struct device *dev = nor->dev;
+	u8 sr_cr[2];
+	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;
+	}
+
+	if (ret & CR_QUAD_EN_SPAN)
+		return 0;
+
+	sr_cr[1] = ret | CR_QUAD_EN_SPAN;
+
+	/* Keep the current value of the Status Register. */
+	ret = read_sr(nor);
+	if (ret < 0) {
+		dev_err(dev, "error while reading status register\n");
+		return -EINVAL;
+	}
+	sr_cr[0] = ret;
+
+	ret = write_sr_cr(nor, sr_cr);
+	if (ret)
+		return ret;
+
+	/* Read back and check it. */
+	ret = read_cr(nor);
+	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
+		dev_err(nor->dev, "Spansion Quad bit not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * 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.
+ *
+ * This is one of the procedures to set the QE bit described in the SFDP
+ * (JESD216 rev B) specification but no manufacturer using this procedure has
+ * been identified yet, hence the name of the function.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int sr2_bit7_quad_enable(struct spi_nor *nor)
+{
+	u8 sr2;
+	int ret;
+
+	/* Check current Quad Enable bit value. */
+	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
+	if (ret)
+		return ret;
+	if (sr2 & SR2_QUAD_EN_BIT7)
+		return 0;
+
+	/* Update the Quad Enable bit. */
+	sr2 |= SR2_QUAD_EN_BIT7;
+
+	write_enable(nor);
+
+	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
+	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");
+		return ret;
+	}
+
+	/* Read back and check it. */
+	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
+	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
+		dev_err(nor->dev, "SR2 Quad bit not set\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
 
 /* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -2002,248 +2240,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
 	return ret;
 }
 
-/**
- * macronix_quad_enable() - set QE bit in Status Register.
- * @nor:	pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Status Register.
- *
- * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int macronix_quad_enable(struct spi_nor *nor)
-{
-	int ret, val;
-
-	val = read_sr(nor);
-	if (val < 0)
-		return val;
-	if (val & SR_QUAD_EN_MX)
-		return 0;
-
-	write_enable(nor);
-
-	write_sr(nor, val | SR_QUAD_EN_MX);
-
-	ret = spi_nor_wait_till_ready(nor);
-	if (ret)
-		return ret;
-
-	ret = read_sr(nor);
-	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
-		dev_err(nor->dev, "Macronix Quad bit not set\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-/*
- * Write status Register and configuration register with 2 bytes
- * The first byte will be written to the status register, while the
- * second byte will be written to the configuration register.
- * Return negative if error occurred.
- */
-static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
-{
-	int ret;
-
-	write_enable(nor);
-
-	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
-	if (ret < 0) {
-		dev_err(nor->dev,
-			"error while writing configuration register\n");
-		return -EINVAL;
-	}
-
-	ret = spi_nor_wait_till_ready(nor);
-	if (ret) {
-		dev_err(nor->dev,
-			"timeout while writing configuration register\n");
-		return ret;
-	}
-
-	return 0;
-}
-
-/**
- * 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 read_cr() returns 0xFF.
- *
- * 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[2] = {0, CR_QUAD_EN_SPAN};
-	int ret;
-
-	ret = write_sr_cr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	/* read back and check it */
-	ret = read_cr(nor);
-	if (!(ret > 0 && (ret & 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'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories not supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
-{
-	u8 sr_cr[2];
-	int ret;
-
-	/* Keep the current value of the Status Register. */
-	ret = read_sr(nor);
-	if (ret < 0) {
-		dev_err(nor->dev, "error while reading status register\n");
-		return -EINVAL;
-	}
-	sr_cr[0] = ret;
-	sr_cr[1] = CR_QUAD_EN_SPAN;
-
-	return write_sr_cr(nor, sr_cr);
-}
-
-/**
- * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
- * @nor:	pointer to a 'struct spi_nor'
- *
- * Set the Quad Enable (QE) bit in the Configuration Register.
- * This function should be used with QSPI memories supporting the Read
- * Configuration Register (35h) instruction.
- *
- * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
- * memories.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int spansion_read_cr_quad_enable(struct spi_nor *nor)
-{
-	struct device *dev = nor->dev;
-	u8 sr_cr[2];
-	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;
-	}
-
-	if (ret & CR_QUAD_EN_SPAN)
-		return 0;
-
-	sr_cr[1] = ret | CR_QUAD_EN_SPAN;
-
-	/* Keep the current value of the Status Register. */
-	ret = read_sr(nor);
-	if (ret < 0) {
-		dev_err(dev, "error while reading status register\n");
-		return -EINVAL;
-	}
-	sr_cr[0] = ret;
-
-	ret = write_sr_cr(nor, sr_cr);
-	if (ret)
-		return ret;
-
-	/* Read back and check it. */
-	ret = read_cr(nor);
-	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
-		dev_err(nor->dev, "Spansion Quad bit not set\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-/**
- * 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.
- *
- * This is one of the procedures to set the QE bit described in the SFDP
- * (JESD216 rev B) specification but no manufacturer using this procedure has
- * been identified yet, hence the name of the function.
- *
- * Return: 0 on success, -errno otherwise.
- */
-static int sr2_bit7_quad_enable(struct spi_nor *nor)
-{
-	u8 sr2;
-	int ret;
-
-	/* Check current Quad Enable bit value. */
-	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
-	if (ret)
-		return ret;
-	if (sr2 & SR2_QUAD_EN_BIT7)
-		return 0;
-
-	/* Update the Quad Enable bit. */
-	sr2 |= SR2_QUAD_EN_BIT7;
-
-	write_enable(nor);
-
-	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
-	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");
-		return ret;
-	}
-
-	/* Read back and check it. */
-	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
-	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
-		dev_err(nor->dev, "SR2 Quad bit not set\n");
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int spi_nor_check(struct spi_nor *nor)
 {
 	if (!nor->dev || !nor->read || !nor->write ||
@@ -2318,6 +2314,57 @@ spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
 	pp->proto = proto;
 }
 
+static int spi_nor_hwcaps2cmd(u32 hwcaps, const int table[][2], size_t size)
+{
+	size_t i;
+
+	for (i = 0; i < size; i++)
+		if (table[i][0] == (int)hwcaps)
+			return table[i][1];
+
+	return -EINVAL;
+}
+
+static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
+{
+	static const int hwcaps_read2cmd[][2] = {
+		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
+		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
+		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
+		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
+		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
+		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
+		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
+		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
+		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
+		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
+		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
+		{ SNOR_HWCAPS_READ_1_1_8,	SNOR_CMD_READ_1_1_8 },
+		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
+		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
+		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
+	};
+
+	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
+				  ARRAY_SIZE(hwcaps_read2cmd));
+}
+
+static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
+{
+	static const int hwcaps_pp2cmd[][2] = {
+		{ SNOR_HWCAPS_PP,		SNOR_CMD_PP },
+		{ SNOR_HWCAPS_PP_1_1_4,		SNOR_CMD_PP_1_1_4 },
+		{ SNOR_HWCAPS_PP_1_4_4,		SNOR_CMD_PP_1_4_4 },
+		{ SNOR_HWCAPS_PP_4_4_4,		SNOR_CMD_PP_4_4_4 },
+		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
+		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
+		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
+	};
+
+	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
+				  ARRAY_SIZE(hwcaps_pp2cmd));
+}
+
 /*
  * Serial Flash Discoverable Parameters (SFDP) parsing.
  */
@@ -2527,8 +2574,6 @@ static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
 	{BFPT_DWORD(9), 16},
 };
 
-static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
-
 /**
  * spi_nor_set_erase_type() - set a SPI NOR erase type
  * @erase:	pointer to a structure that describes a SPI NOR erase type
@@ -3342,57 +3387,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
 	return 0;
 }
 
-static int spi_nor_hwcaps2cmd(u32 hwcaps, const int table[][2], size_t size)
-{
-	size_t i;
-
-	for (i = 0; i < size; i++)
-		if (table[i][0] == (int)hwcaps)
-			return table[i][1];
-
-	return -EINVAL;
-}
-
-static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
-{
-	static const int hwcaps_read2cmd[][2] = {
-		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
-		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
-		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
-		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
-		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
-		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
-		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
-		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
-		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
-		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
-		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
-		{ SNOR_HWCAPS_READ_1_1_8,	SNOR_CMD_READ_1_1_8 },
-		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
-		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
-		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
-	};
-
-	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
-				  ARRAY_SIZE(hwcaps_read2cmd));
-}
-
-static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
-{
-	static const int hwcaps_pp2cmd[][2] = {
-		{ SNOR_HWCAPS_PP,		SNOR_CMD_PP },
-		{ SNOR_HWCAPS_PP_1_1_4,		SNOR_CMD_PP_1_1_4 },
-		{ SNOR_HWCAPS_PP_1_4_4,		SNOR_CMD_PP_1_4_4 },
-		{ SNOR_HWCAPS_PP_4_4_4,		SNOR_CMD_PP_4_4_4 },
-		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
-		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
-		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
-	};
-
-	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
-				  ARRAY_SIZE(hwcaps_pp2cmd));
-}
-
 static int spi_nor_select_read(struct spi_nor *nor,
 			       const struct spi_nor_flash_parameter *params,
 			       u32 shared_hwcaps)
@@ -3669,6 +3663,18 @@ void spi_nor_restore(struct spi_nor *nor)
 }
 EXPORT_SYMBOL_GPL(spi_nor_restore);
 
+static const struct flash_info *spi_nor_match_id(const char *name)
+{
+	const struct flash_info *id = spi_nor_ids;
+
+	while (id->name) {
+		if (!strcmp(name, id->name))
+			return id;
+		id++;
+	}
+	return NULL;
+}
+
 int spi_nor_scan(struct spi_nor *nor, const char *name,
 		 const struct spi_nor_hwcaps *hwcaps)
 {
@@ -3872,18 +3878,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
-static const struct flash_info *spi_nor_match_id(const char *name)
-{
-	const struct flash_info *id = spi_nor_ids;
-
-	while (id->name) {
-		if (!strcmp(name, id->name))
-			return id;
-		id++;
-	}
-	return NULL;
-}
-
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
 MODULE_AUTHOR("Mike Lavender");
-- 
2.17.1

Re: [PATCH v3 2/5] mtd: spi-nor: Avoid forward declaration of internal functions

[Tudor.Ambarus]

On 12/06/2018 12:41 PM, Boris Brezillon wrote:
> Reorganize the code to kill forward declarations of spi_nor_match_id()
> macronix_quad_enable() and spi_nor_hwcaps_read2cmd().
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> ---
> Changes in v3:
> - Group all _quad_enable() implems
> - Move spi_nor_hwcaps_xx2cmd() just after spi_nor_set_pp_settings()
> 
> Changes in v2:
> - None
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 614 +++++++++++++++++-----------------
>  1 file changed, 304 insertions(+), 310 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index d099aee075e9..b440f29637ab 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -289,8 +289,6 @@ struct flash_info {
>  
>  #define JEDEC_MFR(info)	((info)->id[0])
>  
> -static const struct flash_info *spi_nor_match_id(const char *name);
> -
>  /*
>   * Read the status register, returning its value in the location
>   * Return the status register value.
> @@ -1395,7 +1393,247 @@ static int spi_nor_is_locked(struct mtd_info *mtd, loff_t ofs, uint64_t len)
>  	return ret;
>  }
>  
> -static int macronix_quad_enable(struct spi_nor *nor);
> +/*
> + * Write status Register and configuration register with 2 bytes
> + * The first byte will be written to the status register, while the
> + * second byte will be written to the configuration register.
> + * Return negative if error occurred.
> + */
> +static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
> +{
> +	int ret;
> +
> +	write_enable(nor);
> +
> +	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
> +	if (ret < 0) {
> +		dev_err(nor->dev,
> +			"error while writing configuration register\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = spi_nor_wait_till_ready(nor);
> +	if (ret) {
> +		dev_err(nor->dev,
> +			"timeout while writing configuration register\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * macronix_quad_enable() - set QE bit in Status Register.
> + * @nor:	pointer to a 'struct spi_nor'
> + *
> + * Set the Quad Enable (QE) bit in the Status Register.
> + *
> + * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int macronix_quad_enable(struct spi_nor *nor)
> +{
> +	int ret, val;
> +
> +	val = read_sr(nor);
> +	if (val < 0)
> +		return val;
> +	if (val & SR_QUAD_EN_MX)
> +		return 0;
> +
> +	write_enable(nor);
> +
> +	write_sr(nor, val | SR_QUAD_EN_MX);
> +
> +	ret = spi_nor_wait_till_ready(nor);
> +	if (ret)
> +		return ret;
> +
> +	ret = read_sr(nor);
> +	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
> +		dev_err(nor->dev, "Macronix Quad bit not set\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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 read_cr() returns 0xFF.
> + *
> + * 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[2] = {0, CR_QUAD_EN_SPAN};
> +	int ret;
> +
> +	ret = write_sr_cr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	/* read back and check it */
> +	ret = read_cr(nor);
> +	if (!(ret > 0 && (ret & 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'
> + *
> + * Set the Quad Enable (QE) bit in the Configuration Register.
> + * This function should be used with QSPI memories not supporting the Read
> + * Configuration Register (35h) instruction.
> + *
> + * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> + * memories.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
> +{
> +	u8 sr_cr[2];
> +	int ret;
> +
> +	/* Keep the current value of the Status Register. */
> +	ret = read_sr(nor);
> +	if (ret < 0) {
> +		dev_err(nor->dev, "error while reading status register\n");
> +		return -EINVAL;
> +	}
> +	sr_cr[0] = ret;
> +	sr_cr[1] = CR_QUAD_EN_SPAN;
> +
> +	return write_sr_cr(nor, sr_cr);
> +}
> +
> +/**
> + * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
> + * @nor:	pointer to a 'struct spi_nor'
> + *
> + * Set the Quad Enable (QE) bit in the Configuration Register.
> + * This function should be used with QSPI memories supporting the Read
> + * Configuration Register (35h) instruction.
> + *
> + * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> + * memories.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spansion_read_cr_quad_enable(struct spi_nor *nor)
> +{
> +	struct device *dev = nor->dev;
> +	u8 sr_cr[2];
> +	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;
> +	}
> +
> +	if (ret & CR_QUAD_EN_SPAN)
> +		return 0;
> +
> +	sr_cr[1] = ret | CR_QUAD_EN_SPAN;
> +
> +	/* Keep the current value of the Status Register. */
> +	ret = read_sr(nor);
> +	if (ret < 0) {
> +		dev_err(dev, "error while reading status register\n");
> +		return -EINVAL;
> +	}
> +	sr_cr[0] = ret;
> +
> +	ret = write_sr_cr(nor, sr_cr);
> +	if (ret)
> +		return ret;
> +
> +	/* Read back and check it. */
> +	ret = read_cr(nor);
> +	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
> +		dev_err(nor->dev, "Spansion Quad bit not set\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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.
> + *
> + * This is one of the procedures to set the QE bit described in the SFDP
> + * (JESD216 rev B) specification but no manufacturer using this procedure has
> + * been identified yet, hence the name of the function.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int sr2_bit7_quad_enable(struct spi_nor *nor)
> +{
> +	u8 sr2;
> +	int ret;
> +
> +	/* Check current Quad Enable bit value. */
> +	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> +	if (ret)
> +		return ret;
> +	if (sr2 & SR2_QUAD_EN_BIT7)
> +		return 0;
> +
> +	/* Update the Quad Enable bit. */
> +	sr2 |= SR2_QUAD_EN_BIT7;
> +
> +	write_enable(nor);
> +
> +	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
> +	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");
> +		return ret;
> +	}
> +
> +	/* Read back and check it. */
> +	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> +	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
> +		dev_err(nor->dev, "SR2 Quad bit not set\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
>  
>  /* Used when the "_ext_id" is two bytes at most */
>  #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
> @@ -2002,248 +2240,6 @@ static int spi_nor_write(struct mtd_info *mtd, loff_t to, size_t len,
>  	return ret;
>  }
>  
> -/**
> - * macronix_quad_enable() - set QE bit in Status Register.
> - * @nor:	pointer to a 'struct spi_nor'
> - *
> - * Set the Quad Enable (QE) bit in the Status Register.
> - *
> - * bit 6 of the Status Register is the QE bit for Macronix like QSPI memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int macronix_quad_enable(struct spi_nor *nor)
> -{
> -	int ret, val;
> -
> -	val = read_sr(nor);
> -	if (val < 0)
> -		return val;
> -	if (val & SR_QUAD_EN_MX)
> -		return 0;
> -
> -	write_enable(nor);
> -
> -	write_sr(nor, val | SR_QUAD_EN_MX);
> -
> -	ret = spi_nor_wait_till_ready(nor);
> -	if (ret)
> -		return ret;
> -
> -	ret = read_sr(nor);
> -	if (!(ret > 0 && (ret & SR_QUAD_EN_MX))) {
> -		dev_err(nor->dev, "Macronix Quad bit not set\n");
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
> -/*
> - * Write status Register and configuration register with 2 bytes
> - * The first byte will be written to the status register, while the
> - * second byte will be written to the configuration register.
> - * Return negative if error occurred.
> - */
> -static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
> -{
> -	int ret;
> -
> -	write_enable(nor);
> -
> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR, sr_cr, 2);
> -	if (ret < 0) {
> -		dev_err(nor->dev,
> -			"error while writing configuration register\n");
> -		return -EINVAL;
> -	}
> -
> -	ret = spi_nor_wait_till_ready(nor);
> -	if (ret) {
> -		dev_err(nor->dev,
> -			"timeout while writing configuration register\n");
> -		return ret;
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * 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 read_cr() returns 0xFF.
> - *
> - * 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[2] = {0, CR_QUAD_EN_SPAN};
> -	int ret;
> -
> -	ret = write_sr_cr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	/* read back and check it */
> -	ret = read_cr(nor);
> -	if (!(ret > 0 && (ret & 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'
> - *
> - * Set the Quad Enable (QE) bit in the Configuration Register.
> - * This function should be used with QSPI memories not supporting the Read
> - * Configuration Register (35h) instruction.
> - *
> - * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> - * memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spansion_no_read_cr_quad_enable(struct spi_nor *nor)
> -{
> -	u8 sr_cr[2];
> -	int ret;
> -
> -	/* Keep the current value of the Status Register. */
> -	ret = read_sr(nor);
> -	if (ret < 0) {
> -		dev_err(nor->dev, "error while reading status register\n");
> -		return -EINVAL;
> -	}
> -	sr_cr[0] = ret;
> -	sr_cr[1] = CR_QUAD_EN_SPAN;
> -
> -	return write_sr_cr(nor, sr_cr);
> -}
> -
> -/**
> - * spansion_read_cr_quad_enable() - set QE bit in Configuration Register.
> - * @nor:	pointer to a 'struct spi_nor'
> - *
> - * Set the Quad Enable (QE) bit in the Configuration Register.
> - * This function should be used with QSPI memories supporting the Read
> - * Configuration Register (35h) instruction.
> - *
> - * bit 1 of the Configuration Register is the QE bit for Spansion like QSPI
> - * memories.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int spansion_read_cr_quad_enable(struct spi_nor *nor)
> -{
> -	struct device *dev = nor->dev;
> -	u8 sr_cr[2];
> -	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;
> -	}
> -
> -	if (ret & CR_QUAD_EN_SPAN)
> -		return 0;
> -
> -	sr_cr[1] = ret | CR_QUAD_EN_SPAN;
> -
> -	/* Keep the current value of the Status Register. */
> -	ret = read_sr(nor);
> -	if (ret < 0) {
> -		dev_err(dev, "error while reading status register\n");
> -		return -EINVAL;
> -	}
> -	sr_cr[0] = ret;
> -
> -	ret = write_sr_cr(nor, sr_cr);
> -	if (ret)
> -		return ret;
> -
> -	/* Read back and check it. */
> -	ret = read_cr(nor);
> -	if (!(ret > 0 && (ret & CR_QUAD_EN_SPAN))) {
> -		dev_err(nor->dev, "Spansion Quad bit not set\n");
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
> -/**
> - * 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.
> - *
> - * This is one of the procedures to set the QE bit described in the SFDP
> - * (JESD216 rev B) specification but no manufacturer using this procedure has
> - * been identified yet, hence the name of the function.
> - *
> - * Return: 0 on success, -errno otherwise.
> - */
> -static int sr2_bit7_quad_enable(struct spi_nor *nor)
> -{
> -	u8 sr2;
> -	int ret;
> -
> -	/* Check current Quad Enable bit value. */
> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> -	if (ret)
> -		return ret;
> -	if (sr2 & SR2_QUAD_EN_BIT7)
> -		return 0;
> -
> -	/* Update the Quad Enable bit. */
> -	sr2 |= SR2_QUAD_EN_BIT7;
> -
> -	write_enable(nor);
> -
> -	ret = nor->write_reg(nor, SPINOR_OP_WRSR2, &sr2, 1);
> -	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");
> -		return ret;
> -	}
> -
> -	/* Read back and check it. */
> -	ret = nor->read_reg(nor, SPINOR_OP_RDSR2, &sr2, 1);
> -	if (!(ret > 0 && (sr2 & SR2_QUAD_EN_BIT7))) {
> -		dev_err(nor->dev, "SR2 Quad bit not set\n");
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> -}
> -
>  static int spi_nor_check(struct spi_nor *nor)
>  {
>  	if (!nor->dev || !nor->read || !nor->write ||
> @@ -2318,6 +2314,57 @@ spi_nor_set_pp_settings(struct spi_nor_pp_command *pp,
>  	pp->proto = proto;
>  }
>  
> +static int spi_nor_hwcaps2cmd(u32 hwcaps, const int table[][2], size_t size)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < size; i++)
> +		if (table[i][0] == (int)hwcaps)
> +			return table[i][1];
> +
> +	return -EINVAL;
> +}
> +
> +static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
> +{
> +	static const int hwcaps_read2cmd[][2] = {
> +		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
> +		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
> +		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
> +		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
> +		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
> +		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
> +		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
> +		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
> +		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
> +		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
> +		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
> +		{ SNOR_HWCAPS_READ_1_1_8,	SNOR_CMD_READ_1_1_8 },
> +		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
> +		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
> +		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
> +	};
> +
> +	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
> +				  ARRAY_SIZE(hwcaps_read2cmd));
> +}
> +
> +static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
> +{
> +	static const int hwcaps_pp2cmd[][2] = {
> +		{ SNOR_HWCAPS_PP,		SNOR_CMD_PP },
> +		{ SNOR_HWCAPS_PP_1_1_4,		SNOR_CMD_PP_1_1_4 },
> +		{ SNOR_HWCAPS_PP_1_4_4,		SNOR_CMD_PP_1_4_4 },
> +		{ SNOR_HWCAPS_PP_4_4_4,		SNOR_CMD_PP_4_4_4 },
> +		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
> +		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
> +		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
> +	};
> +
> +	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
> +				  ARRAY_SIZE(hwcaps_pp2cmd));
> +}
> +
>  /*
>   * Serial Flash Discoverable Parameters (SFDP) parsing.
>   */
> @@ -2527,8 +2574,6 @@ static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
>  	{BFPT_DWORD(9), 16},
>  };
>  
> -static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
> -
>  /**
>   * spi_nor_set_erase_type() - set a SPI NOR erase type
>   * @erase:	pointer to a structure that describes a SPI NOR erase type
> @@ -3342,57 +3387,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
>  	return 0;
>  }
>  
> -static int spi_nor_hwcaps2cmd(u32 hwcaps, const int table[][2], size_t size)
> -{
> -	size_t i;
> -
> -	for (i = 0; i < size; i++)
> -		if (table[i][0] == (int)hwcaps)
> -			return table[i][1];
> -
> -	return -EINVAL;
> -}
> -
> -static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
> -{
> -	static const int hwcaps_read2cmd[][2] = {
> -		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
> -		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
> -		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
> -		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
> -		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
> -		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
> -		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
> -		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
> -		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
> -		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
> -		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
> -		{ SNOR_HWCAPS_READ_1_1_8,	SNOR_CMD_READ_1_1_8 },
> -		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
> -		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
> -		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
> -	};
> -
> -	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
> -				  ARRAY_SIZE(hwcaps_read2cmd));
> -}
> -
> -static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
> -{
> -	static const int hwcaps_pp2cmd[][2] = {
> -		{ SNOR_HWCAPS_PP,		SNOR_CMD_PP },
> -		{ SNOR_HWCAPS_PP_1_1_4,		SNOR_CMD_PP_1_1_4 },
> -		{ SNOR_HWCAPS_PP_1_4_4,		SNOR_CMD_PP_1_4_4 },
> -		{ SNOR_HWCAPS_PP_4_4_4,		SNOR_CMD_PP_4_4_4 },
> -		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
> -		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
> -		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
> -	};
> -
> -	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
> -				  ARRAY_SIZE(hwcaps_pp2cmd));
> -}
> -
>  static int spi_nor_select_read(struct spi_nor *nor,
>  			       const struct spi_nor_flash_parameter *params,
>  			       u32 shared_hwcaps)
> @@ -3669,6 +3663,18 @@ void spi_nor_restore(struct spi_nor *nor)
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_restore);
>  
> +static const struct flash_info *spi_nor_match_id(const char *name)
> +{
> +	const struct flash_info *id = spi_nor_ids;
> +
> +	while (id->name) {
> +		if (!strcmp(name, id->name))
> +			return id;
> +		id++;
> +	}
> +	return NULL;
> +}
> +
>  int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		 const struct spi_nor_hwcaps *hwcaps)
>  {
> @@ -3872,18 +3878,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>  
> -static const struct flash_info *spi_nor_match_id(const char *name)
> -{
> -	const struct flash_info *id = spi_nor_ids;
> -
> -	while (id->name) {
> -		if (!strcmp(name, id->name))
> -			return id;
> -		id++;
> -	}
> -	return NULL;
> -}
> -
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
>  MODULE_AUTHOR("Mike Lavender");
> 

Re: [v3, 2/5] mtd: spi-nor: Avoid forward declaration of internal functions

[Boris Brezillon]

On Thu, 2018-12-06 at 10:41:17 UTC, Boris Brezillon wrote:
> Reorganize the code to kill forward declarations of spi_nor_match_id()
> macronix_quad_enable() and spi_nor_hwcaps_read2cmd().
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next.

Boris

[PATCH v3 3/5] mtd: spi-nor: Stop passing flash_info around

[Boris Brezillon]

Some functions called from spi_nor_scan() need a flash_info object.
Let's assign nor->info early on to avoid passing info as an extra
argument to each of these sub-functions.

We also stop passing a flash_info object to set_4byte() and use
nor->info directly.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- Merged 2 patches doing pretty much the same thing
- Drop local flash_info vars and dereference nor->info directly

Changes in v2:
- New patch
---
 drivers/mtd/spi-nor/spi-nor.c | 38 +++++++++++++++++------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index b440f29637ab..7d6e8e264cd6 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -434,15 +434,14 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)
 				      ARRAY_SIZE(spi_nor_3to4_erase));
 }
 
-static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
-				      const struct flash_info *info)
+static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
 {
 	/* Do some manufacturer fixups first */
-	switch (JEDEC_MFR(info)) {
+	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 = info->sector_size;
+		nor->mtd.erasesize = nor->info->sector_size;
 		break;
 
 	default:
@@ -467,14 +466,13 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
 }
 
 /* Enable/disable 4-byte addressing mode. */
-static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
-		     int enable)
+static int set_4byte(struct spi_nor *nor, int enable)
 {
 	int status;
 	bool need_wren = false;
 	u8 cmd;
 
-	switch (JEDEC_MFR(info)) {
+	switch (JEDEC_MFR(nor->info)) {
 	case SNOR_MFR_ST:
 	case SNOR_MFR_MICRON:
 		/* Some Micron need WREN command; all will accept it */
@@ -491,7 +489,7 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
 			write_disable(nor);
 
 		if (!status && !enable &&
-		    JEDEC_MFR(info) == SNOR_MFR_WINBOND) {
+		    JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
 			/*
 			 * On Winbond W25Q256FV, leaving 4byte mode causes
 			 * the Extended Address Register to be set to 1, so all
@@ -2251,7 +2249,7 @@ static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
-static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
+static int s3an_nor_scan(struct spi_nor *nor)
 {
 	int ret;
 	u8 val;
@@ -2282,7 +2280,7 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
 		/* Flash in Power of 2 mode */
 		nor->page_size = (nor->page_size == 264) ? 256 : 512;
 		nor->mtd.writebufsize = nor->page_size;
-		nor->mtd.size = 8 * nor->page_size * info->n_sectors;
+		nor->mtd.size = 8 * nor->page_size * nor->info->n_sectors;
 		nor->mtd.erasesize = 8 * nor->page_size;
 	} else {
 		/* Flash in Default addressing mode */
@@ -3270,10 +3268,10 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 }
 
 static int spi_nor_init_params(struct spi_nor *nor,
-			       const struct flash_info *info,
 			       struct spi_nor_flash_parameter *params)
 {
 	struct spi_nor_erase_map *map = &nor->erase_map;
+	const struct flash_info *info = nor->info;
 	u8 i, erase_mask;
 
 	/* Set legacy flash parameters as default. */
@@ -3539,10 +3537,11 @@ 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 flash_info *info,
+static int spi_nor_setup(struct spi_nor *nor,
 			 const struct spi_nor_flash_parameter *params,
 			 const struct spi_nor_hwcaps *hwcaps)
 {
+	const struct flash_info *info = nor->info;
 	u32 ignored_mask, shared_mask;
 	bool enable_quad_io;
 	int err;
@@ -3635,7 +3634,7 @@ static int spi_nor_init(struct spi_nor *nor)
 		 */
 		WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
 			  "enabling reset hack; may not recover from unexpected reboots\n");
-		set_4byte(nor, nor->info, 1);
+		set_4byte(nor, 1);
 	}
 
 	return 0;
@@ -3659,7 +3658,7 @@ void spi_nor_restore(struct spi_nor *nor)
 	/* restore the addressing mode */
 	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
 	    nor->flags & SNOR_F_BROKEN_RESET)
-		set_4byte(nor, nor->info, 0);
+		set_4byte(nor, 0);
 }
 EXPORT_SYMBOL_GPL(spi_nor_restore);
 
@@ -3727,6 +3726,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		}
 	}
 
+	nor->info = info;
+
 	mutex_init(&nor->lock);
 
 	/*
@@ -3738,7 +3739,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		nor->flags |=  SNOR_F_READY_XSR_RDY;
 
 	/* Parse the Serial Flash Discoverable Parameters table. */
-	ret = spi_nor_init_params(nor, info, &params);
+	ret = spi_nor_init_params(nor, &params);
 	if (ret)
 		return ret;
 
@@ -3815,7 +3816,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	 * - set the SPI protocols for register and memory accesses.
 	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
 	 */
-	ret = spi_nor_setup(nor, info, &params, hwcaps);
+	ret = spi_nor_setup(nor, &params, hwcaps);
 	if (ret)
 		return ret;
 
@@ -3835,7 +3836,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		nor->flags |= SNOR_F_4B_OPCODES;
 
 	if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES)
-		spi_nor_set_4byte_opcodes(nor, info);
+		spi_nor_set_4byte_opcodes(nor);
 
 	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
 		dev_err(dev, "address width is too large: %u\n",
@@ -3844,13 +3845,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	}
 
 	if (info->flags & SPI_S3AN) {
-		ret = s3an_nor_scan(info, nor);
+		ret = s3an_nor_scan(nor);
 		if (ret)
 			return ret;
 	}
 
 	/* Send all the required SPI flash commands to initialize device */
-	nor->info = info;
 	ret = spi_nor_init(nor);
 	if (ret)
 		return ret;
-- 
2.17.1

Re: [PATCH v3 3/5] mtd: spi-nor: Stop passing flash_info around

[Tudor.Ambarus]

On 12/06/2018 12:41 PM, Boris Brezillon wrote:
> Some functions called from spi_nor_scan() need a flash_info object.
> Let's assign nor->info early on to avoid passing info as an extra
> argument to each of these sub-functions.
> 
> We also stop passing a flash_info object to set_4byte() and use
> nor->info directly.

Should I apply this on top of git://git.infradead.org/linux-mtd.git spi-nor/next
branch?

Applying: mtd: spi-nor: Stop passing flash_info around
error: patch failed: drivers/mtd/spi-nor/spi-nor.c:3659
error: drivers/mtd/spi-nor/spi-nor.c: patch does not apply

> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
> Changes in v3:
> - Merged 2 patches doing pretty much the same thing
> - Drop local flash_info vars and dereference nor->info directly
> 
> Changes in v2:
> - New patch
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 38 +++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index b440f29637ab..7d6e8e264cd6 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -434,15 +434,14 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)
>  				      ARRAY_SIZE(spi_nor_3to4_erase));
>  }
>  
> -static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> -				      const struct flash_info *info)
> +static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
>  {
>  	/* Do some manufacturer fixups first */
> -	switch (JEDEC_MFR(info)) {
> +	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 = info->sector_size;
> +		nor->mtd.erasesize = nor->info->sector_size;
>  		break;
>  
>  	default:
> @@ -467,14 +466,13 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>  }
>  
>  /* Enable/disable 4-byte addressing mode. */
> -static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
> -		     int enable)
> +static int set_4byte(struct spi_nor *nor, int enable)
>  {
>  	int status;
>  	bool need_wren = false;
>  	u8 cmd;
>  
> -	switch (JEDEC_MFR(info)) {
> +	switch (JEDEC_MFR(nor->info)) {
>  	case SNOR_MFR_ST:
>  	case SNOR_MFR_MICRON:
>  		/* Some Micron need WREN command; all will accept it */
> @@ -491,7 +489,7 @@ static int set_4byte(struct spi_nor *nor, const struct flash_info *info,
>  			write_disable(nor);
>  
>  		if (!status && !enable &&
> -		    JEDEC_MFR(info) == SNOR_MFR_WINBOND) {
> +		    JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
>  			/*
>  			 * On Winbond W25Q256FV, leaving 4byte mode causes
>  			 * the Extended Address Register to be set to 1, so all
> @@ -2251,7 +2249,7 @@ static int spi_nor_check(struct spi_nor *nor)
>  	return 0;
>  }
>  
> -static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
> +static int s3an_nor_scan(struct spi_nor *nor)
>  {
>  	int ret;
>  	u8 val;
> @@ -2282,7 +2280,7 @@ static int s3an_nor_scan(const struct flash_info *info, struct spi_nor *nor)
>  		/* Flash in Power of 2 mode */
>  		nor->page_size = (nor->page_size == 264) ? 256 : 512;
>  		nor->mtd.writebufsize = nor->page_size;
> -		nor->mtd.size = 8 * nor->page_size * info->n_sectors;
> +		nor->mtd.size = 8 * nor->page_size * nor->info->n_sectors;
>  		nor->mtd.erasesize = 8 * nor->page_size;
>  	} else {
>  		/* Flash in Default addressing mode */
> @@ -3270,10 +3268,10 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  }
>  
>  static int spi_nor_init_params(struct spi_nor *nor,
> -			       const struct flash_info *info,
>  			       struct spi_nor_flash_parameter *params)
>  {
>  	struct spi_nor_erase_map *map = &nor->erase_map;
> +	const struct flash_info *info = nor->info;
>  	u8 i, erase_mask;
>  
>  	/* Set legacy flash parameters as default. */
> @@ -3539,10 +3537,11 @@ 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 flash_info *info,
> +static int spi_nor_setup(struct spi_nor *nor,
>  			 const struct spi_nor_flash_parameter *params,
>  			 const struct spi_nor_hwcaps *hwcaps)
>  {
> +	const struct flash_info *info = nor->info;
>  	u32 ignored_mask, shared_mask;
>  	bool enable_quad_io;
>  	int err;
> @@ -3635,7 +3634,7 @@ static int spi_nor_init(struct spi_nor *nor)
>  		 */
>  		WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
>  			  "enabling reset hack; may not recover from unexpected reboots\n");
> -		set_4byte(nor, nor->info, 1);
> +		set_4byte(nor, 1);
>  	}
>  
>  	return 0;
> @@ -3659,7 +3658,7 @@ void spi_nor_restore(struct spi_nor *nor)
>  	/* restore the addressing mode */
>  	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
>  	    nor->flags & SNOR_F_BROKEN_RESET)
> -		set_4byte(nor, nor->info, 0);
> +		set_4byte(nor, 0);
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_restore);
>  
> @@ -3727,6 +3726,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		}
>  	}
>  
> +	nor->info = info;
> +
>  	mutex_init(&nor->lock);
>  
>  	/*
> @@ -3738,7 +3739,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		nor->flags |=  SNOR_F_READY_XSR_RDY;
>  
>  	/* Parse the Serial Flash Discoverable Parameters table. */
> -	ret = spi_nor_init_params(nor, info, &params);
> +	ret = spi_nor_init_params(nor, &params);
>  	if (ret)
>  		return ret;
>  
> @@ -3815,7 +3816,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	 * - set the SPI protocols for register and memory accesses.
>  	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
>  	 */
> -	ret = spi_nor_setup(nor, info, &params, hwcaps);
> +	ret = spi_nor_setup(nor, &params, hwcaps);
>  	if (ret)
>  		return ret;
>  
> @@ -3835,7 +3836,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		nor->flags |= SNOR_F_4B_OPCODES;
>  
>  	if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES)
> -		spi_nor_set_4byte_opcodes(nor, info);
> +		spi_nor_set_4byte_opcodes(nor);
>  
>  	if (nor->addr_width > SPI_NOR_MAX_ADDR_WIDTH) {
>  		dev_err(dev, "address width is too large: %u\n",
> @@ -3844,13 +3845,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	}
>  
>  	if (info->flags & SPI_S3AN) {
> -		ret = s3an_nor_scan(info, nor);
> +		ret = s3an_nor_scan(nor);
>  		if (ret)
>  			return ret;
>  	}
>  
>  	/* Send all the required SPI flash commands to initialize device */
> -	nor->info = info;
>  	ret = spi_nor_init(nor);
>  	if (ret)
>  		return ret;
> 

Re: [PATCH v3 3/5] mtd: spi-nor: Stop passing flash_info around

[Boris Brezillon]

On Thu, 6 Dec 2018 11:20:49 +0000
<Tudor.Ambarus@microchip.com> wrote:

> On 12/06/2018 12:41 PM, Boris Brezillon wrote:
> > Some functions called from spi_nor_scan() need a flash_info object.
> > Let's assign nor->info early on to avoid passing info as an extra
> > argument to each of these sub-functions.
> > 
> > We also stop passing a flash_info object to set_4byte() and use
> > nor->info directly.  
> 
> Should I apply this on top of git://git.infradead.org/linux-mtd.git spi-nor/next
> branch?
> 
> Applying: mtd: spi-nor: Stop passing flash_info around
> error: patch failed: drivers/mtd/spi-nor/spi-nor.c:3659
> error: drivers/mtd/spi-nor/spi-nor.c: patch does not apply

Oops. I forgot to mention that this series now depends on [1] (I
re-ordered patches to get [1] merged as soon as possible).

[1]http://patchwork.ozlabs.org/project/linux-mtd/list/?series=80112

Re: [PATCH v3 3/5] mtd: spi-nor: Stop passing flash_info around

[Tudor.Ambarus]

On 12/06/2018 12:41 PM, Boris Brezillon wrote:
> @@ -3539,10 +3537,11 @@ 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 flash_info *info,
> +static int spi_nor_setup(struct spi_nor *nor,
>  			 const struct spi_nor_flash_parameter *params,
>  			 const struct spi_nor_hwcaps *hwcaps)
>  {
> +	const struct flash_info *info = nor->info;

please drop this local variable and use nor->info->sector_size instead

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Re: [v3,3/5] mtd: spi-nor: Stop passing flash_info around

[Boris Brezillon]

On Thu, 2018-12-06 at 10:41:18 UTC, Boris Brezillon wrote:
> Some functions called from spi_nor_scan() need a flash_info object.
> Let's assign nor->info early on to avoid passing info as an extra
> argument to each of these sub-functions.
> 
> We also stop passing a flash_info object to set_4byte() and use
> nor->info directly.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next.

Boris

[PATCH v3 4/5] mtd: spi-nor: Make the enable argument passed to set_byte() a bool

[Boris Brezillon]

No need to use an integer when the value is either true or false.
Make it a boolean.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
Changes in v3:
- Add Tudor's R-b

Changes in v2:
- None
---
 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 7d6e8e264cd6..db31a81a68b2 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -466,7 +466,7 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
 }
 
 /* Enable/disable 4-byte addressing mode. */
-static int set_4byte(struct spi_nor *nor, int enable)
+static int set_4byte(struct spi_nor *nor, bool enable)
 {
 	int status;
 	bool need_wren = false;
@@ -3634,7 +3634,7 @@ static int spi_nor_init(struct spi_nor *nor)
 		 */
 		WARN_ONCE(nor->flags & SNOR_F_BROKEN_RESET,
 			  "enabling reset hack; may not recover from unexpected reboots\n");
-		set_4byte(nor, 1);
+		set_4byte(nor, true);
 	}
 
 	return 0;
@@ -3658,7 +3658,7 @@ void spi_nor_restore(struct spi_nor *nor)
 	/* restore the addressing mode */
 	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES) &&
 	    nor->flags & SNOR_F_BROKEN_RESET)
-		set_4byte(nor, 0);
+		set_4byte(nor, false);
 }
 EXPORT_SYMBOL_GPL(spi_nor_restore);
 
-- 
2.17.1

Re: [v3, 4/5] mtd: spi-nor: Make the enable argument passed to set_byte() a bool

[Boris Brezillon]

On Thu, 2018-12-06 at 10:41:19 UTC, Boris Brezillon wrote:
> No need to use an integer when the value is either true or false.
> Make it a boolean.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next.

Boris

[PATCH v3 5/5] mtd: spi-nor: Add an SPDX tag to spi-nor.{c,h}

[Boris Brezillon]

Add SPDX tags to replace the license boiler-plate and fix the
MODULE_LICENSE() definition in spi-nor.c to match the license text
(GPL v2).

Interestingly, spi-nor.h and spi-nor.c do not use the same license
(GPL v2+ for spi-nor.h, GPL v2 for spi-nor.c).

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v3:
- Add an SPDX tag to spi-nor.h

Changes in v2:
- New patch
---
 drivers/mtd/spi-nor/spi-nor.c | 7 ++-----
 include/linux/mtd/spi-nor.h   | 6 +-----
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index db31a81a68b2..2e9306e1c52a 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -1,13 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Based on m25p80.c, by Mike Lavender (mike@steroidmicros.com), with
  * influence from lart.c (Abraham Van Der Merwe) and mtd_dataflash.c
  *
  * Copyright (C) 2005, Intec Automation Inc.
  * Copyright (C) 2014, Freescale Semiconductor, Inc.
- *
- * This code is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
  */
 
 #include <linux/err.h>
@@ -3878,7 +3875,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 }
 EXPORT_SYMBOL_GPL(spi_nor_scan);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
 MODULE_AUTHOR("Mike Lavender");
 MODULE_DESCRIPTION("framework for SPI NOR");
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 981d628305a2..5f177aa39f68 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -1,10 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
 /*
  * Copyright (C) 2014 Freescale Semiconductor, Inc.
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
  */
 
 #ifndef __LINUX_MTD_SPI_NOR_H
-- 
2.17.1

Re: [PATCH v3 5/5] mtd: spi-nor: Add an SPDX tag to spi-nor.{c,h}

[Tudor.Ambarus]

On 12/06/2018 12:41 PM, Boris Brezillon wrote:
> Add SPDX tags to replace the license boiler-plate and fix the
> MODULE_LICENSE() definition in spi-nor.c to match the license text
> (GPL v2).
> 
> Interestingly, spi-nor.h and spi-nor.c do not use the same license
> (GPL v2+ for spi-nor.h, GPL v2 for spi-nor.c).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

> ---
> Changes in v3:
> - Add an SPDX tag to spi-nor.h
> 
> Changes in v2:
> - New patch
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 7 ++-----
>  include/linux/mtd/spi-nor.h   | 6 +-----
>  2 files changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index db31a81a68b2..2e9306e1c52a 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -1,13 +1,10 @@
> +// SPDX-License-Identifier: GPL-2.0
>  /*
>   * Based on m25p80.c, by Mike Lavender (mike@steroidmicros.com), with
>   * influence from lart.c (Abraham Van Der Merwe) and mtd_dataflash.c
>   *
>   * Copyright (C) 2005, Intec Automation Inc.
>   * Copyright (C) 2014, Freescale Semiconductor, Inc.
> - *
> - * This code is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
>   */
>  
>  #include <linux/err.h>
> @@ -3878,7 +3875,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  }
>  EXPORT_SYMBOL_GPL(spi_nor_scan);
>  
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("GPL v2");
>  MODULE_AUTHOR("Huang Shijie <shijie8@gmail.com>");
>  MODULE_AUTHOR("Mike Lavender");
>  MODULE_DESCRIPTION("framework for SPI NOR");
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 981d628305a2..5f177aa39f68 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -1,10 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
>  /*
>   * Copyright (C) 2014 Freescale Semiconductor, Inc.
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License as published by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
>   */
>  
>  #ifndef __LINUX_MTD_SPI_NOR_H
> 

Re: [v3,5/5] mtd: spi-nor: Add an SPDX tag to spi-nor.{c,h}

[Boris Brezillon]

On Thu, 2018-12-06 at 10:41:20 UTC, Boris Brezillon wrote:
> Add SPDX tags to replace the license boiler-plate and fix the
> MODULE_LICENSE() definition in spi-nor.c to match the license text
> (GPL v2).
> 
> Interestingly, spi-nor.h and spi-nor.c do not use the same license
> (GPL v2+ for spi-nor.h, GPL v2 for spi-nor.c).
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Applied to http://git.infradead.org/linux-mtd.git spi-nor/next.

Boris