mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

PATCH RESEND In review Michel Stempin 3 participants 8 messages Jul 15, 2013 – Aug 21, 2013 Memory technology devices (MTD)

lore

Share & tools

lore

Michel Stempin

author

2013-07-15 10:14 UTC #0 lore raw

PATCH RESEND 1/1 mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) and Pm25LQ032 (32 Mbit) SPI Flash chips.

This patch addresses two generations of PMC SPI Flash chips:

- Pm25LV512 and Pm25LV010: these have 4KB sectors and 32KB blocks. The 4KB sector erase uses a non-standard opcode (0xd7). They do not support JEDEC RDID (0x9f), and so they can only be detected by matching their name string with pre-configured platform data. Because of the cascaded acquisitions, the datasheet is no longer available on the current manufacturer's website, although it is still commonly used in some recent wireless routers
(<https://forum.openwrt.org/viewtopic.php?pid=186360#p186360>). The only public datasheet available seems to be on GeoCities:
<http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf>

- Pm25LQ032: a newer generation flash, with 4KB sectors and 32KB blocks. It uses the standard erase and JEDEC read-ID opcodes. Manufacturer's datasheet is here:
<http://www.chingistek.com/img/Product_Files/Pm25LQ032C%20datasheet%20v1.6.1.pdf>

This patch is resent in order to take into account both Brian Norris remarks and this upstream patch:

commit e534ee4f9ca29fdb38eea4b0c53f2154fbd8c1ee

Author: Krzysztof Mazur <[email protected]>
Date:   Fri Feb 22 15:51:05 2013 +0100

mtd: m25p80: introduce SST_WRITE flag for SST byte programming

Not all SST devices implement the SST byte programming command. Some devices (like SST25VF064C) implement only standard m25p80 page write command.

Now SPI flash devices that need sst_write() are explicitly marked with new SST_WRITE flag and the decision to use sst_write() is based on this flag instead of manufacturer id.

Signed-off-by Krzysztof Mazur <[email protected]>

Signed-off-by Artem Bityutskiy <[email protected]>

Signed-off-by Gabor Juhos <[email protected]>

Signed-off-by Michel Stempin <[email protected]>

---
 drivers/mtd/devices/m25p80.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/mtd/devices/m25p80.c +10 -0

index 2f3d2a5..55c8b5f 100644
@@ -43,6 +43,7 @@
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
 #define	OPCODE_PP		0x02	/* Page program (up to 256 bytes) */
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
+#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
 #define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
 #define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */

@@ -682,6 +683,7 @@ struct flash_info {
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */
 #define	M25P_NO_ERASE	0x02		/* No erase command needed */
 #define	SST_WRITE	0x04		/* use SST byte programming */
+#define	SECT_4K_PMC	0x08		/* OPCODE_BE_4K_PMC works uniformly */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\

@@ -762,6 +764,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024, 256, 0) },
 	{ "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) },
 
+	/* PMC */
+	{ "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
+	{ "pm25lv010", INFO(0, 0, 32 * 1024, 4, SECT_4K_PMC) },
+	{ "pm25lq032", INFO(0x7f9d46, 0, 64 * 1024,  64, SECT_4K) },
+
 	/* Spansion -- single (large) sector size only, at least
 	 * for the chips listed here (without boot sectors).
 	 */

@@ -1014,6 +1021,9 @@ static int m25p_probe(struct spi_device *spi)
 	if (info->flags & SECT_4K) {
 		flash->erase_opcode = OPCODE_BE_4K;
 		flash->mtd.erasesize = 4096;
+	} else if (info->flags & SECT_4K_PMC) {
+		flash->erase_opcode = OPCODE_BE_4K_PMC;
+		flash->mtd.erasesize = 4096;
 	} else {
 		flash->erase_opcode = OPCODE_SE;
 		flash->mtd.erasesize = info->sector_size;

Signature · 1 line

1.7.10.4

Brian Norris

2013-08-21 07:28 UTC #1 lore raw

+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.

But in the meantime, I fixed a conflict with stuff that's been applied since you submitted this patch, and applied to l2-mtd.git. Thanks!

Brian

Quoted 89 lines · Michel Stempin

Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) and Pm25LQ032 (32 Mbit) SPI Flash chips.
This patch addresses two generations of PMC SPI Flash chips:
- Pm25LV512 and Pm25LV010: these have 4KB sectors and 32KB blocks. The 4KB sector erase uses a non-standard opcode (0xd7). They do not support JEDEC RDID (0x9f), and so they can only be detected by matching their name string with pre-configured platform data. Because of the cascaded acquisitions, the datasheet is no longer available on the current manufacturer's website, although it is still commonly used in some recent wireless routers
(<https://forum.openwrt.org/viewtopic.php?pid=186360#p186360>). The only public datasheet available seems to be on GeoCities:
   <http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf>
- Pm25LQ032: a newer generation flash, with 4KB sectors and 32KB blocks. It uses the standard erase and JEDEC read-ID opcodes. Manufacturer's datasheet is here:
   <http://www.chingistek.com/img/Product_Files/Pm25LQ032C%20datasheet%20v1.6.1.pdf>
This patch is resent in order to take into account both Brian Norris remarks and this upstream patch:
commit e534ee4f9ca29fdb38eea4b0c53f2154fbd8c1ee
Author: Krzysztof Mazur <[email protected]>
Date:   Fri Feb 22 15:51:05 2013 +0100
mtd: m25p80: introduce SST_WRITE flag for SST byte programming
Not all SST devices implement the SST byte programming command. Some devices (like SST25VF064C) implement only standard m25p80 page write command.
Now SPI flash devices that need sst_write() are explicitly marked with new SST_WRITE flag and the decision to use sst_write() is based on this flag instead of manufacturer id.
Signed-off-by Krzysztof Mazur <[email protected]>
Signed-off-by Artem Bityutskiy <[email protected]>
Signed-off-by Gabor Juhos <[email protected]>
Signed-off-by Michel Stempin <[email protected]>
---
 drivers/mtd/devices/m25p80.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/mtd/devices/m25p80.c +10 -0
index 2f3d2a5..55c8b5f 100644
@@ -43,6 +43,7 @@
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high frequency) */
 #define	OPCODE_PP		0x02	/* Page program (up to 256 bytes) */
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
+#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips */
 #define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
 #define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
 #define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
@@ -682,6 +683,7 @@ struct flash_info {
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly */
 #define	M25P_NO_ERASE	0x02		/* No erase command needed */
 #define	SST_WRITE	0x04		/* use SST byte programming */
+#define	SECT_4K_PMC	0x08		/* OPCODE_BE_4K_PMC works uniformly */
 };
 
 #define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags)	\
@@ -762,6 +764,11 @@ static const struct spi_device_id m25p_ids[] = {
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024, 256, 0) },
 	{ "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) },
 
+	/* PMC */
+	{ "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
+	{ "pm25lv010", INFO(0, 0, 32 * 1024, 4, SECT_4K_PMC) },
+	{ "pm25lq032", INFO(0x7f9d46, 0, 64 * 1024,  64, SECT_4K) },
+
 	/* Spansion -- single (large) sector size only, at least
 	 * for the chips listed here (without boot sectors).
 	 */
@@ -1014,6 +1021,9 @@ static int m25p_probe(struct spi_device *spi)
 	if (info->flags & SECT_4K) {
 		flash->erase_opcode = OPCODE_BE_4K;
 		flash->mtd.erasesize = 4096;
+	} else if (info->flags & SECT_4K_PMC) {
+		flash->erase_opcode = OPCODE_BE_4K_PMC;
+		flash->mtd.erasesize = 4096;
 	} else {
 		flash->erase_opcode = OPCODE_SE;
 		flash->mtd.erasesize = info->sector_size;

Marek Vasut

2013-08-21 07:42 UTC #2 lore raw

Dear Brian Norris,

Quoted 3 lines

+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.

Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can just read the chip jedec ID and determine if it's a PMC part according to that. Then if it is PMC AND the SECT_4K flag is set, there is no need to add another flag at all, no?

Quoted 56 lines

But in the meantime, I fixed a conflict with stuff that's been applied since you submitted this patch, and applied to l2-mtd.git. Thanks!
Brian
Quoted 50 lines · Michel Stempin
Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) and Pm25LQ032 (32 Mbit) SPI Flash chips.
This patch addresses two generations of PMC SPI Flash chips:
- Pm25LV512 and Pm25LV010: these have 4KB sectors and 32KB
blocks. The 4KB sector erase uses a non-standard opcode (0xd7). They do not support JEDEC RDID (0x9f), and so they can only be detected by matching their name string with pre-configured platform data. Because of the cascaded acquisitions, the datasheet is no longer available on the current manufacturer's website, although it is still commonly used in some recent wireless routers
(<https://forum.openwrt.org/viewtopic.php?pid=186360#p186360>). The only public datasheet available seems to be on GeoCities:
   <http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf>
- Pm25LQ032: a newer generation flash, with 4KB sectors and 32KB
blocks. It uses the standard erase and JEDEC read-ID opcodes. Manufacturer's datasheet is here:
   <http://www.chingistek.com/img/Product_Files/Pm25LQ032C%20datasheet%20
   v1.6.1.pdf>
This patch is resent in order to take into account both Brian Norris remarks and this upstream patch:
commit e534ee4f9ca29fdb38eea4b0c53f2154fbd8c1ee
Author: Krzysztof Mazur <[email protected]>
Date:   Fri Feb 22 15:51:05 2013 +0100
mtd: m25p80: introduce SST_WRITE flag for SST byte programming
Not all SST devices implement the SST byte programming command. Some devices (like SST25VF064C) implement only standard m25p80 page write command.
Now SPI flash devices that need sst_write() are explicitly marked with new SST_WRITE flag and the decision to use sst_write() is based on this flag instead of manufacturer id.
Signed-off-by Krzysztof Mazur <[email protected]>
Signed-off-by Artem Bityutskiy <[email protected]>
Signed-off-by Gabor Juhos <[email protected]>
Signed-off-by Michel Stempin <[email protected]>
---
 drivers/mtd/devices/m25p80.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Quoted 7 lines

drivers/mtd/devices/m25p80.c line 44
Quoted 7 lines
drivers/mtd/devices/m25p80.c +0 -0
index 2f3d2a5..55c8b5f 100644
@@ -43,6 +43,7 @@
 
 #define	OPCODE_FAST_READ	0x0b	/* Read data bytes (high 

frequency) */

Quoted 1 line

drivers/mtd/devices/m25p80.c line 45
Quoted 1 line
drivers/mtd/devices/m25p80.c +0 -0
 #define	OPCODE_PP		0x02	/* Page program (up to 256 

bytes) */

Quoted 3 lines

drivers/mtd/devices/m25p80.c line 48
Quoted 3 lines
drivers/mtd/devices/m25p80.c +1 -0
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
 
+#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips 

Quoted 4 lines

Quoted 4 lines
drivers/mtd/devices/m25p80.c +0 -0
 #define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
#define OPCODE_CHIP_ERASE 0xc7 /* Erase whole flash chip */
#define OPCODE_SE 0xd8 /* Sector erase (usually 64KiB)

Quoted 4 lines

drivers/mtd/devices/m25p80.c struct flash_info { · line 684
Quoted 4 lines
drivers/mtd/devices/m25p80.c +0 -0
@@ -682,6 +683,7 @@ struct flash_info {
 
 #define	SECT_4K		0x01		/* OPCODE_BE_4K works uniformly 

Quoted 4 lines

drivers/mtd/devices/m25p80.c struct flash_info {

Quoted 4 lines

drivers/mtd/devices/m25p80.c +1 -0

 #define	M25P_NO_ERASE	0x02		/* No erase command needed */
 #define	SST_WRITE	0x04		/* use SST byte programming */
 
+#define	SECT_4K_PMC	0x08		/* OPCODE_BE_4K_PMC works

uniformly */

Quoted 28 lines

drivers/mtd/devices/m25p80.c +0 -0

};

#define INFO(_jedec_id, _ext_id, _sector_size, _n_sectors, _flags) \

drivers/mtd/devices/m25p80.c +5 -0

@@ -762,6 +764,11 @@ static const struct spi_device_id m25p_ids[] = {
 
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024, 256, 0) },
 	{ "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) },
 
+	/* PMC */
+	{ "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
+	{ "pm25lv010", INFO(0, 0, 32 * 1024, 4, SECT_4K_PMC) },
+	{ "pm25lq032", INFO(0x7f9d46, 0, 64 * 1024,  64, SECT_4K) },
+
 
 	/* Spansion -- single (large) sector size only, at least

drivers/mtd/devices/m25p80.c +0 -0

 	 * for the chips listed here (without boot sectors).
 	 */

@@ -1014,6 +1021,9 @@ static int m25p_probe(struct spi_device *spi)
 
 	if (info->flags & SECT_4K) {
 
 		flash->erase_opcode = OPCODE_BE_4K;
 		flash->mtd.erasesize = 4096;

Quoted 8 lines

drivers/mtd/devices/m25p80.c static int m25p_probe(struct spi_device *spi) · line 1029
Quoted 8 lines
drivers/mtd/devices/m25p80.c +3 -0
+	} else if (info->flags & SECT_4K_PMC) {
+		flash->erase_opcode = OPCODE_BE_4K_PMC;
+		flash->mtd.erasesize = 4096;
drivers/mtd/devices/m25p80.c +0 -0
 	} else {
 
 		flash->erase_opcode = OPCODE_SE;
 		flash->mtd.erasesize = info->sector_size;

Best regards,
Marek Vasut

Brian Norris

2013-08-21 07:59 UTC #3 lore raw

Quoted 8 lines · Marek Vasut

Dear Brian Norris,
Quoted 3 lines
+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.
Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can

Separating manufacturer from SECT_4K sounds good, but it really doesn't buy us much. See my next comments.

Quoted 3 lines

just read the chip jedec ID and determine if it's a PMC part according to that. Then if it is PMC AND the SECT_4K flag is set, there is no need to add another flag at all, no?

IIUC, Michel's comment applies:

"They do not support JEDEC RDID (0x9f), and so they can only be detected by matching their name string with pre-configured platform data."

So we cannot use RDID to identify by manufacturer. In fact, this same point screws up any attempt at manufacturer-based property detection for non-JEDEC devices. I guess we just can't expect much from such devices.

So we would have to introduce two flags to the table: one to flag the manufacturer and one to flag the opcode. Not necessary, IMO.

Quoted 1 line

Quoted 1 line · Michel Stempin

[...]

Quoted 8 lines

drivers/mtd/devices/m25p80.c static const struct spi_device_id m25p_ids[] = { · line 770

Quoted 8 lines

drivers/mtd/devices/m25p80.c +3 -0

@@ -762,6 +764,11 @@ static const struct spi_device_id m25p_ids[] = {
 
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024, 256, 0) },
 	{ "n25q256a", INFO(0x20ba19, 0, 64 * 1024, 512, SECT_4K) },
 
+	/* PMC */
+	{ "pm25lv512", INFO(0, 0, 32 * 1024, 2, SECT_4K_PMC) },
+	{ "pm25lv010", INFO(0, 0, 32 * 1024, 4, SECT_4K_PMC) },

Note that only the non-JEDEC chips needed the old commands.

Quoted 8 lines

Quoted 8 lines
drivers/mtd/devices/m25p80.c static const struct spi_device_id m25p_ids[] = { · line 774
Quoted 8 lines
drivers/mtd/devices/m25p80.c +2 -0
+	{ "pm25lq032", INFO(0x7f9d46, 0, 64 * 1024,  64, SECT_4K) },
+
 
 	/* Spansion -- single (large) sector size only, at least
drivers/mtd/devices/m25p80.c +0 -0
 	 * for the chips listed here (without boot sectors).
 	 */

Brian

Marek Vasut

2013-08-21 08:07 UTC #4 lore raw

Dear Brian Norris,

Quoted 7 lines · Marek Vasut

Quoted 6 lines
Dear Brian Norris,
Quoted 3 lines
+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.

Quoted 6 lines

Quoted 3 lines
Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can
Separating manufacturer from SECT_4K sounds good, but it really doesn't buy us much. See my next comments.

I see, that's really bad news. Thanks for the explanation!

I guess there really is nothing much we can do about such parts. But then if we take device tree probe into consideration, we might actually want to match the part name to discern the PMS device. Or am I talking complete nonsense?

Best regards,
Marek Vasut

Brian Norris

2013-08-21 08:30 UTC #5 lore raw

Quoted 9 lines · Marek Vasut

Quoted 9 lines · Marek Vasut
Quoted 7 lines
Quoted 3 lines
+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.
Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can

Quoted 8 lines

Quoted 2 lines
Separating manufacturer from SECT_4K sounds good, but it really doesn't buy us much. See my next comments.
I see, that's really bad news. Thanks for the explanation!
I guess there really is nothing much we can do about such parts. But then if we take device tree probe into consideration, we might actually want to match the part name to discern the PMS device. Or am I talking complete nonsense?

I don't think the device tree probe really gives us anything different than the platform_device probe (a non-JEDEC device can be matched via device-tree "compatible" property or via platform_device "name" property, I think?). So in either case, are you suggesting a string comparison for "pm25" on the spi_device_id.name field? Seems a bit like nonsense :)

Additionally, this still doesn't solve the problem that the old PMC chips need the special opcode, but the newer one doesn't.

Brian

Marek Vasut

2013-08-21 13:11 UTC #6 lore raw

Hi Brian,

Quoted 20 lines · Marek Vasut

Quoted 12 lines
Quoted 11 lines · Marek Vasut
Quoted 7 lines
Quoted 3 lines
+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.
Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can
Separating manufacturer from SECT_4K sounds good, but it really doesn't buy us much. See my next comments.
Quoted 6 lines
I see, that's really bad news. Thanks for the explanation!
I guess there really is nothing much we can do about such parts. But then if we take device tree probe into consideration, we might actually want to match the part name to discern the PMS device. Or am I talking complete nonsense?

Quoted 6 lines

I don't think the device tree probe really gives us anything different than the platform_device probe (a non-JEDEC device can be matched via device-tree "compatible" property or via platform_device "name" property, I think?). So in either case, are you suggesting a string comparison for "pm25" on the spi_device_id.name field? Seems a bit like nonsense :)

Yeah, you're right.

Quoted 2 lines

Additionally, this still doesn't solve the problem that the old PMC chips need the special opcode, but the newer one doesn't.

One would have to match the full part name, but that's already happening. Please ignore me, I had my coffee only now.

Best regards,
Marek Vasut

Brian Norris

2013-08-21 19:48 UTC #7 lore raw

Quoted 27 lines · Marek Vasut

Quoted 20 lines · Marek Vasut
Quoted 12 lines
Quoted 11 lines · Marek Vasut
Quoted 7 lines
Quoted 3 lines
+ Marek, since he's been reviewing (with dismay?) the increase in macro flags in this driver. If there are any objections, I can amend/drop the patch.
Hmmm ... this SECT_4K_PMC seems too combined to me. Why don't we use the SECT_4K flag and another flag to indicate it's a PMC part? Even better, I recall you can
Separating manufacturer from SECT_4K sounds good, but it really doesn't buy us much. See my next comments.
Quoted 6 lines
I see, that's really bad news. Thanks for the explanation!
I guess there really is nothing much we can do about such parts. But then if we take device tree probe into consideration, we might actually want to match the part name to discern the PMS device. Or am I talking complete nonsense?
Quoted 6 lines
I don't think the device tree probe really gives us anything different than the platform_device probe (a non-JEDEC device can be matched via device-tree "compatible" property or via platform_device "name" property, I think?). So in either case, are you suggesting a string comparison for "pm25" on the spi_device_id.name field? Seems a bit like nonsense :)

Quoted 7 lines

Yeah, you're right.
Quoted 2 lines
Additionally, this still doesn't solve the problem that the old PMC chips need the special opcode, but the newer one doesn't.
One would have to match the full part name, but that's already happening. Please ignore me, I had my coffee only now.

No problem. Then I'm leaving the patch queued as-is. Thanks for taking a look.

Brian