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

Groups > linux.kernel > #1624469 > unrolled thread

[PATCH v6 0/4] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories

Back to article view | Back to linux.kernel

Contents

  [PATCH v6 0/4] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories Cyrille Pitchen <cyrille.pitchen@atmel.com> - 2017-04-16 23:50 +0200
    [RFC PATCH v6 4/4] mtd: spi-nor: add spi_nor_init() function Cyrille Pitchen <cyrille.pitchen@atmel.com> - 2017-04-16 23:50 +0200
      Re: [RFC PATCH v6 4/4] mtd: spi-nor: add spi_nor_init() function Marek Vasut <marek.vasut@gmail.com> - 2017-04-17 00:40 +0200
    Re: [PATCH v6 1/4] mtd: spi-nor: introduce more SPI protocols and the  Dual Transfer Mode Marek Vasut <marek.vasut@gmail.com> - 2017-04-17 00:40 +0200

#1624469 — [PATCH v6 0/4] mtd: spi-nor: parse SFDP tables to setup (Q)SPI memories

Hi all,

based on git-hub/spi-nor.
The 4 patches have passed the checkpatch test.

DISCLAIMER: despite what the subjet claims, I've removed the SFDP patches from this
version since they are still RFC/WIP. However I've chosen not to change the subjet
line so it's easier to make the link between this series and its pervious versions.
You can still find the SFDP patches in v5.

Patches 3 and 4 are RFC and are not really needed for now. They are
provided to help Kamal with his own series about adding system
suspend/resume support to the spi-nor framework.


Some words about the reason why I kept the quad_enable() hook inside the
'struct spi_nor_flash_parameter':

Before patch 1:

static int set_quad_mode(struct spi_nor *nor, const struct flash_info *info)
{
	int status;

	switch (JEDEC_MFR(info)) {
	case SNOR_MFR_MACRONIX:
		status = macronix_quad_enable(nor);
		if (status) {
			dev_err(nor->dev, "Macronix quad-read not enabled\n");
			return -EINVAL;
		}
		return status;
	case SNOR_MFR_MICRON:
		return 0;
	default:
		status = spansion_quad_enable(nor);
		if (status) {
			dev_err(nor->dev, "Spansion quad-read not enabled\n");
			return -EINVAL;
		}
		return status;
	}
}
[...]

int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
{

	[...]
	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
		ret = set_quad_mode(nor, info);
		if (ret) {
			dev_err(dev, "quad mode not supported\n");
			return ret;
		}
		nor->flash_read = SPI_NOR_QUAD;
	[...]
}

Since the 'enum read_mode' has been replaced by the
'struct spi_nor_hwcaps', we have to update the if () statement from
spi_nor_scan() because SPI_NOR_QUAD no longer exists.

The original if () statement, checked the memory hardware capabilities
(info->flags & SPI_NOR_QUAD_READ) and the controller harware capabilities
(mode == SPI_NOR_QUAD) to decide whether or not some *_quad_enable()
function had to be called.

After patch 1, things are split into 2 phases:

- phase 1 handled by spi_nor_init_params() to discover the hardware
  capabilities of the SPI flash memory and to initilize the
  'struct spi_nor_flash_parameter' accordingly. For now this structure
  is iniatlized only base on info and JEDEC_MFR(info).
  However we already know that later some flash parameters will be
  overridden when parsing the SFDP tables from spi_nor_init_params().
  Especially the choice of the *_quad_enable() function will be
  directly given by a dedicated field within the Basic Flash Parameter
  Table. That's why params->quad_enable() is set from
  spi_nor_init_params() and not later. It's one of the flash parameters
  after all!
  However at this step, we don't know yet whether or not
  params->quad_enable() should be called.

- phase 2 handled by spi_nor_setup() selects the right settings (best
  matches) for Fast Read, Page Program and Sector Erase operations
  based on the hardware capabilities supported by both the (Q)SPI flash
  memory and the (Q)SPI controller.
  If any Quad SPI protocol (SPI x-y-4) is selected either for Fast Read
  or for Page Progam or for both operations, we must set the QE bit by
 calling params->quad_enable().


I've chosen to integrate ->quad_enable() in the
'struct spi_nor_flash_parameter' also in patch 1 and not in a dedicated
patch because I think we should stay backward compatible and keep the same
features as befor: adding quad_enable() in a dedicated patch would have
not respect the logic of spi_nor_init_params() VS spi_nor_setup() and/or
may have introduce bug if bisecting between patch 1 and patch 1-bis.

Sorry for this LONG explaination but it seemed that some of my choices
needed to be justified some more. I hope those detailed explainations
whould help to clarify what I've done :)



original cover-letter:
"""
This new series of patchs aims to upgrade to spi-nor framework. We need to
take into account latest SPI memories which cannot be handled correctly by
the current implementation.

For instance, SPI NOR memories like Spansion S25FS512S or Macronix
MX25U4035 support only the Fast Read 1-4-4 (EBh) command but not the
Fast Read 1-1-4 (6Bh) command. However the current spi-nor framework
supports only Fast Read 1-1-4 (6Bh).

Also the Spansion S25FS512S memory (and others) may use a non-uniform
sector erase map, whereas the spi-nor framework assumes that a single
sector erase size and opcode can be used anywhere inside the data array.
This assumption is no longer valid.

Then parsing SFDP tables is an attempt to solve many of those issues by
discovering dynamically most of the parameters and settings of the SPI NOR
memory:
- the flash size
- the page size for Page Program commands
- the supported Fast Read commands with the associated opcodes and number
  of mode/wait-state (dummy) cycles.
- the supported Sector/Block Erase commands with the associated opcodes
  and sizes.
- the erase sector map (for non-uniform memory).


Besides, most QSPI controllers from the different vendors are capable to
support the SPI 1-2-2 and 1-4-4 protocols but the spi-nor framework was
not ready to use them. This series also fixes this issue and computes the
best match between the hardware capabilies of both the SPI memory and the
SPI controler (master) to select the right opcodes and dummy cycles for
Fast Read and Page Program operations.

The new 'struct spi_nor_hwcaps' uses a bitmask to describe all the
supported hardware capabilies and makes the difference between Fast Read
and Page Program operations and also between the different SPI protocols
(SPI 1-1-2 vs SPI 1-2-2 or SPI 1-1-4 vs SPI 1-4-4).
"""

Best Regards,

Cyrille


ChangeLog:

v5 -> v6
- rebase onto the 'next' branch of github/spi-nor to patch the new
  stm32_quadspi driver.
- remove many unnecessary parentheses and add some new lines.
- add the 'const' keywork to some 'struct spi_nor_hwcaps' initialized on
  the stack.
- fix a typo in some comment.
- change the encoding scheme of the 'enum spi_nor_protocol' as suggested
  by Marek to simplify the extraction of the number of bits (I/O lines)
  for instruction, address and data clock cycles.
- extact the nor->flash_quad_enable() hook from spi-nor.h from patch 1 and
  place it into the new dedicated patch 3 as suggested by Marek.
  Patch 3 is a tiny transitionnal patch needed by patch 4 which is based
  on an original patch from Kamal Dasu.

v4 -> v5
- rework the whole series.
- introduce support for Octo SPI protocols.
- introduce support for Double Transfer Rate (DTR) protocols.
- introduce support for memories with non-uniform sector erase sizes.

v3 -> v4
- replace dev_info() by dev_dbg() in patch 1.
- split former patch 2 into 2 patches:
  + new patch 2 deals with the rename of SPINOR_OP_READ4_* macros
  + new patch 3 deals with the alternative methode to support memory
> 16MiB
- add comment in patch 3 to describe the dichotomic search performed by
  spi_nor_convert_opcode().
- change return type from int to void for m25p80_proto2nbits() in patch 6.
- remove former patches 8 & 9 from the v2 series: the support of the
  Macronix mx66l1g45g memory will be sent in a separated patch.

v2 -> v3
- tested with new samples: Micron n25q512, n25q01g and Macronix
  mx25v1635f, mx25l3235f, mx25l3273f.
- add "Reviewed-by: Jagan Teki <jagan@openedev.com>" on patch 1.
- add "Tested-by: Vignesh R <vigneshr@ti.com>" on patch 2.
- fix some checkpatch warnings.
- add call of spi_nor_wait_till_ready() in spansion_new_quad_enable()
  and sr2_bit7_quad_enable(), as suggested by Joel Esponde on patch 6.
- test JESD216 rev A (minor 5) instead of rev B (minor 6) with the return
  code of spi_nor_parse_sfdp() from spi_nor_init_params() on patch 6.
  The seven additional DWORDs of the Basic Flash Parameter Table were
  introduced in rev A, not rev B, so the 15th DWORD was already available
  in rev A. The 15th DWORD provides us with the Quad Enable Requirements
  (QER) bits.
  Basic Flash Parameter Table size:
  + JESD216 :  9 DWORDS
  + JESD216A: 16 DWORDS
  + JESD216B: 16 DWORDS

v1 -> v2
- fix patch 3 to resolve compiler errors on hisi-sfc.c and cadence-quadspi.c
  drivers


Cyrille Pitchen (4):
  mtd: spi-nor: introduce more SPI protocols and the Dual Transfer Mode
  mtd: m25p80: add support of SPI 1-2-2 and 1-4-4 protocols
  mtd: spi-nor: transitional patch
  mtd: spi-nor: add spi_nor_init() function

 drivers/mtd/devices/m25p80.c          | 121 +++++---
 drivers/mtd/spi-nor/aspeed-smc.c      |  23 +-
 drivers/mtd/spi-nor/atmel-quadspi.c   |  83 ++++--
 drivers/mtd/spi-nor/cadence-quadspi.c |  18 +-
 drivers/mtd/spi-nor/fsl-quadspi.c     |   6 +-
 drivers/mtd/spi-nor/hisi-sfc.c        |  31 +-
 drivers/mtd/spi-nor/intel-spi.c       |   7 +-
 drivers/mtd/spi-nor/mtk-quadspi.c     |  15 +-
 drivers/mtd/spi-nor/nxp-spifi.c       |  22 +-
 drivers/mtd/spi-nor/spi-nor.c         | 530 ++++++++++++++++++++++++++--------
 drivers/mtd/spi-nor/stm32-quadspi.c   |  27 +-
 include/linux/mtd/spi-nor.h           | 168 ++++++++++-
 12 files changed, 815 insertions(+), 236 deletions(-)

-- 
2.9.3

[toc] | [next] | [standalone]

#1624470 — [RFC PATCH v6 4/4] mtd: spi-nor: add spi_nor_init() function

This patch extracts some chunks from spi_nor_scan() and moves them into
a new spi_nor_init() function.

Indeed, spi_nor_init() regroups all the required SPI flash commands to be
sent to the SPI flash memory before performing any runtime operations
(Fast Read, Page Program, Sector Erase, ...). Hence spi_nor_init():
1) removes the flash protection if applicable for certain vendors.
2) sets the Quad Enable bit, if needed, before using Quad SPI protocols.
3) makes the memory enter its (stateful) 4-byte address mode, if needed,
   for SPI flash memory > 128Mbits not supporting the 4-byte address
   instruction set.

spi_nor_scan() now ends by calling spi_nor_init() once the probe phase has
completed. Further patches could also use spi_nor_init() to implement the
mtd->_resume() handler for the spi-nor framework.

Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 71 ++++++++++++++++++++++++++++---------------
 include/linux/mtd/spi-nor.h   |  8 +++++
 2 files changed, 55 insertions(+), 24 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 2377fe54f8b9..523fd9d85f57 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -255,6 +255,8 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
 	nor->read_opcode = spi_nor_convert_3to4_read(nor->read_opcode);
 	nor->program_opcode = spi_nor_convert_3to4_program(nor->program_opcode);
 	nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
+
+	nor->flags |= SNOR_F_4B_OPCODES;
 }
 
 /* Enable/disable 4-byte addressing mode. */
@@ -1836,6 +1838,45 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
 	return 0;
 }
 
+static int spi_nor_init(struct spi_nor *nor)
+{
+	const struct flash_info *info = nor->info;
+	struct device *dev = nor->dev;
+	int ret;
+
+	/*
+	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
+	 * with the software protection bits set
+	 */
+
+	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
+	    JEDEC_MFR(info) == SNOR_MFR_SST ||
+	    info->flags & SPI_NOR_HAS_LOCK) {
+		write_enable(nor);
+		write_sr(nor, 0);
+		spi_nor_wait_till_ready(nor);
+	}
+
+	/* Set the Quad Enable bit, if needed. */
+	if (nor->flash_quad_enable) {
+		ret = nor->flash_quad_enable(nor);
+		if (ret) {
+			dev_err(dev, "quad mode not supported\n");
+			return ret;
+		}
+	}
+
+	/*
+	 * For SPI flash memories above 128Mib, enter the 4-byte address mode
+	 * only if the 4-byte address instruction set is not supported.
+	 */
+	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES))
+		set_4byte(nor, info, 1);
+
+	return 0;
+}
+
 int spi_nor_scan(struct spi_nor *nor, const char *name,
 		 const struct spi_nor_hwcaps *hwcaps)
 {
@@ -1903,20 +1944,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (ret)
 		return ret;
 
-	/*
-	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
-	 * with the software protection bits set
-	 */
-
-	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
-	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
-	    JEDEC_MFR(info) == SNOR_MFR_SST ||
-	    info->flags & SPI_NOR_HAS_LOCK) {
-		write_enable(nor);
-		write_sr(nor, 0);
-		spi_nor_wait_till_ready(nor);
-	}
-
 	if (!mtd->name)
 		mtd->name = dev_name(dev);
 	mtd->priv = nor;
@@ -1987,14 +2014,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	if (ret)
 		return ret;
 
-	if (nor->flash_quad_enable) {
-		ret = nor->flash_quad_enable(nor);
-		if (ret) {
-			dev_err(dev, "quad mode not supported\n");
-			return ret;
-		}
-	}
-
 	if (info->addr_width)
 		nor->addr_width = info->addr_width;
 	else if (mtd->size > 0x1000000) {
@@ -2003,8 +2022,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
 		    info->flags & SPI_NOR_4B_OPCODES)
 			spi_nor_set_4byte_opcodes(nor, info);
-		else
-			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
 	}
@@ -2021,6 +2038,12 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 			return ret;
 	}
 
+	/* Send all the required SPI flash commands to initialize the memory. */
+	nor->info = info;
+	ret = spi_nor_init(nor);
+	if (ret)
+		return ret;
+
 	dev_info(dev, "%s (%lld Kbytes)\n", info->name,
 			(long long)mtd->size >> 10);
 
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index e0c58d9d9286..618ecac9337a 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -214,9 +214,16 @@ enum spi_nor_option_flags {
 	SNOR_F_NO_OP_CHIP_ERASE	= BIT(2),
 	SNOR_F_S3AN_ADDR_DEFAULT = BIT(3),
 	SNOR_F_READY_XSR_RDY	= BIT(4),
+	SNOR_F_4B_OPCODES	= BIT(5),
 };
 
 /**
+ * struct flash_info -	Forward declaration of a structure used internally by
+ *			spi_nor_scan() and spi_nor_init().
+ */
+struct flash_info;
+
+/**
  * struct spi_nor - Structure for defining a the SPI NOR layer
  * @mtd:		point to a mtd_info structure
  * @lock:		the lock for the read/write/erase/lock/unlock operations
@@ -255,6 +262,7 @@ struct spi_nor {
 	struct mtd_info		mtd;
 	struct mutex		lock;
 	struct device		*dev;
+	const struct flash_info	*info;
 	u32			page_size;
 	u8			addr_width;
 	u8			erase_opcode;
-- 
2.9.3

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

#1624476 — Re: [RFC PATCH v6 4/4] mtd: spi-nor: add spi_nor_init() function

On 04/16/2017 11:41 PM, Cyrille Pitchen wrote:
> This patch extracts some chunks from spi_nor_scan() and moves them into
> a new spi_nor_init() function.
> 
> Indeed, spi_nor_init() regroups all the required SPI flash commands to be
> sent to the SPI flash memory before performing any runtime operations
> (Fast Read, Page Program, Sector Erase, ...). Hence spi_nor_init():
> 1) removes the flash protection if applicable for certain vendors.
> 2) sets the Quad Enable bit, if needed, before using Quad SPI protocols.
> 3) makes the memory enter its (stateful) 4-byte address mode, if needed,
>    for SPI flash memory > 128Mbits not supporting the 4-byte address
>    instruction set.
> 
> spi_nor_scan() now ends by calling spi_nor_init() once the probe phase has
> completed. Further patches could also use spi_nor_init() to implement the
> mtd->_resume() handler for the spi-nor framework.
> 
> Signed-off-by: Kamal Dasu <kdasu.kdev@gmail.com>
> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 71 ++++++++++++++++++++++++++++---------------
>  include/linux/mtd/spi-nor.h   |  8 +++++
>  2 files changed, 55 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 2377fe54f8b9..523fd9d85f57 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -255,6 +255,8 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
>  	nor->read_opcode = spi_nor_convert_3to4_read(nor->read_opcode);
>  	nor->program_opcode = spi_nor_convert_3to4_program(nor->program_opcode);
>  	nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
> +
> +	nor->flags |= SNOR_F_4B_OPCODES;
>  }
>  
>  /* Enable/disable 4-byte addressing mode. */
> @@ -1836,6 +1838,45 @@ static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
>  	return 0;
>  }
>  
> +static int spi_nor_init(struct spi_nor *nor)
> +{
> +	const struct flash_info *info = nor->info;
> +	struct device *dev = nor->dev;
> +	int ret;
> +
> +	/*
> +	 * Atmel, SST, Intel/Numonyx, and others serial NOR tend to power up
> +	 * with the software protection bits set
> +	 */
> +
> +	if (JEDEC_MFR(info) == SNOR_MFR_ATMEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_INTEL ||
> +	    JEDEC_MFR(info) == SNOR_MFR_SST ||
> +	    info->flags & SPI_NOR_HAS_LOCK) {
> +		write_enable(nor);
> +		write_sr(nor, 0);
> +		spi_nor_wait_till_ready(nor);
> +	}
> +
> +	/* Set the Quad Enable bit, if needed. */
> +	if (nor->flash_quad_enable) {
> +		ret = nor->flash_quad_enable(nor);
> +		if (ret) {
> +			dev_err(dev, "quad mode not supported\n");

Do we care about undoing changes (like doing write_disable()) in case of
failure ?

Also, if we have flash_quad_enable() and it fails, should we fall back
to 1-1-1 or something ?

> +			return ret;
> +		}
> +	}
> +
> +	/*
> +	 * For SPI flash memories above 128Mib, enter the 4-byte address mode
> +	 * only if the 4-byte address instruction set is not supported.
> +	 */
> +	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES))
> +		set_4byte(nor, info, 1);
> +
> +	return 0;
> +}
> +

[...]

-- 
Best regards,
Marek Vasut

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

#1624477 — Re: [PATCH v6 1/4] mtd: spi-nor: introduce more SPI protocols and the Dual Transfer Mode

On 04/16/2017 11:41 PM, Cyrille Pitchen wrote:
> This patch changes the prototype of spi_nor_scan(): its 3rd parameter
> is replaced by a 'struct spi_nor_hwcaps' pointer, which tells the spi-nor
> framework about the actual hardware capabilities supported by the SPI
> controller and its driver.
> 
> Besides, this patch also introduces a new 'struct spi_nor_flash_parameter'
> telling the spi-nor framework about the hardware capabilities supported by
> the SPI flash memory and the associated settings required to use those
> hardware caps.
> 
> Currently the 'struct spi_nor_flash_parameter' is filled with legacy
> values but a later patch will allow to fill it dynamically by reading the
> JESD216 Serial Flash Discoverable Parameter (SFDP) tables from the SPI
> memory.
> 
> With both structures, the spi-nor framework can now compute the best
> match between hardware caps supported by both the (Q)SPI memory and
> controller hence selecting the relevant SPI protocols and op codes for
> (Fast) Read and Page Program operations.
> 
> The 'struct spi_nor_flash_parameter' also provides the spi-nor framework
> with the number of dummy cycles to be used with each Fast Read commands.
> 
> Finally the 'struct spi_nor_flash_parameter', through the optional
> .quad_enable() hook, tells the spi-nor framework how to set the Quad
> Enable (QE) bit of the QSPI memory to enable its Quad SPI features.
> For now the .quad_enable() hook is set with the exact same functions as
> already used before this patch: the right function was and is still
> chosen only based on the memory manufacturer. In further patches, this
> choice will be made also based on the memory part. Indeed, we already
> know that some manufacturers, like Spansion, have updated their procedure
> to set the QE bit with their latest QSPI memories. The new procedure is
> safer but not supported by the oldest memory parts.
> The SFDP table could be one solution to select the relevant procedure but
> this issue will be addressed in further dedicated patches.

This IMO could've been factored out into separate small patch, so that
this patch isn't such a blob of multiple changes.

Otherwise, minor nits below ...

> Signed-off-by: Cyrille Pitchen <cyrille.pitchen@atmel.com>
> ---
>  drivers/mtd/devices/m25p80.c          |  18 +-
>  drivers/mtd/spi-nor/aspeed-smc.c      |  23 +-
>  drivers/mtd/spi-nor/atmel-quadspi.c   |  83 ++++--
>  drivers/mtd/spi-nor/cadence-quadspi.c |  18 +-
>  drivers/mtd/spi-nor/fsl-quadspi.c     |   6 +-
>  drivers/mtd/spi-nor/hisi-sfc.c        |  31 ++-
>  drivers/mtd/spi-nor/intel-spi.c       |   7 +-
>  drivers/mtd/spi-nor/mtk-quadspi.c     |  15 +-
>  drivers/mtd/spi-nor/nxp-spifi.c       |  22 +-
>  drivers/mtd/spi-nor/spi-nor.c         | 472 +++++++++++++++++++++++++++-------
>  drivers/mtd/spi-nor/stm32-quadspi.c   |  27 +-
>  include/linux/mtd/spi-nor.h           | 158 +++++++++++-
>  12 files changed, 686 insertions(+), 194 deletions(-)
> 
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index c4df3b1bded0..34f33f6b88b4 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -111,10 +111,10 @@ static ssize_t m25p80_write(struct spi_nor *nor, loff_t to, size_t len,
>  
>  static inline unsigned int m25p80_rx_nbits(struct spi_nor *nor)
>  {
> -	switch (nor->flash_read) {
> -	case SPI_NOR_DUAL:
> +	switch (nor->read_proto) {
> +	case SNOR_PROTO_1_1_2:
>  		return 2;
> -	case SPI_NOR_QUAD:
> +	case SNOR_PROTO_1_1_4:
>  		return 4;

Won't this become something like:

rx_bits = (nor->read_proto >> SNOR_PROTO_INST_SHIFT) & SNOR_PROTO_INST_MASK;

return rx_bits >= 1 ? rx_bits : 0;

And then you don't have to ever touch it once you add new SNOR_PROTO_1_1_x ?

Why do we return 0 by default though ? SPI_NBITS_SINGLE = 1 ... hmmm.

>  	default:
>  		return 0;
> @@ -196,7 +196,11 @@ static int m25p_probe(struct spi_device *spi)
>  	struct flash_platform_data	*data;
>  	struct m25p *flash;
>  	struct spi_nor *nor;
> -	enum read_mode mode = SPI_NOR_NORMAL;
> +	struct spi_nor_hwcaps hwcaps = {
> +		.mask = SNOR_HWCAPS_READ |
> +			SNOR_HWCAPS_READ_FAST |
> +			SNOR_HWCAPS_PP,
> +	};
>  	char *flash_name;
>  	int ret;


[...]

> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 36684ca7aa24..3bb2adff306b 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -150,24 +150,6 @@ static int read_cr(struct spi_nor *nor)
>  }
>  
>  /*
> - * Dummy Cycle calculation for different type of read.
> - * It can be used to support more commands with
> - * different dummy cycle requirements.
> - */
> -static inline int spi_nor_read_dummy_cycles(struct spi_nor *nor)
> -{
> -	switch (nor->flash_read) {
> -	case SPI_NOR_FAST:
> -	case SPI_NOR_DUAL:
> -	case SPI_NOR_QUAD:
> -		return 8;
> -	case SPI_NOR_NORMAL:
> -		return 0;
> -	}
> -	return 0;
> -}
> -
> -/*
>   * Write status register 1 byte
>   * Returns negative if error occurred.
>   */
> @@ -221,6 +203,10 @@ static inline u8 spi_nor_convert_3to4_read(u8 opcode)
>  		{ SPINOR_OP_READ_1_2_2,	SPINOR_OP_READ_1_2_2_4B },
>  		{ SPINOR_OP_READ_1_1_4,	SPINOR_OP_READ_1_1_4_4B },
>  		{ SPINOR_OP_READ_1_4_4,	SPINOR_OP_READ_1_4_4_4B },
> +
> +		{ SPINOR_OP_READ_1_1_1_DTR,	SPINOR_OP_READ_1_1_1_DTR_4B },
> +		{ SPINOR_OP_READ_1_2_2_DTR,	SPINOR_OP_READ_1_2_2_DTR_4B },
> +		{ SPINOR_OP_READ_1_4_4_DTR,	SPINOR_OP_READ_1_4_4_DTR_4B },

If you moved this into separate patch, that patch would be a nice
example for future generations on how to add new protocols ...

>  	};
>  
>  	return spi_nor_convert_opcode(opcode, spi_nor_3to4_read,

[...]

> +struct spi_nor_flash_parameter {
> +	u64				size;
> +	u32				page_size;
> +
> +	struct spi_nor_hwcaps		hwcaps;
> +	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
> +	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
> +
> +	int (*quad_enable)(struct spi_nor *nor);
> +};
> +
> +

One newline too many here :)

> +static void
> +spi_nor_set_read_settings(struct spi_nor_read_command *read,
> +			  u8 num_mode_clocks,
> +			  u8 num_wait_states,
> +			  u8 opcode,
> +			  enum spi_nor_protocol proto)
> +{
> +	read->num_mode_clocks = num_mode_clocks;
> +	read->num_wait_states = num_wait_states;
> +	read->opcode = opcode;
> +	read->proto = proto;
> +}

[...]

-- 
Best regards,
Marek Vasut

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web