All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
@ 2017-09-11  9:41 Bin Meng
  2017-09-11  9:41 ` [PATCH v2 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT Bin Meng
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

This series does several bug fixes and clean ups against the intel-spi
spi-nor driver, as well as enhancements to make the driver independent
on the underlying BIOS/bootloader.

At present the driver uses the HW sequencer for the read/write/erase on
all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
sequencer relies on some programmed register settings and hence creates
unneeded dependencies with the underlying BIOS/bootloader. For example,
the driver unfortunately does not work as expected when booting from
Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
does not set up some SPI controller settings to make the driver happy.
Now such limitation has been removed with this series.

Changes in v2:
- Add stable kernel tags in the commit message (patch [03/10])
- Fix typo of 'operatoin' (patch [10/10])
- Add Mika Westerberg's 'Acked-by' tag

Bin Meng (10):
  spi-nor: intel-spi: Fix number of protected range registers for
    BYT/LPT
  spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle
  spi-nor: intel-spi: Fix broken software sequencing codes
  spi-nor: intel-spi: Check transfer length in the HW/SW cycle
  spi-nor: intel-spi: Use SW sequencer for BYT/LPT
  spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in
    intel_spi_write()
  spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS
  spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW
  spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi'
  spi-nor: intel-spi: Fall back to use SW sequencer to erase

 drivers/mtd/spi-nor/intel-spi.c | 209 +++++++++++++++++++++++++++++-----------
 1 file changed, 151 insertions(+), 58 deletions(-)

-- 
2.9.2

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle Bin Meng
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

The number of protected range registers is not the same on BYT/LPT/
BXT. GPR0 only exists on Apollo Lake and its offset is reserved on
other platforms.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 8a596bf..e5b52e8 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -67,8 +67,6 @@
 #define PR_LIMIT_MASK			(0x3fff << PR_LIMIT_SHIFT)
 #define PR_RPE				BIT(15)
 #define PR_BASE_MASK			0x3fff
-/* Last PR is GPR0 */
-#define PR_NUM				(5 + 1)
 
 /* Offsets are from @ispi->sregs */
 #define SSFSTS_CTL			0x00
@@ -96,14 +94,17 @@
 #define BYT_BCR				0xfc
 #define BYT_BCR_WPD			BIT(0)
 #define BYT_FREG_NUM			5
+#define BYT_PR_NUM			5
 
 #define LPT_PR				0x74
 #define LPT_SSFSTS_CTL			0x90
 #define LPT_FREG_NUM			5
+#define LPT_PR_NUM			5
 
 #define BXT_PR				0x84
 #define BXT_SSFSTS_CTL			0xa0
 #define BXT_FREG_NUM			12
+#define BXT_PR_NUM			6
 
 #define INTEL_SPI_TIMEOUT		5000 /* ms */
 #define INTEL_SPI_FIFO_SZ		64
@@ -117,6 +118,7 @@
  * @pregs: Start of protection registers
  * @sregs: Start of software sequencer registers
  * @nregions: Maximum number of regions
+ * @pr_num: Maximum number of protected range registers
  * @writeable: Is the chip writeable
  * @swseq: Use SW sequencer in register reads/writes
  * @erase_64k: 64k erase supported
@@ -132,6 +134,7 @@ struct intel_spi {
 	void __iomem *pregs;
 	void __iomem *sregs;
 	size_t nregions;
+	size_t pr_num;
 	bool writeable;
 	bool swseq;
 	bool erase_64k;
@@ -167,7 +170,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
 	for (i = 0; i < ispi->nregions; i++)
 		dev_dbg(ispi->dev, "FREG(%d)=0x%08x\n", i,
 			readl(ispi->base + FREG(i)));
-	for (i = 0; i < PR_NUM; i++)
+	for (i = 0; i < ispi->pr_num; i++)
 		dev_dbg(ispi->dev, "PR(%d)=0x%08x\n", i,
 			readl(ispi->pregs + PR(i)));
 
@@ -182,7 +185,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
 		dev_dbg(ispi->dev, "BCR=0x%08x\n", readl(ispi->base + BYT_BCR));
 
 	dev_dbg(ispi->dev, "Protected regions:\n");
-	for (i = 0; i < PR_NUM; i++) {
+	for (i = 0; i < ispi->pr_num; i++) {
 		u32 base, limit;
 
 		value = readl(ispi->pregs + PR(i));
@@ -286,6 +289,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->sregs = ispi->base + BYT_SSFSTS_CTL;
 		ispi->pregs = ispi->base + BYT_PR;
 		ispi->nregions = BYT_FREG_NUM;
+		ispi->pr_num = BYT_PR_NUM;
 
 		if (writeable) {
 			/* Disable write protection */
@@ -305,12 +309,14 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->sregs = ispi->base + LPT_SSFSTS_CTL;
 		ispi->pregs = ispi->base + LPT_PR;
 		ispi->nregions = LPT_FREG_NUM;
+		ispi->pr_num = LPT_PR_NUM;
 		break;
 
 	case INTEL_SPI_BXT:
 		ispi->sregs = ispi->base + BXT_SSFSTS_CTL;
 		ispi->pregs = ispi->base + BXT_PR;
 		ispi->nregions = BXT_FREG_NUM;
+		ispi->pr_num = BXT_PR_NUM;
 		ispi->erase_64k = true;
 		break;
 
@@ -652,7 +658,7 @@ static bool intel_spi_is_protected(const struct intel_spi *ispi,
 {
 	int i;
 
-	for (i = 0; i < PR_NUM; i++) {
+	for (i = 0; i < ispi->pr_num; i++) {
 		u32 pr_base, pr_limit, pr_value;
 
 		pr_value = readl(ispi->pregs + PR(i));
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
  2017-09-11  9:41 ` [PATCH v2 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 03/10] spi-nor: intel-spi: Fix broken software sequencing codes Bin Meng
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

intel_spi_hw_cycle() and intel_spi_sw_cycle() don't use the parameter
'buf' at all. Remove it.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index e5b52e8..07626ca 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -377,8 +377,7 @@ static int intel_spi_opcode_index(struct intel_spi *ispi, u8 opcode)
 	return -EINVAL;
 }
 
-static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, u8 *buf,
-			      int len)
+static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 {
 	u32 val, status;
 	int ret;
@@ -418,8 +417,7 @@ static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, u8 *buf,
 	return 0;
 }
 
-static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, u8 *buf,
-			      int len)
+static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 {
 	u32 val, status;
 	int ret;
@@ -456,9 +454,9 @@ static int intel_spi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	writel(0, ispi->base + FADDR);
 
 	if (ispi->swseq)
-		ret = intel_spi_sw_cycle(ispi, opcode, buf, len);
+		ret = intel_spi_sw_cycle(ispi, opcode, len);
 	else
-		ret = intel_spi_hw_cycle(ispi, opcode, buf, len);
+		ret = intel_spi_hw_cycle(ispi, opcode, len);
 
 	if (ret)
 		return ret;
@@ -486,8 +484,8 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 		return ret;
 
 	if (ispi->swseq)
-		return intel_spi_sw_cycle(ispi, opcode, buf, len);
-	return intel_spi_hw_cycle(ispi, opcode, buf, len);
+		return intel_spi_sw_cycle(ispi, opcode, len);
+	return intel_spi_hw_cycle(ispi, opcode, len);
 }
 
 static ssize_t intel_spi_read(struct spi_nor *nor, loff_t from, size_t len,
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 03/10] spi-nor: intel-spi: Fix broken software sequencing codes
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
  2017-09-11  9:41 ` [PATCH v2 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT Bin Meng
  2017-09-11  9:41 ` [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 04/10] spi-nor: intel-spi: Check transfer length in the HW/SW cycle Bin Meng
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese, stable

There are two bugs in current intel_spi_sw_cycle():

- The 'data byte count' field should be the number of bytes
  transferred minus 1
- SSFSTS_CTL is the offset from ispi->sregs, not ispi->base

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Cc: <stable@vger.kernel.org> # v4.11+
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

---

Changes in v2:
- Add stable kernel tags in the commit message

 drivers/mtd/spi-nor/intel-spi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 07626ca..263c6ab 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -426,7 +426,7 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 	if (ret < 0)
 		return ret;
 
-	val = (len << SSFSTS_CTL_DBC_SHIFT) | SSFSTS_CTL_DS;
+	val = ((len - 1) << SSFSTS_CTL_DBC_SHIFT) | SSFSTS_CTL_DS;
 	val |= ret << SSFSTS_CTL_COP_SHIFT;
 	val |= SSFSTS_CTL_FCERR | SSFSTS_CTL_FDONE;
 	val |= SSFSTS_CTL_SCGO;
@@ -436,7 +436,7 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 	if (ret)
 		return ret;
 
-	status = readl(ispi->base + SSFSTS_CTL);
+	status = readl(ispi->sregs + SSFSTS_CTL);
 	if (status & SSFSTS_CTL_FCERR)
 		return -EIO;
 	else if (status & SSFSTS_CTL_AEL)
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 04/10] spi-nor: intel-spi: Check transfer length in the HW/SW cycle
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (2 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 03/10] spi-nor: intel-spi: Fix broken software sequencing codes Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 05/10] spi-nor: intel-spi: Use SW sequencer for BYT/LPT Bin Meng
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

Intel SPI controller only has a 64 bytes FIFO. This adds a sanity
check before triggering any HW/SW sequencer work.

Additionally for the SW sequencer, if given data length is zero,
we should not mark the 'Data Cycle' bit.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 263c6ab..c4a9de6 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -399,6 +399,9 @@ static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 		return -EINVAL;
 	}
 
+	if (len > INTEL_SPI_FIFO_SZ)
+		return -EINVAL;
+
 	val |= (len - 1) << HSFSTS_CTL_FDBC_SHIFT;
 	val |= HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
 	val |= HSFSTS_CTL_FGO;
@@ -419,14 +422,19 @@ static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 
 static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 {
-	u32 val, status;
+	u32 val = 0, status;
 	int ret;
 
 	ret = intel_spi_opcode_index(ispi, opcode);
 	if (ret < 0)
 		return ret;
 
-	val = ((len - 1) << SSFSTS_CTL_DBC_SHIFT) | SSFSTS_CTL_DS;
+	if (len > INTEL_SPI_FIFO_SZ)
+		return -EINVAL;
+
+	/* Only mark 'Data Cycle' bit when there is data to be transferred */
+	if (len > 0)
+		val = ((len - 1) << SSFSTS_CTL_DBC_SHIFT) | SSFSTS_CTL_DS;
 	val |= ret << SSFSTS_CTL_COP_SHIFT;
 	val |= SSFSTS_CTL_FCERR | SSFSTS_CTL_FDONE;
 	val |= SSFSTS_CTL_SCGO;
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 05/10] spi-nor: intel-spi: Use SW sequencer for BYT/LPT
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (3 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 04/10] spi-nor: intel-spi: Check transfer length in the HW/SW cycle Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 06/10] spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in intel_spi_write() Bin Meng
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

Baytrail/Lynx Point SPI controller's HW sequencer only supports basic
operations. This is determined by the chipset design, however current
codes try to use register values in OPMENU0/OPMENU1 to see whether SW
sequencer should be used, which is wrong. In fact OPMENU0/OPMENU1 can
remain unprogrammed by some bootloaders.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

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

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index c4a9de6..d0237fe 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -290,6 +290,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->pregs = ispi->base + BYT_PR;
 		ispi->nregions = BYT_FREG_NUM;
 		ispi->pr_num = BYT_PR_NUM;
+		ispi->swseq = true;
 
 		if (writeable) {
 			/* Disable write protection */
@@ -310,6 +311,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->pregs = ispi->base + LPT_PR;
 		ispi->nregions = LPT_FREG_NUM;
 		ispi->pr_num = LPT_PR_NUM;
+		ispi->swseq = true;
 		break;
 
 	case INTEL_SPI_BXT:
@@ -324,12 +326,24 @@ static int intel_spi_init(struct intel_spi *ispi)
 		return -EINVAL;
 	}
 
-	/* Disable #SMI generation */
+	/* Disable #SMI generation from HW sequencer */
 	val = readl(ispi->base + HSFSTS_CTL);
 	val &= ~HSFSTS_CTL_FSMIE;
 	writel(val, ispi->base + HSFSTS_CTL);
 
 	/*
+	 * Some controllers can only do basic operations using hardware
+	 * sequencer. All other operations are supposed to be carried out
+	 * using software sequencer.
+	 */
+	if (ispi->swseq) {
+		/* Disable #SMI generation from SW sequencer */
+		val = readl(ispi->sregs + SSFSTS_CTL);
+		val &= ~SSFSTS_CTL_FSMIE;
+		writel(val, ispi->sregs + SSFSTS_CTL);
+	}
+
+	/*
 	 * BIOS programs allowed opcodes and then locks down the register.
 	 * So read back what opcodes it decided to support. That's the set
 	 * we are going to support as well.
@@ -337,13 +351,6 @@ static int intel_spi_init(struct intel_spi *ispi)
 	opmenu0 = readl(ispi->sregs + OPMENU0);
 	opmenu1 = readl(ispi->sregs + OPMENU1);
 
-	/*
-	 * Some controllers can only do basic operations using hardware
-	 * sequencer. All other operations are supposed to be carried out
-	 * using software sequencer. If we find that BIOS has programmed
-	 * opcodes for the software sequencer we use that over the hardware
-	 * sequencer.
-	 */
 	if (opmenu0 && opmenu1) {
 		for (i = 0; i < ARRAY_SIZE(ispi->opcodes) / 2; i++) {
 			ispi->opcodes[i] = opmenu0 >> i * 8;
@@ -353,13 +360,6 @@ static int intel_spi_init(struct intel_spi *ispi)
 		val = readl(ispi->sregs + PREOP_OPTYPE);
 		ispi->preopcodes[0] = val;
 		ispi->preopcodes[1] = val >> 8;
-
-		/* Disable #SMI generation from SW sequencer */
-		val = readl(ispi->sregs + SSFSTS_CTL);
-		val &= ~SSFSTS_CTL_FSMIE;
-		writel(val, ispi->sregs + SSFSTS_CTL);
-
-		ispi->swseq = true;
 	}
 
 	intel_spi_dump_regs(ispi);
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 06/10] spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in intel_spi_write()
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (4 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 05/10] spi-nor: intel-spi: Use SW sequencer for BYT/LPT Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 07/10] spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS Bin Meng
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

So far intel_spi_write() uses the HW sequencer to do the write. But
the HW sequencer register HSFSTS_CTL does not have such a field for
'Atomic Cycle Sequence', remove it.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index d0237fe..757b9f1 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -572,11 +572,6 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
 		val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
 		val |= (block_size - 1) << HSFSTS_CTL_FDBC_SHIFT;
 		val |= HSFSTS_CTL_FCYCLE_WRITE;
-
-		/* Write enable */
-		if (ispi->preopcodes[1] == SPINOR_OP_WREN)
-			val |= SSFSTS_CTL_SPOP;
-		val |= SSFSTS_CTL_ACS;
 		writel(val, ispi->base + HSFSTS_CTL);
 
 		ret = intel_spi_write_block(ispi, write_buf, block_size);
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 07/10] spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (5 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 06/10] spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in intel_spi_write() Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 08/10] spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW Bin Meng
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

At present the driver relies on valid OPMENU0/OPMENU1 register values
that are programmed by BIOS to function correctly. However in a real
world it's absolutely legitimate for a bootloader to leave these two
registers untouched. Intel FSP for Baytrail exactly does like this.
When we are booting from any Intel FSP based bootloaders like U-Boot,
the driver refuses to work.

We can of course program various flash opcodes in the OPMENU0/OPMENU1
registers, and such workaround can be added in either the bootloader
codes, or the kernel driver itself.

But a graceful solution would be to update the kernel driver to remove
such limitation of OPMENU0/1 register dependency. The SPI controller
settings are not locked under such configuration. So we can first check
the controller locking status, and if it is not locked that means the
driver job can be fulfilled by using a chosen OPMENU index to set up
the flash opcode every time.

While we are here, the missing 'Atomic Cycle Sequence' handling in the
SW sequencer codes is also added.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 91 +++++++++++++++++++++++++++++------------
 1 file changed, 65 insertions(+), 26 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 757b9f1..07146ab 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -88,6 +88,11 @@
 #define OPMENU0				0x08
 #define OPMENU1				0x0c
 
+#define OPTYPE_READ_NO_ADDR		0
+#define OPTYPE_WRITE_NO_ADDR		1
+#define OPTYPE_READ_WITH_ADDR		2
+#define OPTYPE_WRITE_WITH_ADDR		3
+
 /* CPU specifics */
 #define BYT_PR				0x74
 #define BYT_SSFSTS_CTL			0x90
@@ -120,6 +125,7 @@
  * @nregions: Maximum number of regions
  * @pr_num: Maximum number of protected range registers
  * @writeable: Is the chip writeable
+ * @locked: Is SPI setting locked
  * @swseq: Use SW sequencer in register reads/writes
  * @erase_64k: 64k erase supported
  * @opcodes: Opcodes which are supported. This are programmed by BIOS
@@ -136,6 +142,7 @@ struct intel_spi {
 	size_t nregions;
 	size_t pr_num;
 	bool writeable;
+	bool locked;
 	bool swseq;
 	bool erase_64k;
 	u8 opcodes[8];
@@ -343,23 +350,29 @@ static int intel_spi_init(struct intel_spi *ispi)
 		writel(val, ispi->sregs + SSFSTS_CTL);
 	}
 
-	/*
-	 * BIOS programs allowed opcodes and then locks down the register.
-	 * So read back what opcodes it decided to support. That's the set
-	 * we are going to support as well.
-	 */
-	opmenu0 = readl(ispi->sregs + OPMENU0);
-	opmenu1 = readl(ispi->sregs + OPMENU1);
+	/* Check controller's lock status */
+	val = readl(ispi->base + HSFSTS_CTL);
+	ispi->locked = !!(val & HSFSTS_CTL_FLOCKDN);
 
-	if (opmenu0 && opmenu1) {
-		for (i = 0; i < ARRAY_SIZE(ispi->opcodes) / 2; i++) {
-			ispi->opcodes[i] = opmenu0 >> i * 8;
-			ispi->opcodes[i + 4] = opmenu1 >> i * 8;
-		}
+	if (ispi->locked) {
+		/*
+		 * BIOS programs allowed opcodes and then locks down the
+		 * register. So read back what opcodes it decided to support.
+		 * That's the set we are going to support as well.
+		 */
+		opmenu0 = readl(ispi->sregs + OPMENU0);
+		opmenu1 = readl(ispi->sregs + OPMENU1);
 
-		val = readl(ispi->sregs + PREOP_OPTYPE);
-		ispi->preopcodes[0] = val;
-		ispi->preopcodes[1] = val >> 8;
+		if (opmenu0 && opmenu1) {
+			for (i = 0; i < ARRAY_SIZE(ispi->opcodes) / 2; i++) {
+				ispi->opcodes[i] = opmenu0 >> i * 8;
+				ispi->opcodes[i + 4] = opmenu1 >> i * 8;
+			}
+
+			val = readl(ispi->sregs + PREOP_OPTYPE);
+			ispi->preopcodes[0] = val;
+			ispi->preopcodes[1] = val >> 8;
+		}
 	}
 
 	intel_spi_dump_regs(ispi);
@@ -367,14 +380,25 @@ static int intel_spi_init(struct intel_spi *ispi)
 	return 0;
 }
 
-static int intel_spi_opcode_index(struct intel_spi *ispi, u8 opcode)
+static int intel_spi_opcode_index(struct intel_spi *ispi, u8 opcode, int optype)
 {
 	int i;
+	int preop;
 
-	for (i = 0; i < ARRAY_SIZE(ispi->opcodes); i++)
-		if (ispi->opcodes[i] == opcode)
-			return i;
-	return -EINVAL;
+	if (ispi->locked) {
+		for (i = 0; i < ARRAY_SIZE(ispi->opcodes); i++)
+			if (ispi->opcodes[i] == opcode)
+				return i;
+
+		return -EINVAL;
+	}
+
+	/* The lock is off, so just use index 0 */
+	writel(opcode, ispi->sregs + OPMENU0);
+	preop = readw(ispi->sregs + PREOP_OPTYPE);
+	writel(optype << 16 | preop, ispi->sregs + PREOP_OPTYPE);
+
+	return 0;
 }
 
 static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, int len)
@@ -420,12 +444,14 @@ static int intel_spi_hw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 	return 0;
 }
 
-static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
+static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len,
+			      int optype)
 {
 	u32 val = 0, status;
+	u16 preop;
 	int ret;
 
-	ret = intel_spi_opcode_index(ispi, opcode);
+	ret = intel_spi_opcode_index(ispi, opcode, optype);
 	if (ret < 0)
 		return ret;
 
@@ -438,6 +464,12 @@ static int intel_spi_sw_cycle(struct intel_spi *ispi, u8 opcode, int len)
 	val |= ret << SSFSTS_CTL_COP_SHIFT;
 	val |= SSFSTS_CTL_FCERR | SSFSTS_CTL_FDONE;
 	val |= SSFSTS_CTL_SCGO;
+	preop = readw(ispi->sregs + PREOP_OPTYPE);
+	if (preop) {
+		val |= SSFSTS_CTL_ACS;
+		if (preop >> 8)
+			val |= SSFSTS_CTL_SPOP;
+	}
 	writel(val, ispi->sregs + SSFSTS_CTL);
 
 	ret = intel_spi_wait_sw_busy(ispi);
@@ -462,7 +494,8 @@ static int intel_spi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	writel(0, ispi->base + FADDR);
 
 	if (ispi->swseq)
-		ret = intel_spi_sw_cycle(ispi, opcode, len);
+		ret = intel_spi_sw_cycle(ispi, opcode, len,
+					 OPTYPE_READ_NO_ADDR);
 	else
 		ret = intel_spi_hw_cycle(ispi, opcode, len);
 
@@ -479,10 +512,15 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 
 	/*
 	 * This is handled with atomic operation and preop code in Intel
-	 * controller so skip it here now.
+	 * controller so skip it here now. If the controller is not locked,
+	 * program the opcode to the PREOP register for later use.
 	 */
-	if (opcode == SPINOR_OP_WREN)
+	if (opcode == SPINOR_OP_WREN) {
+		if (!ispi->locked)
+			writel(opcode, ispi->sregs + PREOP_OPTYPE);
+
 		return 0;
+	}
 
 	writel(0, ispi->base + FADDR);
 
@@ -492,7 +530,8 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 		return ret;
 
 	if (ispi->swseq)
-		return intel_spi_sw_cycle(ispi, opcode, len);
+		return intel_spi_sw_cycle(ispi, opcode, len,
+					  OPTYPE_WRITE_NO_ADDR);
 	return intel_spi_hw_cycle(ispi, opcode, len);
 }
 
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 08/10] spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (6 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 07/10] spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:41 ` [PATCH v2 09/10] spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi' Bin Meng
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

There is no code that alters the HSFSTS register content in between
in intel_spi_write(). Remove the unnecessary RW to save some cycles.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

 drivers/mtd/spi-nor/intel-spi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 07146ab..91ceef7 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -611,7 +611,6 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
 		val |= HSFSTS_CTL_AEL | HSFSTS_CTL_FCERR | HSFSTS_CTL_FDONE;
 		val |= (block_size - 1) << HSFSTS_CTL_FDBC_SHIFT;
 		val |= HSFSTS_CTL_FCYCLE_WRITE;
-		writel(val, ispi->base + HSFSTS_CTL);
 
 		ret = intel_spi_write_block(ispi, write_buf, block_size);
 		if (ret) {
@@ -620,8 +619,8 @@ static ssize_t intel_spi_write(struct spi_nor *nor, loff_t to, size_t len,
 		}
 
 		/* Start the write now */
-		val = readl(ispi->base + HSFSTS_CTL);
-		writel(val | HSFSTS_CTL_FGO, ispi->base + HSFSTS_CTL);
+		val |= HSFSTS_CTL_FGO;
+		writel(val, ispi->base + HSFSTS_CTL);
 
 		ret = intel_spi_wait_hw_busy(ispi);
 		if (ret) {
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 09/10] spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi'
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (7 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 08/10] spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW Bin Meng
@ 2017-09-11  9:41 ` Bin Meng
  2017-09-11  9:42 ` [PATCH v2 10/10] spi-nor: intel-spi: Fall back to use SW sequencer to erase Bin Meng
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:41 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

The ispi->swseq is used for register access. Let's rename it to
swseq_reg to better describe its usage.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---

Changes in v2: None

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

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 91ceef7..5e7a389 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -126,7 +126,7 @@
  * @pr_num: Maximum number of protected range registers
  * @writeable: Is the chip writeable
  * @locked: Is SPI setting locked
- * @swseq: Use SW sequencer in register reads/writes
+ * @swseq_reg: Use SW sequencer in register reads/writes
  * @erase_64k: 64k erase supported
  * @opcodes: Opcodes which are supported. This are programmed by BIOS
  *           before it locks down the controller.
@@ -143,7 +143,7 @@ struct intel_spi {
 	size_t pr_num;
 	bool writeable;
 	bool locked;
-	bool swseq;
+	bool swseq_reg;
 	bool erase_64k;
 	u8 opcodes[8];
 	u8 preopcodes[2];
@@ -224,7 +224,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
 	}
 
 	dev_dbg(ispi->dev, "Using %cW sequencer for register access\n",
-		ispi->swseq ? 'S' : 'H');
+		ispi->swseq_reg ? 'S' : 'H');
 }
 
 /* Reads max INTEL_SPI_FIFO_SZ bytes from the device fifo */
@@ -297,7 +297,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->pregs = ispi->base + BYT_PR;
 		ispi->nregions = BYT_FREG_NUM;
 		ispi->pr_num = BYT_PR_NUM;
-		ispi->swseq = true;
+		ispi->swseq_reg = true;
 
 		if (writeable) {
 			/* Disable write protection */
@@ -318,7 +318,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 		ispi->pregs = ispi->base + LPT_PR;
 		ispi->nregions = LPT_FREG_NUM;
 		ispi->pr_num = LPT_PR_NUM;
-		ispi->swseq = true;
+		ispi->swseq_reg = true;
 		break;
 
 	case INTEL_SPI_BXT:
@@ -343,7 +343,7 @@ static int intel_spi_init(struct intel_spi *ispi)
 	 * sequencer. All other operations are supposed to be carried out
 	 * using software sequencer.
 	 */
-	if (ispi->swseq) {
+	if (ispi->swseq_reg) {
 		/* Disable #SMI generation from SW sequencer */
 		val = readl(ispi->sregs + SSFSTS_CTL);
 		val &= ~SSFSTS_CTL_FSMIE;
@@ -493,7 +493,7 @@ static int intel_spi_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	/* Address of the first chip */
 	writel(0, ispi->base + FADDR);
 
-	if (ispi->swseq)
+	if (ispi->swseq_reg)
 		ret = intel_spi_sw_cycle(ispi, opcode, len,
 					 OPTYPE_READ_NO_ADDR);
 	else
@@ -529,7 +529,7 @@ static int intel_spi_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
 	if (ret)
 		return ret;
 
-	if (ispi->swseq)
+	if (ispi->swseq_reg)
 		return intel_spi_sw_cycle(ispi, opcode, len,
 					  OPTYPE_WRITE_NO_ADDR);
 	return intel_spi_hw_cycle(ispi, opcode, len);
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 10/10] spi-nor: intel-spi: Fall back to use SW sequencer to erase
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (8 preceding siblings ...)
  2017-09-11  9:41 ` [PATCH v2 09/10] spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi' Bin Meng
@ 2017-09-11  9:42 ` Bin Meng
  2017-09-11 17:44   ` Joakim Tjernlund
  2017-10-11  8:06 ` Cyrille Pitchen
  11 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-11  9:42 UTC (permalink / raw)
  To: Mika Westerberg, Cyrille Pitchen, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

According to the datasheet, the HW sequencer has a predefined list
of opcodes, with only the erase opcode being programmable in LVSCC
and UVSCC registers. If these registers don't contain a valid erase
opcode (eg: BIOS does not program it), erase cannot be done using
the HW sequencer, even though the erase operation does not report
any error, the flash remains not erased.

If such register setting is detected, let's fall back to use the SW
sequencer to erase instead.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

---

Changes in v2:
- Fix typo of 'operatoin'

 drivers/mtd/spi-nor/intel-spi.c | 50 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
index 5e7a389..ef034d8 100644
--- a/drivers/mtd/spi-nor/intel-spi.c
+++ b/drivers/mtd/spi-nor/intel-spi.c
@@ -111,6 +111,13 @@
 #define BXT_FREG_NUM			12
 #define BXT_PR_NUM			6
 
+#define LVSCC				0xc4
+#define UVSCC				0xc8
+#define ERASE_OPCODE_SHIFT		8
+#define ERASE_OPCODE_MASK		(0xff << ERASE_OPCODE_SHIFT)
+#define ERASE_64K_OPCODE_SHIFT		16
+#define ERASE_64K_OPCODE_MASK		(0xff << ERASE_OPCODE_SHIFT)
+
 #define INTEL_SPI_TIMEOUT		5000 /* ms */
 #define INTEL_SPI_FIFO_SZ		64
 
@@ -127,6 +134,7 @@
  * @writeable: Is the chip writeable
  * @locked: Is SPI setting locked
  * @swseq_reg: Use SW sequencer in register reads/writes
+ * @swseq_erase: Use SW sequencer in erase operation
  * @erase_64k: 64k erase supported
  * @opcodes: Opcodes which are supported. This are programmed by BIOS
  *           before it locks down the controller.
@@ -144,6 +152,7 @@ struct intel_spi {
 	bool writeable;
 	bool locked;
 	bool swseq_reg;
+	bool swseq_erase;
 	bool erase_64k;
 	u8 opcodes[8];
 	u8 preopcodes[2];
@@ -191,6 +200,9 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
 	if (ispi->info->type == INTEL_SPI_BYT)
 		dev_dbg(ispi->dev, "BCR=0x%08x\n", readl(ispi->base + BYT_BCR));
 
+	dev_dbg(ispi->dev, "LVSCC=0x%08x\n", readl(ispi->base + LVSCC));
+	dev_dbg(ispi->dev, "UVSCC=0x%08x\n", readl(ispi->base + UVSCC));
+
 	dev_dbg(ispi->dev, "Protected regions:\n");
 	for (i = 0; i < ispi->pr_num; i++) {
 		u32 base, limit;
@@ -225,6 +237,8 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
 
 	dev_dbg(ispi->dev, "Using %cW sequencer for register access\n",
 		ispi->swseq_reg ? 'S' : 'H');
+	dev_dbg(ispi->dev, "Using %cW sequencer for erase operation\n",
+		ispi->swseq_erase ? 'S' : 'H');
 }
 
 /* Reads max INTEL_SPI_FIFO_SZ bytes from the device fifo */
@@ -288,7 +302,7 @@ static int intel_spi_wait_sw_busy(struct intel_spi *ispi)
 
 static int intel_spi_init(struct intel_spi *ispi)
 {
-	u32 opmenu0, opmenu1, val;
+	u32 opmenu0, opmenu1, lvscc, uvscc, val;
 	int i;
 
 	switch (ispi->info->type) {
@@ -339,6 +353,24 @@ static int intel_spi_init(struct intel_spi *ispi)
 	writel(val, ispi->base + HSFSTS_CTL);
 
 	/*
+	 * Determine whether erase operation should use HW or SW sequencer.
+	 *
+	 * The HW sequencer has a predefined list of opcodes, with only the
+	 * erase opcode being programmable in LVSCC and UVSCC registers.
+	 * If these registers don't contain a valid erase opcode, erase
+	 * cannot be done using HW sequencer.
+	 */
+	lvscc = readl(ispi->base + LVSCC);
+	uvscc = readl(ispi->base + UVSCC);
+	if (!(lvscc & ERASE_OPCODE_MASK) || !(uvscc & ERASE_OPCODE_MASK))
+		ispi->swseq_erase = true;
+	/* SPI controller on Intel BXT supports 64K erase opcode */
+	if (ispi->info->type == INTEL_SPI_BXT && !ispi->swseq_erase)
+		if (!(lvscc & ERASE_64K_OPCODE_MASK) ||
+		    !(uvscc & ERASE_64K_OPCODE_MASK))
+			ispi->erase_64k = false;
+
+	/*
 	 * Some controllers can only do basic operations using hardware
 	 * sequencer. All other operations are supposed to be carried out
 	 * using software sequencer.
@@ -665,6 +697,22 @@ static int intel_spi_erase(struct spi_nor *nor, loff_t offs)
 		erase_size = SZ_4K;
 	}
 
+	if (ispi->swseq_erase) {
+		while (len > 0) {
+			writel(offs, ispi->base + FADDR);
+
+			ret = intel_spi_sw_cycle(ispi, nor->erase_opcode,
+						 0, OPTYPE_WRITE_WITH_ADDR);
+			if (ret)
+				return ret;
+
+			offs += erase_size;
+			len -= erase_size;
+		}
+
+		return 0;
+	}
+
 	while (len > 0) {
 		writel(offs, ispi->base + FADDR);
 
-- 
2.9.2

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
@ 2017-09-11 17:44   ` Joakim Tjernlund
  2017-09-11  9:41 ` [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle Bin Meng
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2017-09-11 17:44 UTC (permalink / raw)
  To: linux-kernel, linux-mtd, bmeng.cn, mika.westerberg,
	cyrille.pitchen, dwmw2, computersforpeace, boris.brezillon,
	marek.vasut, richard
  Cc: sr

On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
> This series does several bug fixes and clean ups against the intel-spi
> spi-nor driver, as well as enhancements to make the driver independent
> on the underlying BIOS/bootloader.
> 
> At present the driver uses the HW sequencer for the read/write/erase on
> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
> sequencer relies on some programmed register settings and hence creates
> unneeded dependencies with the underlying BIOS/bootloader. For example,
> the driver unfortunately does not work as expected when booting from
> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
> does not set up some SPI controller settings to make the driver happy.
> Now such limitation has been removed with this series.

Hi Bin

Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1 
and the mtd driver seems to only map the first of those flashes. Is this intentional or
are we missing something?

 Jocke

> 
> Changes in v2:
> - Add stable kernel tags in the commit message (patch [03/10])
> - Fix typo of 'operatoin' (patch [10/10])
> - Add Mika Westerberg's 'Acked-by' tag
> 
> Bin Meng (10):
>   spi-nor: intel-spi: Fix number of protected range registers for
>     BYT/LPT
>   spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle
>   spi-nor: intel-spi: Fix broken software sequencing codes
>   spi-nor: intel-spi: Check transfer length in the HW/SW cycle
>   spi-nor: intel-spi: Use SW sequencer for BYT/LPT
>   spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in
>     intel_spi_write()
>   spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS
>   spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW
>   spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi'
>   spi-nor: intel-spi: Fall back to use SW sequencer to erase
> 
>  drivers/mtd/spi-nor/intel-spi.c | 209 +++++++++++++++++++++++++++++-----------
>  1 file changed, 151 insertions(+), 58 deletions(-)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
@ 2017-09-11 17:44   ` Joakim Tjernlund
  0 siblings, 0 replies; 18+ messages in thread
From: Joakim Tjernlund @ 2017-09-11 17:44 UTC (permalink / raw)
  To: linux-kernel, linux-mtd, bmeng.cn, mika.westerberg,
	cyrille.pitchen, dwmw2, computersforpeace, boris.brezillon,
	marek.vasut, richard
  Cc: sr

On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
> This series does several bug fixes and clean ups against the intel-spi
> spi-nor driver, as well as enhancements to make the driver independent
> on the underlying BIOS/bootloader.
> 
> At present the driver uses the HW sequencer for the read/write/erase on
> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
> sequencer relies on some programmed register settings and hence creates
> unneeded dependencies with the underlying BIOS/bootloader. For example,
> the driver unfortunately does not work as expected when booting from
> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
> does not set up some SPI controller settings to make the driver happy.
> Now such limitation has been removed with this series.

Hi Bin

Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1 
and the mtd driver seems to only map the first of those flashes. Is this intentional or
are we missing something?

 Jocke

> 
> Changes in v2:
> - Add stable kernel tags in the commit message (patch [03/10])
> - Fix typo of 'operatoin' (patch [10/10])
> - Add Mika Westerberg's 'Acked-by' tag
> 
> Bin Meng (10):
>   spi-nor: intel-spi: Fix number of protected range registers for
>     BYT/LPT
>   spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle
>   spi-nor: intel-spi: Fix broken software sequencing codes
>   spi-nor: intel-spi: Check transfer length in the HW/SW cycle
>   spi-nor: intel-spi: Use SW sequencer for BYT/LPT
>   spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in
>     intel_spi_write()
>   spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS
>   spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW
>   spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi'
>   spi-nor: intel-spi: Fall back to use SW sequencer to erase
> 
>  drivers/mtd/spi-nor/intel-spi.c | 209 +++++++++++++++++++++++++++++-----------
>  1 file changed, 151 insertions(+), 58 deletions(-)
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
  2017-09-11 17:44   ` Joakim Tjernlund
@ 2017-09-13  2:11     ` Bin Meng
  -1 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-13  2:11 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: linux-kernel, linux-mtd, mika.westerberg, cyrille.pitchen, dwmw2,
	computersforpeace, boris.brezillon, marek.vasut, richard, sr

Hi Joakim,

On Tue, Sep 12, 2017 at 1:44 AM, Joakim Tjernlund
<Joakim.Tjernlund@infinera.com> wrote:
> On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
>> This series does several bug fixes and clean ups against the intel-spi
>> spi-nor driver, as well as enhancements to make the driver independent
>> on the underlying BIOS/bootloader.
>>
>> At present the driver uses the HW sequencer for the read/write/erase on
>> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
>> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
>> sequencer relies on some programmed register settings and hence creates
>> unneeded dependencies with the underlying BIOS/bootloader. For example,
>> the driver unfortunately does not work as expected when booting from
>> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
>> does not set up some SPI controller settings to make the driver happy.
>> Now such limitation has been removed with this series.
>
> Hi Bin
>
> Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1
> and the mtd driver seems to only map the first of those flashes. Is this intentional or
> are we missing something?
>

All the boards I have tested only have one SPI flash. Mika, any comments?

Regards,
Bin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
@ 2017-09-13  2:11     ` Bin Meng
  0 siblings, 0 replies; 18+ messages in thread
From: Bin Meng @ 2017-09-13  2:11 UTC (permalink / raw)
  To: Joakim Tjernlund
  Cc: linux-kernel, linux-mtd, mika.westerberg, cyrille.pitchen, dwmw2,
	computersforpeace, boris.brezillon, marek.vasut, richard, sr

Hi Joakim,

On Tue, Sep 12, 2017 at 1:44 AM, Joakim Tjernlund
<Joakim.Tjernlund@infinera.com> wrote:
> On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
>> This series does several bug fixes and clean ups against the intel-spi
>> spi-nor driver, as well as enhancements to make the driver independent
>> on the underlying BIOS/bootloader.
>>
>> At present the driver uses the HW sequencer for the read/write/erase on
>> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
>> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
>> sequencer relies on some programmed register settings and hence creates
>> unneeded dependencies with the underlying BIOS/bootloader. For example,
>> the driver unfortunately does not work as expected when booting from
>> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
>> does not set up some SPI controller settings to make the driver happy.
>> Now such limitation has been removed with this series.
>
> Hi Bin
>
> Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1
> and the mtd driver seems to only map the first of those flashes. Is this intentional or
> are we missing something?
>

All the boards I have tested only have one SPI flash. Mika, any comments?

Regards,
Bin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
  2017-09-13  2:11     ` Bin Meng
@ 2017-09-13  9:47       ` mika.westerberg
  -1 siblings, 0 replies; 18+ messages in thread
From: mika.westerberg @ 2017-09-13  9:47 UTC (permalink / raw)
  To: Bin Meng
  Cc: Joakim Tjernlund, linux-kernel, linux-mtd, cyrille.pitchen,
	dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard,
	sr

On Wed, Sep 13, 2017 at 10:11:21AM +0800, Bin Meng wrote:
> Hi Joakim,
> 
> On Tue, Sep 12, 2017 at 1:44 AM, Joakim Tjernlund
> <Joakim.Tjernlund@infinera.com> wrote:
> > On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
> >> This series does several bug fixes and clean ups against the intel-spi
> >> spi-nor driver, as well as enhancements to make the driver independent
> >> on the underlying BIOS/bootloader.
> >>
> >> At present the driver uses the HW sequencer for the read/write/erase on
> >> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
> >> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
> >> sequencer relies on some programmed register settings and hence creates
> >> unneeded dependencies with the underlying BIOS/bootloader. For example,
> >> the driver unfortunately does not work as expected when booting from
> >> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
> >> does not set up some SPI controller settings to make the driver happy.
> >> Now such limitation has been removed with this series.
> >
> > Hi Bin
> >
> > Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1
> > and the mtd driver seems to only map the first of those flashes. Is this intentional or
> > are we missing something?
> >
> 
> All the boards I have tested only have one SPI flash. Mika, any comments?

So I don't have such boards either.

However, I think the other CS is mapped to bit 24 of the flash address.
So once you try to address higher than 16MB it should activate the other
CS instead. Not 100% sure, though but for example Intel C620 chipset
datasheet [1] seems to have additional bits in address register (there is
also another CS for TPM).

[1] https://www.intel.com/content/www/us/en/chipsets/c620-series-chipset-datasheet.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
@ 2017-09-13  9:47       ` mika.westerberg
  0 siblings, 0 replies; 18+ messages in thread
From: mika.westerberg @ 2017-09-13  9:47 UTC (permalink / raw)
  To: Bin Meng
  Cc: Joakim Tjernlund, linux-kernel, linux-mtd, cyrille.pitchen,
	dwmw2, computersforpeace, boris.brezillon, marek.vasut, richard,
	sr

On Wed, Sep 13, 2017 at 10:11:21AM +0800, Bin Meng wrote:
> Hi Joakim,
> 
> On Tue, Sep 12, 2017 at 1:44 AM, Joakim Tjernlund
> <Joakim.Tjernlund@infinera.com> wrote:
> > On Mon, 2017-09-11 at 02:41 -0700, Bin Meng wrote:
> >> This series does several bug fixes and clean ups against the intel-spi
> >> spi-nor driver, as well as enhancements to make the driver independent
> >> on the underlying BIOS/bootloader.
> >>
> >> At present the driver uses the HW sequencer for the read/write/erase on
> >> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
> >> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
> >> sequencer relies on some programmed register settings and hence creates
> >> unneeded dependencies with the underlying BIOS/bootloader. For example,
> >> the driver unfortunately does not work as expected when booting from
> >> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
> >> does not set up some SPI controller settings to make the driver happy.
> >> Now such limitation has been removed with this series.
> >
> > Hi Bin
> >
> > Just starting to test these on Rangeley and got a question: We have two SPI flashes on CS0 resp. CS1
> > and the mtd driver seems to only map the first of those flashes. Is this intentional or
> > are we missing something?
> >
> 
> All the boards I have tested only have one SPI flash. Mika, any comments?

So I don't have such boards either.

However, I think the other CS is mapped to bit 24 of the flash address.
So once you try to address higher than 16MB it should activate the other
CS instead. Not 100% sure, though but for example Intel C620 chipset
datasheet [1] seems to have additional bits in address register (there is
also another CS for TPM).

[1] https://www.intel.com/content/www/us/en/chipsets/c620-series-chipset-datasheet.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements
  2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
                   ` (10 preceding siblings ...)
  2017-09-11 17:44   ` Joakim Tjernlund
@ 2017-10-11  8:06 ` Cyrille Pitchen
  11 siblings, 0 replies; 18+ messages in thread
From: Cyrille Pitchen @ 2017-10-11  8:06 UTC (permalink / raw)
  To: Bin Meng, Mika Westerberg, Marek Vasut, Boris Brezillon,
	Brian Norris, Richard Weinberger, David Woodhouse, linux-mtd,
	linux-kernel
  Cc: Stefan Roese

Le 11/09/2017 à 11:41, Bin Meng a écrit :
> This series does several bug fixes and clean ups against the intel-spi
> spi-nor driver, as well as enhancements to make the driver independent
> on the underlying BIOS/bootloader.
> 
> At present the driver uses the HW sequencer for the read/write/erase on
> all supported platforms, read_reg/write_reg for BXT, and the SW sequencer
> for read_reg/write_reg for BYT/LPT. The way the driver uses the HW and SW
> sequencer relies on some programmed register settings and hence creates
> unneeded dependencies with the underlying BIOS/bootloader. For example,
> the driver unfortunately does not work as expected when booting from
> Intel Baytrail FSP based bootloaders like U-Boot, as the Baytrail FSP
> does not set up some SPI controller settings to make the driver happy.
> Now such limitation has been removed with this series.
> 
> Changes in v2:
> - Add stable kernel tags in the commit message (patch [03/10])
> - Fix typo of 'operatoin' (patch [10/10])
> - Add Mika Westerberg's 'Acked-by' tag
> 
> Bin Meng (10):
>   spi-nor: intel-spi: Fix number of protected range registers for
>     BYT/LPT
>   spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle
>   spi-nor: intel-spi: Fix broken software sequencing codes
>   spi-nor: intel-spi: Check transfer length in the HW/SW cycle
>   spi-nor: intel-spi: Use SW sequencer for BYT/LPT
>   spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in
>     intel_spi_write()
>   spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS
>   spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW
>   spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi'
>   spi-nor: intel-spi: Fall back to use SW sequencer to erase
> 
>  drivers/mtd/spi-nor/intel-spi.c | 209 +++++++++++++++++++++++++++++-----------
>  1 file changed, 151 insertions(+), 58 deletions(-)
> 
Series applied to the spi-nor/next branch of l2-mtd

Thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2017-10-11  8:08 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-11  9:41 [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng
2017-09-11  9:41 ` [PATCH v2 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT Bin Meng
2017-09-11  9:41 ` [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle Bin Meng
2017-09-11  9:41 ` [PATCH v2 03/10] spi-nor: intel-spi: Fix broken software sequencing codes Bin Meng
2017-09-11  9:41 ` [PATCH v2 04/10] spi-nor: intel-spi: Check transfer length in the HW/SW cycle Bin Meng
2017-09-11  9:41 ` [PATCH v2 05/10] spi-nor: intel-spi: Use SW sequencer for BYT/LPT Bin Meng
2017-09-11  9:41 ` [PATCH v2 06/10] spi-nor: intel-spi: Remove 'Atomic Cycle Sequence' in intel_spi_write() Bin Meng
2017-09-11  9:41 ` [PATCH v2 07/10] spi-nor: intel-spi: Don't assume OPMENU0/1 to be programmed by BIOS Bin Meng
2017-09-11  9:41 ` [PATCH v2 08/10] spi-nor: intel-spi: Remove the unnecessary HSFSTS register RW Bin Meng
2017-09-11  9:41 ` [PATCH v2 09/10] spi-nor: intel-spi: Rename swseq to swseq_reg in 'struct intel_spi' Bin Meng
2017-09-11  9:42 ` [PATCH v2 10/10] spi-nor: intel-spi: Fall back to use SW sequencer to erase Bin Meng
2017-09-11 17:44 ` [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Joakim Tjernlund
2017-09-11 17:44   ` Joakim Tjernlund
2017-09-13  2:11   ` Bin Meng
2017-09-13  2:11     ` Bin Meng
2017-09-13  9:47     ` mika.westerberg
2017-09-13  9:47       ` mika.westerberg
2017-10-11  8:06 ` Cyrille Pitchen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.