[PATCH 0/5] mtd: spi-nor: move manuf out of the core - batch 0

[PATCH 0/5] mtd: spi-nor: move manuf out of the core - batch 0

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

This series is a prerequisite for the effort of moving the
manufacturer specific code out of the SPI NOR core.

The scope is to move all [FLASH-SPECIFIC] parameters and settings
from 'struct spi_nor' to 'struct spi_nor_flash_parameter'. We will
have a clear separation between the SPI NOR layer and the flash
parameters and settings.

'struct spi_nor_flash_parameter' describes the hardware capabilities
and associated settings of the SPI NOR flash memory. It includes
legacy flash parameters and settings that can be overwritten by the
spi_nor_fixups hooks, or dynamically when parsing the JESD216
Serial Flash Discoverable Parameters (SFDP) tables. All SFDP params
and settings will fit inside 'struct spi_nor_flash_parameter'.

Tested uniform and non-uniform erase on sst26vf064b flash using the
atmel-quadspi driver.

In order to test this, you'll have to merge v5.3-rc5 in spi-nor/next.
This patch depends on
'commit 834de5c1aa76 ("mtd: spi-nor: Fix the disabling of write protection at init")

Tudor Ambarus (5):
  mtd: spi-nor: Regroup flash parameter and settings
  mtd: spi-nor: Use nor->params
  mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
  mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'
  mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'

 drivers/mtd/spi-nor/spi-nor.c | 236 ++++++++++++++++-----------------------
 include/linux/mtd/spi-nor.h   | 254 ++++++++++++++++++++++++++++--------------
 2 files changed, 269 insertions(+), 221 deletions(-)

-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 1/5] mtd: spi-nor: Regroup flash parameter and settings

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The scope is to move all [FLASH-SPECIFIC] parameters and settings
from 'struct spi_nor' to 'struct spi_nor_flash_parameter'.

'struct spi_nor_flash_parameter' describes the hardware capabilities
and associated settings of the SPI NOR flash memory. It includes
legacy flash parameters and settings that can be overwritten by the
spi_nor_fixups hooks, or dynamically when parsing the JESD216
Serial Flash Discoverable Parameters (SFDP) tables. All SFDP params
and settings will fit inside 'struct spi_nor_flash_parameter'.

Move spi_nor_hwcaps related code to avoid forward declarations.
Add a forward declaration that we can't avoid: 'struct spi_nor' will
be used in 'struct spi_nor_flash_parameter'.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c |  65 ------------
 include/linux/mtd/spi-nor.h   | 239 +++++++++++++++++++++++++++++-------------
 2 files changed, 164 insertions(+), 140 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 0597cb8257b0..d35dc6a97521 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -40,71 +40,6 @@
 #define SPI_NOR_MAX_ID_LEN	6
 #define SPI_NOR_MAX_ADDR_WIDTH	4
 
-struct spi_nor_read_command {
-	u8			num_mode_clocks;
-	u8			num_wait_states;
-	u8			opcode;
-	enum spi_nor_protocol	proto;
-};
-
-struct spi_nor_pp_command {
-	u8			opcode;
-	enum spi_nor_protocol	proto;
-};
-
-enum spi_nor_read_command_index {
-	SNOR_CMD_READ,
-	SNOR_CMD_READ_FAST,
-	SNOR_CMD_READ_1_1_1_DTR,
-
-	/* Dual SPI */
-	SNOR_CMD_READ_1_1_2,
-	SNOR_CMD_READ_1_2_2,
-	SNOR_CMD_READ_2_2_2,
-	SNOR_CMD_READ_1_2_2_DTR,
-
-	/* Quad SPI */
-	SNOR_CMD_READ_1_1_4,
-	SNOR_CMD_READ_1_4_4,
-	SNOR_CMD_READ_4_4_4,
-	SNOR_CMD_READ_1_4_4_DTR,
-
-	/* Octal SPI */
-	SNOR_CMD_READ_1_1_8,
-	SNOR_CMD_READ_1_8_8,
-	SNOR_CMD_READ_8_8_8,
-	SNOR_CMD_READ_1_8_8_DTR,
-
-	SNOR_CMD_READ_MAX
-};
-
-enum spi_nor_pp_command_index {
-	SNOR_CMD_PP,
-
-	/* Quad SPI */
-	SNOR_CMD_PP_1_1_4,
-	SNOR_CMD_PP_1_4_4,
-	SNOR_CMD_PP_4_4_4,
-
-	/* Octal SPI */
-	SNOR_CMD_PP_1_1_8,
-	SNOR_CMD_PP_1_8_8,
-	SNOR_CMD_PP_8_8_8,
-
-	SNOR_CMD_PP_MAX
-};
-
-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);
-};
-
 struct sfdp_parameter_header {
 	u8		id_lsb;
 	u8		minor;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 3075ac73b171..77ba692d9348 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -334,6 +334,165 @@ struct spi_nor_erase_map {
 };
 
 /**
+ * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
+ * supported by the SPI controller (bus master).
+ * @mask:		the bitmask listing all the supported hw capabilies
+ */
+struct spi_nor_hwcaps {
+	u32	mask;
+};
+
+/*
+ *(Fast) Read capabilities.
+ * MUST be ordered by priority: the higher bit position, the higher priority.
+ * As a matter of performances, it is relevant to use Octal SPI protocols first,
+ * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
+ * (Slow) Read.
+ */
+#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
+#define SNOR_HWCAPS_READ		BIT(0)
+#define SNOR_HWCAPS_READ_FAST		BIT(1)
+#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
+
+#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
+#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
+#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
+#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
+#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
+
+#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
+#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
+#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
+#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
+#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
+
+#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
+#define SNOR_HWCAPS_READ_1_1_8		BIT(11)
+#define SNOR_HWCAPS_READ_1_8_8		BIT(12)
+#define SNOR_HWCAPS_READ_8_8_8		BIT(13)
+#define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
+
+/*
+ * Page Program capabilities.
+ * MUST be ordered by priority: the higher bit position, the higher priority.
+ * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
+ * legacy SPI 1-1-1 protocol.
+ * Note that Dual Page Programs are not supported because there is no existing
+ * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
+ * implements such commands.
+ */
+#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
+#define SNOR_HWCAPS_PP		BIT(16)
+
+#define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
+#define SNOR_HWCAPS_PP_1_1_4	BIT(17)
+#define SNOR_HWCAPS_PP_1_4_4	BIT(18)
+#define SNOR_HWCAPS_PP_4_4_4	BIT(19)
+
+#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
+#define SNOR_HWCAPS_PP_1_1_8	BIT(20)
+#define SNOR_HWCAPS_PP_1_8_8	BIT(21)
+#define SNOR_HWCAPS_PP_8_8_8	BIT(22)
+
+#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
+				 SNOR_HWCAPS_READ_4_4_4 |	\
+				 SNOR_HWCAPS_READ_8_8_8 |	\
+				 SNOR_HWCAPS_PP_4_4_4 |		\
+				 SNOR_HWCAPS_PP_8_8_8)
+
+#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
+				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
+				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
+				 SNOR_HWCAPS_READ_1_8_8_DTR)
+
+#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
+				 SNOR_HWCAPS_PP_MASK)
+
+struct spi_nor_read_command {
+	u8			num_mode_clocks;
+	u8			num_wait_states;
+	u8			opcode;
+	enum spi_nor_protocol	proto;
+};
+
+struct spi_nor_pp_command {
+	u8			opcode;
+	enum spi_nor_protocol	proto;
+};
+
+enum spi_nor_read_command_index {
+	SNOR_CMD_READ,
+	SNOR_CMD_READ_FAST,
+	SNOR_CMD_READ_1_1_1_DTR,
+
+	/* Dual SPI */
+	SNOR_CMD_READ_1_1_2,
+	SNOR_CMD_READ_1_2_2,
+	SNOR_CMD_READ_2_2_2,
+	SNOR_CMD_READ_1_2_2_DTR,
+
+	/* Quad SPI */
+	SNOR_CMD_READ_1_1_4,
+	SNOR_CMD_READ_1_4_4,
+	SNOR_CMD_READ_4_4_4,
+	SNOR_CMD_READ_1_4_4_DTR,
+
+	/* Octal SPI */
+	SNOR_CMD_READ_1_1_8,
+	SNOR_CMD_READ_1_8_8,
+	SNOR_CMD_READ_8_8_8,
+	SNOR_CMD_READ_1_8_8_DTR,
+
+	SNOR_CMD_READ_MAX
+};
+
+enum spi_nor_pp_command_index {
+	SNOR_CMD_PP,
+
+	/* Quad SPI */
+	SNOR_CMD_PP_1_1_4,
+	SNOR_CMD_PP_1_4_4,
+	SNOR_CMD_PP_4_4_4,
+
+	/* Octal SPI */
+	SNOR_CMD_PP_1_1_8,
+	SNOR_CMD_PP_1_8_8,
+	SNOR_CMD_PP_8_8_8,
+
+	SNOR_CMD_PP_MAX
+};
+
+/* Forward declaration that will be used in 'struct spi_nor_flash_parameter' */
+struct spi_nor;
+
+/**
+ * struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
+ * Includes legacy flash parameters and settings that can be overwritten
+ * by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
+ * Serial Flash Discoverable Parameters (SFDP) tables.
+ *
+ * @size:		the flash memory density in bytes.
+ * @page_size:		the page size of the SPI NOR flash memory.
+ * @hwcaps:		describes the read and page program hardware
+ *			capabilities.
+ * @reads:		read capabilities ordered by priority: the higher index
+ *                      in the array, the higher priority.
+ * @page_programs:	page program capabilities ordered by priority: the
+ *                      higher index in the array, the higher priority.
+ * @quad_enable:	enables SPI NOR quad mode.
+ */
+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);
+};
+
+/**
  * struct flash_info - Forward declaration of a structure used internally by
  *		       spi_nor_scan()
  */
@@ -379,6 +538,10 @@ struct flash_info;
  * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
  * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
  *			the SPI NOR Status Register.
+ * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
+ *                      The structure includes legacy flash parameters and
+ *                      settings that can be overwritten by the spi_nor_fixups
+ *                      hooks, or dynamically when parsing the SFDP tables.
  * @priv:		the private data
  */
 struct spi_nor {
@@ -418,6 +581,7 @@ struct spi_nor {
 	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*clear_sr_bp)(struct spi_nor *nor);
+	struct spi_nor_flash_parameter params;
 
 	void *priv;
 };
@@ -463,81 +627,6 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
 }
 
 /**
- * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
- * supported by the SPI controller (bus master).
- * @mask:		the bitmask listing all the supported hw capabilies
- */
-struct spi_nor_hwcaps {
-	u32	mask;
-};
-
-/*
- *(Fast) Read capabilities.
- * MUST be ordered by priority: the higher bit position, the higher priority.
- * As a matter of performances, it is relevant to use Octal SPI protocols first,
- * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
- * (Slow) Read.
- */
-#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
-#define SNOR_HWCAPS_READ		BIT(0)
-#define SNOR_HWCAPS_READ_FAST		BIT(1)
-#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
-
-#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
-#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
-#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
-#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
-#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
-
-#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
-#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
-#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
-#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
-#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
-
-#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
-#define SNOR_HWCAPS_READ_1_1_8		BIT(11)
-#define SNOR_HWCAPS_READ_1_8_8		BIT(12)
-#define SNOR_HWCAPS_READ_8_8_8		BIT(13)
-#define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
-
-/*
- * Page Program capabilities.
- * MUST be ordered by priority: the higher bit position, the higher priority.
- * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
- * legacy SPI 1-1-1 protocol.
- * Note that Dual Page Programs are not supported because there is no existing
- * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
- * implements such commands.
- */
-#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
-#define SNOR_HWCAPS_PP		BIT(16)
-
-#define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
-#define SNOR_HWCAPS_PP_1_1_4	BIT(17)
-#define SNOR_HWCAPS_PP_1_4_4	BIT(18)
-#define SNOR_HWCAPS_PP_4_4_4	BIT(19)
-
-#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
-#define SNOR_HWCAPS_PP_1_1_8	BIT(20)
-#define SNOR_HWCAPS_PP_1_8_8	BIT(21)
-#define SNOR_HWCAPS_PP_8_8_8	BIT(22)
-
-#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
-				 SNOR_HWCAPS_READ_4_4_4 |	\
-				 SNOR_HWCAPS_READ_8_8_8 |	\
-				 SNOR_HWCAPS_PP_4_4_4 |		\
-				 SNOR_HWCAPS_PP_8_8_8)
-
-#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
-				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
-				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
-				 SNOR_HWCAPS_READ_1_8_8_DTR)
-
-#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
-				 SNOR_HWCAPS_PP_MASK)
-
-/**
  * spi_nor_scan() - scan the SPI NOR
  * @nor:	the spi_nor structure
  * @name:	the chip type name
-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 1/5] mtd: spi-nor: Regroup flash parameter and settings

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:35 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> The scope is to move all [FLASH-SPECIFIC] parameters and settings
> from 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
> 
> 'struct spi_nor_flash_parameter' describes the hardware capabilities
> and associated settings of the SPI NOR flash memory. It includes
> legacy flash parameters and settings that can be overwritten by the
> spi_nor_fixups hooks, or dynamically when parsing the JESD216
> Serial Flash Discoverable Parameters (SFDP) tables. All SFDP params
> and settings will fit inside 'struct spi_nor_flash_parameter'.
> 
> Move spi_nor_hwcaps related code to avoid forward declarations.
> Add a forward declaration that we can't avoid: 'struct spi_nor' will
> be used in 'struct spi_nor_flash_parameter'.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/spi-nor/spi-nor.c |  65 ------------
>  include/linux/mtd/spi-nor.h   | 239 +++++++++++++++++++++++++++++-------------
>  2 files changed, 164 insertions(+), 140 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0597cb8257b0..d35dc6a97521 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -40,71 +40,6 @@
>  #define SPI_NOR_MAX_ID_LEN	6
>  #define SPI_NOR_MAX_ADDR_WIDTH	4
>  
> -struct spi_nor_read_command {
> -	u8			num_mode_clocks;
> -	u8			num_wait_states;
> -	u8			opcode;
> -	enum spi_nor_protocol	proto;
> -};
> -
> -struct spi_nor_pp_command {
> -	u8			opcode;
> -	enum spi_nor_protocol	proto;
> -};
> -
> -enum spi_nor_read_command_index {
> -	SNOR_CMD_READ,
> -	SNOR_CMD_READ_FAST,
> -	SNOR_CMD_READ_1_1_1_DTR,
> -
> -	/* Dual SPI */
> -	SNOR_CMD_READ_1_1_2,
> -	SNOR_CMD_READ_1_2_2,
> -	SNOR_CMD_READ_2_2_2,
> -	SNOR_CMD_READ_1_2_2_DTR,
> -
> -	/* Quad SPI */
> -	SNOR_CMD_READ_1_1_4,
> -	SNOR_CMD_READ_1_4_4,
> -	SNOR_CMD_READ_4_4_4,
> -	SNOR_CMD_READ_1_4_4_DTR,
> -
> -	/* Octal SPI */
> -	SNOR_CMD_READ_1_1_8,
> -	SNOR_CMD_READ_1_8_8,
> -	SNOR_CMD_READ_8_8_8,
> -	SNOR_CMD_READ_1_8_8_DTR,
> -
> -	SNOR_CMD_READ_MAX
> -};
> -
> -enum spi_nor_pp_command_index {
> -	SNOR_CMD_PP,
> -
> -	/* Quad SPI */
> -	SNOR_CMD_PP_1_1_4,
> -	SNOR_CMD_PP_1_4_4,
> -	SNOR_CMD_PP_4_4_4,
> -
> -	/* Octal SPI */
> -	SNOR_CMD_PP_1_1_8,
> -	SNOR_CMD_PP_1_8_8,
> -	SNOR_CMD_PP_8_8_8,
> -
> -	SNOR_CMD_PP_MAX
> -};
> -
> -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);
> -};
> -
>  struct sfdp_parameter_header {
>  	u8		id_lsb;
>  	u8		minor;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 3075ac73b171..77ba692d9348 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -334,6 +334,165 @@ struct spi_nor_erase_map {
>  };
>  
>  /**
> + * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
> + * supported by the SPI controller (bus master).
> + * @mask:		the bitmask listing all the supported hw capabilies
> + */
> +struct spi_nor_hwcaps {
> +	u32	mask;
> +};
> +
> +/*
> + *(Fast) Read capabilities.
> + * MUST be ordered by priority: the higher bit position, the higher priority.
> + * As a matter of performances, it is relevant to use Octal SPI protocols first,
> + * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
> + * (Slow) Read.
> + */
> +#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
> +#define SNOR_HWCAPS_READ		BIT(0)
> +#define SNOR_HWCAPS_READ_FAST		BIT(1)
> +#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
> +
> +#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
> +#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
> +#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
> +#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
> +#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
> +
> +#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
> +#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
> +#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
> +#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
> +#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
> +
> +#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
> +#define SNOR_HWCAPS_READ_1_1_8		BIT(11)
> +#define SNOR_HWCAPS_READ_1_8_8		BIT(12)
> +#define SNOR_HWCAPS_READ_8_8_8		BIT(13)
> +#define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
> +
> +/*
> + * Page Program capabilities.
> + * MUST be ordered by priority: the higher bit position, the higher priority.
> + * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
> + * legacy SPI 1-1-1 protocol.
> + * Note that Dual Page Programs are not supported because there is no existing
> + * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
> + * implements such commands.
> + */
> +#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
> +#define SNOR_HWCAPS_PP		BIT(16)
> +
> +#define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
> +#define SNOR_HWCAPS_PP_1_1_4	BIT(17)
> +#define SNOR_HWCAPS_PP_1_4_4	BIT(18)
> +#define SNOR_HWCAPS_PP_4_4_4	BIT(19)
> +
> +#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
> +#define SNOR_HWCAPS_PP_1_1_8	BIT(20)
> +#define SNOR_HWCAPS_PP_1_8_8	BIT(21)
> +#define SNOR_HWCAPS_PP_8_8_8	BIT(22)
> +
> +#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
> +				 SNOR_HWCAPS_READ_4_4_4 |	\
> +				 SNOR_HWCAPS_READ_8_8_8 |	\
> +				 SNOR_HWCAPS_PP_4_4_4 |		\
> +				 SNOR_HWCAPS_PP_8_8_8)
> +
> +#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
> +				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
> +				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
> +				 SNOR_HWCAPS_READ_1_8_8_DTR)
> +
> +#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
> +				 SNOR_HWCAPS_PP_MASK)
> +
> +struct spi_nor_read_command {
> +	u8			num_mode_clocks;
> +	u8			num_wait_states;
> +	u8			opcode;
> +	enum spi_nor_protocol	proto;
> +};
> +
> +struct spi_nor_pp_command {
> +	u8			opcode;
> +	enum spi_nor_protocol	proto;
> +};
> +
> +enum spi_nor_read_command_index {
> +	SNOR_CMD_READ,
> +	SNOR_CMD_READ_FAST,
> +	SNOR_CMD_READ_1_1_1_DTR,
> +
> +	/* Dual SPI */
> +	SNOR_CMD_READ_1_1_2,
> +	SNOR_CMD_READ_1_2_2,
> +	SNOR_CMD_READ_2_2_2,
> +	SNOR_CMD_READ_1_2_2_DTR,
> +
> +	/* Quad SPI */
> +	SNOR_CMD_READ_1_1_4,
> +	SNOR_CMD_READ_1_4_4,
> +	SNOR_CMD_READ_4_4_4,
> +	SNOR_CMD_READ_1_4_4_DTR,
> +
> +	/* Octal SPI */
> +	SNOR_CMD_READ_1_1_8,
> +	SNOR_CMD_READ_1_8_8,
> +	SNOR_CMD_READ_8_8_8,
> +	SNOR_CMD_READ_1_8_8_DTR,
> +
> +	SNOR_CMD_READ_MAX
> +};
> +
> +enum spi_nor_pp_command_index {
> +	SNOR_CMD_PP,
> +
> +	/* Quad SPI */
> +	SNOR_CMD_PP_1_1_4,
> +	SNOR_CMD_PP_1_4_4,
> +	SNOR_CMD_PP_4_4_4,
> +
> +	/* Octal SPI */
> +	SNOR_CMD_PP_1_1_8,
> +	SNOR_CMD_PP_1_8_8,
> +	SNOR_CMD_PP_8_8_8,
> +
> +	SNOR_CMD_PP_MAX
> +};
> +
> +/* Forward declaration that will be used in 'struct spi_nor_flash_parameter' */
> +struct spi_nor;
> +
> +/**
> + * struct spi_nor_flash_parameter - SPI NOR flash parameters and settings.
> + * Includes legacy flash parameters and settings that can be overwritten
> + * by the spi_nor_fixups hooks, or dynamically when parsing the JESD216
> + * Serial Flash Discoverable Parameters (SFDP) tables.
> + *
> + * @size:		the flash memory density in bytes.
> + * @page_size:		the page size of the SPI NOR flash memory.
> + * @hwcaps:		describes the read and page program hardware
> + *			capabilities.
> + * @reads:		read capabilities ordered by priority: the higher index
> + *                      in the array, the higher priority.
> + * @page_programs:	page program capabilities ordered by priority: the
> + *                      higher index in the array, the higher priority.
> + * @quad_enable:	enables SPI NOR quad mode.
> + */
> +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);
> +};
> +
> +/**
>   * struct flash_info - Forward declaration of a structure used internally by
>   *		       spi_nor_scan()
>   */
> @@ -379,6 +538,10 @@ struct flash_info;
>   * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>   * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
>   *			the SPI NOR Status Register.
> + * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
> + *                      The structure includes legacy flash parameters and
> + *                      settings that can be overwritten by the spi_nor_fixups
> + *                      hooks, or dynamically when parsing the SFDP tables.
>   * @priv:		the private data
>   */
>  struct spi_nor {
> @@ -418,6 +581,7 @@ struct spi_nor {
>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*quad_enable)(struct spi_nor *nor);
>  	int (*clear_sr_bp)(struct spi_nor *nor);
> +	struct spi_nor_flash_parameter params;
>  
>  	void *priv;
>  };
> @@ -463,81 +627,6 @@ static inline struct device_node *spi_nor_get_flash_node(struct spi_nor *nor)
>  }
>  
>  /**
> - * struct spi_nor_hwcaps - Structure for describing the hardware capabilies
> - * supported by the SPI controller (bus master).
> - * @mask:		the bitmask listing all the supported hw capabilies
> - */
> -struct spi_nor_hwcaps {
> -	u32	mask;
> -};
> -
> -/*
> - *(Fast) Read capabilities.
> - * MUST be ordered by priority: the higher bit position, the higher priority.
> - * As a matter of performances, it is relevant to use Octal SPI protocols first,
> - * then Quad SPI protocols before Dual SPI protocols, Fast Read and lastly
> - * (Slow) Read.
> - */
> -#define SNOR_HWCAPS_READ_MASK		GENMASK(14, 0)
> -#define SNOR_HWCAPS_READ		BIT(0)
> -#define SNOR_HWCAPS_READ_FAST		BIT(1)
> -#define SNOR_HWCAPS_READ_1_1_1_DTR	BIT(2)
> -
> -#define SNOR_HWCAPS_READ_DUAL		GENMASK(6, 3)
> -#define SNOR_HWCAPS_READ_1_1_2		BIT(3)
> -#define SNOR_HWCAPS_READ_1_2_2		BIT(4)
> -#define SNOR_HWCAPS_READ_2_2_2		BIT(5)
> -#define SNOR_HWCAPS_READ_1_2_2_DTR	BIT(6)
> -
> -#define SNOR_HWCAPS_READ_QUAD		GENMASK(10, 7)
> -#define SNOR_HWCAPS_READ_1_1_4		BIT(7)
> -#define SNOR_HWCAPS_READ_1_4_4		BIT(8)
> -#define SNOR_HWCAPS_READ_4_4_4		BIT(9)
> -#define SNOR_HWCAPS_READ_1_4_4_DTR	BIT(10)
> -
> -#define SNOR_HWCAPS_READ_OCTAL		GENMASK(14, 11)
> -#define SNOR_HWCAPS_READ_1_1_8		BIT(11)
> -#define SNOR_HWCAPS_READ_1_8_8		BIT(12)
> -#define SNOR_HWCAPS_READ_8_8_8		BIT(13)
> -#define SNOR_HWCAPS_READ_1_8_8_DTR	BIT(14)
> -
> -/*
> - * Page Program capabilities.
> - * MUST be ordered by priority: the higher bit position, the higher priority.
> - * Like (Fast) Read capabilities, Octal/Quad SPI protocols are preferred to the
> - * legacy SPI 1-1-1 protocol.
> - * Note that Dual Page Programs are not supported because there is no existing
> - * JEDEC/SFDP standard to define them. Also at this moment no SPI flash memory
> - * implements such commands.
> - */
> -#define SNOR_HWCAPS_PP_MASK	GENMASK(22, 16)
> -#define SNOR_HWCAPS_PP		BIT(16)
> -
> -#define SNOR_HWCAPS_PP_QUAD	GENMASK(19, 17)
> -#define SNOR_HWCAPS_PP_1_1_4	BIT(17)
> -#define SNOR_HWCAPS_PP_1_4_4	BIT(18)
> -#define SNOR_HWCAPS_PP_4_4_4	BIT(19)
> -
> -#define SNOR_HWCAPS_PP_OCTAL	GENMASK(22, 20)
> -#define SNOR_HWCAPS_PP_1_1_8	BIT(20)
> -#define SNOR_HWCAPS_PP_1_8_8	BIT(21)
> -#define SNOR_HWCAPS_PP_8_8_8	BIT(22)
> -
> -#define SNOR_HWCAPS_X_X_X	(SNOR_HWCAPS_READ_2_2_2 |	\
> -				 SNOR_HWCAPS_READ_4_4_4 |	\
> -				 SNOR_HWCAPS_READ_8_8_8 |	\
> -				 SNOR_HWCAPS_PP_4_4_4 |		\
> -				 SNOR_HWCAPS_PP_8_8_8)
> -
> -#define SNOR_HWCAPS_DTR		(SNOR_HWCAPS_READ_1_1_1_DTR |	\
> -				 SNOR_HWCAPS_READ_1_2_2_DTR |	\
> -				 SNOR_HWCAPS_READ_1_4_4_DTR |	\
> -				 SNOR_HWCAPS_READ_1_8_8_DTR)
> -
> -#define SNOR_HWCAPS_ALL		(SNOR_HWCAPS_READ_MASK |	\
> -				 SNOR_HWCAPS_PP_MASK)
> -
> -/**
>   * spi_nor_scan() - scan the SPI NOR
>   * @nor:	the spi_nor structure
>   * @name:	the chip type name


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 2/5] mtd: spi-nor: Use nor->params

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

The Flash parameters and settings are now stored in 'struct spi_nor'.
Use this instead of the stack allocated params.

Few functions stop passing pointer to params, as they can get it from
'struct spi_nor'. spi_nor_parse_sfdp() and children will keep passing
pointer to params because of the roll-back mechanism: in case the
parsing of SFDP fails, the legacy flash parameter and settings will be
restored.

Zeroizing params is no longer needed because all SPI NOR users kzalloc
'struct spi_nor'.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 46 ++++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index d35dc6a97521..e9b9cd70a999 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2974,16 +2974,13 @@ static int spi_nor_spimem_check_pp(struct spi_nor *nor,
  * spi_nor_spimem_adjust_hwcaps - Find optimal Read/Write protocol
  *                                based on SPI controller capabilities
  * @nor:        pointer to a 'struct spi_nor'
- * @params:     pointer to the 'struct spi_nor_flash_parameter'
- *              representing SPI NOR flash capabilities
  * @hwcaps:     pointer to resulting capabilities after adjusting
  *              according to controller and flash's capability
  */
 static void
-spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor,
-			     const struct spi_nor_flash_parameter *params,
-			     u32 *hwcaps)
+spi_nor_spimem_adjust_hwcaps(struct spi_nor *nor, u32 *hwcaps)
 {
+	struct spi_nor_flash_parameter *params =  &nor->params;
 	unsigned int cap;
 
 	/* DTR modes are not supported yet, mask them all. */
@@ -4129,16 +4126,13 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 	return err;
 }
 
-static int spi_nor_init_params(struct spi_nor *nor,
-			       struct spi_nor_flash_parameter *params)
+static int spi_nor_init_params(struct spi_nor *nor)
 {
+	struct spi_nor_flash_parameter *params = &nor->params;
 	struct spi_nor_erase_map *map = &nor->erase_map;
 	const struct flash_info *info = nor->info;
 	u8 i, erase_mask;
 
-	/* Set legacy flash parameters as default. */
-	memset(params, 0, sizeof(*params));
-
 	/* Set SPI NOR sizes. */
 	params->size = (u64)info->sector_size * info->n_sectors;
 	params->page_size = info->page_size;
@@ -4255,7 +4249,6 @@ static int spi_nor_init_params(struct spi_nor *nor,
 }
 
 static int spi_nor_select_read(struct spi_nor *nor,
-			       const struct spi_nor_flash_parameter *params,
 			       u32 shared_hwcaps)
 {
 	int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_READ_MASK) - 1;
@@ -4268,7 +4261,7 @@ static int spi_nor_select_read(struct spi_nor *nor,
 	if (cmd < 0)
 		return -EINVAL;
 
-	read = &params->reads[cmd];
+	read = &nor->params.reads[cmd];
 	nor->read_opcode = read->opcode;
 	nor->read_proto = read->proto;
 
@@ -4287,7 +4280,6 @@ static int spi_nor_select_read(struct spi_nor *nor,
 }
 
 static int spi_nor_select_pp(struct spi_nor *nor,
-			     const struct spi_nor_flash_parameter *params,
 			     u32 shared_hwcaps)
 {
 	int cmd, best_match = fls(shared_hwcaps & SNOR_HWCAPS_PP_MASK) - 1;
@@ -4300,7 +4292,7 @@ static int spi_nor_select_pp(struct spi_nor *nor,
 	if (cmd < 0)
 		return -EINVAL;
 
-	pp = &params->page_programs[cmd];
+	pp = &nor->params.page_programs[cmd];
 	nor->program_opcode = pp->opcode;
 	nor->write_proto = pp->proto;
 	return 0;
@@ -4407,9 +4399,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
 }
 
 static int spi_nor_setup(struct spi_nor *nor,
-			 const struct spi_nor_flash_parameter *params,
 			 const struct spi_nor_hwcaps *hwcaps)
 {
+	struct spi_nor_flash_parameter *params = &nor->params;
 	u32 ignored_mask, shared_mask;
 	bool enable_quad_io;
 	int err;
@@ -4426,7 +4418,7 @@ static int spi_nor_setup(struct spi_nor *nor,
 		 * need to discard some of them based on what the SPI
 		 * controller actually supports (using spi_mem_supports_op()).
 		 */
-		spi_nor_spimem_adjust_hwcaps(nor, params, &shared_mask);
+		spi_nor_spimem_adjust_hwcaps(nor, &shared_mask);
 	} else {
 		/*
 		 * SPI n-n-n protocols are not supported when the SPI
@@ -4442,7 +4434,7 @@ static int spi_nor_setup(struct spi_nor *nor,
 	}
 
 	/* Select the (Fast) Read command. */
-	err = spi_nor_select_read(nor, params, shared_mask);
+	err = spi_nor_select_read(nor, shared_mask);
 	if (err) {
 		dev_err(nor->dev,
 			"can't select read settings supported by both the SPI controller and memory.\n");
@@ -4450,7 +4442,7 @@ static int spi_nor_setup(struct spi_nor *nor,
 	}
 
 	/* Select the Page Program command. */
-	err = spi_nor_select_pp(nor, params, shared_mask);
+	err = spi_nor_select_pp(nor, shared_mask);
 	if (err) {
 		dev_err(nor->dev,
 			"can't select write settings supported by both the SPI controller and memory.\n");
@@ -4553,11 +4545,11 @@ static const struct flash_info *spi_nor_match_id(const char *name)
 int spi_nor_scan(struct spi_nor *nor, const char *name,
 		 const struct spi_nor_hwcaps *hwcaps)
 {
-	struct spi_nor_flash_parameter params;
 	const struct flash_info *info = NULL;
 	struct device *dev = nor->dev;
 	struct mtd_info *mtd = &nor->mtd;
 	struct device_node *np = spi_nor_get_flash_node(nor);
+	struct spi_nor_flash_parameter *params = &nor->params;
 	int ret;
 	int i;
 
@@ -4639,7 +4631,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		nor->clear_sr_bp = spi_nor_clear_sr_bp;
 
 	/* Parse the Serial Flash Discoverable Parameters table. */
-	ret = spi_nor_init_params(nor, &params);
+	ret = spi_nor_init_params(nor);
 	if (ret)
 		return ret;
 
@@ -4649,7 +4641,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	mtd->type = MTD_NORFLASH;
 	mtd->writesize = 1;
 	mtd->flags = MTD_CAP_NORFLASH;
-	mtd->size = params.size;
+	mtd->size = params->size;
 	mtd->_erase = spi_nor_erase;
 	mtd->_read = spi_nor_read;
 	mtd->_resume = spi_nor_resume;
@@ -4688,18 +4680,18 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 		mtd->flags |= MTD_NO_ERASE;
 
 	mtd->dev.parent = dev;
-	nor->page_size = params.page_size;
+	nor->page_size = params->page_size;
 	mtd->writebufsize = nor->page_size;
 
 	if (np) {
 		/* If we were instantiated by DT, use it */
 		if (of_property_read_bool(np, "m25p,fast-read"))
-			params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+			params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
 		else
-			params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+			params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
 	} else {
 		/* If we weren't instantiated by DT, default to fast-read */
-		params.hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
+		params->hwcaps.mask |= SNOR_HWCAPS_READ_FAST;
 	}
 
 	if (of_property_read_bool(np, "broken-flash-reset"))
@@ -4707,7 +4699,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 
 	/* Some devices cannot do fast-read, no matter what DT tells us */
 	if (info->flags & SPI_NOR_NO_FR)
-		params.hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
+		params->hwcaps.mask &= ~SNOR_HWCAPS_READ_FAST;
 
 	/*
 	 * Configure the SPI memory:
@@ -4716,7 +4708,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	 * - set the SPI protocols for register and memory accesses.
 	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
 	 */
-	ret = spi_nor_setup(nor, &params, hwcaps);
+	ret = spi_nor_setup(nor, hwcaps);
 	if (ret)
 		return ret;
 
-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 2/5] mtd: spi-nor: Use nor->params

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:37 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> The Flash parameters and settings are now stored in 'struct spi_nor'.
> Use this instead of the stack allocated params.
> 
> Few functions stop passing pointer to params, as they can get it from
> 'struct spi_nor'. spi_nor_parse_sfdp() and children will keep passing
> pointer to params because of the roll-back mechanism: in case the
> parsing of SFDP fails, the legacy flash parameter and settings will be
> restored.
> 
> Zeroizing params is no longer needed because all SPI NOR users kzalloc

  ^ Zeroing

With this fixed, you can add

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> 'struct spi_nor'.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 3/5] mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

All flash parameters and settings should reside inside
'struct spi_nor_flash_parameter'. Drop the local copy of
quad_enable() and use the one from 'struct spi_nor_flash_parameter'.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 38 ++++++++++++++++++++++----------------
 include/linux/mtd/spi-nor.h   |  2 --
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index e9b9cd70a999..6bd104c29cd9 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4403,7 +4403,6 @@ static int spi_nor_setup(struct spi_nor *nor,
 {
 	struct spi_nor_flash_parameter *params = &nor->params;
 	u32 ignored_mask, shared_mask;
-	bool enable_quad_io;
 	int err;
 
 	/*
@@ -4457,17 +4456,27 @@ static int spi_nor_setup(struct spi_nor *nor,
 		return err;
 	}
 
-	/* Enable Quad I/O if needed. */
-	enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 ||
-			  spi_nor_get_protocol_width(nor->write_proto) == 4);
-	if (enable_quad_io && params->quad_enable)
-		nor->quad_enable = params->quad_enable;
-	else
-		nor->quad_enable = NULL;
-
 	return 0;
 }
 
+/**
+ * spi_nor_quad_enable() - enable Quad I/O if needed.
+ * @nor:                pointer to a 'struct spi_nor'
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_quad_enable(struct spi_nor *nor)
+{
+	if (!nor->params.quad_enable)
+		return 0;
+
+	if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
+	      spi_nor_get_protocol_width(nor->write_proto) == 4))
+		return 0;
+
+	return nor->params.quad_enable(nor);
+}
+
 static int spi_nor_init(struct spi_nor *nor)
 {
 	int err;
@@ -4484,12 +4493,10 @@ static int spi_nor_init(struct spi_nor *nor)
 		}
 	}
 
-	if (nor->quad_enable) {
-		err = nor->quad_enable(nor);
-		if (err) {
-			dev_err(nor->dev, "quad mode not supported\n");
-			return err;
-		}
+	err = spi_nor_quad_enable(nor);
+	if (err) {
+		dev_err(nor->dev, "quad mode not supported\n");
+		return err;
 	}
 
 	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
@@ -4706,7 +4713,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	 * - select op codes for (Fast) Read, Page Program and Sector Erase.
 	 * - set the number of dummy cycles (mode cycles + wait states).
 	 * - set the SPI protocols for register and memory accesses.
-	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
 	 */
 	ret = spi_nor_setup(nor, hwcaps);
 	if (ret)
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 77ba692d9348..17787238f0e9 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -535,7 +535,6 @@ struct flash_info;
  * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
  * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
  *			completely locked
- * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
  * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
  *			the SPI NOR Status Register.
  * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
@@ -579,7 +578,6 @@ struct spi_nor {
 	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
-	int (*quad_enable)(struct spi_nor *nor);
 	int (*clear_sr_bp)(struct spi_nor *nor);
 	struct spi_nor_flash_parameter params;
 
-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 3/5] mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:39 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> All flash parameters and settings should reside inside
> 'struct spi_nor_flash_parameter'. Drop the local copy of
> quad_enable() and use the one from 'struct spi_nor_flash_parameter'.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 38 ++++++++++++++++++++++----------------
>  include/linux/mtd/spi-nor.h   |  2 --
>  2 files changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index e9b9cd70a999..6bd104c29cd9 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4403,7 +4403,6 @@ static int spi_nor_setup(struct spi_nor *nor,
>  {
>  	struct spi_nor_flash_parameter *params = &nor->params;
>  	u32 ignored_mask, shared_mask;
> -	bool enable_quad_io;
>  	int err;
>  
>  	/*
> @@ -4457,17 +4456,27 @@ static int spi_nor_setup(struct spi_nor *nor,
>  		return err;
>  	}
>  
> -	/* Enable Quad I/O if needed. */
> -	enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> -			  spi_nor_get_protocol_width(nor->write_proto) == 4);
> -	if (enable_quad_io && params->quad_enable)
> -		nor->quad_enable = params->quad_enable;
> -	else
> -		nor->quad_enable = NULL;
> -
>  	return 0;
>  }
>  
> +/**
> + * spi_nor_quad_enable() - enable Quad I/O if needed.
> + * @nor:                pointer to a 'struct spi_nor'
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_quad_enable(struct spi_nor *nor)
> +{
> +	if (!nor->params.quad_enable)
> +		return 0;
> +
> +	if (!(spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> +	      spi_nor_get_protocol_width(nor->write_proto) == 4))
> +		return 0;
> +
> +	return nor->params.quad_enable(nor);
> +}
> +
>  static int spi_nor_init(struct spi_nor *nor)
>  {
>  	int err;
> @@ -4484,12 +4493,10 @@ static int spi_nor_init(struct spi_nor *nor)
>  		}
>  	}
>  
> -	if (nor->quad_enable) {
> -		err = nor->quad_enable(nor);
> -		if (err) {
> -			dev_err(nor->dev, "quad mode not supported\n");
> -			return err;
> -		}
> +	err = spi_nor_quad_enable(nor);
> +	if (err) {
> +		dev_err(nor->dev, "quad mode not supported\n");
> +		return err;
>  	}
>  
>  	if (nor->addr_width == 4 && !(nor->flags & SNOR_F_4B_OPCODES)) {
> @@ -4706,7 +4713,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	 * - select op codes for (Fast) Read, Page Program and Sector Erase.
>  	 * - set the number of dummy cycles (mode cycles + wait states).
>  	 * - set the SPI protocols for register and memory accesses.
> -	 * - set the Quad Enable bit if needed (required by SPI x-y-4 protos).
>  	 */
>  	ret = spi_nor_setup(nor, hwcaps);
>  	if (ret)
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 77ba692d9348..17787238f0e9 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -535,7 +535,6 @@ struct flash_info;
>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
>   *			completely locked
> - * @quad_enable:	[FLASH-SPECIFIC] enables SPI NOR quad mode
>   * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
>   *			the SPI NOR Status Register.
>   * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
> @@ -579,7 +578,6 @@ struct spi_nor {
>  	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> -	int (*quad_enable)(struct spi_nor *nor);
>  	int (*clear_sr_bp)(struct spi_nor *nor);
>  	struct spi_nor_flash_parameter params;
>  


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

All flash parameters and settings should reside inside
'struct spi_nor_flash_parameter'. Move clear_sr_bp() from
'struct spi_nor' to 'struct spi_nor_flash_parameter'.

Rename clear_sr_bp()/disable_block_protection() to better indicate
what the function does.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 47 +++++++++++++++++++++++++++++++++----------
 include/linux/mtd/spi-nor.h   |  5 ++---
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 6bd104c29cd9..15b0b1148bf3 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -4477,20 +4477,45 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
 	return nor->params.quad_enable(nor);
 }
 
+/**
+ * spi_nor_disable_block_protection() - Disable the write block protection
+ * during power-up.
+ * @nor:                pointer to a 'struct spi_nor'
+ *
+ * Some spi-nor flashes are write protected by default after a power-on reset
+ * cycle, in order to avoid inadvertend writes during power-up. Backward
+ * compatibility imposes to disable the write block protection at power-up
+ * by default.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_disable_block_protection(struct spi_nor *nor)
+{
+	if (!nor->params.disable_block_protection)
+		return 0;
+
+	/*
+	 * In case of the legacy quad enable requirements are set, if the
+	 * configuration register Quad Enable bit is one, only the the
+	 * Write Status (01h) command with two data bytes may be used to clear
+	 * the block protection bits.
+	 */
+	if (nor->params.quad_enable == spansion_quad_enable)
+		nor->params.disable_block_protection =
+			spi_nor_spansion_clear_sr_bp;
+
+	return nor->params.disable_block_protection(nor);
+}
+
 static int spi_nor_init(struct spi_nor *nor)
 {
 	int err;
 
-	if (nor->clear_sr_bp) {
-		if (nor->quad_enable == spansion_quad_enable)
-			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
-
-		err = nor->clear_sr_bp(nor);
-		if (err) {
-			dev_err(nor->dev,
-				"fail to clear block protection bits\n");
-			return err;
-		}
+	err = spi_nor_disable_block_protection(nor);
+	if (err) {
+		dev_err(nor->dev,
+			"fail to unlock the flash at init (err = %d)\n", err);
+		return err;
 	}
 
 	err = spi_nor_quad_enable(nor);
@@ -4635,7 +4660,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
 	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
 	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
 	    nor->info->flags & SPI_NOR_HAS_LOCK)
-		nor->clear_sr_bp = spi_nor_clear_sr_bp;
+		nor->params.disable_block_protection = spi_nor_clear_sr_bp;
 
 	/* Parse the Serial Flash Discoverable Parameters table. */
 	ret = spi_nor_init_params(nor);
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 17787238f0e9..399ac34a529d 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -480,6 +480,7 @@ struct spi_nor;
  * @page_programs:	page program capabilities ordered by priority: the
  *                      higher index in the array, the higher priority.
  * @quad_enable:	enables SPI NOR quad mode.
+ * @disable_block_protection: disables block protection during power-up.
  */
 struct spi_nor_flash_parameter {
 	u64				size;
@@ -490,6 +491,7 @@ struct spi_nor_flash_parameter {
 	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
 
 	int (*quad_enable)(struct spi_nor *nor);
+	int (*disable_block_protection)(struct spi_nor *nor);
 };
 
 /**
@@ -535,8 +537,6 @@ struct flash_info;
  * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
  * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
  *			completely locked
- * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
- *			the SPI NOR Status Register.
  * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
  *                      The structure includes legacy flash parameters and
  *                      settings that can be overwritten by the spi_nor_fixups
@@ -578,7 +578,6 @@ struct spi_nor {
 	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
 	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
-	int (*clear_sr_bp)(struct spi_nor *nor);
 	struct spi_nor_flash_parameter params;
 
 	void *priv;
-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Tudor.Ambarus]

On 08/23/2019 06:53 PM, Tudor Ambarus - M18064 wrote:
> +	 * configuration register Quad Enable bit is one, only the the
								^duplication
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:41 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> All flash parameters and settings should reside inside
> 'struct spi_nor_flash_parameter'. Move clear_sr_bp() from
> 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
> 
> Rename clear_sr_bp()/disable_block_protection() to better indicate
> what the function does.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 47 +++++++++++++++++++++++++++++++++----------
>  include/linux/mtd/spi-nor.h   |  5 ++---
>  2 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 6bd104c29cd9..15b0b1148bf3 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4477,20 +4477,45 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
>  	return nor->params.quad_enable(nor);
>  }
>  
> +/**
> + * spi_nor_disable_block_protection() - Disable the write block protection
> + * during power-up.
> + * @nor:                pointer to a 'struct spi_nor'
> + *
> + * Some spi-nor flashes are write protected by default after a power-on reset
> + * cycle, in order to avoid inadvertend writes during power-up. Backward
> + * compatibility imposes to disable the write block protection at power-up
> + * by default.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_disable_block_protection(struct spi_nor *nor)
> +{
> +	if (!nor->params.disable_block_protection)
> +		return 0;
> +
> +	/*
> +	 * In case of the legacy quad enable requirements are set, if the
> +	 * configuration register Quad Enable bit is one, only the the
> +	 * Write Status (01h) command with two data bytes may be used to clear
> +	 * the block protection bits.
> +	 */
> +	if (nor->params.quad_enable == spansion_quad_enable)
> +		nor->params.disable_block_protection =
> +			spi_nor_spansion_clear_sr_bp;
> +
> +	return nor->params.disable_block_protection(nor);
> +}
> +
>  static int spi_nor_init(struct spi_nor *nor)
>  {
>  	int err;
>  
> -	if (nor->clear_sr_bp) {
> -		if (nor->quad_enable == spansion_quad_enable)
> -			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
> -
> -		err = nor->clear_sr_bp(nor);
> -		if (err) {
> -			dev_err(nor->dev,
> -				"fail to clear block protection bits\n");
> -			return err;
> -		}
> +	err = spi_nor_disable_block_protection(nor);
> +	if (err) {
> +		dev_err(nor->dev,
> +			"fail to unlock the flash at init (err = %d)\n", err);
> +		return err;
>  	}
>  
>  	err = spi_nor_quad_enable(nor);
> @@ -4635,7 +4660,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
>  	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
>  	    nor->info->flags & SPI_NOR_HAS_LOCK)
> -		nor->clear_sr_bp = spi_nor_clear_sr_bp;
> +		nor->params.disable_block_protection = spi_nor_clear_sr_bp;
>  
>  	/* Parse the Serial Flash Discoverable Parameters table. */
>  	ret = spi_nor_init_params(nor);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 17787238f0e9..399ac34a529d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -480,6 +480,7 @@ struct spi_nor;
>   * @page_programs:	page program capabilities ordered by priority: the
>   *                      higher index in the array, the higher priority.
>   * @quad_enable:	enables SPI NOR quad mode.
> + * @disable_block_protection: disables block protection during power-up.
>   */
>  struct spi_nor_flash_parameter {
>  	u64				size;
> @@ -490,6 +491,7 @@ struct spi_nor_flash_parameter {
>  	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
>  
>  	int (*quad_enable)(struct spi_nor *nor);
> +	int (*disable_block_protection)(struct spi_nor *nor);
>  };
>  
>  /**
> @@ -535,8 +537,6 @@ struct flash_info;
>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
>   *			completely locked
> - * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
> - *			the SPI NOR Status Register.
>   * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
>   *                      The structure includes legacy flash parameters and
>   *                      settings that can be overwritten by the spi_nor_fixups
> @@ -578,7 +578,6 @@ struct spi_nor {
>  	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> -	int (*clear_sr_bp)(struct spi_nor *nor);
>  	struct spi_nor_flash_parameter params;
>  
>  	void *priv;


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:41 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> All flash parameters and settings should reside inside
> 'struct spi_nor_flash_parameter'. Move clear_sr_bp() from
> 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
> 
> Rename clear_sr_bp()/disable_block_protection() to better indicate
> what the function does.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 47 +++++++++++++++++++++++++++++++++----------
>  include/linux/mtd/spi-nor.h   |  5 ++---
>  2 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 6bd104c29cd9..15b0b1148bf3 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -4477,20 +4477,45 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
>  	return nor->params.quad_enable(nor);
>  }
>  
> +/**
> + * spi_nor_disable_block_protection() - Disable the write block protection
> + * during power-up.
> + * @nor:                pointer to a 'struct spi_nor'
> + *
> + * Some spi-nor flashes are write protected by default after a power-on reset
> + * cycle, in order to avoid inadvertend writes during power-up. Backward
> + * compatibility imposes to disable the write block protection at power-up
> + * by default.
> + *
> + * Return: 0 on success, -errno otherwise.
> + */
> +static int spi_nor_disable_block_protection(struct spi_nor *nor)
> +{
> +	if (!nor->params.disable_block_protection)
> +		return 0;
> +
> +	/*
> +	 * In case of the legacy quad enable requirements are set, if the
> +	 * configuration register Quad Enable bit is one, only the the
> +	 * Write Status (01h) command with two data bytes may be used to clear
> +	 * the block protection bits.
> +	 */
> +	if (nor->params.quad_enable == spansion_quad_enable)
> +		nor->params.disable_block_protection =
> +			spi_nor_spansion_clear_sr_bp;

Hm, doesn't look right to adjust the function pointer just before
calling it. Can't we move that logic earlier (when doing the
default/manufacturer specific init)? Also, as I said in one of my
previous emails, I'd prefer to have this hook moved to
spi_nor_locking_ops and just have a flag to reflect when block
protection can be disabled.

> +
> +	return nor->params.disable_block_protection(nor);
> +}
> +
>  static int spi_nor_init(struct spi_nor *nor)
>  {
>  	int err;
>  
> -	if (nor->clear_sr_bp) {
> -		if (nor->quad_enable == spansion_quad_enable)
> -			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
> -
> -		err = nor->clear_sr_bp(nor);
> -		if (err) {
> -			dev_err(nor->dev,
> -				"fail to clear block protection bits\n");
> -			return err;
> -		}
> +	err = spi_nor_disable_block_protection(nor);
> +	if (err) {
> +		dev_err(nor->dev,
> +			"fail to unlock the flash at init (err = %d)\n", err);
> +		return err;
>  	}
>  
>  	err = spi_nor_quad_enable(nor);
> @@ -4635,7 +4660,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
>  	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
>  	    nor->info->flags & SPI_NOR_HAS_LOCK)
> -		nor->clear_sr_bp = spi_nor_clear_sr_bp;
> +		nor->params.disable_block_protection = spi_nor_clear_sr_bp;
>  
>  	/* Parse the Serial Flash Discoverable Parameters table. */
>  	ret = spi_nor_init_params(nor);
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 17787238f0e9..399ac34a529d 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -480,6 +480,7 @@ struct spi_nor;
>   * @page_programs:	page program capabilities ordered by priority: the
>   *                      higher index in the array, the higher priority.
>   * @quad_enable:	enables SPI NOR quad mode.
> + * @disable_block_protection: disables block protection during power-up.
>   */
>  struct spi_nor_flash_parameter {
>  	u64				size;
> @@ -490,6 +491,7 @@ struct spi_nor_flash_parameter {
>  	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
>  
>  	int (*quad_enable)(struct spi_nor *nor);
> +	int (*disable_block_protection)(struct spi_nor *nor);
>  };
>  
>  /**
> @@ -535,8 +537,6 @@ struct flash_info;
>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
>   *			completely locked
> - * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
> - *			the SPI NOR Status Register.
>   * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
>   *                      The structure includes legacy flash parameters and
>   *                      settings that can be overwritten by the spi_nor_fixups
> @@ -578,7 +578,6 @@ struct spi_nor {
>  	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> -	int (*clear_sr_bp)(struct spi_nor *nor);
>  	struct spi_nor_flash_parameter params;
>  
>  	void *priv;


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Tudor.Ambarus]

On 08/25/2019 04:09 PM, Boris Brezillon wrote:
> On Fri, 23 Aug 2019 15:53:41 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> All flash parameters and settings should reside inside
>> 'struct spi_nor_flash_parameter'. Move clear_sr_bp() from
>> 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
>>
>> Rename clear_sr_bp()/disable_block_protection() to better indicate
>> what the function does.
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 47 +++++++++++++++++++++++++++++++++----------
>>  include/linux/mtd/spi-nor.h   |  5 ++---
>>  2 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index 6bd104c29cd9..15b0b1148bf3 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -4477,20 +4477,45 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
>>  	return nor->params.quad_enable(nor);
>>  }
>>  
>> +/**
>> + * spi_nor_disable_block_protection() - Disable the write block protection
>> + * during power-up.
>> + * @nor:                pointer to a 'struct spi_nor'
>> + *
>> + * Some spi-nor flashes are write protected by default after a power-on reset
>> + * cycle, in order to avoid inadvertend writes during power-up. Backward
>> + * compatibility imposes to disable the write block protection at power-up
>> + * by default.
>> + *
>> + * Return: 0 on success, -errno otherwise.
>> + */
>> +static int spi_nor_disable_block_protection(struct spi_nor *nor)
>> +{
>> +	if (!nor->params.disable_block_protection)
>> +		return 0;
>> +
>> +	/*
>> +	 * In case of the legacy quad enable requirements are set, if the
>> +	 * configuration register Quad Enable bit is one, only the the
>> +	 * Write Status (01h) command with two data bytes may be used to clear
>> +	 * the block protection bits.
>> +	 */
>> +	if (nor->params.quad_enable == spansion_quad_enable)
>> +		nor->params.disable_block_protection =
>> +			spi_nor_spansion_clear_sr_bp;
> 
> Hm, doesn't look right to adjust the function pointer just before
> calling it. Can't we move that logic earlier (when doing the
> default/manufacturer specific init)? Also, as I said in one of my

No, we can't move it earlier to ->default_init() because the pointer to
quad_enable() function can be modified later on, when parsing SFDP. This should
stay here, after the quad_enable() method is known, so after the
spi_nor_init_params() call.


> previous emails, I'd prefer to have this hook moved to
> spi_nor_locking_ops and just have a flag to reflect when block
> protection can be disabled.

yes, I agree, will move.

> 
>> +
>> +	return nor->params.disable_block_protection(nor);
>> +}
>> +
>>  static int spi_nor_init(struct spi_nor *nor)
>>  {
>>  	int err;
>>  
>> -	if (nor->clear_sr_bp) {
>> -		if (nor->quad_enable == spansion_quad_enable)
>> -			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
>> -
>> -		err = nor->clear_sr_bp(nor);
>> -		if (err) {
>> -			dev_err(nor->dev,
>> -				"fail to clear block protection bits\n");
>> -			return err;
>> -		}
>> +	err = spi_nor_disable_block_protection(nor);
>> +	if (err) {
>> +		dev_err(nor->dev,
>> +			"fail to unlock the flash at init (err = %d)\n", err);
>> +		return err;
>>  	}
>>  
>>  	err = spi_nor_quad_enable(nor);
>> @@ -4635,7 +4660,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>>  	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
>>  	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
>>  	    nor->info->flags & SPI_NOR_HAS_LOCK)
>> -		nor->clear_sr_bp = spi_nor_clear_sr_bp;
>> +		nor->params.disable_block_protection = spi_nor_clear_sr_bp;
>>  
>>  	/* Parse the Serial Flash Discoverable Parameters table. */
>>  	ret = spi_nor_init_params(nor);
>> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
>> index 17787238f0e9..399ac34a529d 100644
>> --- a/include/linux/mtd/spi-nor.h
>> +++ b/include/linux/mtd/spi-nor.h
>> @@ -480,6 +480,7 @@ struct spi_nor;
>>   * @page_programs:	page program capabilities ordered by priority: the
>>   *                      higher index in the array, the higher priority.
>>   * @quad_enable:	enables SPI NOR quad mode.
>> + * @disable_block_protection: disables block protection during power-up.
>>   */
>>  struct spi_nor_flash_parameter {
>>  	u64				size;
>> @@ -490,6 +491,7 @@ struct spi_nor_flash_parameter {
>>  	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
>>  
>>  	int (*quad_enable)(struct spi_nor *nor);
>> +	int (*disable_block_protection)(struct spi_nor *nor);
>>  };
>>  
>>  /**
>> @@ -535,8 +537,6 @@ struct flash_info;
>>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
>>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
>>   *			completely locked
>> - * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
>> - *			the SPI NOR Status Register.
>>   * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
>>   *                      The structure includes legacy flash parameters and
>>   *                      settings that can be overwritten by the spi_nor_fixups
>> @@ -578,7 +578,6 @@ struct spi_nor {
>>  	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
>> -	int (*clear_sr_bp)(struct spi_nor *nor);
>>  	struct spi_nor_flash_parameter params;
>>  
>>  	void *priv;
> 
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 4/5] mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'

[Boris Brezillon]

On Sun, 25 Aug 2019 13:19:57 +0000
<Tudor.Ambarus@microchip.com> wrote:

> On 08/25/2019 04:09 PM, Boris Brezillon wrote:
> > On Fri, 23 Aug 2019 15:53:41 +0000
> > <Tudor.Ambarus@microchip.com> wrote:
> >   
> >> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> >>
> >> All flash parameters and settings should reside inside
> >> 'struct spi_nor_flash_parameter'. Move clear_sr_bp() from
> >> 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
> >>
> >> Rename clear_sr_bp()/disable_block_protection() to better indicate
> >> what the function does.
> >>
> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> >> ---
> >>  drivers/mtd/spi-nor/spi-nor.c | 47 +++++++++++++++++++++++++++++++++----------
> >>  include/linux/mtd/spi-nor.h   |  5 ++---
> >>  2 files changed, 38 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> >> index 6bd104c29cd9..15b0b1148bf3 100644
> >> --- a/drivers/mtd/spi-nor/spi-nor.c
> >> +++ b/drivers/mtd/spi-nor/spi-nor.c
> >> @@ -4477,20 +4477,45 @@ static int spi_nor_quad_enable(struct spi_nor *nor)
> >>  	return nor->params.quad_enable(nor);
> >>  }
> >>  
> >> +/**
> >> + * spi_nor_disable_block_protection() - Disable the write block protection
> >> + * during power-up.
> >> + * @nor:                pointer to a 'struct spi_nor'
> >> + *
> >> + * Some spi-nor flashes are write protected by default after a power-on reset
> >> + * cycle, in order to avoid inadvertend writes during power-up. Backward
> >> + * compatibility imposes to disable the write block protection at power-up
> >> + * by default.
> >> + *
> >> + * Return: 0 on success, -errno otherwise.
> >> + */
> >> +static int spi_nor_disable_block_protection(struct spi_nor *nor)
> >> +{
> >> +	if (!nor->params.disable_block_protection)
> >> +		return 0;
> >> +
> >> +	/*
> >> +	 * In case of the legacy quad enable requirements are set, if the
> >> +	 * configuration register Quad Enable bit is one, only the the
> >> +	 * Write Status (01h) command with two data bytes may be used to clear
> >> +	 * the block protection bits.
> >> +	 */
> >> +	if (nor->params.quad_enable == spansion_quad_enable)
> >> +		nor->params.disable_block_protection =
> >> +			spi_nor_spansion_clear_sr_bp;  
> > 
> > Hm, doesn't look right to adjust the function pointer just before
> > calling it. Can't we move that logic earlier (when doing the
> > default/manufacturer specific init)? Also, as I said in one of my  
> 
> No, we can't move it earlier to ->default_init() because the pointer to
> quad_enable() function can be modified later on, when parsing SFDP. This should
> stay here, after the quad_enable() method is known, so after the
> spi_nor_init_params() call.
> 
> 
> > previous emails, I'd prefer to have this hook moved to
> > spi_nor_locking_ops and just have a flag to reflect when block
> > protection can be disabled.  
> 
> yes, I agree, will move.

That won't work: ->locking_ops is const, meaning that you can't update
individual fields (which I consider a good thing). As I see it, the
locking scheme is a package that describe how to lock/unlock blocks, be
it individually, by groups or globally. I really hope we can take this
decision on a per-manufacturer basis, but I fear it might actually be a
per-chip thing.

> 
> >   
> >> +
> >> +	return nor->params.disable_block_protection(nor);
> >> +}
> >> +
> >>  static int spi_nor_init(struct spi_nor *nor)
> >>  {
> >>  	int err;
> >>  
> >> -	if (nor->clear_sr_bp) {
> >> -		if (nor->quad_enable == spansion_quad_enable)
> >> -			nor->clear_sr_bp = spi_nor_spansion_clear_sr_bp;
> >> -
> >> -		err = nor->clear_sr_bp(nor);
> >> -		if (err) {
> >> -			dev_err(nor->dev,
> >> -				"fail to clear block protection bits\n");
> >> -			return err;
> >> -		}
> >> +	err = spi_nor_disable_block_protection(nor);
> >> +	if (err) {
> >> +		dev_err(nor->dev,
> >> +			"fail to unlock the flash at init (err = %d)\n", err);
> >> +		return err;
> >>  	}
> >>  
> >>  	err = spi_nor_quad_enable(nor);
> >> @@ -4635,7 +4660,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> >>  	    JEDEC_MFR(nor->info) == SNOR_MFR_INTEL ||
> >>  	    JEDEC_MFR(nor->info) == SNOR_MFR_SST ||
> >>  	    nor->info->flags & SPI_NOR_HAS_LOCK)
> >> -		nor->clear_sr_bp = spi_nor_clear_sr_bp;
> >> +		nor->params.disable_block_protection = spi_nor_clear_sr_bp;
> >>  
> >>  	/* Parse the Serial Flash Discoverable Parameters table. */
> >>  	ret = spi_nor_init_params(nor);
> >> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> >> index 17787238f0e9..399ac34a529d 100644
> >> --- a/include/linux/mtd/spi-nor.h
> >> +++ b/include/linux/mtd/spi-nor.h
> >> @@ -480,6 +480,7 @@ struct spi_nor;
> >>   * @page_programs:	page program capabilities ordered by priority: the
> >>   *                      higher index in the array, the higher priority.
> >>   * @quad_enable:	enables SPI NOR quad mode.
> >> + * @disable_block_protection: disables block protection during power-up.
> >>   */
> >>  struct spi_nor_flash_parameter {
> >>  	u64				size;
> >> @@ -490,6 +491,7 @@ struct spi_nor_flash_parameter {
> >>  	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
> >>  
> >>  	int (*quad_enable)(struct spi_nor *nor);
> >> +	int (*disable_block_protection)(struct spi_nor *nor);
> >>  };
> >>  
> >>  /**
> >> @@ -535,8 +537,6 @@ struct flash_info;
> >>   * @flash_unlock:	[FLASH-SPECIFIC] unlock a region of the SPI NOR
> >>   * @flash_is_locked:	[FLASH-SPECIFIC] check if a region of the SPI NOR is
> >>   *			completely locked
> >> - * @clear_sr_bp:	[FLASH-SPECIFIC] clears the Block Protection Bits from
> >> - *			the SPI NOR Status Register.
> >>   * @params:		[FLASH-SPECIFIC] SPI-NOR flash parameters and settings.
> >>   *                      The structure includes legacy flash parameters and
> >>   *                      settings that can be overwritten by the spi_nor_fixups
> >> @@ -578,7 +578,6 @@ struct spi_nor {
> >>  	int (*flash_lock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> >>  	int (*flash_unlock)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> >>  	int (*flash_is_locked)(struct spi_nor *nor, loff_t ofs, uint64_t len);
> >> -	int (*clear_sr_bp)(struct spi_nor *nor);
> >>  	struct spi_nor_flash_parameter params;
> >>  
> >>  	void *priv;  
> > 
> >   


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH 5/5] mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'

[Tudor.Ambarus]

From: Tudor Ambarus <tudor.ambarus@microchip.com>

All flash parameters and settings should reside inside
'struct spi_nor_flash_parameter'. Move the SMPT parsed erase map
from 'struct spi_nor' to 'struct spi_nor_flash_parameter'.

Please note that there is a roll-back mechanism for the flash
parameter and settings, for cases when SFDP parser fails. The SFDP
parser receives a Stack allocated copy of nor->params, called
sfdp_params, and uses it to retrieve the serial flash discoverable
parameters. JESD216 SFDP is a standard and has a higher priority
than the legacy initialized flash parameters, so will overwrite the
sfdp_params data when needed. All SFDP code uses the local copy of
nor->params, that will overwrite it in the end, if the parser succeds.

Saving and restoring the nor->params.erase_map is no longer needed,
since the SFDP code does not touch it.

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 40 +++++++++++++++++++++-------------------
 include/linux/mtd/spi-nor.h   |  8 +++++---
 2 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 15b0b1148bf3..f5c1c71caf1b 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -600,7 +600,7 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
 	nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
 
 	if (!spi_nor_has_uniform_erase(nor)) {
-		struct spi_nor_erase_map *map = &nor->erase_map;
+		struct spi_nor_erase_map *map = &nor->params.erase_map;
 		struct spi_nor_erase_type *erase;
 		int i;
 
@@ -1133,7 +1133,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
 				       struct list_head *erase_list,
 				       u64 addr, u32 len)
 {
-	const struct spi_nor_erase_map *map = &nor->erase_map;
+	const struct spi_nor_erase_map *map = &nor->params.erase_map;
 	const struct spi_nor_erase_type *erase, *prev_erase = NULL;
 	struct spi_nor_erase_region *region;
 	struct spi_nor_erase_command *cmd = NULL;
@@ -3328,7 +3328,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 			      const struct sfdp_parameter_header *bfpt_header,
 			      struct spi_nor_flash_parameter *params)
 {
-	struct spi_nor_erase_map *map = &nor->erase_map;
+	struct spi_nor_erase_map *map = &params->erase_map;
 	struct spi_nor_erase_type *erase_type = map->erase_type;
 	struct sfdp_bfpt bfpt;
 	size_t len;
@@ -3409,7 +3409,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
 	 * Erase Types defined in the bfpt table.
 	 */
 	erase_mask = 0;
-	memset(&nor->erase_map, 0, sizeof(nor->erase_map));
+	memset(&params->erase_map, 0, sizeof(params->erase_map));
 	for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_erases); i++) {
 		const struct sfdp_bfpt_erase *er = &sfdp_bfpt_erases[i];
 		u32 erasesize;
@@ -3684,14 +3684,18 @@ spi_nor_region_check_overlay(struct spi_nor_erase_region *region,
 /**
  * spi_nor_init_non_uniform_erase_map() - initialize the non-uniform erase map
  * @nor:	pointer to a 'struct spi_nor'
+ * @params:     pointer to a duplicate 'struct spi_nor_flash_parameter' that is
+ *              used for storing SFDP parsed data
  * @smpt:	pointer to the sector map parameter table
  *
  * Return: 0 on success, -errno otherwise.
  */
-static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
-					      const u32 *smpt)
+static int
+spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
+				   struct spi_nor_flash_parameter *params,
+				   const u32 *smpt)
 {
-	struct spi_nor_erase_map *map = &nor->erase_map;
+	struct spi_nor_erase_map *map = &params->erase_map;
 	struct spi_nor_erase_type *erase = map->erase_type;
 	struct spi_nor_erase_region *region;
 	u64 offset;
@@ -3770,6 +3774,8 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
  * spi_nor_parse_smpt() - parse Sector Map Parameter Table
  * @nor:		pointer to a 'struct spi_nor'
  * @smpt_header:	sector map parameter table header
+ * @params:		pointer to a duplicate 'struct spi_nor_flash_parameter'
+ *                      that is used for storing SFDP parsed data
  *
  * This table is optional, but when available, we parse it to identify the
  * location and size of sectors within the main data array of the flash memory
@@ -3778,7 +3784,8 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
  * Return: 0 on success, -errno otherwise.
  */
 static int spi_nor_parse_smpt(struct spi_nor *nor,
-			      const struct sfdp_parameter_header *smpt_header)
+			      const struct sfdp_parameter_header *smpt_header,
+			      struct spi_nor_flash_parameter *params)
 {
 	const u32 *sector_map;
 	u32 *smpt;
@@ -3807,11 +3814,11 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
 		goto out;
 	}
 
-	ret = spi_nor_init_non_uniform_erase_map(nor, sector_map);
+	ret = spi_nor_init_non_uniform_erase_map(nor, params, sector_map);
 	if (ret)
 		goto out;
 
-	spi_nor_regions_sort_erase_types(&nor->erase_map);
+	spi_nor_regions_sort_erase_types(&params->erase_map);
 	/* fall through */
 out:
 	kfree(smpt);
@@ -3867,7 +3874,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
 		{ 0u /* not used */,		BIT(12) },
 	};
 	struct spi_nor_pp_command *params_pp = params->page_programs;
-	struct spi_nor_erase_map *map = &nor->erase_map;
+	struct spi_nor_erase_map *map = &params->erase_map;
 	struct spi_nor_erase_type *erase_type = map->erase_type;
 	u32 *dwords;
 	size_t len;
@@ -4097,7 +4104,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 
 		switch (SFDP_PARAM_HEADER_ID(param_header)) {
 		case SFDP_SECTOR_MAP_ID:
-			err = spi_nor_parse_smpt(nor, param_header);
+			err = spi_nor_parse_smpt(nor, param_header, params);
 			break;
 
 		case SFDP_4BAIT_ID:
@@ -4129,7 +4136,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
 static int spi_nor_init_params(struct spi_nor *nor)
 {
 	struct spi_nor_flash_parameter *params = &nor->params;
-	struct spi_nor_erase_map *map = &nor->erase_map;
+	struct spi_nor_erase_map *map = &params->erase_map;
 	const struct flash_info *info = nor->info;
 	u8 i, erase_mask;
 
@@ -4229,17 +4236,12 @@ static int spi_nor_init_params(struct spi_nor *nor)
 	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
 	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
 		struct spi_nor_flash_parameter sfdp_params;
-		struct spi_nor_erase_map prev_map;
 
 		memcpy(&sfdp_params, params, sizeof(sfdp_params));
-		memcpy(&prev_map, &nor->erase_map, sizeof(prev_map));
 
 		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
 			nor->addr_width = 0;
 			nor->flags &= ~SNOR_F_4B_OPCODES;
-			/* restore previous erase map */
-			memcpy(&nor->erase_map, &prev_map,
-			       sizeof(nor->erase_map));
 		} else {
 			memcpy(params, &sfdp_params, sizeof(*params));
 		}
@@ -4353,7 +4355,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
 
 static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
 {
-	struct spi_nor_erase_map *map = &nor->erase_map;
+	struct spi_nor_erase_map *map = &nor->params.erase_map;
 	const struct spi_nor_erase_type *erase = NULL;
 	struct mtd_info *mtd = &nor->mtd;
 	int i;
diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
index 399ac34a529d..a3a765c21edc 100644
--- a/include/linux/mtd/spi-nor.h
+++ b/include/linux/mtd/spi-nor.h
@@ -479,6 +479,8 @@ struct spi_nor;
  *                      in the array, the higher priority.
  * @page_programs:	page program capabilities ordered by priority: the
  *                      higher index in the array, the higher priority.
+ * @erase_map:		the erase map parsed from the SFDP Sector Map Parameter
+ *                      Table.
  * @quad_enable:	enables SPI NOR quad mode.
  * @disable_block_protection: disables block protection during power-up.
  */
@@ -490,6 +492,8 @@ struct spi_nor_flash_parameter {
 	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
 	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
 
+	struct spi_nor_erase_map        erase_map;
+
 	int (*quad_enable)(struct spi_nor *nor);
 	int (*disable_block_protection)(struct spi_nor *nor);
 };
@@ -521,7 +525,6 @@ struct flash_info;
  * @read_proto:		the SPI protocol for read operations
  * @write_proto:	the SPI protocol for write operations
  * @reg_proto		the SPI protocol for read_reg/write_reg/erase operations
- * @erase_map:		the erase map of the SPI NOR
  * @prepare:		[OPTIONAL] do some preparations for the
  *			read/write/erase/lock/unlock operations
  * @unprepare:		[OPTIONAL] do some post work after the
@@ -562,7 +565,6 @@ struct spi_nor {
 	enum spi_nor_protocol	reg_proto;
 	bool			sst_write_second;
 	u32			flags;
-	struct spi_nor_erase_map	erase_map;
 
 	int (*prepare)(struct spi_nor *nor, enum spi_nor_ops ops);
 	void (*unprepare)(struct spi_nor *nor, enum spi_nor_ops ops);
@@ -609,7 +611,7 @@ spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
 
 static bool __maybe_unused spi_nor_has_uniform_erase(const struct spi_nor *nor)
 {
-	return !!nor->erase_map.uniform_erase_type;
+	return !!nor->params.erase_map.uniform_erase_type;
 }
 
 static inline void spi_nor_set_flash_node(struct spi_nor *nor,
-- 
2.9.5


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 5/5] mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:43 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> All flash parameters and settings should reside inside
> 'struct spi_nor_flash_parameter'. Move the SMPT parsed erase map
> from 'struct spi_nor' to 'struct spi_nor_flash_parameter'.
> 
> Please note that there is a roll-back mechanism for the flash
> parameter and settings, for cases when SFDP parser fails. The SFDP
> parser receives a Stack allocated copy of nor->params, called
> sfdp_params, and uses it to retrieve the serial flash discoverable
> parameters. JESD216 SFDP is a standard and has a higher priority
> than the legacy initialized flash parameters, so will overwrite the
> sfdp_params data when needed. All SFDP code uses the local copy of
> nor->params, that will overwrite it in the end, if the parser succeds.
> 
> Saving and restoring the nor->params.erase_map is no longer needed,
> since the SFDP code does not touch it.
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>

Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 40 +++++++++++++++++++++-------------------
>  include/linux/mtd/spi-nor.h   |  8 +++++---
>  2 files changed, 26 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 15b0b1148bf3..f5c1c71caf1b 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -600,7 +600,7 @@ static void spi_nor_set_4byte_opcodes(struct spi_nor *nor)
>  	nor->erase_opcode = spi_nor_convert_3to4_erase(nor->erase_opcode);
>  
>  	if (!spi_nor_has_uniform_erase(nor)) {
> -		struct spi_nor_erase_map *map = &nor->erase_map;
> +		struct spi_nor_erase_map *map = &nor->params.erase_map;
>  		struct spi_nor_erase_type *erase;
>  		int i;
>  
> @@ -1133,7 +1133,7 @@ static int spi_nor_init_erase_cmd_list(struct spi_nor *nor,
>  				       struct list_head *erase_list,
>  				       u64 addr, u32 len)
>  {
> -	const struct spi_nor_erase_map *map = &nor->erase_map;
> +	const struct spi_nor_erase_map *map = &nor->params.erase_map;
>  	const struct spi_nor_erase_type *erase, *prev_erase = NULL;
>  	struct spi_nor_erase_region *region;
>  	struct spi_nor_erase_command *cmd = NULL;
> @@ -3328,7 +3328,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  			      const struct sfdp_parameter_header *bfpt_header,
>  			      struct spi_nor_flash_parameter *params)
>  {
> -	struct spi_nor_erase_map *map = &nor->erase_map;
> +	struct spi_nor_erase_map *map = &params->erase_map;
>  	struct spi_nor_erase_type *erase_type = map->erase_type;
>  	struct sfdp_bfpt bfpt;
>  	size_t len;
> @@ -3409,7 +3409,7 @@ static int spi_nor_parse_bfpt(struct spi_nor *nor,
>  	 * Erase Types defined in the bfpt table.
>  	 */
>  	erase_mask = 0;
> -	memset(&nor->erase_map, 0, sizeof(nor->erase_map));
> +	memset(&params->erase_map, 0, sizeof(params->erase_map));
>  	for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_erases); i++) {
>  		const struct sfdp_bfpt_erase *er = &sfdp_bfpt_erases[i];
>  		u32 erasesize;
> @@ -3684,14 +3684,18 @@ spi_nor_region_check_overlay(struct spi_nor_erase_region *region,
>  /**
>   * spi_nor_init_non_uniform_erase_map() - initialize the non-uniform erase map
>   * @nor:	pointer to a 'struct spi_nor'
> + * @params:     pointer to a duplicate 'struct spi_nor_flash_parameter' that is
> + *              used for storing SFDP parsed data
>   * @smpt:	pointer to the sector map parameter table
>   *
>   * Return: 0 on success, -errno otherwise.
>   */
> -static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
> -					      const u32 *smpt)
> +static int
> +spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
> +				   struct spi_nor_flash_parameter *params,
> +				   const u32 *smpt)
>  {
> -	struct spi_nor_erase_map *map = &nor->erase_map;
> +	struct spi_nor_erase_map *map = &params->erase_map;
>  	struct spi_nor_erase_type *erase = map->erase_type;
>  	struct spi_nor_erase_region *region;
>  	u64 offset;
> @@ -3770,6 +3774,8 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>   * spi_nor_parse_smpt() - parse Sector Map Parameter Table
>   * @nor:		pointer to a 'struct spi_nor'
>   * @smpt_header:	sector map parameter table header
> + * @params:		pointer to a duplicate 'struct spi_nor_flash_parameter'
> + *                      that is used for storing SFDP parsed data
>   *
>   * This table is optional, but when available, we parse it to identify the
>   * location and size of sectors within the main data array of the flash memory
> @@ -3778,7 +3784,8 @@ static int spi_nor_init_non_uniform_erase_map(struct spi_nor *nor,
>   * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_parse_smpt(struct spi_nor *nor,
> -			      const struct sfdp_parameter_header *smpt_header)
> +			      const struct sfdp_parameter_header *smpt_header,
> +			      struct spi_nor_flash_parameter *params)
>  {
>  	const u32 *sector_map;
>  	u32 *smpt;
> @@ -3807,11 +3814,11 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
>  		goto out;
>  	}
>  
> -	ret = spi_nor_init_non_uniform_erase_map(nor, sector_map);
> +	ret = spi_nor_init_non_uniform_erase_map(nor, params, sector_map);
>  	if (ret)
>  		goto out;
>  
> -	spi_nor_regions_sort_erase_types(&nor->erase_map);
> +	spi_nor_regions_sort_erase_types(&params->erase_map);
>  	/* fall through */
>  out:
>  	kfree(smpt);
> @@ -3867,7 +3874,7 @@ static int spi_nor_parse_4bait(struct spi_nor *nor,
>  		{ 0u /* not used */,		BIT(12) },
>  	};
>  	struct spi_nor_pp_command *params_pp = params->page_programs;
> -	struct spi_nor_erase_map *map = &nor->erase_map;
> +	struct spi_nor_erase_map *map = &params->erase_map;
>  	struct spi_nor_erase_type *erase_type = map->erase_type;
>  	u32 *dwords;
>  	size_t len;
> @@ -4097,7 +4104,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  
>  		switch (SFDP_PARAM_HEADER_ID(param_header)) {
>  		case SFDP_SECTOR_MAP_ID:
> -			err = spi_nor_parse_smpt(nor, param_header);
> +			err = spi_nor_parse_smpt(nor, param_header, params);
>  			break;
>  
>  		case SFDP_4BAIT_ID:
> @@ -4129,7 +4136,7 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>  static int spi_nor_init_params(struct spi_nor *nor)
>  {
>  	struct spi_nor_flash_parameter *params = &nor->params;
> -	struct spi_nor_erase_map *map = &nor->erase_map;
> +	struct spi_nor_erase_map *map = &params->erase_map;
>  	const struct flash_info *info = nor->info;
>  	u8 i, erase_mask;
>  
> @@ -4229,17 +4236,12 @@ static int spi_nor_init_params(struct spi_nor *nor)
>  	if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
>  	    !(info->flags & SPI_NOR_SKIP_SFDP)) {
>  		struct spi_nor_flash_parameter sfdp_params;
> -		struct spi_nor_erase_map prev_map;
>  
>  		memcpy(&sfdp_params, params, sizeof(sfdp_params));
> -		memcpy(&prev_map, &nor->erase_map, sizeof(prev_map));
>  
>  		if (spi_nor_parse_sfdp(nor, &sfdp_params)) {
>  			nor->addr_width = 0;
>  			nor->flags &= ~SNOR_F_4B_OPCODES;
> -			/* restore previous erase map */
> -			memcpy(&nor->erase_map, &prev_map,
> -			       sizeof(nor->erase_map));
>  		} else {
>  			memcpy(params, &sfdp_params, sizeof(*params));
>  		}
> @@ -4353,7 +4355,7 @@ spi_nor_select_uniform_erase(struct spi_nor_erase_map *map,
>  
>  static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
>  {
> -	struct spi_nor_erase_map *map = &nor->erase_map;
> +	struct spi_nor_erase_map *map = &nor->params.erase_map;
>  	const struct spi_nor_erase_type *erase = NULL;
>  	struct mtd_info *mtd = &nor->mtd;
>  	int i;
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 399ac34a529d..a3a765c21edc 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -479,6 +479,8 @@ struct spi_nor;
>   *                      in the array, the higher priority.
>   * @page_programs:	page program capabilities ordered by priority: the
>   *                      higher index in the array, the higher priority.
> + * @erase_map:		the erase map parsed from the SFDP Sector Map Parameter
> + *                      Table.
>   * @quad_enable:	enables SPI NOR quad mode.
>   * @disable_block_protection: disables block protection during power-up.
>   */
> @@ -490,6 +492,8 @@ struct spi_nor_flash_parameter {
>  	struct spi_nor_read_command	reads[SNOR_CMD_READ_MAX];
>  	struct spi_nor_pp_command	page_programs[SNOR_CMD_PP_MAX];
>  
> +	struct spi_nor_erase_map        erase_map;
> +
>  	int (*quad_enable)(struct spi_nor *nor);
>  	int (*disable_block_protection)(struct spi_nor *nor);
>  };
> @@ -521,7 +525,6 @@ struct flash_info;
>   * @read_proto:		the SPI protocol for read operations
>   * @write_proto:	the SPI protocol for write operations
>   * @reg_proto		the SPI protocol for read_reg/write_reg/erase operations
> - * @erase_map:		the erase map of the SPI NOR
>   * @prepare:		[OPTIONAL] do some preparations for the
>   *			read/write/erase/lock/unlock operations
>   * @unprepare:		[OPTIONAL] do some post work after the
> @@ -562,7 +565,6 @@ struct spi_nor {
>  	enum spi_nor_protocol	reg_proto;
>  	bool			sst_write_second;
>  	u32			flags;
> -	struct spi_nor_erase_map	erase_map;
>  
>  	int (*prepare)(struct spi_nor *nor, enum spi_nor_ops ops);
>  	void (*unprepare)(struct spi_nor *nor, enum spi_nor_ops ops);
> @@ -609,7 +611,7 @@ spi_nor_region_mark_overlay(struct spi_nor_erase_region *region)
>  
>  static bool __maybe_unused spi_nor_has_uniform_erase(const struct spi_nor *nor)
>  {
> -	return !!nor->erase_map.uniform_erase_type;
> +	return !!nor->params.erase_map.uniform_erase_type;
>  }
>  
>  static inline void spi_nor_set_flash_node(struct spi_nor *nor,


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/5] mtd: spi-nor: move manuf out of the core - batch 0

[Boris Brezillon]

On Fri, 23 Aug 2019 15:53:33 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> This series is a prerequisite for the effort of moving the
> manufacturer specific code out of the SPI NOR core.
> 
> The scope is to move all [FLASH-SPECIFIC] parameters and settings
> from 'struct spi_nor' to 'struct spi_nor_flash_parameter'. We will
> have a clear separation between the SPI NOR layer and the flash
> parameters and settings.
> 
> 'struct spi_nor_flash_parameter' describes the hardware capabilities
> and associated settings of the SPI NOR flash memory. It includes
> legacy flash parameters and settings that can be overwritten by the
> spi_nor_fixups hooks, or dynamically when parsing the JESD216
> Serial Flash Discoverable Parameters (SFDP) tables. All SFDP params
> and settings will fit inside 'struct spi_nor_flash_parameter'.

While we're at moving things around, I think it'd make sense to move
all '[DRIVER SPECIFIC]' fields (which are actually SPI NOR controller
driver specific fields) to a separate struct:

struct spi_nor_controller_ops {
	int (*prepare)(struct spi_nor *nor, enum spi_nor_ops ops);
	void (*unprepare)(struct spi_nor *nor, enum spi_nor_ops ops);
	int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
	int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
	ssize_t (*read)(struct spi_nor *nor, loff_t from,
			size_t len, u_char *read_buf);
	ssize_t (*write)(struct spi_nor *nor, loff_t to,
			size_t len, const u_char *write_buf);
	int (*erase)(struct spi_nor *nor, loff_t offs);
};

struct spi_nor {
...
	const struct spi_nor_controller_ops *controller_ops;
...
};

> 
> Tested uniform and non-uniform erase on sst26vf064b flash using the
> atmel-quadspi driver.
> 
> In order to test this, you'll have to merge v5.3-rc5 in spi-nor/next.
> This patch depends on
> 'commit 834de5c1aa76 ("mtd: spi-nor: Fix the disabling of write protection at init")
> 
> Tudor Ambarus (5):
>   mtd: spi-nor: Regroup flash parameter and settings
>   mtd: spi-nor: Use nor->params
>   mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
>   mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'
>   mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'
> 
>  drivers/mtd/spi-nor/spi-nor.c | 236 ++++++++++++++++-----------------------
>  include/linux/mtd/spi-nor.h   | 254 ++++++++++++++++++++++++++++--------------
>  2 files changed, 269 insertions(+), 221 deletions(-)
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 0/5] mtd: spi-nor: move manuf out of the core - batch 0

[Tudor.Ambarus]

On 08/25/2019 02:38 PM, Boris Brezillon wrote:
> External E-Mail
> 
> 
> On Fri, 23 Aug 2019 15:53:33 +0000
> <Tudor.Ambarus@microchip.com> wrote:
> 
>> From: Tudor Ambarus <tudor.ambarus@microchip.com>
>>
>> This series is a prerequisite for the effort of moving the
>> manufacturer specific code out of the SPI NOR core.
>>
>> The scope is to move all [FLASH-SPECIFIC] parameters and settings
>> from 'struct spi_nor' to 'struct spi_nor_flash_parameter'. We will
>> have a clear separation between the SPI NOR layer and the flash
>> parameters and settings.
>>
>> 'struct spi_nor_flash_parameter' describes the hardware capabilities
>> and associated settings of the SPI NOR flash memory. It includes
>> legacy flash parameters and settings that can be overwritten by the
>> spi_nor_fixups hooks, or dynamically when parsing the JESD216
>> Serial Flash Discoverable Parameters (SFDP) tables. All SFDP params
>> and settings will fit inside 'struct spi_nor_flash_parameter'.
> 
> While we're at moving things around, I think it'd make sense to move
> all '[DRIVER SPECIFIC]' fields (which are actually SPI NOR controller
> driver specific fields) to a separate struct:
> 
> struct spi_nor_controller_ops {
> 	int (*prepare)(struct spi_nor *nor, enum spi_nor_ops ops);
> 	void (*unprepare)(struct spi_nor *nor, enum spi_nor_ops ops);
> 	int (*read_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
> 	int (*write_reg)(struct spi_nor *nor, u8 opcode, u8 *buf, int len);
> 	ssize_t (*read)(struct spi_nor *nor, loff_t from,
> 			size_t len, u_char *read_buf);
> 	ssize_t (*write)(struct spi_nor *nor, loff_t to,
> 			size_t len, const u_char *write_buf);
> 	int (*erase)(struct spi_nor *nor, loff_t offs);
> };
> 
> struct spi_nor {
> ...
> 	const struct spi_nor_controller_ops *controller_ops;
> ...
> };

Yep, this is a good idea. I'll make a patch and add your Suggested-by tag.

Thanks!

> 
>>
>> Tested uniform and non-uniform erase on sst26vf064b flash using the
>> atmel-quadspi driver.
>>
>> In order to test this, you'll have to merge v5.3-rc5 in spi-nor/next.
>> This patch depends on
>> 'commit 834de5c1aa76 ("mtd: spi-nor: Fix the disabling of write protection at init")
>>
>> Tudor Ambarus (5):
>>   mtd: spi-nor: Regroup flash parameter and settings
>>   mtd: spi-nor: Use nor->params
>>   mtd: spi-nor: Drop quad_enable() from 'struct spi-nor'
>>   mtd: spi-nor: Move clear_sr_bp() to 'struct spi_nor_flash_parameter'
>>   mtd: spi-nor: Move erase_map to 'struct spi_nor_flash_parameter'
>>
>>  drivers/mtd/spi-nor/spi-nor.c | 236 ++++++++++++++++-----------------------
>>  include/linux/mtd/spi-nor.h   | 254 ++++++++++++++++++++++++++++--------------
>>  2 files changed, 269 insertions(+), 221 deletions(-)
>>
> 
> 
> 
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/