Re: [PATCH v2] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-12-07 Thread Boris Brezillon
Hi Tudor,

On Thu, 6 Dec 2018 14:43:39 +
 wrote:

>  /**
>   * spi_nor_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
>   * @nor: pointer to a 'struct spi_nor'
> @@ -3276,6 +3462,10 @@ static int spi_nor_parse_sfdp(struct spi_nor *nor,
>   err = spi_nor_parse_smpt(nor, param_header);
>   break;
>  
> + case SFDP_4BAIT_ID:
> + err = spi_nor_parse_4bait(nor, param_header, params);
> + break;
> +
>   default:
>   break;
>   }
> @@ -3857,7 +4047,8 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>   (JEDEC_MFR(info) == SNOR_MFR_SPANSION && mtd->size > SZ_16M))
>   nor->flags |= SNOR_F_4B_OPCODES;
>  
> - if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES)
> + if (nor->addr_width == 4 && nor->flags & SNOR_F_4B_OPCODES &&
> + !(nor->flags & SNOR_F_HAS_4BAIT))
>   spi_nor_set_4byte_opcodes(nor);

The list of checks is growing. Maybe we could move those checks
at the beginning of spi_nor_set_4byte_opcodes() so we can
unconditionally call spi_nor_set_4byte_opcodes() here.
Not something I ask you to fix in this patch, just thinking out loud.

The rest of the patch looks good, but I'm pretty sure we'll run into
trouble as soon as we start parsing this 4BAIT section (as has been the
case for all other SFPD sections so far), just because vendors tend to
get this sort of things wrong. I don't have a solution for that other
than the proposed SFDP fixup hooks infrastructure [1] or the more
conservative "don't parse/trust SFDP" approach. Note that, even if we
go for the SFDP fixups approach, we might still break setups where the
4BAIT section of the SFDP table is broken until someone comes with a
fixup for those broken chips.

Since I don't like to block inclusion of new features for purely
theoretical issues, I'm in favor of applying this patch, but be
prepared to receive bug reports during the 4.21-rc cycle ;-).

Regards,

Boris

[1]http://patchwork.ozlabs.org/patch/1008687/


[PATCH v2] mtd: spi-nor: parse SFDP 4-byte Address Instruction Table

2018-12-06 Thread Tudor.Ambarus
From: Cyrille Pitchen 

Add support for SFDP (JESD216B) 4-byte Address Instruction Table. This
table is optional but when available, we parse it to get the 4-byte
address op codes supported by the memory.
Using these op codes is stateless as opposed to entering the 4-byte
address mode or setting the Base Address Register (BAR).

Flashes that have the 4BAIT table declared can now support
SPINOR_OP_PP_1_1_4_4B and SPINOR_OP_PP_1_4_4_4B opcodes.

Tested on MX25L25673G.

Signed-off-by: Cyrille Pitchen 
[tudor.amba...@microchip.com:
- rework erase and page program logic,
- pass DMA-able buffer to spi_nor_read_sfdp(),
- introduce SPI_NOR_HAS_4BAIT
- various minor updates.]
Signed-off-by: Tudor Ambarus 
---
v2:
- introduce SPI_NOR_HAS_4BAIT
- use SNOR_F_4B_OPCODES

This patch depends on:
- "mtd: spi-nor: fix selection of uniform erase type in flexible conf"
  which was already applied as a fix
- "mtd: spi-nor: Add the SNOR_F_4B_OPCODES flag" which will be present
  in spi-nor/next

 drivers/mtd/spi-nor/spi-nor.c | 193 +-
 include/linux/mtd/spi-nor.h   |   1 +
 2 files changed, 193 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index a9508db58d7f..e77efc153736 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -121,6 +121,7 @@ struct sfdp_parameter_header {
 
 #define SFDP_BFPT_ID   0xff00  /* Basic Flash Parameter Table */
 #define SFDP_SECTOR_MAP_ID 0xff81  /* Sector Map Table */
+#define SFDP_4BAIT_ID  0xff84  /* 4-byte Address Instruction Table */
 
 #define SFDP_SIGNATURE 0x50444653U
 #define SFDP_JESD216_MAJOR 1
@@ -3179,6 +3180,191 @@ static int spi_nor_parse_smpt(struct spi_nor *nor,
return ret;
 }
 
+#define SFDP_4BAIT_DWORD_MAX   2
+
+struct sfdp_4bait {
+   /* The hardware capability. */
+   u32 hwcaps;
+
+   /*
+* The  bit in DWORD1 of the 4BAIT tells us whether
+* the associated 4-byte address op code is supported.
+*/
+   u32 supported_bit;
+};
+
+/**
+ * spi_nor_parse_4bait() - parse the 4-Byte Address Instruction Table
+ * @nor:   pointer to a 'struct spi_nor'.
+ * @param_header:  pointer to the 'struct sfdp_parameter_header' describing
+ * the 4-Byte Address Instruction Table length and version.
+ * @params:pointer to the 'struct spi_nor_flash_parameter' to be.
+ *
+ * Return: 0 on success, -errno otherwise.
+ */
+static int spi_nor_parse_4bait(struct spi_nor *nor,
+  const struct sfdp_parameter_header *param_header,
+  struct spi_nor_flash_parameter *params)
+{
+   static const struct sfdp_4bait reads[] = {
+   { SNOR_HWCAPS_READ, BIT(0) },
+   { SNOR_HWCAPS_READ_FAST,BIT(1) },
+   { SNOR_HWCAPS_READ_1_1_2,   BIT(2) },
+   { SNOR_HWCAPS_READ_1_2_2,   BIT(3) },
+   { SNOR_HWCAPS_READ_1_1_4,   BIT(4) },
+   { SNOR_HWCAPS_READ_1_4_4,   BIT(5) },
+   { SNOR_HWCAPS_READ_1_1_1_DTR,   BIT(13) },
+   { SNOR_HWCAPS_READ_1_2_2_DTR,   BIT(14) },
+   { SNOR_HWCAPS_READ_1_4_4_DTR,   BIT(15) },
+   };
+   static const struct sfdp_4bait programs[] = {
+   { SNOR_HWCAPS_PP,   BIT(6) },
+   { SNOR_HWCAPS_PP_1_1_4, BIT(7) },
+   { SNOR_HWCAPS_PP_1_4_4, BIT(8) },
+   };
+   static const struct sfdp_4bait erases[SNOR_ERASE_TYPE_MAX] = {
+   { 0u /* not used */,BIT(9) },
+   { 0u /* not used */,BIT(10) },
+   { 0u /* not used */,BIT(11) },
+   { 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_type *erase_type = map->erase_type;
+   u32 *dwords;
+   size_t len;
+   u32 addr, discard_hwcaps, read_hwcaps, pp_hwcaps, erase_mask;
+   int i, ret;
+
+   if (param_header->major != SFDP_JESD216_MAJOR ||
+   param_header->length < SFDP_4BAIT_DWORD_MAX)
+   return -EINVAL;
+
+   /* Read the 4-byte Address Instruction Table. */
+   len = sizeof(*dwords) * SFDP_4BAIT_DWORD_MAX;
+
+   /* Use a kmalloc'ed bounce buffer to guarantee it is DMA-able. */
+   dwords = kmalloc(len, GFP_KERNEL);
+   if (!dwords)
+   return -ENOMEM;
+
+   addr = SFDP_PARAM_HEADER_PTP(param_header);
+   ret = spi_nor_read_sfdp(nor, addr, len, dwords);
+   if (ret)
+   return ret;
+
+   /* Fix endianness of the 4BAIT DWORDs. */
+   for (i = 0; i < SFDP_4BAIT_DWORD_MAX; i++)
+   dwords[i] = le32_to_cpu(dwords[i]);
+
+   /*
+* Compute the su