Skip to contentSkip to conversation

mtd: spi-nor: add id/id_len for flash_info{}

PATCH Applied Huang Shijie 4 participants 18 messages Aug 12, 2014 – Dec 1, 2014

Subsystems (2)
Memory technology devices (MTD) SPI NOR
lore
Share & tools
lore

Huang Shijie

author
#0 lore raw

PATCH 1/3 mtd: spi-nor: add id/id_len for flash_info{}

This patch adds the id/id_len fields for flash_info{}, and rewrite the INFO to fill them. And at last, we read out 6 bytes in the spi_nor_read_id(), and we use these new fields to parse out the correct flash_info.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +25 -14
index b5ad6be..0b69072 100644
@@ -422,6 +422,8 @@ err:
 	return ret;
 }
 
+#define SPI_NOR_MAX_ID_LEN	6
+
 struct flash_info {
 	/* JEDEC id zero means "no ID" (most older chips); otherwise it has
 	 * a high byte of zero plus three data bytes: the manufacturer id,

@@ -430,6 +432,14 @@ struct flash_info {
 	u32		jedec_id;
 	u16             ext_id;
 
+	/*
+	 * This array stores the ID bytes.
+	 * The first three bytes are the JEDIC id.
+	 * JEDEC id zero means "no ID" (most older chips);
+	 */
+	u8		id[SPI_NOR_MAX_ID_LEN];
+	u8		id_len;
+
 	/* The size listed here is what works with SPINOR_OP_SE, which isn't
 	 * necessarily called a "sector" by the vendor.
 	 */

@@ -450,10 +460,19 @@ struct flash_info {
 #define	USE_FSR			0x80	/* use flag status register */
 };
 
+/* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
 		.jedec_id = (_jedec_id),				\
 		.ext_id = (_ext_id),					\
+		.id = {							\
+			((_jedec_id) >> 16) & 0xff,			\
+			((_jedec_id) >> 8) & 0xff,			\
+			(_jedec_id) & 0xff,				\
+			((_ext_id) >> 8) & 0xff,			\
+			(_ext_id) & 0xff,				\
+			},						\
+		.id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),	\
 		.sector_size = (_sector_size),				\
 		.n_sectors = (_n_sectors),				\
 		.page_size = 256,					\

@@ -642,32 +661,24 @@ EXPORT_SYMBOL_GPL(spi_nor_ids);
 static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor)
 {
 	int			tmp;
-	u8			id[5];
-	u32			jedec;
-	u16                     ext_jedec;
+	u8			id[SPI_NOR_MAX_ID_LEN];
 	struct flash_info	*info;
 
-	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, 5);
+	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
 	if (tmp < 0) {
 		dev_dbg(nor->dev, " error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
 	}
-	jedec = id[0];
-	jedec = jedec << 8;
-	jedec |= id[1];
-	jedec = jedec << 8;
-	jedec |= id[2];
-
-	ext_jedec = id[3] << 8 | id[4];
 
 	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
 		info = (void *)spi_nor_ids[tmp].driver_data;
-		if (info->jedec_id == jedec) {
-			if (info->ext_id == 0 || info->ext_id == ext_jedec)
+		if (info->id_len) {
+			if (!strncmp(info->id, id, info->id_len))
 				return &spi_nor_ids[tmp];
 		}
 	}
-	dev_err(nor->dev, "unrecognized JEDEC id %06x\n", jedec);
+	dev_err(nor->dev, "unrecognized JEDEC id bytes: %02x, %2x, %2x\n",
+		id[0], id[1], id[2]);
 	return ERR_PTR(-ENODEV);
 }
Signature · 1 line
1.7.7.6
author
#1 lore raw

PATCH 2/3 mtd: spi-nor: remove the jedec_id/ext_id

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   98 +++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 53 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +45 -53
index 0b69072..a0923b0 100644
@@ -26,7 +26,38 @@
 /* Define max times to check status register before we give up. */
 #define	MAX_READY_WAIT_JIFFIES	(40 * HZ) /* M25P16 specs 40s max chip erase */
 
-#define JEDEC_MFR(_jedec_id)	((_jedec_id) >> 16)
+#define SPI_NOR_MAX_ID_LEN	6
+
+struct flash_info {
+	/*
+	 * This array stores the ID bytes.
+	 * The first three bytes are the JEDIC id.
+	 * JEDEC id zero means "no ID" (most older chips);
+	 */
+	u8		id[SPI_NOR_MAX_ID_LEN];
+	u8		id_len;
+
+	/* The size listed here is what works with SPINOR_OP_SE, which isn't
+	 * necessarily called a "sector" by the vendor.
+	 */
+	unsigned	sector_size;
+	u16		n_sectors;
+
+	u16		page_size;
+	u16		addr_width;
+
+	u16		flags;
+#define	SECT_4K			0x01	/* SPINOR_OP_BE_4K works uniformly */
+#define	SPI_NOR_NO_ERASE	0x02	/* No erase command needed */
+#define	SST_WRITE		0x04	/* use SST byte programming */
+#define	SPI_NOR_NO_FR		0x08	/* Can't do fastread */
+#define	SECT_4K_PMC		0x10	/* SPINOR_OP_BE_4K_PMC works uniformly */
+#define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
+#define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
+#define	USE_FSR			0x80	/* use flag status register */
+};
+
+#define JEDEC_MFR(info)	((info)->id[0])
 
 /*
  * Read the status register, returning its value in the location

@@ -136,13 +167,14 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 }
 
 /* Enable/disable 4-byte addressing mode. */
-static inline int set_4byte(struct spi_nor *nor, u32 jedec_id, int enable)
+static inline int
+set_4byte(struct spi_nor *nor, struct flash_info *info, int enable)
 {
 	int status;
 	bool need_wren = false;
 	u8 cmd;
 
-	switch (JEDEC_MFR(jedec_id)) {
+	switch (JEDEC_MFR(info)) {
 	case CFI_MFR_ST: /* Micron, actually */
 		/* Some Micron need WREN command; all will accept it */
 		need_wren = true;

@@ -422,49 +454,9 @@ err:
 	return ret;
 }
 
-#define SPI_NOR_MAX_ID_LEN	6
-
-struct flash_info {
-	/* JEDEC id zero means "no ID" (most older chips); otherwise it has
-	 * a high byte of zero plus three data bytes: the manufacturer id,
-	 * then a two byte device id.
-	 */
-	u32		jedec_id;
-	u16             ext_id;
-
-	/*
-	 * This array stores the ID bytes.
-	 * The first three bytes are the JEDIC id.
-	 * JEDEC id zero means "no ID" (most older chips);
-	 */
-	u8		id[SPI_NOR_MAX_ID_LEN];
-	u8		id_len;
-
-	/* The size listed here is what works with SPINOR_OP_SE, which isn't
-	 * necessarily called a "sector" by the vendor.
-	 */
-	unsigned	sector_size;
-	u16		n_sectors;
-
-	u16		page_size;
-	u16		addr_width;
-
-	u16		flags;
-#define	SECT_4K			0x01	/* SPINOR_OP_BE_4K works uniformly */
-#define	SPI_NOR_NO_ERASE	0x02	/* No erase command needed */
-#define	SST_WRITE		0x04	/* use SST byte programming */
-#define	SPI_NOR_NO_FR		0x08	/* Can't do fastread */
-#define	SECT_4K_PMC		0x10	/* SPINOR_OP_BE_4K_PMC works uniformly */
-#define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
-#define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
-#define	USE_FSR			0x80	/* use flag status register */
-};
-
 /* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
-		.jedec_id = (_jedec_id),				\
-		.ext_id = (_ext_id),					\
 		.id = {							\
 			((_jedec_id) >> 16) & 0xff,			\
 			((_jedec_id) >> 8) & 0xff,			\

@@ -889,11 +881,11 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
+static int set_quad_mode(struct spi_nor *nor, struct flash_info *info)
 {
 	int status;
 
-	switch (JEDEC_MFR(jedec_id)) {
+	switch (JEDEC_MFR(info)) {
 	case CFI_MFR_MACRONIX:
 		status = macronix_quad_enable(nor);
 		if (status) {

@@ -966,7 +958,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 
 	info = (void *)id->driver_data;
 
-	if (info->jedec_id) {
+	if (info->id_len) {
 		const struct spi_device_id *jid;
 
 		jid = jedec_probe(nor);

@@ -994,9 +986,9 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	 * up with the software protection bits set
 	 */
 
-	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_ATMEL ||
-	    JEDEC_MFR(info->jedec_id) == CFI_MFR_INTEL ||
-	    JEDEC_MFR(info->jedec_id) == CFI_MFR_SST) {
+	if (JEDEC_MFR(info) == CFI_MFR_ATMEL ||
+	    JEDEC_MFR(info) == CFI_MFR_INTEL ||
+	    JEDEC_MFR(info) == CFI_MFR_SST) {
 		write_enable(nor);
 		write_sr(nor, 0);
 	}

@@ -1014,7 +1006,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	mtd->_read = spi_nor_read;
 
 	/* nor protection support for STmicro chips */
-	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_ST) {
+	if (JEDEC_MFR(info) == CFI_MFR_ST) {
 		mtd->_lock = spi_nor_lock;
 		mtd->_unlock = spi_nor_unlock;
 	}

@@ -1065,7 +1057,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 
 	/* Quad/Dual-read mode takes precedence over fast/normal */
 	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-		ret = set_quad_mode(nor, info->jedec_id);
+		ret = set_quad_mode(nor, info);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
 			return ret;

@@ -1101,7 +1093,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
+		if (JEDEC_MFR(info) == CFI_MFR_AMD) {
 			/* Dedicated 4-byte command set */
 			switch (nor->flash_read) {
 			case SPI_NOR_QUAD:

@@ -1122,7 +1114,7 @@ int spi_nor_scan(struct spi_nor *nor, const struct spi_device_id *id,
 			nor->erase_opcode = SPINOR_OP_SE_4B;
 			mtd->erasesize = info->sector_size;
 		} else
-			set_4byte(nor, info->jedec_id, 1);
+			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
 	}
Signature · 1 line
1.7.7.6
#2 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 5 lines · Huang Shijie

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

This patch does not apply any more. Can you rebase?

Brian

#3 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 8 lines · Brian Norris
Quoted 5 lines · Huang Shijie

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

This patch does not apply any more. Can you rebase?

Hm, I've just noticed that my patch:
"prefer more specific entries from chips database"
http://patchwork.ozlabs.org/patch/401907/
conflicts with this one.

I think the final solution (after somehow applying both patches) should be something like:

for (len = 6; i >= 4; len--) {
    for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
        if (tmp->id_len == len && !strncmp(info->id, id, info->id_len)
            return &spi_nor_ids[tmp];
    }
}

Does it make sense to you?

Huang: I think it'll be the most optimal to me rebase my patch on top on your ones.

Btw. I have two comments regarding 1/3 (can't find it to reply properly):

1) Do we need the if (info->id_len) check?
2) Shouldn't we use memcmp in the if (!strncmp(info->id, id, info->id_len))

Signature · 1 line
Rafał
author
#4 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 17 lines · Rafał Miłecki
Quoted 8 lines · Brian Norris
Quoted 5 lines · Huang Shijie

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

This patch does not apply any more. Can you rebase?

Hm, I've just noticed that my patch:
"prefer more specific entries from chips database"
http://patchwork.ozlabs.org/patch/401907/
conflicts with this one.

I think the final solution (after somehow applying both patches) should be something like:

Quoted 8 lines
for (len = 6; i >= 4; len--) {
    for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
        if (tmp->id_len == len && !strncmp(info->id, id, info->id_len)
            return &spi_nor_ids[tmp];
    }
}

Does it make sense to you?

sorry, I can not understand this code. could you please give me a full patch for your idea? thanks

Quoted 3 lines

Huang: I think it'll be the most optimal to me rebase my patch on top on your ones.

It is okay. I will CC to you with the new patch.

Quoted 4 lines

Btw. I have two comments regarding 1/3 (can't find it to reply properly):

1) Do we need the if (info->id_len) check?

yes. some chips may have short info->id_len, some chips may have long info->id_len.

Quoted 1 line

2) Shouldn't we use memcmp in the if (!strncmp(info->id, id, info->id_len))

I prefer to memcmp too. But i misused with strncmp. You can send a patch to fix it. I can give you my Ack. :)

thanks
Huang Shiji

#5 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 3 lines · Huang Shijie
Quoted 1 line

1) Do we need the if (info->id_len) check?

yes. some chips may have short info->id_len, some chips may have long info->id_len.

Right, thanks.

Quoted 3 lines
Quoted 1 line

2) Shouldn't we use memcmp in the if (!strncmp(info->id, id, info->id_len))

I prefer to memcmp too. But i misused with strncmp. You can send a patch to fix it. I can give you my Ack. :)

Well, you were sending V2 2 hours later anyway... you could fix that in it :|

Signature · 1 line
Rafał
author
#6 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 2 lines · Rafał Miłecki

Well, you were sending V2 2 hours later anyway... you could fix that in it :|

okay.
I will fix it, and send out a new version.

thanks
Huang Shijie

author
#7 lore raw

Re: [PATCH 2/3] mtd: spi-nor: remove the jedec_id/ext_id

In reply to Brian Norris
Quoted 8 lines · Brian Norris
Quoted 5 lines · Huang Shijie

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

This patch does not apply any more. Can you rebase?

ok, no preblem.

thanks
Huang Shijie

author
#8 lore raw

PATCH 1/1 mtd: spi-nor: remove the jedec_id/ext_id

In reply to Brian Norris

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

---
rebase this patch on the latest l2-mtd

---
 drivers/mtd/spi-nor/spi-nor.c |   98 +++++++++++++++++++----------------------
 1 files changed, 45 insertions(+), 53 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +45 -53
index 8d6b832..182dd42 100644
@@ -26,7 +26,38 @@
 /* Define max times to check status register before we give up. */
 #define	MAX_READY_WAIT_JIFFIES	(40 * HZ) /* M25P16 specs 40s max chip erase */
 
-#define JEDEC_MFR(_jedec_id)	((_jedec_id) >> 16)
+#define SPI_NOR_MAX_ID_LEN	6
+
+struct flash_info {
+	/*
+	 * This array stores the ID bytes.
+	 * The first three bytes are the JEDIC id.
+	 * JEDEC id zero means "no ID" (most older chips);
+	 */
+	u8		id[SPI_NOR_MAX_ID_LEN];
+	u8		id_len;
+
+	/* The size listed here is what works with SPINOR_OP_SE, which isn't
+	 * necessarily called a "sector" by the vendor.
+	 */
+	unsigned	sector_size;
+	u16		n_sectors;
+
+	u16		page_size;
+	u16		addr_width;
+
+	u16		flags;
+#define	SECT_4K			0x01	/* SPINOR_OP_BE_4K works uniformly */
+#define	SPI_NOR_NO_ERASE	0x02	/* No erase command needed */
+#define	SST_WRITE		0x04	/* use SST byte programming */
+#define	SPI_NOR_NO_FR		0x08	/* Can't do fastread */
+#define	SECT_4K_PMC		0x10	/* SPINOR_OP_BE_4K_PMC works uniformly */
+#define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
+#define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
+#define	USE_FSR			0x80	/* use flag status register */
+};
+
+#define JEDEC_MFR(info)	((info)->id[0])
 
 static const struct spi_device_id *spi_nor_match_id(const char *name);
 

@@ -138,13 +169,14 @@ static inline struct spi_nor *mtd_to_spi_nor(struct mtd_info *mtd)
 }
 
 /* Enable/disable 4-byte addressing mode. */
-static inline int set_4byte(struct spi_nor *nor, u32 jedec_id, int enable)
+static inline int
+set_4byte(struct spi_nor *nor, struct flash_info *info, int enable)
 {
 	int status;
 	bool need_wren = false;
 	u8 cmd;
 
-	switch (JEDEC_MFR(jedec_id)) {
+	switch (JEDEC_MFR(info)) {
 	case CFI_MFR_ST: /* Micron, actually */
 		/* Some Micron need WREN command; all will accept it */
 		need_wren = true;

@@ -413,49 +445,9 @@ err:
 	return ret;
 }
 
-#define SPI_NOR_MAX_ID_LEN	6
-
-struct flash_info {
-	/* JEDEC id zero means "no ID" (most older chips); otherwise it has
-	 * a high byte of zero plus three data bytes: the manufacturer id,
-	 * then a two byte device id.
-	 */
-	u32		jedec_id;
-	u16             ext_id;
-
-	/*
-	 * This array stores the ID bytes.
-	 * The first three bytes are the JEDIC id.
-	 * JEDEC id zero means "no ID" (most older chips);
-	 */
-	u8		id[SPI_NOR_MAX_ID_LEN];
-	u8		id_len;
-
-	/* The size listed here is what works with SPINOR_OP_SE, which isn't
-	 * necessarily called a "sector" by the vendor.
-	 */
-	unsigned	sector_size;
-	u16		n_sectors;
-
-	u16		page_size;
-	u16		addr_width;
-
-	u16		flags;
-#define	SECT_4K			0x01	/* SPINOR_OP_BE_4K works uniformly */
-#define	SPI_NOR_NO_ERASE	0x02	/* No erase command needed */
-#define	SST_WRITE		0x04	/* use SST byte programming */
-#define	SPI_NOR_NO_FR		0x08	/* Can't do fastread */
-#define	SECT_4K_PMC		0x10	/* SPINOR_OP_BE_4K_PMC works uniformly */
-#define	SPI_NOR_DUAL_READ	0x20    /* Flash supports Dual Read */
-#define	SPI_NOR_QUAD_READ	0x40    /* Flash supports Quad Read */
-#define	USE_FSR			0x80	/* use flag status register */
-};
-
 /* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
-		.jedec_id = (_jedec_id),				\
-		.ext_id = (_ext_id),					\
 		.id = {							\
 			((_jedec_id) >> 16) & 0xff,			\
 			((_jedec_id) >> 8) & 0xff,			\

@@ -872,11 +864,11 @@ static int spansion_quad_enable(struct spi_nor *nor)
 	return 0;
 }
 
-static int set_quad_mode(struct spi_nor *nor, u32 jedec_id)
+static int set_quad_mode(struct spi_nor *nor, struct flash_info *info)
 {
 	int status;
 
-	switch (JEDEC_MFR(jedec_id)) {
+	switch (JEDEC_MFR(info)) {
 	case CFI_MFR_MACRONIX:
 		status = macronix_quad_enable(nor);
 		if (status) {

@@ -925,7 +917,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	info = (void *)id->driver_data;
 
-	if (info->jedec_id) {
+	if (info->id_len) {
 		const struct spi_device_id *jid;
 
 		jid = spi_nor_read_id(nor);

@@ -953,9 +945,9 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	 * up with the software protection bits set
 	 */
 
-	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_ATMEL ||
-	    JEDEC_MFR(info->jedec_id) == CFI_MFR_INTEL ||
-	    JEDEC_MFR(info->jedec_id) == CFI_MFR_SST) {
+	if (JEDEC_MFR(info) == CFI_MFR_ATMEL ||
+	    JEDEC_MFR(info) == CFI_MFR_INTEL ||
+	    JEDEC_MFR(info) == CFI_MFR_SST) {
 		write_enable(nor);
 		write_sr(nor, 0);
 	}

@@ -970,7 +962,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	mtd->_read = spi_nor_read;
 
 	/* nor protection support for STmicro chips */
-	if (JEDEC_MFR(info->jedec_id) == CFI_MFR_ST) {
+	if (JEDEC_MFR(info) == CFI_MFR_ST) {
 		mtd->_lock = spi_nor_lock;
 		mtd->_unlock = spi_nor_unlock;
 	}

@@ -1023,7 +1015,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 
 	/* Quad/Dual-read mode takes precedence over fast/normal */
 	if (mode == SPI_NOR_QUAD && info->flags & SPI_NOR_QUAD_READ) {
-		ret = set_quad_mode(nor, info->jedec_id);
+		ret = set_quad_mode(nor, info);
 		if (ret) {
 			dev_err(dev, "quad mode not supported\n");
 			return ret;

@@ -1059,7 +1051,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 	else if (mtd->size > 0x1000000) {
 		/* enable 4-byte addressing if the device exceeds 16MiB */
 		nor->addr_width = 4;
-		if (JEDEC_MFR(info->jedec_id) == CFI_MFR_AMD) {
+		if (JEDEC_MFR(info) == CFI_MFR_AMD) {
 			/* Dedicated 4-byte command set */
 			switch (nor->flash_read) {
 			case SPI_NOR_QUAD:

@@ -1080,7 +1072,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name, enum read_mode mode)
 			nor->erase_opcode = SPINOR_OP_SE_4B;
 			mtd->erasesize = info->sector_size;
 		} else
-			set_4byte(nor, info->jedec_id, 1);
+			set_4byte(nor, info, 1);
 	} else {
 		nor->addr_width = 3;
 	}
Signature · 1 line
1.7.7.6
Applied
#9 lore raw

Re: [PATCH] mtd: spi-nor: remove the jedec_id/ext_id

Quoted 7 lines · Huang Shijie

The "id" array contains all the information about the JEDEC and the manufacturer ID info, this patch remove the jedec_id/ext_id from the flash_info.

Signed-off-by Huang Shijie <[email protected]>

---
rebase this patch on the latest l2-mtd

Applied to l2-mtd.git. Thanks!

Brian

author
#10 lore raw

PATCH 3/3 mtd: spi-nor: add support for s25fl128s

In reply to Huang Shijie

We need to store the six bytes ID for s25fl128s, since it shares the same five bytes with s25fl129p1.

This patch adds a macro INFO6 which is used for the six bytes ID flash, and adds a new item for the s25fl128s.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +18 -0
index a0923b0..c130bf7 100644
@@ -471,6 +471,23 @@ err:
 		.flags = (_flags),					\
 	})
 
+#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
+	((kernel_ulong_t)&(struct flash_info) {				\
+		.id = {							\
+			((_jedec_id) >> 16) & 0xff,			\
+			((_jedec_id) >> 8) & 0xff,			\
+			(_jedec_id) & 0xff,				\
+			((_ext_id) >> 16) & 0xff,			\
+			((_ext_id) >> 8) & 0xff,			\
+			(_ext_id) & 0xff,				\
+			},						\
+		.id_len = 6,						\
+		.sector_size = (_sector_size),				\
+		.n_sectors = (_n_sectors),				\
+		.page_size = 256,					\
+		.flags = (_flags),					\
+	})
+
 #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width, _flags)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
 		.sector_size = (_sector_size),				\

@@ -565,6 +582,7 @@ const struct spi_device_id spi_nor_ids[] = {
 	{ "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
 	{ "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
 	{ "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
+	{ "s25fl128s",	INFO6(0x012018, 0x4d0180, 64 * 1024, 256, SPI_NOR_QUAD_READ) },
 	{ "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64, 0) },
 	{ "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256, 0) },
 	{ "s25sl004a",  INFO(0x010212,      0,  64 * 1024,   8, 0) },
Signature · 1 line
1.7.7.6
#11 lore raw

Re: [PATCH 3/3] mtd: spi-nor: add support for s25fl128s

Hi Brian,

Quoted 40 lines · Huang Shijie

We need to store the six bytes ID for s25fl128s, since it shares the same five bytes with s25fl129p1.

This patch adds a macro INFO6 which is used for the six bytes ID flash, and adds a new item for the s25fl128s.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +17 -0
index a0923b0..c130bf7 100644
@@ -471,6 +471,23 @@ err:
                .flags = (_flags),                                      \
        })
 
+#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)    \
+       ((kernel_ulong_t)&(struct flash_info) {                         \
+               .id = {                                                 \
+                       ((_jedec_id) >> 16) & 0xff,                     \
+                       ((_jedec_id) >> 8) & 0xff,                      \
+                       (_jedec_id) & 0xff,                             \
+                       ((_ext_id) >> 16) & 0xff,                       \
+                       ((_ext_id) >> 8) & 0xff,                        \
+                       (_ext_id) & 0xff,                               \
+                       },                                              \
+               .id_len = 6,                                            \
+               .sector_size = (_sector_size),                          \
+               .n_sectors = (_n_sectors),                              \
+               .page_size = 256,                                       \
+               .flags = (_flags),                                      \
+       })
+
 #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width, _flags)  \
        ((kernel_ulong_t)&(struct flash_info) {                         \
                .sector_size = (_sector_size),                          \

drivers/mtd/spi-nor/spi-nor.c const struct spi_device_id spi_nor_ids[] = { · line 588

Quoted 7 lines
drivers/mtd/spi-nor/spi-nor.c +1 -0
@@ -565,6 +582,7 @@ const struct spi_device_id spi_nor_ids[] = {
        { "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
        { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
        { "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
+       { "s25fl128s",  INFO6(0x012018, 0x4d0180, 64 * 1024, 256, SPI_NOR_QUAD_READ) },
        { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64, 0) },
        { "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256, 0) },
        { "s25sl004a",  INFO(0x010212,      0,  64 * 1024,   8, 0) },

We have a s25fl128s flash on imx6sx-sdb board and this patch would be helpful.

Do you see any issues with it?

Thanks,

Fabio Estevam

#12 lore raw

Re: [PATCH 3/3] mtd: spi-nor: add support for s25fl128s

Hi Huang/Brian,

Quoted 53 lines · Fabio Estevam

Hi Brian,

Quoted 40 lines · Huang Shijie

We need to store the six bytes ID for s25fl128s, since it shares the same five bytes with s25fl129p1.

This patch adds a macro INFO6 which is used for the six bytes ID flash, and adds a new item for the s25fl128s.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +17 -0
index a0923b0..c130bf7 100644
@@ -471,6 +471,23 @@ err:
                .flags = (_flags),                                      \
        })
 
+#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)    \
+       ((kernel_ulong_t)&(struct flash_info) {                         \
+               .id = {                                                 \
+                       ((_jedec_id) >> 16) & 0xff,                     \
+                       ((_jedec_id) >> 8) & 0xff,                      \
+                       (_jedec_id) & 0xff,                             \
+                       ((_ext_id) >> 16) & 0xff,                       \
+                       ((_ext_id) >> 8) & 0xff,                        \
+                       (_ext_id) & 0xff,                               \
+                       },                                              \
+               .id_len = 6,                                            \
+               .sector_size = (_sector_size),                          \
+               .n_sectors = (_n_sectors),                              \
+               .page_size = 256,                                       \
+               .flags = (_flags),                                      \
+       })
+
 #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width, _flags)  \
        ((kernel_ulong_t)&(struct flash_info) {                         \
                .sector_size = (_sector_size),                          \

drivers/mtd/spi-nor/spi-nor.c const struct spi_device_id spi_nor_ids[] = { · line 588

Quoted 7 lines
drivers/mtd/spi-nor/spi-nor.c +1 -0
@@ -565,6 +582,7 @@ const struct spi_device_id spi_nor_ids[] = {
        { "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
        { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
        { "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
+       { "s25fl128s",  INFO6(0x012018, 0x4d0180, 64 * 1024, 256, SPI_NOR_QUAD_READ) },
        { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64, 0) },
        { "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256, 0) },
        { "s25sl004a",  INFO(0x010212,      0,  64 * 1024,   8, 0) },

We have a s25fl128s flash on imx6sx-sdb board and this patch would be helpful.

Quoted 1 line

Do you see any issues with it?

Is there still issues preventing this patch to be merged?

I would like to make use of it on a mx6sx-sdb board.

Regards,

Fabio Estevam

#13 lore raw

Re: [PATCH 3/3] mtd: spi-nor: add support for s25fl128s

Quoted 56 lines · Fabio Estevam

Hi Huang/Brian,

Quoted 53 lines · Fabio Estevam

Hi Brian,

Quoted 40 lines · Huang Shijie

We need to store the six bytes ID for s25fl128s, since it shares the same five bytes with s25fl129p1.

This patch adds a macro INFO6 which is used for the six bytes ID flash, and adds a new item for the s25fl128s.

Signed-off-by Huang Shijie <[email protected]>
---
 drivers/mtd/spi-nor/spi-nor.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +17 -0
index a0923b0..c130bf7 100644
@@ -471,6 +471,23 @@ err:
                .flags = (_flags),                                      \
        })
 
+#define INFO6(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)    \
+       ((kernel_ulong_t)&(struct flash_info) {                         \
+               .id = {                                                 \
+                       ((_jedec_id) >> 16) & 0xff,                     \
+                       ((_jedec_id) >> 8) & 0xff,                      \
+                       (_jedec_id) & 0xff,                             \
+                       ((_ext_id) >> 16) & 0xff,                       \
+                       ((_ext_id) >> 8) & 0xff,                        \
+                       (_ext_id) & 0xff,                               \
+                       },                                              \
+               .id_len = 6,                                            \
+               .sector_size = (_sector_size),                          \
+               .n_sectors = (_n_sectors),                              \
+               .page_size = 256,                                       \
+               .flags = (_flags),                                      \
+       })
+
 #define CAT25_INFO(_sector_size, _n_sectors, _page_size, _addr_width, _flags)  \
        ((kernel_ulong_t)&(struct flash_info) {                         \
                .sector_size = (_sector_size),                          \

drivers/mtd/spi-nor/spi-nor.c const struct spi_device_id spi_nor_ids[] = { · line 588

Quoted 7 lines
drivers/mtd/spi-nor/spi-nor.c +1 -0
@@ -565,6 +582,7 @@ const struct spi_device_id spi_nor_ids[] = {
        { "s70fl01gs",  INFO(0x010221, 0x4d00, 256 * 1024, 256, 0) },
        { "s25sl12800", INFO(0x012018, 0x0300, 256 * 1024,  64, 0) },
        { "s25sl12801", INFO(0x012018, 0x0301,  64 * 1024, 256, 0) },
+       { "s25fl128s",  INFO6(0x012018, 0x4d0180, 64 * 1024, 256, SPI_NOR_QUAD_READ) },
        { "s25fl129p0", INFO(0x012018, 0x4d00, 256 * 1024,  64, 0) },
        { "s25fl129p1", INFO(0x012018, 0x4d01,  64 * 1024, 256, 0) },
        { "s25sl004a",  INFO(0x010212,      0,  64 * 1024,   8, 0) },

We have a s25fl128s flash on imx6sx-sdb board and this patch would be helpful.

Quoted 3 lines
Quoted 1 line

Do you see any issues with it?

Is there still issues preventing this patch to be merged?

Two things:

(1) My time (I'll get to it)

(2) The spi_nor_ids[] table is still not in great shape. We haven't fully agreed on the fallout of this change, and how people should be binding against the m25p80 driver in the future:

commit a5b7616c55e188fe3d6ef686bef402d4703ecb62
Author: Ben Hutchings <[email protected]>
Date: Tue Sep 30 03:14:55 2014 +0100

mtd: m25p80,spi-nor: Fix module aliases for m25p80

In the meantime, I'm not too keen on modifying this table. But I'll probably merge patches soon anyway.

Brian

Applied
#14 lore raw

Re: [PATCH 3/3] mtd: spi-nor: add support for s25fl128s

In reply to Huang Shijie
Quoted 7 lines · Huang Shijie

We need to store the six bytes ID for s25fl128s, since it shares the same five bytes with s25fl129p1.

This patch adds a macro INFO6 which is used for the six bytes ID flash, and adds a new item for the s25fl128s.

Signed-off-by Huang Shijie <[email protected]>

Applied to l2-mtd.git. Thanks.

Brian

#15 lore raw

PATCH v2 1/3 mtd: spi-nor: add id/id_len for flash_info{}

In reply to Huang Shijie

From: Huang Shijie <[email protected]>

This patch adds the id/id_len fields for flash_info{}, and rewrite the INFO to fill them. And at last, we read out 6 bytes in the spi_nor_read_id(), and we use these new fields to parse out the correct flash_info.

Signed-off-by Huang Shijie <[email protected]>
Signed-off-by Rafał Miłecki <[email protected]>

---
V2: s/strncmp/memcmp/ as we don't compare strings at all

---
 drivers/mtd/spi-nor/spi-nor.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +25 -14
index eafaeeb..b49efee 100644
@@ -413,6 +413,8 @@ err:
 	return ret;
 }
 
+#define SPI_NOR_MAX_ID_LEN	6
+
 struct flash_info {
 	/* JEDEC id zero means "no ID" (most older chips); otherwise it has
 	 * a high byte of zero plus three data bytes: the manufacturer id,

@@ -421,6 +423,14 @@ struct flash_info {
 	u32		jedec_id;
 	u16             ext_id;
 
+	/*
+	 * This array stores the ID bytes.
+	 * The first three bytes are the JEDIC id.
+	 * JEDEC id zero means "no ID" (most older chips);
+	 */
+	u8		id[SPI_NOR_MAX_ID_LEN];
+	u8		id_len;
+
 	/* The size listed here is what works with SPINOR_OP_SE, which isn't
 	 * necessarily called a "sector" by the vendor.
 	 */

@@ -441,10 +451,19 @@ struct flash_info {
 #define	USE_FSR			0x80	/* use flag status register */
 };
 
+/* Used when the "_ext_id" is two bytes at most */
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
 	((kernel_ulong_t)&(struct flash_info) {				\
 		.jedec_id = (_jedec_id),				\
 		.ext_id = (_ext_id),					\
+		.id = {							\
+			((_jedec_id) >> 16) & 0xff,			\
+			((_jedec_id) >> 8) & 0xff,			\
+			(_jedec_id) & 0xff,				\
+			((_ext_id) >> 8) & 0xff,			\
+			(_ext_id) & 0xff,				\
+			},						\
+		.id_len = (!(_jedec_id) ? 0 : (3 + ((_ext_id) ? 2 : 0))),	\
 		.sector_size = (_sector_size),				\
 		.n_sectors = (_n_sectors),				\
 		.page_size = 256,					\

@@ -636,32 +655,24 @@ static const struct spi_device_id spi_nor_ids[] = {
 static const struct spi_device_id *spi_nor_read_id(struct spi_nor *nor)
 {
 	int			tmp;
-	u8			id[5];
-	u32			jedec;
-	u16                     ext_jedec;
+	u8			id[SPI_NOR_MAX_ID_LEN];
 	struct flash_info	*info;
 
-	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, 5);
+	tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
 	if (tmp < 0) {
 		dev_dbg(nor->dev, " error %d reading JEDEC ID\n", tmp);
 		return ERR_PTR(tmp);
 	}
-	jedec = id[0];
-	jedec = jedec << 8;
-	jedec |= id[1];
-	jedec = jedec << 8;
-	jedec |= id[2];
-
-	ext_jedec = id[3] << 8 | id[4];
 
 	for (tmp = 0; tmp < ARRAY_SIZE(spi_nor_ids) - 1; tmp++) {
 		info = (void *)spi_nor_ids[tmp].driver_data;
-		if (info->jedec_id == jedec) {
-			if (info->ext_id == 0 || info->ext_id == ext_jedec)
+		if (info->id_len) {
+			if (!memcmp(info->id, id, info->id_len))
 				return &spi_nor_ids[tmp];
 		}
 	}
-	dev_err(nor->dev, "unrecognized JEDEC id %06x\n", jedec);
+	dev_err(nor->dev, "unrecognized JEDEC id bytes: %02x, %2x, %2x\n",
+		id[0], id[1], id[2]);
 	return ERR_PTR(-ENODEV);
 }
Signature · 1 line
1.8.4.5
author
#16 lore raw
Quoted 8 lines · Rafał Miłecki

From: Huang Shijie <[email protected]>

This patch adds the id/id_len fields for flash_info{}, and rewrite the INFO to fill them. And at last, we read out 6 bytes in the spi_nor_read_id(), and we use these new fields to parse out the correct flash_info.

Signed-off-by Huang Shijie <[email protected]>
Signed-off-by Rafał Miłecki <[email protected]>

thanks a lot.

I was just about to send out the new patch, then i saw this patch. :)

thanks
Huang Shijie

Applied
#17 lore raw
In reply to Rafał Miłecki
Quoted 10 lines · Rafał Miłecki

From: Huang Shijie <[email protected]>

This patch adds the id/id_len fields for flash_info{}, and rewrite the INFO to fill them. And at last, we read out 6 bytes in the spi_nor_read_id(), and we use these new fields to parse out the correct flash_info.

Signed-off-by Huang Shijie <[email protected]>
Signed-off-by Rafał Miłecki <[email protected]>

---
V2: s/strncmp/memcmp/ as we don't compare strings at all

Applied to l2-mtd.git. Thanks!

Brian