[PATCH v3 00/13] mtd: st_spi_fsm: Align with ST's internal development

[PATCH v3 00/13] mtd: st_spi_fsm: Align with ST's internal development

[Lee Jones]

Hi Brian, all,
 
 [x] Bulid test
 [x] Bisectable
 [x] Smatch
 [x] Sparse

v3:
  Further aligned with upstream changes as suggested by Peter Griffin.
  The clk_ignore_unused kernel command line parameter is due to be turned
  off on STiH4* platforms, so we no longer give the driver the chance to
  opt-out of fetching the EMI clock.  We also now provide syscfg offsets
  inside the driver, as it's simpler and reduces the cost of having lots
  of extra DT properties.
 
v2:
  These are the last remaining patches in the set, all rebased and
  with the DT documentation requested after the last submission.
 
v1:
  This patch-set updates ST's FSM SPI-NOR driver with all the internal
  goodness which has happened since the initial (now upstreamed) snapshot
  was taken. It covers just over 6 months worth of internal development
  and bug-fixes. A final whitespace clean-up is also appended to the set
  - to make it look pretty and stuff. :)

Angus Clark (3):
  mtd: st_spi_fsm: Add support for N25Q512 and N25Q00A devices
  mtd: st_spi_fsm: Update Spansion device entries
  mtd: st_spi_fsm: Improve busy wait handling

Lee Jones (9):
  mtd: st_spi_fsm: Extend fsm_clear_fifo to handle unwanted bytes
  mtd: st_spi_fsm: Obtain and use EMI clock
  mtd: st_spi_fsm: dt-bindings: Deprecate generic compatible string
  mtd: st_spi_fsm: Fetch boot device locations from DT match tables
  mtd: st_spi_fsm: Fix [-Wsign-compare] build warning
  mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs
  mtd: st_spi_fsm: General tidy-up
  ARM: STi: stih416: Use new platform specific compatible string
  ARM: STi: stih416: Supply EMI clock reference to FSM SPI NOR

Nunzio Raciti (1):
  mtd: st_spi_fsm: Add support for Micron N25Q512A

 Documentation/devicetree/bindings/mtd/st-fsm.txt |  20 +-
 arch/arm/boot/dts/stih416.dtsi                   |   6 +-
 drivers/mtd/devices/st_spi_fsm.c                 | 747 +++++++++++++++--------
 include/dt-bindings/clock/stih416-clks.h         |   1 +
 4 files changed, 519 insertions(+), 255 deletions(-)

-- 
1.9.1

[PATCH v3 01/13] mtd: st_spi_fsm: Extend fsm_clear_fifo to handle unwanted bytes

[Lee Jones]

Under certain conditions, the SPI-FSM Controller can be left in a state where
the data FIFO is not entirely empty.  This can lead to problems where subsequent
data transfers appear to have been shifted by a number of unidentified bytes.

One simple example would be an errant FSM sequence which loaded more data to the
FIFO than was read by the host.  Another more interesting case results from an
obscure artefact in the FSM Controller.  When switching from data transfers in
x4 or x2 mode to data transfers in x1 mode, extraneous bytes will appear in the
FIFO, unless the previous data transfer was a multiple of 32 cycles (i.e. 8
bytes for x2, and 16 bytes for x4).  This applies equally whether FSM is being
operated directly by a S/W driver, or by the SPI boot-controller in FSM-Boot
mode.  Furthermore, data in the FIFO not only survive a transition between
FSM-Boot and FSM, but also a S/W reset of IP block [1].

By taking certain precautions, it is possible to prevent the driver from causing
this type of problem (e.g. ensuring that the host and programmed sequence
agree on the transfer size, and restricting transfer sizes to multiples of
32-cycles [2]).  However, at the point the driver is loaded, no assumptions can be
made regarding the state of the FIFO.  Even if previous S/W drivers have behaved
correctly, it is impossible to control the number of transactions serviced by
the controller operating in FSM-Boot.

To address this problem, we ensure the FIFO is cleared during initialisation,
before performing any FSM operations.  Previously, the fsm_clear_fifo() code was
capable of detecting and clearing any unwanted 32-bit words from the FIFO.  This
patch extends the capability to handle an arbitrary number of bytes present in
the FIFO [3].  Now that the issue is better understood, we also remove the calls
to fsm_clear_fifo() following the fsm_read() and fsm_write() operations.

The process of actually clearing the FIFO deserves a mention.  While the FIFO
may contain any number of bytes, the SPI_FAST_SEQ_STA register only reports the
number of complete 32-bit words present.  Furthermore, data can only be drained
from the FIFO by reading complete 32-bit words.  With this in mind, a two stage
process is used to the clear the FIFO:

    1. Read any complete 32-bit words from the FIFO, as reported by the
           SPI_FAST_SEQ_STA register.

    2. Mop up any remaining bytes.  At this point, it is not known if there
           are 0, 1, 2, or 3 bytes in the FIFO.  To handle all cases, a dummy
           FSM sequence is used to load one byte at a time, until a complete
           32-bit word is formed; at most, 4 bytes will need to be loaded.

[1] Although this issue has existed since early versions of the SPI-FSM
    controller, its full extent only emerged recently as a consequence of the
    targetpacks starting to use FSM-Boot(x4) as the default configuration.

[2] The requirement to restrict transfers to multiples of 32 cycles was found
    empirically back when DUAL and QUAD mode support was added.  The current
    analysis now gives a satisfactory explanation for this requirement.

[3] Theoretically, it is possible for the FIFO to contain an arbitrary number of
    bits.  However, since there are no known use-cases that leave incomplete
    bytes in the FIFO, only words and bytes are considered here.

Signed-off-by: Angus Clark <angus.clark@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 95 +++++++++++++++++++++++++++++++++-------
 1 file changed, 79 insertions(+), 16 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index d252514..6e4d3bfe 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -663,6 +663,23 @@ static struct stfsm_seq stfsm_seq_write_status = {
 		    SEQ_CFG_STARTSEQ),
 };
 
+/* Dummy sequence to read one byte of data from flash into the FIFO */
+static const struct stfsm_seq stfsm_seq_load_fifo_byte = {
+	.data_size = TRANSFER_SIZE(1),
+	.seq_opc[0] = (SEQ_OPC_PADS_1 |
+		       SEQ_OPC_CYCLES(8) |
+		       SEQ_OPC_OPCODE(SPINOR_OP_RDID)),
+	.seq = {
+		STFSM_INST_CMD1,
+		STFSM_INST_DATA_READ,
+		STFSM_INST_STOP,
+	},
+	.seq_cfg = (SEQ_CFG_PADS_1 |
+		    SEQ_CFG_READNOTWRITE |
+		    SEQ_CFG_CSDEASSERT |
+		    SEQ_CFG_STARTSEQ),
+};
+
 static int stfsm_n25q_en_32bit_addr_seq(struct stfsm_seq *seq)
 {
 	seq->seq_opc[0] = (SEQ_OPC_PADS_1 | SEQ_OPC_CYCLES(8) |
@@ -695,22 +712,6 @@ static inline uint32_t stfsm_fifo_available(struct stfsm *fsm)
 	return (readl(fsm->base + SPI_FAST_SEQ_STA) >> 5) & 0x7f;
 }
 
-static void stfsm_clear_fifo(struct stfsm *fsm)
-{
-	uint32_t avail;
-
-	for (;;) {
-		avail = stfsm_fifo_available(fsm);
-		if (!avail)
-			break;
-
-		while (avail) {
-			readl(fsm->base + SPI_FAST_SEQ_DATA_REG);
-			avail--;
-		}
-	}
-}
-
 static inline void stfsm_load_seq(struct stfsm *fsm,
 				  const struct stfsm_seq *seq)
 {
@@ -772,6 +773,68 @@ static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
 	}
 }
 
+/*
+ * Clear the data FIFO
+ *
+ * Typically, this is only required during driver initialisation, where no
+ * assumptions can be made regarding the state of the FIFO.
+ *
+ * The process of clearing the FIFO is complicated by fact that while it is
+ * possible for the FIFO to contain an arbitrary number of bytes [1], the
+ * SPI_FAST_SEQ_STA register only reports the number of complete 32-bit words
+ * present.  Furthermore, data can only be drained from the FIFO by reading
+ * complete 32-bit words.
+ *
+ * With this in mind, a two stage process is used to the clear the FIFO:
+ *
+ *     1. Read any complete 32-bit words from the FIFO, as reported by the
+ *        SPI_FAST_SEQ_STA register.
+ *
+ *     2. Mop up any remaining bytes.  At this point, it is not known if there
+ *        are 0, 1, 2, or 3 bytes in the FIFO.  To handle all cases, a dummy FSM
+ *        sequence is used to load one byte at a time, until a complete 32-bit
+ *        word is formed; at most, 4 bytes will need to be loaded.
+ *
+ * [1] It is theoretically possible for the FIFO to contain an arbitrary number
+ *     of bits.  However, since there are no known use-cases that leave
+ *     incomplete bytes in the FIFO, only words and bytes are considered here.
+ */
+static void stfsm_clear_fifo(struct stfsm *fsm)
+{
+	const struct stfsm_seq *seq = &stfsm_seq_load_fifo_byte;
+	uint32_t words, i;
+
+	/* 1. Clear any 32-bit words */
+	words = stfsm_fifo_available(fsm);
+	if (words) {
+		for (i = 0; i < words; i++)
+			readl(fsm->base + SPI_FAST_SEQ_DATA_REG);
+		dev_dbg(fsm->dev, "cleared %d words from FIFO\n", words);
+	}
+
+	/*
+	 * 2. Clear any remaining bytes
+	 *    - Load the FIFO, one byte at a time, until a complete 32-bit word
+	 *      is available.
+	 */
+	for (i = 0, words = 0; i < 4 && !words; i++) {
+		stfsm_load_seq(fsm, seq);
+		stfsm_wait_seq(fsm);
+		words = stfsm_fifo_available(fsm);
+	}
+
+	/*    - A single word must be available now */
+	if (words != 1) {
+		dev_err(fsm->dev, "failed to clear bytes from the data FIFO\n");
+		return;
+	}
+
+	/*    - Read the 32-bit word */
+	readl(fsm->base + SPI_FAST_SEQ_DATA_REG);
+
+	dev_dbg(fsm->dev, "cleared %d byte(s) from the data FIFO\n", 4 - i);
+}
+
 static int stfsm_write_fifo(struct stfsm *fsm, const uint32_t *buf,
 			    uint32_t size)
 {
-- 
1.9.1

[PATCH v3 02/13] mtd: st_spi_fsm: Obtain and use EMI clock

[Lee Jones]

ST's Common Clk Framework is now available. This patch ensures the FSM
makes use of it by obtaining and enabling the EMI clock. If system fails
to provide the EMI clock, we bomb out.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 6e4d3bfe..fac0fe9 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -24,6 +24,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/of.h>
+#include <linux/clk.h>
 
 #include "serial_flash_cmds.h"
 
@@ -262,6 +263,7 @@ struct stfsm {
 	struct mtd_info		mtd;
 	struct mutex		lock;
 	struct flash_info       *info;
+	struct clk              *clk;
 
 	uint32_t                configuration;
 	uint32_t                fifo_dir_delay;
@@ -1906,8 +1908,7 @@ static void stfsm_set_freq(struct stfsm *fsm, uint32_t spi_freq)
 	uint32_t emi_freq;
 	uint32_t clk_div;
 
-	/* TODO: Make this dynamic */
-	emi_freq = STFSM_DEFAULT_EMI_FREQ;
+	emi_freq = clk_get_rate(fsm->clk);
 
 	/*
 	 * Calculate clk_div - values between 2 and 128
@@ -2057,6 +2058,18 @@ static int stfsm_probe(struct platform_device *pdev)
 		return PTR_ERR(fsm->base);
 	}
 
+	fsm->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(fsm->clk)) {
+		dev_err(fsm->dev, "Couldn't find EMI clock.\n");
+		return PTR_ERR(fsm->clk);
+	}
+
+	ret = clk_prepare_enable(fsm->clk);
+	if (ret) {
+		dev_err(fsm->dev, "Failed to enable EMI clock.\n");
+		return ret;
+	}
+
 	mutex_init(&fsm->lock);
 
 	ret = stfsm_init(fsm);
@@ -2121,6 +2134,28 @@ static int stfsm_remove(struct platform_device *pdev)
 	return mtd_device_unregister(&fsm->mtd);
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int stfsmfsm_suspend(struct device *dev)
+{
+	struct stfsm *fsm = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(fsm->clk);
+
+	return 0;
+}
+
+static int stfsmfsm_resume(struct device *dev)
+{
+	struct stfsm *fsm = dev_get_drvdata(dev);
+
+	clk_prepare_enable(fsm->clk);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(stfsm_pm_ops, stfsmfsm_suspend, stfsmfsm_resume);
+
 static const struct of_device_id stfsm_match[] = {
 	{ .compatible = "st,spi-fsm", },
 	{},
@@ -2134,6 +2169,7 @@ static struct platform_driver stfsm_driver = {
 		.name	= "st-spi-fsm",
 		.owner	= THIS_MODULE,
 		.of_match_table = stfsm_match,
+		.pm     = &stfsm_pm_ops,
 	},
 };
 module_platform_driver(stfsm_driver);
-- 
1.9.1

[PATCH v3 03/13] mtd: st_spi_fsm: dt-bindings: Deprecate generic compatible string

[Lee Jones]

This driver now obtains platform information via DT matching, which requires
a compatible string per platform.  This change introduces the new specific
strings and deprecates the old generic one.

We also take out all of the old, unused properties which are no longer
required.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 Documentation/devicetree/bindings/mtd/st-fsm.txt | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/st-fsm.txt b/Documentation/devicetree/bindings/mtd/st-fsm.txt
index c248939..4e3a16c 100644
--- a/Documentation/devicetree/bindings/mtd/st-fsm.txt
+++ b/Documentation/devicetree/bindings/mtd/st-fsm.txt
@@ -1,26 +1,20 @@
 * ST-Microelectronics SPI FSM Serial (NOR) Flash Controller
 
 Required properties:
-  - compatible : Should be "st,spi-fsm"
+  - compatible : "st,spi-fsm" is now DEPRECATED
+		 Should be one of;
+		   "st,stid127-spi-fsm"
+		   "st,stih407-spi-fsm"
+		   "st,stih416-spi-fsm"
   - reg        : Contains register's location and length.
-  - reg-names  : Should contain the reg names "spi-fsm"
   - interrupts : The interrupt number
   - pinctrl-0  : Standard Pinctrl phandle (see: pinctrl/pinctrl-bindings.txt)
-
-Optional properties:
-  - st,syscfg          : Phandle to boot-device system configuration registers
-  - st,boot-device-reg : Address of the aforementioned boot-device register(s)
-  - st,boot-device-spi : Expected boot-device value if booted via this device
+  - st,syscfg  : Phandle to boot-device system configuration registers
 
 Example:
 	spifsm: spifsm at fe902000{
-	        compatible         = "st,spi-fsm";
+		compatible         = "st,stih407-spi-fsm";
 	        reg                =  <0xfe902000 0x1000>;
-	        reg-names          = "spi-fsm";
 	        pinctrl-0          = <&pinctrl_fsm>;
 		st,syscfg	   = <&syscfg_rear>;
-	        st,boot-device-reg = <0x958>;
-	        st,boot-device-spi = <0x1a>;
-		status = "okay";
 	};
-
-- 
1.9.1

[PATCH v3 03/13] mtd: st_spi_fsm: dt-bindings: Deprecate generic compatible string

[Brian Norris]

+ devicetree

Please include devicetree at vger.kernel.org whenever you send DT patches
like this.

On Mon, Dec 15, 2014 at 11:59:10AM +0000, Lee Jones wrote:
> This driver now obtains platform information via DT matching, which requires
> a compatible string per platform.  This change introduces the new specific
> strings and deprecates the old generic one.
> 
> We also take out all of the old, unused properties which are no longer
> required.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  Documentation/devicetree/bindings/mtd/st-fsm.txt | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/st-fsm.txt b/Documentation/devicetree/bindings/mtd/st-fsm.txt
> index c248939..4e3a16c 100644
> --- a/Documentation/devicetree/bindings/mtd/st-fsm.txt
> +++ b/Documentation/devicetree/bindings/mtd/st-fsm.txt
> @@ -1,26 +1,20 @@
>  * ST-Microelectronics SPI FSM Serial (NOR) Flash Controller
>  
>  Required properties:
> -  - compatible : Should be "st,spi-fsm"
> +  - compatible : "st,spi-fsm" is now DEPRECATED

See my comments on patch 4.

In light of that, it may make sense to support both, so DTS files could
list:

	compatible = "st,stid127-spi-fsm", "st,spi-fsm";

But I'm not really privy to whatever discussion determined that you
needed to deprecate the current binding. If there really are no users of
the current upstream binding, then maybe your current deprecation plan
can work fine.

> +		 Should be one of;
> +		   "st,stid127-spi-fsm"
> +		   "st,stih407-spi-fsm"
> +		   "st,stih416-spi-fsm"
>    - reg        : Contains register's location and length.
> -  - reg-names  : Should contain the reg names "spi-fsm"
>    - interrupts : The interrupt number
>    - pinctrl-0  : Standard Pinctrl phandle (see: pinctrl/pinctrl-bindings.txt)
> -
> -Optional properties:
> -  - st,syscfg          : Phandle to boot-device system configuration registers
> -  - st,boot-device-reg : Address of the aforementioned boot-device register(s)
> -  - st,boot-device-spi : Expected boot-device value if booted via this device
> +  - st,syscfg  : Phandle to boot-device system configuration registers
>  
>  Example:
>  	spifsm: spifsm at fe902000{
> -	        compatible         = "st,spi-fsm";
> +		compatible         = "st,stih407-spi-fsm";
>  	        reg                =  <0xfe902000 0x1000>;
> -	        reg-names          = "spi-fsm";
>  	        pinctrl-0          = <&pinctrl_fsm>;
>  		st,syscfg	   = <&syscfg_rear>;
> -	        st,boot-device-reg = <0x958>;
> -	        st,boot-device-spi = <0x1a>;
> -		status = "okay";
>  	};
> -

Brian

[PATCH v3 04/13] mtd: st_spi_fsm: Fetch boot device locations from DT match tables

[Lee Jones]

To trim down on the amount of properties used by this driver and to conform
to the newly agreed method of acquiring syscfg registers/offsets, we now
obtain this information using match tables.

In the process we are deprecating the old generic compatible string and
providing 3 shiny new ones for each of the support platforms.  The
deprecated compatible string will be removed in due course.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 75 ++++++++++++++++++++++++++++++----------
 1 file changed, 57 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index fac0fe9..d82394a 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -29,6 +29,21 @@
 #include "serial_flash_cmds.h"
 
 /*
+ * FSM SPI Boot Mode Registers/Masks
+ */
+#define STID127_SYSCON_BOOT_DEV_REG	0x0D8
+#define STID127_SYSCON_BOOT_DEV_SPI	0x068
+#define STID127_SYSCON_BOOT_DEV_MASK	0x07C
+
+#define STIH407_SYSCON_BOOT_DEV_REG	0x8C4
+#define STIH407_SYSCON_BOOT_DEV_SPI	0x068
+#define STIH407_SYSCON_BOOT_DEV_MASK	0x07C
+
+#define STIH416_SYSCON_BOOT_DEV_REG	0x958
+#define STIH416_SYSCON_BOOT_DEV_SPI	0x01A
+#define STIH416_SYSCON_BOOT_DEV_MASK	0x01F
+
+/*
  * FSM SPI Controller Registers
  */
 #define SPI_CLOCKDIV			0x0010
@@ -288,6 +303,12 @@ struct seq_rw_config {
 	uint8_t         dummy_cycles;   /* No. of DUMMY cycles */
 };
 
+struct stfsm_boot_dev {
+	uint32_t		reg;
+	uint32_t		spi;
+	uint32_t		mask;
+};
+
 /* SPI Flash Device Table */
 struct flash_info {
 	char            *name;
@@ -313,6 +334,24 @@ struct flash_info {
 	int             (*config)(struct stfsm *);
 };
 
+static struct stfsm_boot_dev stfsm_stid127_data = {
+	.reg  = STID127_SYSCON_BOOT_DEV_REG,
+	.spi  = STID127_SYSCON_BOOT_DEV_SPI,
+	.mask = STID127_SYSCON_BOOT_DEV_MASK,
+};
+
+static struct stfsm_boot_dev stfsm_stih407_data = {
+	.reg  = STIH407_SYSCON_BOOT_DEV_REG,
+	.spi  = STIH407_SYSCON_BOOT_DEV_SPI,
+	.mask = STIH407_SYSCON_BOOT_DEV_MASK,
+};
+
+static struct stfsm_boot_dev stfsm_stih416_data = {
+	.reg  = STIH416_SYSCON_BOOT_DEV_REG,
+	.spi  = STIH416_SYSCON_BOOT_DEV_SPI,
+	.mask = STIH416_SYSCON_BOOT_DEV_MASK,
+};
+
 static int stfsm_n25q_config(struct stfsm *fsm);
 static int stfsm_mx25_config(struct stfsm *fsm);
 static int stfsm_s25fl_config(struct stfsm *fsm);
@@ -1977,14 +2016,23 @@ static int stfsm_init(struct stfsm *fsm)
 	return 0;
 }
 
+static const struct of_device_id stfsm_match[] = {
+	{ .compatible = "st,spi-fsm" }, /* DEPRECATED */
+	{ .compatible = "st,stid127-spi-fsm", .data = &stfsm_stid127_data },
+	{ .compatible = "st,stih407-spi-fsm", .data = &stfsm_stih407_data },
+	{ .compatible = "st,stih416-spi-fsm", .data = &stfsm_stih416_data },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stfsm_match);
+
 static void stfsm_fetch_platform_configs(struct platform_device *pdev)
 {
 	struct stfsm *fsm = platform_get_drvdata(pdev);
 	struct device_node *np = pdev->dev.of_node;
+	const struct stfsm_boot_dev *boot_dev;
+	const struct of_device_id *match;
 	struct regmap *regmap;
-	uint32_t boot_device_reg;
-	uint32_t boot_device_spi;
-	uint32_t boot_device;     /* Value we read from *boot_device_reg */
+	uint32_t boot_device;    /* Value we read from the boot dev mode pins */
 	int ret;
 
 	/* Booting from SPI NOR Flash is the default */
@@ -1998,21 +2046,18 @@ static void stfsm_fetch_platform_configs(struct platform_device *pdev)
 
 	fsm->reset_por = of_property_read_bool(np, "st,reset-por");
 
-	/* Where in the syscon the boot device information lives */
-	ret = of_property_read_u32(np, "st,boot-device-reg", &boot_device_reg);
-	if (ret)
+	match = of_match_node(stfsm_match, np);
+	if (!match)
 		goto boot_device_fail;
+	boot_dev = match->data;
 
-	/* Boot device value when booted from SPI NOR */
-	ret = of_property_read_u32(np, "st,boot-device-spi", &boot_device_spi);
+	ret = regmap_read(regmap, boot_dev->reg, &boot_device);
 	if (ret)
 		goto boot_device_fail;
 
-	ret = regmap_read(regmap, boot_device_reg, &boot_device);
-	if (ret)
-		goto boot_device_fail;
+	boot_device &= boot_dev->mask;
 
-	if (boot_device != boot_device_spi)
+	if (boot_device != boot_dev->spi)
 		fsm->booted_from_spi = false;
 
 	return;
@@ -2156,12 +2201,6 @@ static int stfsmfsm_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(stfsm_pm_ops, stfsmfsm_suspend, stfsmfsm_resume);
 
-static const struct of_device_id stfsm_match[] = {
-	{ .compatible = "st,spi-fsm", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, stfsm_match);
-
 static struct platform_driver stfsm_driver = {
 	.probe		= stfsm_probe,
 	.remove		= stfsm_remove,
-- 
1.9.1

[PATCH v3 04/13] mtd: st_spi_fsm: Fetch boot device locations from DT match tables

[Brian Norris]

+ devicetree

On Mon, Dec 15, 2014 at 11:59:11AM +0000, Lee Jones wrote:
> To trim down on the amount of properties used by this driver and to conform
> to the newly agreed method of acquiring syscfg registers/offsets, we now
> obtain this information using match tables.
> 
> In the process we are deprecating the old generic compatible string and
> providing 3 shiny new ones for each of the support platforms.  The
> deprecated compatible string will be removed in due course.

I'm not sure how this agreement was done, but by default, we expect not
to break backwards compatibility. This patch does just that.

> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/mtd/devices/st_spi_fsm.c | 75 ++++++++++++++++++++++++++++++----------
>  1 file changed, 57 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
> index fac0fe9..d82394a 100644
> --- a/drivers/mtd/devices/st_spi_fsm.c
> +++ b/drivers/mtd/devices/st_spi_fsm.c
> @@ -29,6 +29,21 @@
>  #include "serial_flash_cmds.h"
>  
>  /*
> + * FSM SPI Boot Mode Registers/Masks
> + */
> +#define STID127_SYSCON_BOOT_DEV_REG	0x0D8
> +#define STID127_SYSCON_BOOT_DEV_SPI	0x068
> +#define STID127_SYSCON_BOOT_DEV_MASK	0x07C
> +
> +#define STIH407_SYSCON_BOOT_DEV_REG	0x8C4
> +#define STIH407_SYSCON_BOOT_DEV_SPI	0x068
> +#define STIH407_SYSCON_BOOT_DEV_MASK	0x07C
> +
> +#define STIH416_SYSCON_BOOT_DEV_REG	0x958
> +#define STIH416_SYSCON_BOOT_DEV_SPI	0x01A
> +#define STIH416_SYSCON_BOOT_DEV_MASK	0x01F
> +
> +/*
>   * FSM SPI Controller Registers
>   */
>  #define SPI_CLOCKDIV			0x0010
> @@ -288,6 +303,12 @@ struct seq_rw_config {
>  	uint8_t         dummy_cycles;   /* No. of DUMMY cycles */
>  };
>  
> +struct stfsm_boot_dev {
> +	uint32_t		reg;
> +	uint32_t		spi;
> +	uint32_t		mask;
> +};
> +
>  /* SPI Flash Device Table */
>  struct flash_info {
>  	char            *name;
> @@ -313,6 +334,24 @@ struct flash_info {
>  	int             (*config)(struct stfsm *);
>  };
>  
> +static struct stfsm_boot_dev stfsm_stid127_data = {
> +	.reg  = STID127_SYSCON_BOOT_DEV_REG,
> +	.spi  = STID127_SYSCON_BOOT_DEV_SPI,
> +	.mask = STID127_SYSCON_BOOT_DEV_MASK,
> +};
> +
> +static struct stfsm_boot_dev stfsm_stih407_data = {
> +	.reg  = STIH407_SYSCON_BOOT_DEV_REG,
> +	.spi  = STIH407_SYSCON_BOOT_DEV_SPI,
> +	.mask = STIH407_SYSCON_BOOT_DEV_MASK,
> +};
> +
> +static struct stfsm_boot_dev stfsm_stih416_data = {
> +	.reg  = STIH416_SYSCON_BOOT_DEV_REG,
> +	.spi  = STIH416_SYSCON_BOOT_DEV_SPI,
> +	.mask = STIH416_SYSCON_BOOT_DEV_MASK,
> +};
> +
>  static int stfsm_n25q_config(struct stfsm *fsm);
>  static int stfsm_mx25_config(struct stfsm *fsm);
>  static int stfsm_s25fl_config(struct stfsm *fsm);
> @@ -1977,14 +2016,23 @@ static int stfsm_init(struct stfsm *fsm)
>  	return 0;
>  }
>  
> +static const struct of_device_id stfsm_match[] = {
> +	{ .compatible = "st,spi-fsm" }, /* DEPRECATED */
> +	{ .compatible = "st,stid127-spi-fsm", .data = &stfsm_stid127_data },
> +	{ .compatible = "st,stih407-spi-fsm", .data = &stfsm_stih407_data },
> +	{ .compatible = "st,stih416-spi-fsm", .data = &stfsm_stih416_data },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stfsm_match);
> +
>  static void stfsm_fetch_platform_configs(struct platform_device *pdev)
>  {
>  	struct stfsm *fsm = platform_get_drvdata(pdev);
>  	struct device_node *np = pdev->dev.of_node;
> +	const struct stfsm_boot_dev *boot_dev;
> +	const struct of_device_id *match;
>  	struct regmap *regmap;
> -	uint32_t boot_device_reg;
> -	uint32_t boot_device_spi;
> -	uint32_t boot_device;     /* Value we read from *boot_device_reg */
> +	uint32_t boot_device;    /* Value we read from the boot dev mode pins */
>  	int ret;
>  
>  	/* Booting from SPI NOR Flash is the default */
> @@ -1998,21 +2046,18 @@ static void stfsm_fetch_platform_configs(struct platform_device *pdev)
>  
>  	fsm->reset_por = of_property_read_bool(np, "st,reset-por");
>  
> -	/* Where in the syscon the boot device information lives */
> -	ret = of_property_read_u32(np, "st,boot-device-reg", &boot_device_reg);
> -	if (ret)
> +	match = of_match_node(stfsm_match, np);
> +	if (!match)
>  		goto boot_device_fail;
> +	boot_dev = match->data;
>  
> -	/* Boot device value when booted from SPI NOR */
> -	ret = of_property_read_u32(np, "st,boot-device-spi", &boot_device_spi);
> +	ret = regmap_read(regmap, boot_dev->reg, &boot_device);


^^^ NULL pointer dereference for any existing DTB that uses the
"st,spi-fsm" compatible string. So I'll NAK this patch as-is. Either we
need to completely drop the "deprecated" compatible property (what's the
point binding to it, if we're going to OOPS on it anyway), or else we
need to maintain both methods (at least for whatever deprecation period
is deemed acceptable).

>  	if (ret)
>  		goto boot_device_fail;
>  
> -	ret = regmap_read(regmap, boot_device_reg, &boot_device);
> -	if (ret)
> -		goto boot_device_fail;
> +	boot_device &= boot_dev->mask;
>  
> -	if (boot_device != boot_device_spi)
> +	if (boot_device != boot_dev->spi)
>  		fsm->booted_from_spi = false;
>  
>  	return;
> @@ -2156,12 +2201,6 @@ static int stfsmfsm_resume(struct device *dev)
>  
>  static SIMPLE_DEV_PM_OPS(stfsm_pm_ops, stfsmfsm_suspend, stfsmfsm_resume);
>  
> -static const struct of_device_id stfsm_match[] = {
> -	{ .compatible = "st,spi-fsm", },
> -	{},
> -};
> -MODULE_DEVICE_TABLE(of, stfsm_match);
> -
>  static struct platform_driver stfsm_driver = {
>  	.probe		= stfsm_probe,
>  	.remove		= stfsm_remove,

Brian

[PATCH v3 04/13] mtd: st_spi_fsm: Fetch boot device locations from DT match tables

[Lee Jones]

On Mon, 12 Jan 2015, Brian Norris wrote:

> + devicetree
> 
> On Mon, Dec 15, 2014 at 11:59:11AM +0000, Lee Jones wrote:
> > To trim down on the amount of properties used by this driver and to conform
> > to the newly agreed method of acquiring syscfg registers/offsets, we now
> > obtain this information using match tables.
> > 
> > In the process we are deprecating the old generic compatible string and
> > providing 3 shiny new ones for each of the support platforms.  The
> > deprecated compatible string will be removed in due course.
> 
> I'm not sure how this agreement was done, but by default, we expect not
> to break backwards compatibility. This patch does just that.
> 
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  drivers/mtd/devices/st_spi_fsm.c | 75 ++++++++++++++++++++++++++++++----------
> >  1 file changed, 57 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
> > index fac0fe9..d82394a 100644
> > --- a/drivers/mtd/devices/st_spi_fsm.c
> > +++ b/drivers/mtd/devices/st_spi_fsm.c
> > @@ -29,6 +29,21 @@
> >  #include "serial_flash_cmds.h"
> >  
> >  /*
> > + * FSM SPI Boot Mode Registers/Masks
> > + */
> > +#define STID127_SYSCON_BOOT_DEV_REG	0x0D8
> > +#define STID127_SYSCON_BOOT_DEV_SPI	0x068
> > +#define STID127_SYSCON_BOOT_DEV_MASK	0x07C
> > +
> > +#define STIH407_SYSCON_BOOT_DEV_REG	0x8C4
> > +#define STIH407_SYSCON_BOOT_DEV_SPI	0x068
> > +#define STIH407_SYSCON_BOOT_DEV_MASK	0x07C
> > +
> > +#define STIH416_SYSCON_BOOT_DEV_REG	0x958
> > +#define STIH416_SYSCON_BOOT_DEV_SPI	0x01A
> > +#define STIH416_SYSCON_BOOT_DEV_MASK	0x01F
> > +
> > +/*
> >   * FSM SPI Controller Registers
> >   */
> >  #define SPI_CLOCKDIV			0x0010
> > @@ -288,6 +303,12 @@ struct seq_rw_config {
> >  	uint8_t         dummy_cycles;   /* No. of DUMMY cycles */
> >  };
> >  
> > +struct stfsm_boot_dev {
> > +	uint32_t		reg;
> > +	uint32_t		spi;
> > +	uint32_t		mask;
> > +};
> > +
> >  /* SPI Flash Device Table */
> >  struct flash_info {
> >  	char            *name;
> > @@ -313,6 +334,24 @@ struct flash_info {
> >  	int             (*config)(struct stfsm *);
> >  };
> >  
> > +static struct stfsm_boot_dev stfsm_stid127_data = {
> > +	.reg  = STID127_SYSCON_BOOT_DEV_REG,
> > +	.spi  = STID127_SYSCON_BOOT_DEV_SPI,
> > +	.mask = STID127_SYSCON_BOOT_DEV_MASK,
> > +};
> > +
> > +static struct stfsm_boot_dev stfsm_stih407_data = {
> > +	.reg  = STIH407_SYSCON_BOOT_DEV_REG,
> > +	.spi  = STIH407_SYSCON_BOOT_DEV_SPI,
> > +	.mask = STIH407_SYSCON_BOOT_DEV_MASK,
> > +};
> > +
> > +static struct stfsm_boot_dev stfsm_stih416_data = {
> > +	.reg  = STIH416_SYSCON_BOOT_DEV_REG,
> > +	.spi  = STIH416_SYSCON_BOOT_DEV_SPI,
> > +	.mask = STIH416_SYSCON_BOOT_DEV_MASK,
> > +};
> > +
> >  static int stfsm_n25q_config(struct stfsm *fsm);
> >  static int stfsm_mx25_config(struct stfsm *fsm);
> >  static int stfsm_s25fl_config(struct stfsm *fsm);
> > @@ -1977,14 +2016,23 @@ static int stfsm_init(struct stfsm *fsm)
> >  	return 0;
> >  }
> >  
> > +static const struct of_device_id stfsm_match[] = {
> > +	{ .compatible = "st,spi-fsm" }, /* DEPRECATED */
> > +	{ .compatible = "st,stid127-spi-fsm", .data = &stfsm_stid127_data },
> > +	{ .compatible = "st,stih407-spi-fsm", .data = &stfsm_stih407_data },
> > +	{ .compatible = "st,stih416-spi-fsm", .data = &stfsm_stih416_data },
> > +	{},
> > +};
> > +MODULE_DEVICE_TABLE(of, stfsm_match);
> > +
> >  static void stfsm_fetch_platform_configs(struct platform_device *pdev)
> >  {
> >  	struct stfsm *fsm = platform_get_drvdata(pdev);
> >  	struct device_node *np = pdev->dev.of_node;
> > +	const struct stfsm_boot_dev *boot_dev;
> > +	const struct of_device_id *match;
> >  	struct regmap *regmap;
> > -	uint32_t boot_device_reg;
> > -	uint32_t boot_device_spi;
> > -	uint32_t boot_device;     /* Value we read from *boot_device_reg */
> > +	uint32_t boot_device;    /* Value we read from the boot dev mode pins */
> >  	int ret;
> >  
> >  	/* Booting from SPI NOR Flash is the default */
> > @@ -1998,21 +2046,18 @@ static void stfsm_fetch_platform_configs(struct platform_device *pdev)
> >  
> >  	fsm->reset_por = of_property_read_bool(np, "st,reset-por");
> >  
> > -	/* Where in the syscon the boot device information lives */
> > -	ret = of_property_read_u32(np, "st,boot-device-reg", &boot_device_reg);
> > -	if (ret)
> > +	match = of_match_node(stfsm_match, np);
> > +	if (!match)
> >  		goto boot_device_fail;
> > +	boot_dev = match->data;
> >  
> > -	/* Boot device value when booted from SPI NOR */
> > -	ret = of_property_read_u32(np, "st,boot-device-spi", &boot_device_spi);
> > +	ret = regmap_read(regmap, boot_dev->reg, &boot_device);
> 
> 
> ^^^ NULL pointer dereference for any existing DTB that uses the
> "st,spi-fsm" compatible string. So I'll NAK this patch as-is. Either we
> need to completely drop the "deprecated" compatible property (what's the
> point binding to it, if we're going to OOPS on it anyway), or else we
> need to maintain both methods (at least for whatever deprecation period
> is deemed acceptable).

Great spot Brian.

So, as I explained in reply to the cover-letter, this driver isn't
being used in anger yet.  As the driver is still considered
in-progress I take the view that it's perfectly okay to remove the old
compatible string and not match on it at all.

I'll make the necessary changes and re-submit.

> >  	if (ret)
> >  		goto boot_device_fail;
> >  
> > -	ret = regmap_read(regmap, boot_device_reg, &boot_device);
> > -	if (ret)
> > -		goto boot_device_fail;
> > +	boot_device &= boot_dev->mask;
> >  
> > -	if (boot_device != boot_device_spi)
> > +	if (boot_device != boot_dev->spi)
> >  		fsm->booted_from_spi = false;
> >  
> >  	return;
> > @@ -2156,12 +2201,6 @@ static int stfsmfsm_resume(struct device *dev)
> >  
> >  static SIMPLE_DEV_PM_OPS(stfsm_pm_ops, stfsmfsm_suspend, stfsmfsm_resume);
> >  
> > -static const struct of_device_id stfsm_match[] = {
> > -	{ .compatible = "st,spi-fsm", },
> > -	{},
> > -};
> > -MODULE_DEVICE_TABLE(of, stfsm_match);
> > -
> >  static struct platform_driver stfsm_driver = {
> >  	.probe		= stfsm_probe,
> >  	.remove		= stfsm_remove,
> 
> Brian

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 05/13] mtd: st_spi_fsm: Fix [-Wsign-compare] build warning

[Lee Jones]

drivers/mtd/devices/st_spi_fsm.c:1647:17:
  warning: comparison between signed and unsigned integer expressions

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index d82394a..63a84d2 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -1625,11 +1625,11 @@ static int stfsm_write(struct stfsm *fsm, const uint8_t *buf,
 	uint32_t size_lb;
 	uint32_t size_mop;
 	uint32_t tmp[4];
+	uint32_t i;
 	uint32_t page_buf[FLASH_PAGESIZE_32];
 	uint8_t *t = (uint8_t *)&tmp;
 	const uint8_t *p;
 	int ret;
-	int i;
 
 	dev_dbg(fsm->dev, "writing %d bytes to 0x%08x\n", size, offset);
 
-- 
1.9.1

[PATCH v3 06/13] mtd: st_spi_fsm: Add support for Micron N25Q512A

[Lee Jones]

From: Nunzio Raciti <nunzio.raciti@st.com>

This patch adds support for the Micron N25Q512A device as required
by the B2147 (STiD127) board.

Signed-off-by: Nunzio Raciti <nunzio.raciti@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 63a84d2..7e6cd26 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -413,6 +413,8 @@ static struct flash_info flash_types[] = {
 	  stfsm_n25q_config },
 	{ "n25q256", 0x20ba19, 0, 64 * 1024,  512,
 	  N25Q_FLAG | FLASH_FLAG_32BIT_ADDR, 108, stfsm_n25q_config },
+	{ "n25q512", 0x20ba20, 0, 64 * 1024,  1024,
+	  N25Q_FLAG | FLASH_FLAG_32BIT_ADDR, 108, stfsm_n25q_config},
 
 	/*
 	 * Spansion S25FLxxxP
-- 
1.9.1

[PATCH v3 07/13] mtd: st_spi_fsm: Add support for N25Q512 and N25Q00A devices

[Lee Jones]

From: Angus Clark <angus.clark@st.com>

This patch adds support for the Micron N25Q512 and N25Q00A Serial Flash devices.
Unlike previous Micron devices, it is now mandatory to check the Flags Status
Register following a Write or Erase operation.  The N25Q512A device presents a
further complication in that different variants of the device use different
opcodes for the WRITE_1_4_4 operation.  Since there is no easy way to determine
at runtime which variant is being used, FLASH_CAPS_WRITE_1_4_4 support is
removed for N25Q512 devices, resulting in WRITE_1_1_4 being used instead.

The following devices have been tested:

    b2000C + N25Q512A13GSF40G
    b2000C + N25Q00AA13GSF40G
    b2147A + N25Q512A83GSF40X

Signed-off-by: Angus Clark <angus.clark@st.com>
Signed-off-by: Carmelo Amoroso <carmelo.amoroso@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 96 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 91 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 7e6cd26..4fac169 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -240,12 +240,25 @@
 #define S25FL_STATUS_E_ERR     0x20
 #define S25FL_STATUS_P_ERR     0x40
 
+/* N25Q - READ/WRITE/CLEAR NON/VOLATILE STATUS/CONFIG Registers */
+#define N25Q_CMD_RFSR          0x70
+#define N25Q_CMD_CLFSR         0x50
 #define N25Q_CMD_WRVCR         0x81
 #define N25Q_CMD_RDVCR         0x85
 #define N25Q_CMD_RDVECR        0x65
 #define N25Q_CMD_RDNVCR        0xb5
 #define N25Q_CMD_WRNVCR        0xb1
 
+/* N25Q Flags Status Register: Error Flags */
+#define N25Q_FLAGS_ERR_ERASE   BIT(5)
+#define N25Q_FLAGS_ERR_PROG    BIT(4)
+#define N25Q_FLAGS_ERR_VPP     BIT(3)
+#define N25Q_FLAGS_ERR_PROT    BIT(1)
+#define N25Q_FLAGS_ERROR	(N25Q_FLAGS_ERR_ERASE   | \
+				N25Q_FLAGS_ERR_PROG    | \
+				N25Q_FLAGS_ERR_VPP     | \
+				N25Q_FLAGS_ERR_PROT)
+
 #define FLASH_PAGESIZE         256			/* In Bytes    */
 #define FLASH_PAGESIZE_32      (FLASH_PAGESIZE / 4)	/* In uint32_t */
 #define FLASH_MAX_BUSY_WAIT    (300 * HZ)	/* Maximum 'CHIPERASE' time */
@@ -257,6 +270,7 @@
 #define CFG_WRITE_TOGGLE_32BIT_ADDR    0x00000002
 #define CFG_ERASESEC_TOGGLE_32BIT_ADDR 0x00000008
 #define CFG_S25FL_CHECK_ERROR_FLAGS    0x00000010
+#define CFG_N25Q_CHECK_ERROR_FLAGS     0x00000020
 
 struct stfsm_seq {
 	uint32_t data_size;
@@ -399,6 +413,7 @@ static struct flash_info flash_types[] = {
 	  (MX25_FLAG | FLASH_FLAG_32BIT_ADDR | FLASH_FLAG_RESET), 70,
 	  stfsm_mx25_config},
 
+	/* Micron N25Qxxx */
 #define N25Q_FLAG (FLASH_FLAG_READ_WRITE       |	\
 		   FLASH_FLAG_READ_FAST         |	\
 		   FLASH_FLAG_READ_1_1_2        |	\
@@ -411,10 +426,29 @@ static struct flash_info flash_types[] = {
 		   FLASH_FLAG_WRITE_1_4_4)
 	{ "n25q128", 0x20ba18, 0, 64 * 1024,  256, N25Q_FLAG, 108,
 	  stfsm_n25q_config },
-	{ "n25q256", 0x20ba19, 0, 64 * 1024,  512,
-	  N25Q_FLAG | FLASH_FLAG_32BIT_ADDR, 108, stfsm_n25q_config },
-	{ "n25q512", 0x20ba20, 0, 64 * 1024,  1024,
-	  N25Q_FLAG | FLASH_FLAG_32BIT_ADDR, 108, stfsm_n25q_config},
+
+	/* Micron N25Q256/N25Q512/N25Q00A (32-bit ADDR devices)
+	*
+	* Versions are available with or without a dedicated RESET# pin
+	* (e.g. N25Q512A83GSF40G vs. N25Q512A13GSF40G).  To complicate matters,
+	* the versions that include a RESET# pin (Feature Set = 8) require a
+	* different opcode for the FLASH_CMD_WRITE_1_4_4 command.
+	* Unfortunately it is not possible to determine easily at run-time
+	* which version is being used.  We therefore remove support for
+	* FLASH_FLAG_WRITE_1_4_4 (falling back to FLASH_FLAG_WRITE_1_1_4), and
+	* defer overall support for RESET# to the board-level platform/Device
+	* Tree property "reset-signal".
+	*/
+#define N25Q_32BIT_ADDR_FLAG  ((N25Q_FLAG              |	\
+				FLASH_FLAG_32BIT_ADDR  |	\
+				FLASH_FLAG_RESET)      &	\
+			       ~FLASH_FLAG_WRITE_1_4_4)
+	{ "n25q256", 0x20ba19,      0, 64 * 1024,   512,
+	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
+	{ "n25q512", 0x20ba20, 0x1000, 64 * 1024,  1024,
+	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
+	{ "n25q00a", 0x20ba21, 0x1000, 64 * 1024,  2048,
+	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
 
 	/*
 	 * Spansion S25FLxxxP
@@ -892,6 +926,30 @@ static int stfsm_write_fifo(struct stfsm *fsm, const uint32_t *buf,
 	return size;
 }
 
+static int n25q_clear_flags(struct stfsm *fsm)
+{
+	struct stfsm_seq seq = {
+		.seq_opc[0] = (SEQ_OPC_PADS_1 |
+			       SEQ_OPC_CYCLES(8) |
+			       SEQ_OPC_OPCODE(N25Q_CMD_CLFSR) |
+			       SEQ_OPC_CSDEASSERT),
+		.seq = {
+			STFSM_INST_CMD1,
+			STFSM_INST_STOP,
+		},
+		.seq_cfg = (SEQ_CFG_PADS_1 |
+			    SEQ_CFG_READNOTWRITE |
+			    SEQ_CFG_CSDEASSERT |
+			    SEQ_CFG_STARTSEQ),
+	};
+
+	stfsm_load_seq(fsm, &seq);
+
+	stfsm_wait_seq(fsm);
+
+	return 0;
+}
+
 static int stfsm_enter_32bit_addr(struct stfsm *fsm, int enter)
 {
 	struct stfsm_seq *seq = &fsm->stfsm_seq_en_32bit_addr;
@@ -1252,10 +1310,18 @@ static int stfsm_mx25_config(struct stfsm *fsm)
 static int stfsm_n25q_config(struct stfsm *fsm)
 {
 	uint32_t flags = fsm->info->flags;
-	uint8_t vcr;
+	uint8_t vcr, sta;
 	int ret = 0;
 	bool soc_reset;
 
+	/*
+	 * Check/Clear Error Flags
+	 */
+	fsm->configuration |= CFG_N25Q_CHECK_ERROR_FLAGS;
+	stfsm_read_status(fsm, N25Q_CMD_RFSR, &sta, 1);
+	if (sta & N25Q_FLAGS_ERROR)
+		n25q_clear_flags(fsm);
+
 	/* Configure 'READ' sequence */
 	if (flags & FLASH_FLAG_32BIT_ADDR)
 		ret = stfsm_search_prepare_rw_seq(fsm, &fsm->stfsm_seq_read,
@@ -1631,6 +1697,7 @@ static int stfsm_write(struct stfsm *fsm, const uint8_t *buf,
 	uint32_t page_buf[FLASH_PAGESIZE_32];
 	uint8_t *t = (uint8_t *)&tmp;
 	const uint8_t *p;
+	uint8_t sta;
 	int ret;
 
 	dev_dbg(fsm->dev, "writing %d bytes to 0x%08x\n", size, offset);
@@ -1701,6 +1768,15 @@ static int stfsm_write(struct stfsm *fsm, const uint8_t *buf,
 	if (ret && fsm->configuration & CFG_S25FL_CHECK_ERROR_FLAGS)
 		stfsm_s25fl_clear_status_reg(fsm);
 
+	/* N25Q: Check/Clear Error Flags */
+	if (fsm->configuration & CFG_N25Q_CHECK_ERROR_FLAGS) {
+		stfsm_read_status(fsm, N25Q_CMD_RFSR, &sta, 1);
+		if (sta & N25Q_FLAGS_ERROR) {
+			n25q_clear_flags(fsm);
+			ret = -EPROTO;
+		}
+	}
+
 	/* Exit 32-bit address mode, if required */
 	if (fsm->configuration & CFG_WRITE_TOGGLE_32BIT_ADDR)
 		stfsm_enter_32bit_addr(fsm, 0);
@@ -1743,6 +1819,7 @@ static int stfsm_mtd_read(struct mtd_info *mtd, loff_t from, size_t len,
 static int stfsm_erase_sector(struct stfsm *fsm, uint32_t offset)
 {
 	struct stfsm_seq *seq = &stfsm_seq_erase_sector;
+	uint8_t sta;
 	int ret;
 
 	dev_dbg(fsm->dev, "erasing sector at 0x%08x\n", offset);
@@ -1763,6 +1840,15 @@ static int stfsm_erase_sector(struct stfsm *fsm, uint32_t offset)
 	if (ret && fsm->configuration & CFG_S25FL_CHECK_ERROR_FLAGS)
 		stfsm_s25fl_clear_status_reg(fsm);
 
+	/* N25Q: Check/Clear Error Flags */
+	if (fsm->configuration & CFG_N25Q_CHECK_ERROR_FLAGS) {
+		stfsm_read_status(fsm, N25Q_CMD_RFSR, &sta, 1);
+		if (sta & N25Q_FLAGS_ERROR) {
+			n25q_clear_flags(fsm);
+			ret = -EPROTO;
+		}
+	}
+
 	/* Exit 32-bit address mode, if required */
 	if (fsm->configuration & CFG_ERASESEC_TOGGLE_32BIT_ADDR)
 		stfsm_enter_32bit_addr(fsm, 0);
-- 
1.9.1

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Lee Jones]

The previous code was based on 3-byte JEDEC IDs, with a possible 2-byte
extension.  However, devices are now emerging that return 6 or more bytes of
READID data and the additional bytes are required to differentiate between
variants or generations of similar devices.

This patch refactors the device table and JEDEC probe code to handle arbitrary
length READIDs, with the standard JEDEC definition now becoming a special case.
Functionally, there should be no change in behaviour.  A subsequent patch will
update the table with extended READIDs where applicable.

Signed-off-by: Angus Clark <angus.clark@st.com>
Signed-off-by: Carmelo Amoroso <carmelo.amoroso@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 206 ++++++++++++++++++++++-----------------
 1 file changed, 119 insertions(+), 87 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 4fac169..949745a 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -21,6 +21,7 @@
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
 #include <linux/sched.h>
+#include <linux/sort.h>
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/of.h>
@@ -236,6 +237,11 @@
 #define FLASH_STATUS_BP2       0x10
 #define FLASH_STATUS_SRWP0     0x80
 #define FLASH_STATUS_TIMEOUT   0xff
+
+/* Maximum READID length */
+#define MAX_READID_LEN         6
+#define MAX_READID_LEN_ALIGNED ALIGN(MAX_READID_LEN, 4)
+
 /* S25FL Error Flags */
 #define S25FL_STATUS_E_ERR     0x20
 #define S25FL_STATUS_P_ERR     0x40
@@ -326,13 +332,9 @@ struct stfsm_boot_dev {
 /* SPI Flash Device Table */
 struct flash_info {
 	char            *name;
-	/*
-	 * JEDEC id zero means "no ID" (most older chips); otherwise it has
-	 * a high byte of zero plus three data bytes: the manufacturer id,
-	 * then a two byte device id.
-	 */
-	u32             jedec_id;
-	u16             ext_id;
+	/* READID data, as returned by 'SPINOR_OP_RDID' (0x9f). */
+	u8              readid[MAX_READID_LEN];
+	int             readid_len;
 	/*
 	 * The size listed here is what works with SPINOR_OP_SE, which isn't
 	 * necessarily called a "sector" by the vendor.
@@ -366,6 +368,37 @@ static struct stfsm_boot_dev stfsm_stih416_data = {
 	.mask = STIH416_SYSCON_BOOT_DEV_MASK,
 };
 
+/* Device with standard 3-byte JEDEC ID */
+#define JEDEC_INFO(_name, _jedec_id, _sector_size, _n_sectors,	\
+		   _flags, _max_freq, _config)			\
+	{							\
+		.name = (_name),				\
+		.readid[0] = ((_jedec_id) >> 16 & 0xff),	\
+		.readid[1] = ((_jedec_id) >>  8 & 0xff),	\
+		.readid[2] = ((_jedec_id) >>  0 & 0xff),	\
+		.readid_len = 3,				\
+		.sector_size = (_sector_size),			\
+		.n_sectors = (_n_sectors),			\
+		.flags = (_flags),				\
+		.max_freq = (_max_freq),			\
+		.config = (_config)				\
+	}
+
+/* Device with arbitrary-length READID */
+#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
+#define RDID_INFO(_name, _readid, _readid_len, _sector_size,	\
+		  _n_sectors, _flags, _max_freq, _config)	\
+	{							\
+		.name = (_name),				\
+		.readid = _readid,				\
+		.readid_len = _readid_len,			\
+		.sector_size = (_sector_size),			\
+		.n_sectors = (_n_sectors),			\
+		.flags = (_flags),				\
+		.max_freq = (_max_freq),			\
+		.config = (_config)				\
+	}
+
 static int stfsm_n25q_config(struct stfsm *fsm);
 static int stfsm_mx25_config(struct stfsm *fsm);
 static int stfsm_s25fl_config(struct stfsm *fsm);
@@ -378,19 +411,19 @@ static struct flash_info flash_types[] = {
 	 * (eg faster operating frequency)
 	 */
 #define M25P_FLAG (FLASH_FLAG_READ_WRITE | FLASH_FLAG_READ_FAST)
-	{ "m25p40",  0x202013, 0,  64 * 1024,   8, M25P_FLAG, 25, NULL },
-	{ "m25p80",  0x202014, 0,  64 * 1024,  16, M25P_FLAG, 25, NULL },
-	{ "m25p16",  0x202015, 0,  64 * 1024,  32, M25P_FLAG, 25, NULL },
-	{ "m25p32",  0x202016, 0,  64 * 1024,  64, M25P_FLAG, 50, NULL },
-	{ "m25p64",  0x202017, 0,  64 * 1024, 128, M25P_FLAG, 50, NULL },
-	{ "m25p128", 0x202018, 0, 256 * 1024,  64, M25P_FLAG, 50, NULL },
+	JEDEC_INFO("m25p40",  0x202013,  64 * 1024,   8, M25P_FLAG, 25, NULL),
+	JEDEC_INFO("m25p80",  0x202014,  64 * 1024,  16, M25P_FLAG, 25, NULL),
+	JEDEC_INFO("m25p16",  0x202015,  64 * 1024,  32, M25P_FLAG, 25, NULL),
+	JEDEC_INFO("m25p32",  0x202016,  64 * 1024,  64, M25P_FLAG, 50, NULL),
+	JEDEC_INFO("m25p64",  0x202017,  64 * 1024, 128, M25P_FLAG, 50, NULL),
+	JEDEC_INFO("m25p128", 0x202018, 256 * 1024,  64, M25P_FLAG, 50, NULL),
 
 #define M25PX_FLAG (FLASH_FLAG_READ_WRITE      |	\
 		    FLASH_FLAG_READ_FAST        |	\
 		    FLASH_FLAG_READ_1_1_2       |	\
 		    FLASH_FLAG_WRITE_1_1_2)
-	{ "m25px32", 0x207116, 0,  64 * 1024,  64, M25PX_FLAG, 75, NULL },
-	{ "m25px64", 0x207117, 0,  64 * 1024, 128, M25PX_FLAG, 75, NULL },
+	JEDEC_INFO("m25px32", 0x207116,  64 * 1024,  64, M25PX_FLAG, 75, NULL),
+	JEDEC_INFO("m25px64", 0x207117,  64 * 1024, 128, M25PX_FLAG, 75, NULL),
 
 	/* Macronix MX25xxx
 	 *     - Support for 'FLASH_FLAG_WRITE_1_4_4' is omitted for devices
@@ -403,15 +436,12 @@ static struct flash_info flash_types[] = {
 		   FLASH_FLAG_READ_1_1_4        |	\
 		   FLASH_FLAG_SE_4K             |	\
 		   FLASH_FLAG_SE_32K)
-	{ "mx25l3255e",  0xc29e16, 0, 64 * 1024, 64,
-	  (MX25_FLAG | FLASH_FLAG_WRITE_1_4_4), 86,
-	  stfsm_mx25_config},
-	{ "mx25l25635e", 0xc22019, 0, 64*1024, 512,
-	  (MX25_FLAG | FLASH_FLAG_32BIT_ADDR | FLASH_FLAG_RESET), 70,
-	  stfsm_mx25_config },
-	{ "mx25l25655e", 0xc22619, 0, 64*1024, 512,
-	  (MX25_FLAG | FLASH_FLAG_32BIT_ADDR | FLASH_FLAG_RESET), 70,
-	  stfsm_mx25_config},
+	JEDEC_INFO("mx25l3255e",  0xc29e16, 64 * 1024, 64,
+		   (MX25_FLAG | FLASH_FLAG_WRITE_1_4_4), 86, stfsm_mx25_config),
+	JEDEC_INFO("mx25l25635e", 0xc22019, 64 * 1024, 512,
+		   (MX25_FLAG | FLASH_FLAG_RESET), 70, stfsm_mx25_config),
+	JEDEC_INFO("mx25l25655e", 0xc22619, 64 * 1024, 512,
+		   (MX25_FLAG | FLASH_FLAG_RESET), 70, stfsm_mx25_config),
 
 	/* Micron N25Qxxx */
 #define N25Q_FLAG (FLASH_FLAG_READ_WRITE       |	\
@@ -424,8 +454,8 @@ static struct flash_info flash_types[] = {
 		   FLASH_FLAG_WRITE_1_2_2       |	\
 		   FLASH_FLAG_WRITE_1_1_4       |	\
 		   FLASH_FLAG_WRITE_1_4_4)
-	{ "n25q128", 0x20ba18, 0, 64 * 1024,  256, N25Q_FLAG, 108,
-	  stfsm_n25q_config },
+	JEDEC_INFO("n25q128", 0x20ba18, 64 * 1024,  256,
+		   N25Q_FLAG, 108, stfsm_n25q_config),
 
 	/* Micron N25Q256/N25Q512/N25Q00A (32-bit ADDR devices)
 	*
@@ -443,12 +473,12 @@ static struct flash_info flash_types[] = {
 				FLASH_FLAG_32BIT_ADDR  |	\
 				FLASH_FLAG_RESET)      &	\
 			       ~FLASH_FLAG_WRITE_1_4_4)
-	{ "n25q256", 0x20ba19,      0, 64 * 1024,   512,
-	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
-	{ "n25q512", 0x20ba20, 0x1000, 64 * 1024,  1024,
-	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
-	{ "n25q00a", 0x20ba21, 0x1000, 64 * 1024,  2048,
-	  N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config},
+	JEDEC_INFO("n25q256", 0x20ba19, 64 * 1024,   512,
+		   N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config),
+	RDID_INFO("n25q512", RDID({0x20, 0xba, 0x20, 0x10, 0x00}), 5, 64 * 1024,
+		  1024, N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config),
+	RDID_INFO("n25q00a", RDID({0x20, 0xba, 0x21, 0x10, 0x00}), 5, 64 * 1024,
+		  2048, N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config),
 
 	/*
 	 * Spansion S25FLxxxP
@@ -461,12 +491,12 @@ static struct flash_info flash_types[] = {
 			FLASH_FLAG_READ_1_4_4   |	\
 			FLASH_FLAG_WRITE_1_1_4  |	\
 			FLASH_FLAG_READ_FAST)
-	{ "s25fl032p",  0x010215, 0x4d00,  64 * 1024,  64, S25FLXXXP_FLAG, 80,
-	  stfsm_s25fl_config},
-	{ "s25fl129p0", 0x012018, 0x4d00, 256 * 1024,  64, S25FLXXXP_FLAG, 80,
-	  stfsm_s25fl_config },
-	{ "s25fl129p1", 0x012018, 0x4d01,  64 * 1024, 256, S25FLXXXP_FLAG, 80,
-	  stfsm_s25fl_config },
+	RDID_INFO("s25fl032p", RDID({0x01, 0x02, 0x15, 0x4d, 0x00}), 5,
+		  64 * 1024,  64, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl129p0", RDID({0x01, 0x20, 0x18, 0x4d, 0x00}), 5,
+		  256 * 1024,  64, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl129p1", RDID({0x01, 0x20, 0x18, 0x4d, 0x01}), 5,
+		  64 * 1024, 256, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
 
 	/*
 	 * Spansion S25FLxxxS
@@ -480,25 +510,24 @@ static struct flash_info flash_types[] = {
 #define S25FLXXXS_FLAG (S25FLXXXP_FLAG         |	\
 			FLASH_FLAG_RESET        |	\
 			FLASH_FLAG_DYB_LOCKING)
-	{ "s25fl128s0", 0x012018, 0x0300,  256 * 1024, 64, S25FLXXXS_FLAG, 80,
-	  stfsm_s25fl_config },
-	{ "s25fl128s1", 0x012018, 0x0301,  64 * 1024, 256, S25FLXXXS_FLAG, 80,
-	  stfsm_s25fl_config },
-	{ "s25fl256s0", 0x010219, 0x4d00, 256 * 1024, 128,
-	  S25FLXXXS_FLAG | FLASH_FLAG_32BIT_ADDR, 80, stfsm_s25fl_config },
-	{ "s25fl256s1", 0x010219, 0x4d01,  64 * 1024, 512,
-	  S25FLXXXS_FLAG | FLASH_FLAG_32BIT_ADDR, 80, stfsm_s25fl_config },
-
-	/* Winbond -- w25x "blocks" are 64K, "sectors" are 4KiB */
+	RDID_INFO("s25fl128s0", RDID({0x01, 0x20, 0x18, 0x03, 0x00}), 5,
+		  256 * 1024, 64, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl128s1", RDID({0x01, 0x20, 0x18, 0x03, 0x01}), 5,
+		  64 * 1024, 256, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl256s0", RDID({0x01, 0x02, 0x19, 0x4d, 0x00}), 5,
+		  256 * 1024, 128, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl256s1", RDID({0x01, 0x02, 0x19, 0x4d, 0x01}), 5,
+		  64 * 1024, 512, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
+
 #define W25X_FLAG (FLASH_FLAG_READ_WRITE       |	\
 		   FLASH_FLAG_READ_FAST         |	\
 		   FLASH_FLAG_READ_1_1_2        |	\
 		   FLASH_FLAG_WRITE_1_1_2)
-	{ "w25x40",  0xef3013, 0,  64 * 1024,   8, W25X_FLAG, 75, NULL },
-	{ "w25x80",  0xef3014, 0,  64 * 1024,  16, W25X_FLAG, 75, NULL },
-	{ "w25x16",  0xef3015, 0,  64 * 1024,  32, W25X_FLAG, 75, NULL },
-	{ "w25x32",  0xef3016, 0,  64 * 1024,  64, W25X_FLAG, 75, NULL },
-	{ "w25x64",  0xef3017, 0,  64 * 1024, 128, W25X_FLAG, 75, NULL },
+	JEDEC_INFO("w25x40", 0xef3013, 64 * 1024,   8, W25X_FLAG, 75, NULL),
+	JEDEC_INFO("w25x80", 0xef3014, 64 * 1024,  16, W25X_FLAG, 75, NULL),
+	JEDEC_INFO("w25x16", 0xef3015, 64 * 1024,  32, W25X_FLAG, 75, NULL),
+	JEDEC_INFO("w25x32", 0xef3016, 64 * 1024,  64, W25X_FLAG, 75, NULL),
+	JEDEC_INFO("w25x64", 0xef3017, 64 * 1024, 128, W25X_FLAG, 75, NULL),
 
 	/* Winbond -- w25q "blocks" are 64K, "sectors" are 4KiB */
 #define W25Q_FLAG (FLASH_FLAG_READ_WRITE       |	\
@@ -508,17 +537,16 @@ static struct flash_info flash_types[] = {
 		   FLASH_FLAG_READ_1_1_4        |	\
 		   FLASH_FLAG_READ_1_4_4        |	\
 		   FLASH_FLAG_WRITE_1_1_4)
-	{ "w25q80",  0xef4014, 0,  64 * 1024,  16, W25Q_FLAG, 80,
-	  stfsm_w25q_config },
-	{ "w25q16",  0xef4015, 0,  64 * 1024,  32, W25Q_FLAG, 80,
-	  stfsm_w25q_config },
-	{ "w25q32",  0xef4016, 0,  64 * 1024,  64, W25Q_FLAG, 80,
-	  stfsm_w25q_config },
-	{ "w25q64",  0xef4017, 0,  64 * 1024, 128, W25Q_FLAG, 80,
-	  stfsm_w25q_config },
-
-	/* Sentinel */
-	{ NULL, 0x000000, 0, 0, 0, 0, 0, NULL },
+	JEDEC_INFO("w25q80", 0xef4014, 64 * 1024,  16,
+		   W25Q_FLAG, 80, stfsm_w25q_config),
+	JEDEC_INFO("w25q16", 0xef4015, 64 * 1024,  32,
+		   W25Q_FLAG, 80, stfsm_w25q_config),
+	JEDEC_INFO("w25q32", 0xef4016, 64 * 1024,  64,
+		   W25Q_FLAG, 80, stfsm_w25q_config),
+	JEDEC_INFO("w25q64", 0xef4017, 64 * 1024, 128,
+		   W25Q_FLAG, 80, stfsm_w25q_config),
+
+	{ },
 };
 
 /*
@@ -649,7 +677,7 @@ static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
 #define W25Q_STATUS_QE			(0x1 << 1)
 
 static struct stfsm_seq stfsm_seq_read_jedec = {
-	.data_size = TRANSFER_SIZE(8),
+	.data_size = TRANSFER_SIZE(MAX_READID_LEN_ALIGNED),
 	.seq_opc[0] = (SEQ_OPC_PADS_1 |
 		       SEQ_OPC_CYCLES(8) |
 		       SEQ_OPC_OPCODE(SPINOR_OP_RDID)),
@@ -1967,45 +1995,49 @@ out1:
 static void stfsm_read_jedec(struct stfsm *fsm, uint8_t *jedec)
 {
 	const struct stfsm_seq *seq = &stfsm_seq_read_jedec;
-	uint32_t tmp[2];
+	uint32_t tmp[MAX_READID_LEN_ALIGNED / 4];
 
 	stfsm_load_seq(fsm, seq);
 
-	stfsm_read_fifo(fsm, tmp, 8);
+	stfsm_read_fifo(fsm, tmp, MAX_READID_LEN_ALIGNED);
 
-	memcpy(jedec, tmp, 5);
+	memcpy(jedec, tmp, MAX_READID_LEN);
 
 	stfsm_wait_seq(fsm);
 }
 
+static int stfsm_cmp_flash_info_readid_len(const void *a, const void *b)
+{
+	return ((struct flash_info *)b)->readid_len -
+		((struct flash_info *)a)->readid_len;
+}
+
 static struct flash_info *stfsm_jedec_probe(struct stfsm *fsm)
 {
-	struct flash_info	*info;
-	u16                     ext_jedec;
-	u32			jedec;
-	u8			id[5];
+	uint8_t readid[MAX_READID_LEN];
+	char readid_str[MAX_READID_LEN * 3 + 1];
+	struct flash_info *info;
 
-	stfsm_read_jedec(fsm, id);
+	stfsm_read_jedec(fsm, readid);
 
-	jedec     = id[0] << 16 | id[1] << 8 | id[2];
-	/*
-	 * JEDEC also defines an optional "extended device information"
-	 * string for after vendor-specific data, after the three bytes
-	 * we use here. Supporting some chips might require using it.
-	 */
-	ext_jedec = id[3] << 8  | id[4];
+	hex_dump_to_buffer(readid, MAX_READID_LEN, 16, 1,
+			   readid_str, sizeof(readid_str), 0);
 
-	dev_dbg(fsm->dev, "JEDEC =  0x%08x [%02x %02x %02x %02x %02x]\n",
-		jedec, id[0], id[1], id[2], id[3], id[4]);
+	dev_dbg(fsm->dev, "READID = %s\n", readid_str);
+
+	/* The 'readid' may match multiple entries in the table.  To ensure we
+	 * retrieve the most specific match, the table is sorted in order of
+	 * 'readid_len'.
+	 */
+	sort(flash_types, ARRAY_SIZE(flash_types) - 1,
+	     sizeof(struct flash_info), stfsm_cmp_flash_info_readid_len, NULL);
 
 	for (info = flash_types; info->name; info++) {
-		if (info->jedec_id == jedec) {
-			if (info->ext_id && info->ext_id != ext_jedec)
-				continue;
+		if (memcmp(info->readid, readid, info->readid_len) == 0)
 			return info;
-		}
 	}
-	dev_err(fsm->dev, "Unrecognized JEDEC id %06x\n", jedec);
+
+	dev_err(fsm->dev, "Unrecognized READID [%s]\n", readid_str);
 
 	return NULL;
 }
-- 
1.9.1

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Brian Norris]

On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> The previous code was based on 3-byte JEDEC IDs, with a possible 2-byte
> extension.  However, devices are now emerging that return 6 or more bytes of
> READID data and the additional bytes are required to differentiate between
> variants or generations of similar devices.
> 
> This patch refactors the device table and JEDEC probe code to handle arbitrary
> length READIDs, with the standard JEDEC definition now becoming a special case.
> Functionally, there should be no change in behaviour.  A subsequent patch will
> update the table with extended READIDs where applicable.

BTW, how's that promise going, where you work on adapting this driver to
the spi-nor framework? We've already done some of this same work there.

> +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */

What? What needs "protected"?

...

Brian

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Lee Jones]

On Mon, 12 Jan 2015, Brian Norris wrote:

> On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> > The previous code was based on 3-byte JEDEC IDs, with a possible 2-byte
> > extension.  However, devices are now emerging that return 6 or more bytes of
> > READID data and the additional bytes are required to differentiate between
> > variants or generations of similar devices.
> > 
> > This patch refactors the device table and JEDEC probe code to handle arbitrary
> > length READIDs, with the standard JEDEC definition now becoming a special case.
> > Functionally, there should be no change in behaviour.  A subsequent patch will
> > update the table with extended READIDs where applicable.
> 
> BTW, how's that promise going, where you work on adapting this driver to
> the spi-nor framework? We've already done some of this same work there.

I have pushed this point within ST and someone has agreed to do the
work.  Last I heard it relied on these patches, but I'll ask again.

> > +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
> 
> What? What needs "protected"?

You're asking me questions I can't answer I'm afraid and Angus has now
left the building.  I guess he thinks __VA_ARGS__ will prevent some
kind of overflow?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Brian Norris]

On Wed, Jan 21, 2015 at 01:02:04PM +0000, Lee Jones wrote:
> On Mon, 12 Jan 2015, Brian Norris wrote:
> > On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> > > The previous code was based on 3-byte JEDEC IDs, with a possible 2-byte
> > > extension.  However, devices are now emerging that return 6 or more bytes of
> > > READID data and the additional bytes are required to differentiate between
> > > variants or generations of similar devices.
> > > 
> > > This patch refactors the device table and JEDEC probe code to handle arbitrary
> > > length READIDs, with the standard JEDEC definition now becoming a special case.
> > > Functionally, there should be no change in behaviour.  A subsequent patch will
> > > update the table with extended READIDs where applicable.
> > 
> > BTW, how's that promise going, where you work on adapting this driver to
> > the spi-nor framework? We've already done some of this same work there.
> 
> I have pushed this point within ST and someone has agreed to do the
> work.  Last I heard it relied on these patches, but I'll ask again.

OK.

> > > +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
> > 
> > What? What needs "protected"?
> 
> You're asking me questions I can't answer I'm afraid and Angus has now
> left the building.  I guess he thinks __VA_ARGS__ will prevent some
> kind of overflow?

If you don't understand your own code, how can I be expected to maintain
it? This one's pretty trivial and harmless, but an accumulation of
answers like this don't exactly put me in a good mood.

FWIW, I expect the comment has nothing to do with the __VA_ARGS__; it's
just commenting that he has placed a macro around the array just in case
somebody needs/wants to rearrange formats later. That way, we don't
necessarily have to rewrite the whole table, but can just change the
macros.

So the __VA_ARGS__ is just there to make the compiler happy (it thinks
an array argument to a macro actually looks like more than one
argument), and the comment is only mildly descriptive of its purpose.

Brian

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Lee Jones]

On Thu, 05 Feb 2015, Brian Norris wrote:

> On Wed, Jan 21, 2015 at 01:02:04PM +0000, Lee Jones wrote:
> > On Mon, 12 Jan 2015, Brian Norris wrote:
> > > On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> > > > The previous code was based on 3-byte JEDEC IDs, with a possible 2-byte
> > > > extension.  However, devices are now emerging that return 6 or more bytes of
> > > > READID data and the additional bytes are required to differentiate between
> > > > variants or generations of similar devices.
> > > > 
> > > > This patch refactors the device table and JEDEC probe code to handle arbitrary
> > > > length READIDs, with the standard JEDEC definition now becoming a special case.
> > > > Functionally, there should be no change in behaviour.  A subsequent patch will
> > > > update the table with extended READIDs where applicable.
> > > 
> > > BTW, how's that promise going, where you work on adapting this driver to
> > > the spi-nor framework? We've already done some of this same work there.
> > 
> > I have pushed this point within ST and someone has agreed to do the
> > work.  Last I heard it relied on these patches, but I'll ask again.
> 
> OK.
> 
> > > > +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
> > > 
> > > What? What needs "protected"?
> > 
> > You're asking me questions I can't answer I'm afraid and Angus has now
> > left the building.  I guess he thinks __VA_ARGS__ will prevent some
> > kind of overflow?
> 
> If you don't understand your own code, how can I be expected to maintain
> it? This one's pretty trivial and harmless, but an accumulation of
> answers like this don't exactly put me in a good mood.

I have never written a line of code that I didn't understand and I
think you are quite aware that this has never been my 'own code'.

With regards to the previous accumulation of 'answers like this'; I
have never been, nor have any desire to be an expert on Memory
Technology Devices.  I was asked to upstream ST's NAND and NOR drivers
with support of the local expert and author; however, for reasons not
under neither of our controls, he has now left the company.  With him
went all of the historical reasoning for all for the questions you've
asked.  This is not my domain and have found it pretty tough to keep
up with all of your demands, both on this and the NAND driver but I
have been pretty subservient and keep up with them I have.  I am not
privy to any of the author's thoughts or reasons for any decisions
taken.  I can only hope that his comments might have some meaning to a
like minded 'expert' such as yourself.  If you don't know something,
then the chances are slight that I will be able to answer your query.

> FWIW, I expect the comment has nothing to do with the __VA_ARGS__; it's
> just commenting that he has placed a macro around the array just in case
> somebody needs/wants to rearrange formats later. That way, we don't
> necessarily have to rewrite the whole table, but can just change the
> macros.
> 
> So the __VA_ARGS__ is just there to make the compiler happy (it thinks
> an array argument to a macro actually looks like more than one
> argument), and the comment is only mildly descriptive of its purpose.

I was present Passing Variable Arguments 101, so I'm aware of what
__VA_ARGS__ and "..." do at a functional level.

So it looks like you had a better idea of what Angus was trying to
achieve than I do.  Perhaps your question should have been more
specific.  I guess it's just the comment that you are unhappy with.  I
can remove it if it makes you happy.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Brian Norris]

On Tue, Feb 10, 2015 at 05:04:33PM +0800, Lee Jones wrote:
> On Thu, 05 Feb 2015, Brian Norris wrote:
> > On Wed, Jan 21, 2015 at 01:02:04PM +0000, Lee Jones wrote:
> > > On Mon, 12 Jan 2015, Brian Norris wrote:
> > > > On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> > > > > +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
> > > > 
> > > > What? What needs "protected"?
> > > 
> > > You're asking me questions I can't answer I'm afraid and Angus has now
> > > left the building.  I guess he thinks __VA_ARGS__ will prevent some
> > > kind of overflow?
> > 
> > If you don't understand your own code, how can I be expected to maintain
> > it? This one's pretty trivial and harmless, but an accumulation of
> > answers like this don't exactly put me in a good mood.
> 
> I have never written a line of code that I didn't understand and I
> think you are quite aware that this has never been my 'own code'.

Stop deflecting. I don't care that you didn't write this code. If you're
asking me to apply this upstream, then you own it. Or else, hire another
expert who is willing to own your drivers.

> With regards to the previous accumulation of 'answers like this'; I
> have never been, nor have any desire to be an expert on Memory
> Technology Devices.  I was asked to upstream ST's NAND and NOR drivers
> with support of the local expert and author; however, for reasons not
> under neither of our controls, he has now left the company.  With him
> went all of the historical reasoning for all for the questions you've
> asked.  This is not my domain and have found it pretty tough to keep
> up with all of your demands, both on this and the NAND driver but I
> have been pretty subservient and keep up with them I have.  I am not
> privy to any of the author's thoughts or reasons for any decisions
> taken.  I can only hope that his comments might have some meaning to a
> like minded 'expert' such as yourself.  If you don't know something,
> then the chances are slight that I will be able to answer your query.

Well, I think we have a fundamentally different view of what upstream
development means, then. My job is not to worry about your company's
internal problems. My job is not to worry about your company's
development schedules. My job is not to be your on-call MTD expert.

My job is to make sure that upstream MTD contains relatively clean,
maintainable code, and to accept patches that generally improve the
codebase (i.e., bug fixes, refactoring, feature improvements).

Recall that at least one prominent kernel developer has suggested that
it's actually not in a maintainer's interests to accept random
contributors' code. The contributor's job is to give the maintainer no
reason to reject his or her code. When you repeatedly claim to not
understand the code you're sending me, that sets off warning bells that
make it significantly harder to handle your code.

Perhaps we'd all be better off if I simply removed your driver from
upstream, and you can convince your company to hire an MTD expert to
handle your upstreaming efforts.

> > FWIW, I expect the comment has nothing to do with the __VA_ARGS__; it's
> > just commenting that he has placed a macro around the array just in case
> > somebody needs/wants to rearrange formats later. That way, we don't
> > necessarily have to rewrite the whole table, but can just change the
> > macros.
> > 
> > So the __VA_ARGS__ is just there to make the compiler happy (it thinks
> > an array argument to a macro actually looks like more than one
> > argument), and the comment is only mildly descriptive of its purpose.
> 
> I was present Passing Variable Arguments 101, so I'm aware of what
> __VA_ARGS__ and "..." do at a functional level.

But this is not a '101' use case, exactly. It's there because of the
peculiarities of macros, it seems. And don't pretend to understand when
you clearly did not when I first asked. (And to be clear: I did not
understand either. That's why I asked.)

> So it looks like you had a better idea of what Angus was trying to
> achieve than I do.  Perhaps your question should have been more
> specific.  I guess it's just the comment that you are unhappy with.  I
> can remove it if it makes you happy.

No, the comment wasn't the problem, exactly. I was a bit confused, and I
wrongly assumed that you understood what you're sending me. Apparently
that was too much to assume.

Brian

[PATCH v3 08/13] mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs

[Lee Jones]

On Mon, 23 Feb 2015, Brian Norris wrote:
> On Tue, Feb 10, 2015 at 05:04:33PM +0800, Lee Jones wrote:
> > On Thu, 05 Feb 2015, Brian Norris wrote:
> > > On Wed, Jan 21, 2015 at 01:02:04PM +0000, Lee Jones wrote:
> > > > On Mon, 12 Jan 2015, Brian Norris wrote:
> > > > > On Mon, Dec 15, 2014 at 11:59:15AM +0000, Lee Jones wrote:
> > > > > > +#define RDID(...) __VA_ARGS__  /* Dummy macro to protect array argument. */
> > > > > 
> > > > > What? What needs "protected"?
> > > > 
> > > > You're asking me questions I can't answer I'm afraid and Angus has now
> > > > left the building.  I guess he thinks __VA_ARGS__ will prevent some
> > > > kind of overflow?
> > > 
> > > If you don't understand your own code, how can I be expected to maintain
> > > it? This one's pretty trivial and harmless, but an accumulation of
> > > answers like this don't exactly put me in a good mood.
> > 
> > I have never written a line of code that I didn't understand and I
> > think you are quite aware that this has never been my 'own code'.
> 
> Stop deflecting. I don't care that you didn't write this code. If you're
> asking me to apply this upstream, then you own it. 

Okay, I own it.  And I have a good grasp of how most of the driver
operates.  However, it is unreasonable to request that someone who
conducts Mainlining activities who is not the author to understand
every single line.

It's now commonplace for an 'author' and 'submitter' to be different
people.  The only difference is that normally the author is still
around to provide support.  Or at least the submitter has access to a
team who have in-depth knowledge of the hardware/driver.

I have submitted drivers and fixes all over the kernel tree and have a
fairly good knowledge of each of the subsystems, but to ask that I
become an expert in all of them -- especially subsystems as complex as
MTD is idealistic and unrealistic at best.

> Or else, hire another expert who is willing to own your drivers.

Silicon chip vendors do not have limitless resources to hire or train
experts to conduct all their upstreaming activities?  These guys are
almost always focused on customer projects/requests and have little
time for upstreaming.  It's people like us to advocate and encourage
pushing code into Mainline, as the cost of upstreaming is usually
dwarfed by the cost of migrating a huge delta to a more recent code
base.  This activity however, has not a good example of that.

> > With regards to the previous accumulation of 'answers like this'; I
> > have never been, nor have any desire to be an expert on Memory
> > Technology Devices.  I was asked to upstream ST's NAND and NOR drivers
> > with support of the local expert and author; however, for reasons not
> > under neither of our controls, he has now left the company.  With him
> > went all of the historical reasoning for all for the questions you've
> > asked.  This is not my domain and have found it pretty tough to keep
> > up with all of your demands, both on this and the NAND driver but I
> > have been pretty subservient and keep up with them I have.  I am not
> > privy to any of the author's thoughts or reasons for any decisions
> > taken.  I can only hope that his comments might have some meaning to a
> > like minded 'expert' such as yourself.  If you don't know something,
> > then the chances are slight that I will be able to answer your query.
> 
> Well, I think we have a fundamentally different view of what upstream
> development means, then. My job is not to worry about your company's
> internal problems. My job is not to worry about your company's
> development schedules. My job is not to be your on-call MTD expert.
> 
> My job is to make sure that upstream MTD contains relatively clean,
> maintainable code, and to accept patches that generally improve the
> codebase (i.e., bug fixes, refactoring, feature improvements).
> 
> Recall that at least one prominent kernel developer has suggested that
> it's actually not in a maintainer's interests to accept random
> contributors' code. The contributor's job is to give the maintainer no
> reason to reject his or her code. When you repeatedly claim to not
> understand the code you're sending me, that sets off warning bells that
> make it significantly harder to handle your code.

At the moment, what you're saying is; if your NOR Contreoller is not
exactly the same as all the others and doesn't fit inside our
mega-driver, it's not welcome.

> Perhaps we'd all be better off if I simply removed your driver from
> upstream, and you can convince your company to hire an MTD expert to
> handle your upstreaming efforts.

See my point above about the lack of limitless funds.

> > > FWIW, I expect the comment has nothing to do with the __VA_ARGS__; it's
> > > just commenting that he has placed a macro around the array just in case
> > > somebody needs/wants to rearrange formats later. That way, we don't
> > > necessarily have to rewrite the whole table, but can just change the
> > > macros.
> > > 
> > > So the __VA_ARGS__ is just there to make the compiler happy (it thinks
> > > an array argument to a macro actually looks like more than one
> > > argument), and the comment is only mildly descriptive of its purpose.
> > 
> > I was present Passing Variable Arguments 101, so I'm aware of what
> > __VA_ARGS__ and "..." do at a functional level.
> 
> But this is not a '101' use case, exactly. It's there because of the
> peculiarities of macros, it seems. And don't pretend to understand when
> you clearly did not when I first asked. (And to be clear: I did not
> understand either. That's why I asked.)

Stop twisting things.  You (unnecessarily) explained to me what
va_args was.  I was saying that I know what it is and how to declare
functions with variable arguments.  Not that I knew what the MACRO was
doing.

> > So it looks like you had a better idea of what Angus was trying to
> > achieve than I do.  Perhaps your question should have been more
> > specific.  I guess it's just the comment that you are unhappy with.  I
> > can remove it if it makes you happy.
> 
> No, the comment wasn't the problem, exactly. I was a bit confused, and I
> wrongly assumed that you understood what you're sending me. Apparently
> that was too much to assume.

Refer to my points above.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 09/13] mtd: st_spi_fsm: Update Spansion device entries

[Lee Jones]

From: Angus Clark <angus.clark@st.com>

This patch updates various Spansion device entries in the flash_types[] table:
  - Define full 6-byte READIDs for S25FL128Sx devices (and fix the 4th
    byte). This allows us to differentiate between S25FL129P and S25FL128S
    devices.
  - Add S25FL128Px device entries.

Signed-off-by: Angus Clark <angus.clark@st.com>
Signed-off-by: Carmelo Amoroso <carmelo.amoroso@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 949745a..7c024ec 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -483,6 +483,7 @@ static struct flash_info flash_types[] = {
 	/*
 	 * Spansion S25FLxxxP
 	 *     - 256KiB and 64KiB sector variants (identified by ext. JEDEC)
+	 *     - S25FL128Px devices do not support DUAL or QUAD I/O
 	 */
 #define S25FLXXXP_FLAG (FLASH_FLAG_READ_WRITE  |	\
 			FLASH_FLAG_READ_1_1_2   |	\
@@ -493,6 +494,12 @@ static struct flash_info flash_types[] = {
 			FLASH_FLAG_READ_FAST)
 	RDID_INFO("s25fl032p", RDID({0x01, 0x02, 0x15, 0x4d, 0x00}), 5,
 		  64 * 1024,  64, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
+	RDID_INFO("s25fl128p1", RDID({0x01, 0x20, 0x18, 0x03, 0x00}), 5,
+		  256 * 1024, 64,
+		  (FLASH_FLAG_READ_WRITE | FLASH_FLAG_READ_FAST), 104, NULL),
+	RDID_INFO("s25fl128p0", RDID({0x01, 0x20, 0x18, 0x03, 0x01}), 5,
+		  64 * 1024, 256,
+		  (FLASH_FLAG_READ_WRITE | FLASH_FLAG_READ_FAST), 104, NULL),
 	RDID_INFO("s25fl129p0", RDID({0x01, 0x20, 0x18, 0x4d, 0x00}), 5,
 		  256 * 1024,  64, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
 	RDID_INFO("s25fl129p1", RDID({0x01, 0x20, 0x18, 0x4d, 0x01}), 5,
@@ -506,17 +513,18 @@ static struct flash_info flash_types[] = {
 	 *       so this information is captured in the board's flags.
 	 *     - Supports 'DYB' sector protection. Depending on variant, sectors
 	 *       may default to locked state on power-on.
+	 *     - S25FL127Sx handled as S25FL128Sx
 	 */
 #define S25FLXXXS_FLAG (S25FLXXXP_FLAG         |	\
 			FLASH_FLAG_RESET        |	\
 			FLASH_FLAG_DYB_LOCKING)
-	RDID_INFO("s25fl128s0", RDID({0x01, 0x20, 0x18, 0x03, 0x00}), 5,
+	RDID_INFO("s25fl128s0", RDID({0x01, 0x20, 0x18, 0x4d, 0x00, 0x80}), 6,
 		  256 * 1024, 64, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
-	RDID_INFO("s25fl128s1", RDID({0x01, 0x20, 0x18, 0x03, 0x01}), 5,
+	RDID_INFO("s25fl128s1", RDID({0x01, 0x20, 0x18, 0x4d, 0x01, 0x80}), 6,
 		  64 * 1024, 256, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
-	RDID_INFO("s25fl256s0", RDID({0x01, 0x02, 0x19, 0x4d, 0x00}), 5,
+	RDID_INFO("s25fl256s0", RDID({0x01, 0x02, 0x19, 0x4d, 0x00, 0x80}), 6,
 		  256 * 1024, 128, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
-	RDID_INFO("s25fl256s1", RDID({0x01, 0x02, 0x19, 0x4d, 0x01}), 5,
+	RDID_INFO("s25fl256s1", RDID({0x01, 0x02, 0x19, 0x4d, 0x01, 0x80}), 6,
 		  64 * 1024, 512, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
 
 #define W25X_FLAG (FLASH_FLAG_READ_WRITE       |	\
-- 
1.9.1

[PATCH v3 10/13] mtd: st_spi_fsm: Improve busy wait handling

[Lee Jones]

From: Angus Clark <angus.clark@st.com>

In this patch, the fsm_wait_busy() function is updated to a take a timeout
parameter.  This allows us to specify different timeout delays depending on
the operation being performed.  Previously, a fixed, worst-case delay
(corresponding to the Chip Erase operation, ~300s!) was used. For the moment,
we have defined conservative delays for each relevant operation, which should
accommodate all existing devices.  In principle, one could set the delays
according the device probed, but there is probably little to be gained.

Signed-off-by: Angus Clark <angus.clark@st.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 7c024ec..3424574 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -267,7 +267,12 @@
 
 #define FLASH_PAGESIZE         256			/* In Bytes    */
 #define FLASH_PAGESIZE_32      (FLASH_PAGESIZE / 4)	/* In uint32_t */
-#define FLASH_MAX_BUSY_WAIT    (300 * HZ)	/* Maximum 'CHIPERASE' time */
+/* Maximum operation times (in ms) */
+#define FLASH_MAX_CHIP_ERASE_MS 500000          /* Chip Erase time */
+#define FLASH_MAX_SEC_ERASE_MS  30000           /* Sector Erase time */
+#define FLASH_MAX_PAGE_WRITE_MS 100             /* Write Page time */
+#define FLASH_MAX_STA_WRITE_MS  4000            /* Write status reg time */
+#define FSM_MAX_WAIT_SEQ_MS     1000            /* FSM execution time */
 
 /*
  * Flags to tweak operation of default read/write/erase routines
@@ -1003,7 +1008,7 @@ static int stfsm_enter_32bit_addr(struct stfsm *fsm, int enter)
 	return 0;
 }
 
-static uint8_t stfsm_wait_busy(struct stfsm *fsm)
+static uint8_t stfsm_wait_busy(struct stfsm *fsm, unsigned int max_time_ms)
 {
 	struct stfsm_seq *seq = &stfsm_seq_read_status_fifo;
 	unsigned long deadline;
@@ -1021,7 +1026,7 @@ static uint8_t stfsm_wait_busy(struct stfsm *fsm)
 	/*
 	 * Repeat until busy bit is deasserted, or timeout, or error (S25FLxxxS)
 	 */
-	deadline = jiffies + FLASH_MAX_BUSY_WAIT;
+	deadline = jiffies + msecs_to_jiffies(max_time_ms);
 	while (!timeout) {
 		if (time_after_eq(jiffies, deadline))
 			timeout = 1;
@@ -1100,7 +1105,7 @@ static int stfsm_write_status(struct stfsm *fsm, uint8_t cmd,
 	stfsm_wait_seq(fsm);
 
 	if (wait_busy)
-		stfsm_wait_busy(fsm);
+		stfsm_wait_busy(fsm, FLASH_MAX_STA_WRITE_MS);
 
 	return 0;
 }
@@ -1497,7 +1502,7 @@ static void stfsm_s25fl_write_dyb(struct stfsm *fsm, uint32_t offs, uint8_t dby)
 	stfsm_load_seq(fsm, &seq);
 	stfsm_wait_seq(fsm);
 
-	stfsm_wait_busy(fsm);
+	stfsm_wait_busy(fsm, FLASH_MAX_STA_WRITE_MS);
 }
 
 static int stfsm_s25fl_clear_status_reg(struct stfsm *fsm)
@@ -1800,7 +1805,7 @@ static int stfsm_write(struct stfsm *fsm, const uint8_t *buf,
 	stfsm_wait_seq(fsm);
 
 	/* Wait for completion */
-	ret = stfsm_wait_busy(fsm);
+	ret = stfsm_wait_busy(fsm, FLASH_MAX_PAGE_WRITE_MS);
 	if (ret && fsm->configuration & CFG_S25FL_CHECK_ERROR_FLAGS)
 		stfsm_s25fl_clear_status_reg(fsm);
 
@@ -1872,7 +1877,7 @@ static int stfsm_erase_sector(struct stfsm *fsm, uint32_t offset)
 	stfsm_wait_seq(fsm);
 
 	/* Wait for completion */
-	ret = stfsm_wait_busy(fsm);
+	ret = stfsm_wait_busy(fsm, FLASH_MAX_SEC_ERASE_MS);
 	if (ret && fsm->configuration & CFG_S25FL_CHECK_ERROR_FLAGS)
 		stfsm_s25fl_clear_status_reg(fsm);
 
@@ -1902,7 +1907,7 @@ static int stfsm_erase_chip(struct stfsm *fsm)
 
 	stfsm_wait_seq(fsm);
 
-	return stfsm_wait_busy(fsm);
+	return stfsm_wait_busy(fsm, FLASH_MAX_CHIP_ERASE_MS);
 }
 
 /*
-- 
1.9.1

[PATCH v3 11/13] mtd: st_spi_fsm: General tidy-up

[Lee Jones]

Due to the nature of the port (lots of copy/paste) much of the white-space
is taken up by spaces instead of tab separators. This patch aims to change
that.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/mtd/devices/st_spi_fsm.c | 280 +++++++++++++++++++--------------------
 1 file changed, 140 insertions(+), 140 deletions(-)

diff --git a/drivers/mtd/devices/st_spi_fsm.c b/drivers/mtd/devices/st_spi_fsm.c
index 3424574..38ba057 100644
--- a/drivers/mtd/devices/st_spi_fsm.c
+++ b/drivers/mtd/devices/st_spi_fsm.c
@@ -97,9 +97,9 @@
 #define SPI_CFG_CS_SETUPHOLD(x)		(((x) & 0xff) << 16)
 #define SPI_CFG_DATA_HOLD(x)		(((x) & 0xff) << 24)
 
-#define SPI_CFG_DEFAULT_MIN_CS_HIGH    SPI_CFG_MIN_CS_HIGH(0x0AA)
-#define SPI_CFG_DEFAULT_CS_SETUPHOLD   SPI_CFG_CS_SETUPHOLD(0xA0)
-#define SPI_CFG_DEFAULT_DATA_HOLD      SPI_CFG_DATA_HOLD(0x00)
+#define SPI_CFG_DEFAULT_MIN_CS_HIGH	SPI_CFG_MIN_CS_HIGH(0x0AA)
+#define SPI_CFG_DEFAULT_CS_SETUPHOLD	SPI_CFG_CS_SETUPHOLD(0xA0)
+#define SPI_CFG_DEFAULT_DATA_HOLD	SPI_CFG_DATA_HOLD(0x00)
 
 /*
  * Register: SPI_FAST_SEQ_TRANSFER_SIZE
@@ -179,7 +179,7 @@
 #define STFSM_OPC_ADD			0x2
 #define STFSM_OPC_STA			0x3
 #define STFSM_OPC_MODE			0x4
-#define STFSM_OPC_DUMMY		0x5
+#define STFSM_OPC_DUMMY			0x5
 #define STFSM_OPC_DATA			0x6
 #define STFSM_OPC_WAIT			0x7
 #define STFSM_OPC_JUMP			0x8
@@ -212,88 +212,87 @@
 #define STFSM_INST_WAIT			STFSM_INSTR(STFSM_OPC_WAIT,	0)
 #define STFSM_INST_STOP			STFSM_INSTR(STFSM_OPC_STOP,	0)
 
-#define STFSM_DEFAULT_EMI_FREQ 100000000UL                        /* 100 MHz */
-#define STFSM_DEFAULT_WR_TIME  (STFSM_DEFAULT_EMI_FREQ * (15/1000)) /* 15ms */
+#define STFSM_DEFAULT_EMI_FREQ		100000000UL	/* 100 MHz */
+#define STFSM_DEFAULT_WR_TIME	(STFSM_DEFAULT_EMI_FREQ * (15/1000)) /* 15ms */
 
-#define STFSM_FLASH_SAFE_FREQ  10000000UL                         /* 10 MHz */
+#define STFSM_FLASH_SAFE_FREQ		10000000UL	/* 10 MHz */
 
-#define STFSM_MAX_WAIT_SEQ_MS  1000     /* FSM execution time */
+#define STFSM_MAX_WAIT_SEQ_MS		1000		/* FSM execution time */
 
 /* S25FLxxxS commands */
-#define S25FL_CMD_WRITE4_1_1_4 0x34
-#define S25FL_CMD_SE4          0xdc
-#define S25FL_CMD_CLSR         0x30
-#define S25FL_CMD_DYBWR                0xe1
-#define S25FL_CMD_DYBRD                0xe0
-#define S25FL_CMD_WRITE4       0x12    /* Note, opcode clashes with
+#define S25FL_CMD_WRITE4_1_1_4		0x34
+#define S25FL_CMD_SE4			0xdc
+#define S25FL_CMD_CLSR			0x30
+#define S25FL_CMD_DYBWR			0xe1
+#define S25FL_CMD_DYBRD			0xe0
+#define S25FL_CMD_WRITE4		0x12    /* Note, opcode clashes with
 					* 'SPINOR_OP_WRITE_1_4_4'
 					* as found on N25Qxxx devices! */
-
 /* Status register */
-#define FLASH_STATUS_BUSY      0x01
-#define FLASH_STATUS_WEL       0x02
-#define FLASH_STATUS_BP0       0x04
-#define FLASH_STATUS_BP1       0x08
-#define FLASH_STATUS_BP2       0x10
-#define FLASH_STATUS_SRWP0     0x80
-#define FLASH_STATUS_TIMEOUT   0xff
+#define FLASH_STATUS_BUSY		0x01
+#define FLASH_STATUS_WEL		0x02
+#define FLASH_STATUS_BP0		0x04
+#define FLASH_STATUS_BP1		0x08
+#define FLASH_STATUS_BP2		0x10
+#define FLASH_STATUS_SRWP0		0x80
+#define FLASH_STATUS_TIMEOUT		0xff
 
 /* Maximum READID length */
-#define MAX_READID_LEN         6
-#define MAX_READID_LEN_ALIGNED ALIGN(MAX_READID_LEN, 4)
+#define MAX_READID_LEN	 		6
+#define MAX_READID_LEN_ALIGNED 		ALIGN(MAX_READID_LEN, 4)
 
 /* S25FL Error Flags */
-#define S25FL_STATUS_E_ERR     0x20
-#define S25FL_STATUS_P_ERR     0x40
+#define S25FL_STATUS_E_ERR		0x20
+#define S25FL_STATUS_P_ERR		0x40
 
 /* N25Q - READ/WRITE/CLEAR NON/VOLATILE STATUS/CONFIG Registers */
-#define N25Q_CMD_RFSR          0x70
-#define N25Q_CMD_CLFSR         0x50
-#define N25Q_CMD_WRVCR         0x81
-#define N25Q_CMD_RDVCR         0x85
-#define N25Q_CMD_RDVECR        0x65
-#define N25Q_CMD_RDNVCR        0xb5
-#define N25Q_CMD_WRNVCR        0xb1
+#define N25Q_CMD_RFSR			0x70
+#define N25Q_CMD_CLFSR			0x50
+#define N25Q_CMD_WRVCR			0x81
+#define N25Q_CMD_RDVCR			0x85
+#define N25Q_CMD_RDVECR			0x65
+#define N25Q_CMD_RDNVCR			0xb5
+#define N25Q_CMD_WRNVCR			0xb1
 
 /* N25Q Flags Status Register: Error Flags */
-#define N25Q_FLAGS_ERR_ERASE   BIT(5)
-#define N25Q_FLAGS_ERR_PROG    BIT(4)
-#define N25Q_FLAGS_ERR_VPP     BIT(3)
-#define N25Q_FLAGS_ERR_PROT    BIT(1)
-#define N25Q_FLAGS_ERROR	(N25Q_FLAGS_ERR_ERASE   | \
-				N25Q_FLAGS_ERR_PROG    | \
-				N25Q_FLAGS_ERR_VPP     | \
-				N25Q_FLAGS_ERR_PROT)
-
-#define FLASH_PAGESIZE         256			/* In Bytes    */
-#define FLASH_PAGESIZE_32      (FLASH_PAGESIZE / 4)	/* In uint32_t */
+#define N25Q_FLAGS_ERR_ERASE		BIT(5)
+#define N25Q_FLAGS_ERR_PROG		BIT(4)
+#define N25Q_FLAGS_ERR_VPP		BIT(3)
+#define N25Q_FLAGS_ERR_PROT		BIT(1)
+#define N25Q_FLAGS_ERROR		(N25Q_FLAGS_ERR_ERASE	| \
+					 N25Q_FLAGS_ERR_PROG	| \
+					 N25Q_FLAGS_ERR_VPP	| \
+					 N25Q_FLAGS_ERR_PROT)
+
+#define FLASH_PAGESIZE			256		     /* In Bytes    */
+#define FLASH_PAGESIZE_32		(FLASH_PAGESIZE / 4) /* In uint32_t */
 /* Maximum operation times (in ms) */
-#define FLASH_MAX_CHIP_ERASE_MS 500000          /* Chip Erase time */
-#define FLASH_MAX_SEC_ERASE_MS  30000           /* Sector Erase time */
-#define FLASH_MAX_PAGE_WRITE_MS 100             /* Write Page time */
-#define FLASH_MAX_STA_WRITE_MS  4000            /* Write status reg time */
-#define FSM_MAX_WAIT_SEQ_MS     1000            /* FSM execution time */
+#define FLASH_MAX_CHIP_ERASE_MS		500000	/* Chip Erase time */
+#define FLASH_MAX_SEC_ERASE_MS		30000	/* Sector Erase time */
+#define FLASH_MAX_PAGE_WRITE_MS		100	/* Write Page time */
+#define FLASH_MAX_STA_WRITE_MS		4000	/* Write status reg time */
+#define FSM_MAX_WAIT_SEQ_MS		1000	/* FSM execution time */
 
 /*
  * Flags to tweak operation of default read/write/erase routines
  */
-#define CFG_READ_TOGGLE_32BIT_ADDR     0x00000001
-#define CFG_WRITE_TOGGLE_32BIT_ADDR    0x00000002
-#define CFG_ERASESEC_TOGGLE_32BIT_ADDR 0x00000008
-#define CFG_S25FL_CHECK_ERROR_FLAGS    0x00000010
-#define CFG_N25Q_CHECK_ERROR_FLAGS     0x00000020
+#define CFG_READ_TOGGLE_32BIT_ADDR		0x00000001
+#define CFG_WRITE_TOGGLE_32BIT_ADDR		0x00000002
+#define CFG_ERASESEC_TOGGLE_32BIT_ADDR		0x00000008
+#define CFG_S25FL_CHECK_ERROR_FLAGS		0x00000010
+#define CFG_N25Q_CHECK_ERROR_FLAGS		0x00000020
 
 struct stfsm_seq {
-	uint32_t data_size;
-	uint32_t addr1;
-	uint32_t addr2;
-	uint32_t addr_cfg;
-	uint32_t seq_opc[5];
-	uint32_t mode;
-	uint32_t dummy;
-	uint32_t status;
-	uint8_t  seq[16];
-	uint32_t seq_cfg;
+	uint32_t 		data_size;
+	uint32_t 		addr1;
+	uint32_t 		addr2;
+	uint32_t 		addr_cfg;
+	uint32_t 		seq_opc[5];
+	uint32_t 		mode;
+	uint32_t 		dummy;
+	uint32_t 		status;
+	uint8_t  		seq[16];
+	uint32_t 		seq_cfg;
 } __packed __aligned(4);
 
 struct stfsm {
@@ -302,30 +301,31 @@ struct stfsm {
 	struct resource		*region;
 	struct mtd_info		mtd;
 	struct mutex		lock;
-	struct flash_info       *info;
-	struct clk              *clk;
-
-	uint32_t                configuration;
-	uint32_t                fifo_dir_delay;
-	bool                    booted_from_spi;
-	bool                    reset_signal;
-	bool                    reset_por;
-
-	struct stfsm_seq stfsm_seq_read;
-	struct stfsm_seq stfsm_seq_write;
-	struct stfsm_seq stfsm_seq_en_32bit_addr;
+	struct flash_info	*info;
+	struct clk		*clk;
+
+	uint32_t		configuration;
+	uint32_t		fifo_dir_delay;
+	bool			booted_from_spi;
+	bool			reset_signal;
+	bool			reset_por;
+
+	struct stfsm_seq 	stfsm_seq_read;
+	struct stfsm_seq 	stfsm_seq_write;
+	struct stfsm_seq 	stfsm_seq_en_32bit_addr;
 };
 
 /* Parameters to configure a READ or WRITE FSM sequence */
 struct seq_rw_config {
-	uint32_t        flags;          /* flags to support config */
-	uint8_t         cmd;            /* FLASH command */
-	int             write;          /* Write Sequence */
-	uint8_t         addr_pads;      /* No. of addr pads (MODE & DUMMY) */
-	uint8_t         data_pads;      /* No. of data pads */
-	uint8_t         mode_data;      /* MODE data */
-	uint8_t         mode_cycles;    /* No. of MODE cycles */
-	uint8_t         dummy_cycles;   /* No. of DUMMY cycles */
+	uint32_t		flags;		/* flags to support config */
+	uint8_t	 		cmd;		/* FLASH command */
+	int			write;		/* Write Sequence */
+	uint8_t	 		addr_pads;	/* No. of addr pads */
+						/* (MODE & DUMMY) */
+	uint8_t	 		data_pads;	/* No. of data pads */
+	uint8_t	 		mode_data;	/* MODE data */
+	uint8_t	 		mode_cycles;	/* No. of MODE cycles */
+	uint8_t	 		dummy_cycles;	/* No. of DUMMY cycles */
 };
 
 struct stfsm_boot_dev {
@@ -336,23 +336,23 @@ struct stfsm_boot_dev {
 
 /* SPI Flash Device Table */
 struct flash_info {
-	char            *name;
-	/* READID data, as returned by 'SPINOR_OP_RDID' (0x9f). */
-	u8              readid[MAX_READID_LEN];
-	int             readid_len;
+	char			*name;
+	/* READID data, as returned by 'FLASH_CMD_RDID' (0x9f). */
+	u8			readid[MAX_READID_LEN];
+	int			readid_len;
 	/*
 	 * The size listed here is what works with SPINOR_OP_SE, which isn't
 	 * necessarily called a "sector" by the vendor.
 	 */
-	unsigned        sector_size;
-	u16             n_sectors;
-	u32             flags;
+	unsigned		sector_size;
+	u16			n_sectors;
+	u32			flags;
 	/*
 	 * Note, where FAST_READ is supported, freq_max specifies the
 	 * FAST_READ frequency, not the READ frequency.
 	 */
-	u32             max_freq;
-	int             (*config)(struct stfsm *);
+	u32			max_freq;
+	int			(*config)(struct stfsm *);
 };
 
 static struct stfsm_boot_dev stfsm_stid127_data = {
@@ -423,9 +423,9 @@ static struct flash_info flash_types[] = {
 	JEDEC_INFO("m25p64",  0x202017,  64 * 1024, 128, M25P_FLAG, 50, NULL),
 	JEDEC_INFO("m25p128", 0x202018, 256 * 1024,  64, M25P_FLAG, 50, NULL),
 
-#define M25PX_FLAG (FLASH_FLAG_READ_WRITE      |	\
-		    FLASH_FLAG_READ_FAST        |	\
-		    FLASH_FLAG_READ_1_1_2       |	\
+#define M25PX_FLAG (FLASH_FLAG_READ_WRITE	|	\
+		    FLASH_FLAG_READ_FAST	|	\
+		    FLASH_FLAG_READ_1_1_2	|	\
 		    FLASH_FLAG_WRITE_1_1_2)
 	JEDEC_INFO("m25px32", 0x207116,  64 * 1024,  64, M25PX_FLAG, 75, NULL),
 	JEDEC_INFO("m25px64", 0x207117,  64 * 1024, 128, M25PX_FLAG, 75, NULL),
@@ -434,12 +434,12 @@ static struct flash_info flash_types[] = {
 	 *     - Support for 'FLASH_FLAG_WRITE_1_4_4' is omitted for devices
 	 *       where operating frequency must be reduced.
 	 */
-#define MX25_FLAG (FLASH_FLAG_READ_WRITE       |	\
-		   FLASH_FLAG_READ_FAST         |	\
-		   FLASH_FLAG_READ_1_1_2        |	\
-		   FLASH_FLAG_READ_1_2_2        |	\
-		   FLASH_FLAG_READ_1_1_4        |	\
-		   FLASH_FLAG_SE_4K             |	\
+#define MX25_FLAG (FLASH_FLAG_READ_WRITE	|	\
+		   FLASH_FLAG_READ_FAST		|	\
+		   FLASH_FLAG_READ_1_1_2	|	\
+		   FLASH_FLAG_READ_1_2_2	|	\
+		   FLASH_FLAG_READ_1_1_4	|	\
+		   FLASH_FLAG_SE_4K		|	\
 		   FLASH_FLAG_SE_32K)
 	JEDEC_INFO("mx25l3255e",  0xc29e16, 64 * 1024, 64,
 		   (MX25_FLAG | FLASH_FLAG_WRITE_1_4_4), 86, stfsm_mx25_config),
@@ -449,20 +449,20 @@ static struct flash_info flash_types[] = {
 		   (MX25_FLAG | FLASH_FLAG_RESET), 70, stfsm_mx25_config),
 
 	/* Micron N25Qxxx */
-#define N25Q_FLAG (FLASH_FLAG_READ_WRITE       |	\
-		   FLASH_FLAG_READ_FAST         |	\
-		   FLASH_FLAG_READ_1_1_2        |	\
-		   FLASH_FLAG_READ_1_2_2        |	\
-		   FLASH_FLAG_READ_1_1_4        |	\
-		   FLASH_FLAG_READ_1_4_4        |	\
-		   FLASH_FLAG_WRITE_1_1_2       |	\
-		   FLASH_FLAG_WRITE_1_2_2       |	\
-		   FLASH_FLAG_WRITE_1_1_4       |	\
+#define N25Q_FLAG (FLASH_FLAG_READ_WRITE	|	\
+		   FLASH_FLAG_READ_FAST		|	\
+		   FLASH_FLAG_READ_1_1_2	|	\
+		   FLASH_FLAG_READ_1_2_2	|	\
+		   FLASH_FLAG_READ_1_1_4	|	\
+		   FLASH_FLAG_READ_1_4_4	|	\
+		   FLASH_FLAG_WRITE_1_1_2	|	\
+		   FLASH_FLAG_WRITE_1_2_2	|	\
+		   FLASH_FLAG_WRITE_1_1_4	|	\
 		   FLASH_FLAG_WRITE_1_4_4)
 	JEDEC_INFO("n25q128", 0x20ba18, 64 * 1024,  256,
 		   N25Q_FLAG, 108, stfsm_n25q_config),
 
-	/* Micron N25Q256/N25Q512/N25Q00A (32-bit ADDR devices)
+       /* Micron N25Q256/N25Q512/N25Q00A (32-bit ADDR devices)
 	*
 	* Versions are available with or without a dedicated RESET# pin
 	* (e.g. N25Q512A83GSF40G vs. N25Q512A13GSF40G).  To complicate matters,
@@ -474,9 +474,9 @@ static struct flash_info flash_types[] = {
 	* defer overall support for RESET# to the board-level platform/Device
 	* Tree property "reset-signal".
 	*/
-#define N25Q_32BIT_ADDR_FLAG  ((N25Q_FLAG              |	\
-				FLASH_FLAG_32BIT_ADDR  |	\
-				FLASH_FLAG_RESET)      &	\
+#define N25Q_32BIT_ADDR_FLAG  ((N25Q_FLAG		|	\
+				FLASH_FLAG_32BIT_ADDR	|	\
+				FLASH_FLAG_RESET)	&	\
 			       ~FLASH_FLAG_WRITE_1_4_4)
 	JEDEC_INFO("n25q256", 0x20ba19, 64 * 1024,   512,
 		   N25Q_32BIT_ADDR_FLAG, 108, stfsm_n25q_config),
@@ -490,12 +490,12 @@ static struct flash_info flash_types[] = {
 	 *     - 256KiB and 64KiB sector variants (identified by ext. JEDEC)
 	 *     - S25FL128Px devices do not support DUAL or QUAD I/O
 	 */
-#define S25FLXXXP_FLAG (FLASH_FLAG_READ_WRITE  |	\
-			FLASH_FLAG_READ_1_1_2   |	\
-			FLASH_FLAG_READ_1_2_2   |	\
-			FLASH_FLAG_READ_1_1_4   |	\
-			FLASH_FLAG_READ_1_4_4   |	\
-			FLASH_FLAG_WRITE_1_1_4  |	\
+#define S25FLXXXP_FLAG (FLASH_FLAG_READ_WRITE	|	\
+			FLASH_FLAG_READ_1_1_2	|	\
+			FLASH_FLAG_READ_1_2_2	|	\
+			FLASH_FLAG_READ_1_1_4	|	\
+			FLASH_FLAG_READ_1_4_4	|	\
+			FLASH_FLAG_WRITE_1_1_4	|	\
 			FLASH_FLAG_READ_FAST)
 	RDID_INFO("s25fl032p", RDID({0x01, 0x02, 0x15, 0x4d, 0x00}), 5,
 		  64 * 1024,  64, S25FLXXXP_FLAG, 80, stfsm_s25fl_config),
@@ -520,8 +520,8 @@ static struct flash_info flash_types[] = {
 	 *       may default to locked state on power-on.
 	 *     - S25FL127Sx handled as S25FL128Sx
 	 */
-#define S25FLXXXS_FLAG (S25FLXXXP_FLAG         |	\
-			FLASH_FLAG_RESET        |	\
+#define S25FLXXXS_FLAG (S25FLXXXP_FLAG	 	|	\
+			FLASH_FLAG_RESET	|	\
 			FLASH_FLAG_DYB_LOCKING)
 	RDID_INFO("s25fl128s0", RDID({0x01, 0x20, 0x18, 0x4d, 0x00, 0x80}), 6,
 		  256 * 1024, 64, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
@@ -532,9 +532,9 @@ static struct flash_info flash_types[] = {
 	RDID_INFO("s25fl256s1", RDID({0x01, 0x02, 0x19, 0x4d, 0x01, 0x80}), 6,
 		  64 * 1024, 512, S25FLXXXS_FLAG, 80, stfsm_s25fl_config),
 
-#define W25X_FLAG (FLASH_FLAG_READ_WRITE       |	\
-		   FLASH_FLAG_READ_FAST         |	\
-		   FLASH_FLAG_READ_1_1_2        |	\
+#define W25X_FLAG (FLASH_FLAG_READ_WRITE	|	\
+		   FLASH_FLAG_READ_FAST	 	|	\
+		   FLASH_FLAG_READ_1_1_2	|	\
 		   FLASH_FLAG_WRITE_1_1_2)
 	JEDEC_INFO("w25x40", 0xef3013, 64 * 1024,   8, W25X_FLAG, 75, NULL),
 	JEDEC_INFO("w25x80", 0xef3014, 64 * 1024,  16, W25X_FLAG, 75, NULL),
@@ -543,12 +543,12 @@ static struct flash_info flash_types[] = {
 	JEDEC_INFO("w25x64", 0xef3017, 64 * 1024, 128, W25X_FLAG, 75, NULL),
 
 	/* Winbond -- w25q "blocks" are 64K, "sectors" are 4KiB */
-#define W25Q_FLAG (FLASH_FLAG_READ_WRITE       |	\
-		   FLASH_FLAG_READ_FAST         |	\
-		   FLASH_FLAG_READ_1_1_2        |	\
-		   FLASH_FLAG_READ_1_2_2        |	\
-		   FLASH_FLAG_READ_1_1_4        |	\
-		   FLASH_FLAG_READ_1_4_4        |	\
+#define W25Q_FLAG (FLASH_FLAG_READ_WRITE	|	\
+		   FLASH_FLAG_READ_FAST		|	\
+		   FLASH_FLAG_READ_1_1_2	|	\
+		   FLASH_FLAG_READ_1_2_2	|	\
+		   FLASH_FLAG_READ_1_1_4	|	\
+		   FLASH_FLAG_READ_1_4_4	|	\
 		   FLASH_FLAG_WRITE_1_1_4)
 	JEDEC_INFO("w25q80", 0xef4014, 64 * 1024,  16,
 		   W25Q_FLAG, 80, stfsm_w25q_config),
@@ -619,7 +619,7 @@ static struct seq_rw_config n25q_read3_configs[] = {
 
 /* N25Q 4-byte Address READ configurations
  *	- use special 4-byte address READ commands (reduces overheads, and
- *        reduces risk of hitting watchdog reset issues).
+ *	reduces risk of hitting watchdog reset issues).
  *	- 'FAST' variants configured for 8 dummy cycles (see note above.)
  */
 static struct seq_rw_config n25q_read4_configs[] = {
@@ -681,7 +681,7 @@ static struct seq_rw_config stfsm_s25fl_read4_configs[] = {
 static struct seq_rw_config stfsm_s25fl_write4_configs[] = {
 	{FLASH_FLAG_WRITE_1_1_4, S25FL_CMD_WRITE4_1_1_4, 1, 1, 4, 0x00, 0, 0},
 	{FLASH_FLAG_READ_WRITE,  S25FL_CMD_WRITE4,       1, 1, 1, 0x00, 0, 0},
-	{0x00,                   0,                      0, 0, 0, 0x00, 0, 0},
+	{0x00,		   0,		      0, 0, 0, 0x00, 0, 0},
 };
 
 /*
@@ -906,12 +906,12 @@ static void stfsm_read_fifo(struct stfsm *fsm, uint32_t *buf, uint32_t size)
  * With this in mind, a two stage process is used to the clear the FIFO:
  *
  *     1. Read any complete 32-bit words from the FIFO, as reported by the
- *        SPI_FAST_SEQ_STA register.
+ *	SPI_FAST_SEQ_STA register.
  *
  *     2. Mop up any remaining bytes.  At this point, it is not known if there
- *        are 0, 1, 2, or 3 bytes in the FIFO.  To handle all cases, a dummy FSM
- *        sequence is used to load one byte at a time, until a complete 32-bit
- *        word is formed; at most, 4 bytes will need to be loaded.
+ *	are 0, 1, 2, or 3 bytes in the FIFO.  To handle all cases, a dummy FSM
+ *	sequence is used to load one byte at a time, until a complete 32-bit
+ *	word is formed;@most, 4 bytes will need to be loaded.
  *
  * [1] It is theoretically possible for the FIFO to contain an arbitrary number
  *     of bits.  However, since there are no known use-cases that leave
@@ -2129,7 +2129,7 @@ static int stfsm_init(struct stfsm *fsm)
 		return ret;
 
 	/* Set timing parameters */
-	writel(SPI_CFG_DEVICE_ST            |
+	writel(SPI_CFG_DEVICE_ST	    |
 	       SPI_CFG_DEFAULT_MIN_CS_HIGH  |
 	       SPI_CFG_DEFAULT_CS_SETUPHOLD |
 	       SPI_CFG_DEFAULT_DATA_HOLD,
@@ -2341,7 +2341,7 @@ static struct platform_driver stfsm_driver = {
 		.name	= "st-spi-fsm",
 		.owner	= THIS_MODULE,
 		.of_match_table = stfsm_match,
-		.pm     = &stfsm_pm_ops,
+		.pm	= &stfsm_pm_ops,
 	},
 };
 module_platform_driver(stfsm_driver);
-- 
1.9.1

[PATCH v3 11/13] mtd: st_spi_fsm: General tidy-up

[Brian Norris]

On Mon, Dec 15, 2014 at 11:59:18AM +0000, Lee Jones wrote:
> Due to the nature of the port (lots of copy/paste) much of the white-space
> is taken up by spaces instead of tab separators. This patch aims to change
> that.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

checkpatch.pl gives several warnings like this on this patch:

    WARNING:SPACE_BEFORE_TAB: please, no space before tabs

Please consider fixing them up.

Brian

[PATCH v3 12/13] ARM: STi: stih416: Use new platform specific compatible string

[Lee Jones]

The FSM SPI NOR driver now obtains syscfg particulars using DT match. In
order for this to happen each platform is required to supply their own
specific compatible string.

We're also remove the old, now unused vendor properties from the node.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416.dtsi | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index 84758d7..dec546e 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -214,13 +214,11 @@
 
 		/* FSM */
 		spifsm: spifsm at fe902000 {
-			compatible	   = "st,spi-fsm";
+			compatible	   = "st,stih416-spi-fsm";
 			reg		   = <0xfe902000 0x1000>;
 			pinctrl-0	   = <&pinctrl_fsm>;
 
 			st,syscfg	   = <&syscfg_rear>;
-			st,boot-device-reg = <0x958>;
-			st,boot-device-spi = <0x1a>;
 
 			status = "disabled";
 		};
-- 
1.9.1

[PATCH v3 13/13] ARM: STi: stih416: Supply EMI clock reference to FSM SPI NOR

[Lee Jones]

While we're at it we're also adding a new human readable define for the
aforementioned clock.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih416.dtsi           | 2 +-
 include/dt-bindings/clock/stih416-clks.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stih416.dtsi b/arch/arm/boot/dts/stih416.dtsi
index dec546e..663e93b 100644
--- a/arch/arm/boot/dts/stih416.dtsi
+++ b/arch/arm/boot/dts/stih416.dtsi
@@ -217,7 +217,7 @@
 			compatible	   = "st,stih416-spi-fsm";
 			reg		   = <0xfe902000 0x1000>;
 			pinctrl-0	   = <&pinctrl_fsm>;
-
+			clocks		   = <&clk_s_a0_ls CLK_EMISS>;
 			st,syscfg	   = <&syscfg_rear>;
 
 			status = "disabled";
diff --git a/include/dt-bindings/clock/stih416-clks.h b/include/dt-bindings/clock/stih416-clks.h
index f9bdbd1..4a46a47e 100644
--- a/include/dt-bindings/clock/stih416-clks.h
+++ b/include/dt-bindings/clock/stih416-clks.h
@@ -7,6 +7,7 @@
 
 /* CLOCKGEN A0 */
 #define CLK_ICN_REG		0
+#define CLK_EMISS		3
 #define CLK_ETH1_PHY		4
 
 /* CLOCKGEN A1 */
-- 
1.9.1

[PATCH v3 00/13] mtd: st_spi_fsm: Align with ST's internal development

[Brian Norris]

On Mon, Dec 15, 2014 at 11:59:07AM +0000, Lee Jones wrote:
> Hi Brian, all,
>  
>  [x] Bulid test
>  [x] Bisectable
>  [x] Smatch
>  [x] Sparse

Picked patches 1, 2, and 5 to l2-mtd.git. I requested changes to patch 3
and possibly 4. The rest look OK, but I don't want to futz with patch
conflicts on the remaining. Please rebase to l2-mtd.git if/when you
resend.

> v3:
>   Further aligned with upstream changes as suggested by Peter Griffin.
>   The clk_ignore_unused kernel command line parameter is due to be turned
>   off on STiH4* platforms, so we no longer give the driver the chance to
>   opt-out of fetching the EMI clock.  We also now provide syscfg offsets
>   inside the driver, as it's simpler and reduces the cost of having lots
>   of extra DT properties.

Typically "simpler" and "reducing the cost of having lots of extra DT
properties" aren't good enough reasons for immediately breaking an
existing DT binding. We usually expect to support both for some period
of time.

> v2:
>   These are the last remaining patches in the set, all rebased and
>   with the DT documentation requested after the last submission.
>  
> v1:
>   This patch-set updates ST's FSM SPI-NOR driver with all the internal
>   goodness which has happened since the initial (now upstreamed) snapshot
>   was taken. It covers just over 6 months worth of internal development
>   and bug-fixes. A final whitespace clean-up is also appended to the set
>   - to make it look pretty and stuff. :)
> 
> Angus Clark (3):
>   mtd: st_spi_fsm: Add support for N25Q512 and N25Q00A devices
>   mtd: st_spi_fsm: Update Spansion device entries
>   mtd: st_spi_fsm: Improve busy wait handling
> 
> Lee Jones (9):
>   mtd: st_spi_fsm: Extend fsm_clear_fifo to handle unwanted bytes
>   mtd: st_spi_fsm: Obtain and use EMI clock
>   mtd: st_spi_fsm: dt-bindings: Deprecate generic compatible string
>   mtd: st_spi_fsm: Fetch boot device locations from DT match tables
>   mtd: st_spi_fsm: Fix [-Wsign-compare] build warning
>   mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs
>   mtd: st_spi_fsm: General tidy-up
>   ARM: STi: stih416: Use new platform specific compatible string
>   ARM: STi: stih416: Supply EMI clock reference to FSM SPI NOR
> 
> Nunzio Raciti (1):
>   mtd: st_spi_fsm: Add support for Micron N25Q512A
> 
>  Documentation/devicetree/bindings/mtd/st-fsm.txt |  20 +-
>  arch/arm/boot/dts/stih416.dtsi                   |   6 +-
>  drivers/mtd/devices/st_spi_fsm.c                 | 747 +++++++++++++++--------
>  include/dt-bindings/clock/stih416-clks.h         |   1 +
>  4 files changed, 519 insertions(+), 255 deletions(-)
> 

Brian

[PATCH v3 00/13] mtd: st_spi_fsm: Align with ST's internal development

[Lee Jones]

On Mon, 12 Jan 2015, Brian Norris wrote:
> On Mon, Dec 15, 2014 at 11:59:07AM +0000, Lee Jones wrote:
> > Hi Brian, all,
> >  
> >  [x] Bulid test
> >  [x] Bisectable
> >  [x] Smatch
> >  [x] Sparse
> 
> Picked patches 1, 2, and 5 to l2-mtd.git. I requested changes to patch 3
> and possibly 4. The rest look OK, but I don't want to futz with patch
> conflicts on the remaining. Please rebase to l2-mtd.git if/when you
> resend.

Sure.  I'll get on to that now.

> > v3:
> >   Further aligned with upstream changes as suggested by Peter Griffin.
> >   The clk_ignore_unused kernel command line parameter is due to be turned
> >   off on STiH4* platforms, so we no longer give the driver the chance to
> >   opt-out of fetching the EMI clock.  We also now provide syscfg offsets
> >   inside the driver, as it's simpler and reduces the cost of having lots
> >   of extra DT properties.
> 
> Typically "simpler" and "reducing the cost of having lots of extra DT
> properties" aren't good enough reasons for immediately breaking an
> existing DT binding. We usually expect to support both for some period
> of time.

Only once a binding has became ABI.  As there are no users of this
driver yet (this will change after this sync'ing patch-set has been
applied), so we can safely consider this binding to be in-progress and
as such, not yet ABI, thus we can do this ol' switch-aroo without the
usual 6 month deprecation period.

> > v2:
> >   These are the last remaining patches in the set, all rebased and
> >   with the DT documentation requested after the last submission.
> >  
> > v1:
> >   This patch-set updates ST's FSM SPI-NOR driver with all the internal
> >   goodness which has happened since the initial (now upstreamed) snapshot
> >   was taken. It covers just over 6 months worth of internal development
> >   and bug-fixes. A final whitespace clean-up is also appended to the set
> >   - to make it look pretty and stuff. :)
> > 
> > Angus Clark (3):
> >   mtd: st_spi_fsm: Add support for N25Q512 and N25Q00A devices
> >   mtd: st_spi_fsm: Update Spansion device entries
> >   mtd: st_spi_fsm: Improve busy wait handling
> > 
> > Lee Jones (9):
> >   mtd: st_spi_fsm: Extend fsm_clear_fifo to handle unwanted bytes
> >   mtd: st_spi_fsm: Obtain and use EMI clock
> >   mtd: st_spi_fsm: dt-bindings: Deprecate generic compatible string
> >   mtd: st_spi_fsm: Fetch boot device locations from DT match tables
> >   mtd: st_spi_fsm: Fix [-Wsign-compare] build warning
> >   mtd: st_spi_fsm: Update the JEDEC probe to handle extended READIDs
> >   mtd: st_spi_fsm: General tidy-up
> >   ARM: STi: stih416: Use new platform specific compatible string
> >   ARM: STi: stih416: Supply EMI clock reference to FSM SPI NOR
> > 
> > Nunzio Raciti (1):
> >   mtd: st_spi_fsm: Add support for Micron N25Q512A
> > 
> >  Documentation/devicetree/bindings/mtd/st-fsm.txt |  20 +-
> >  arch/arm/boot/dts/stih416.dtsi                   |   6 +-
> >  drivers/mtd/devices/st_spi_fsm.c                 | 747 +++++++++++++++--------
> >  include/dt-bindings/clock/stih416-clks.h         |   1 +
> >  4 files changed, 519 insertions(+), 255 deletions(-)
> > 
> 
> Brian

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v3 00/13] mtd: st_spi_fsm: Align with ST's internal development

[Brian Norris]

On Wed, Jan 21, 2015 at 12:49:30PM +0000, Lee Jones wrote:
> On Mon, 12 Jan 2015, Brian Norris wrote:
> > Typically "simpler" and "reducing the cost of having lots of extra DT
> > properties" aren't good enough reasons for immediately breaking an
> > existing DT binding. We usually expect to support both for some period
> > of time.
> 
> Only once a binding has became ABI.  As there are no users of this
> driver yet (this will change after this sync'ing patch-set has been
> applied), so we can safely consider this binding to be in-progress and
> as such, not yet ABI, thus we can do this ol' switch-aroo without the
> usual 6 month deprecation period.

OK, fine with me.

Brian