Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1730240 > unrolled thread

[PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng <bmeng.cn@gmail.com> - 2017-09-11 11:40 +0200
    [PATCH v2 10/10] spi-nor: intel-spi: Fall back to use SW sequencer to erase Bin Meng <bmeng.cn@gmail.com> - 2017-09-11 11:40 +0200
    [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle Bin Meng <bmeng.cn@gmail.com> - 2017-09-11 11:50 +0200
    Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements Bin Meng <bmeng.cn@gmail.com> - 2017-09-13 04:20 +0200
      Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and  enhancements "mika.westerberg@linux.intel.com" <mika.westerberg@linux.intel.com> - 2017-09-13 11:50 +0200

#1730240 — [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements

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

[toc] | [next] | [standalone]

#1730241 — [PATCH v2 10/10] spi-nor: intel-spi: Fall back to use SW sequencer to erase

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

[toc] | [prev] | [next] | [standalone]

#1730247 — [PATCH v2 02/10] spi-nor: intel-spi: Remove useless 'buf' parameter in the HW/SW cycle

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

[toc] | [prev] | [next] | [standalone]

#1731335

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

[toc] | [prev] | [next] | [standalone]

#1731514 — Re: [PATCH v2 00/10] spi-nor: intel-spi: Various fixes and enhancements

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web