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

PATCH In review Michel Stempin 3 participants 7 messages Apr 4, 2013 – Jul 1, 2013 Memory technology devices (MTD)

Share & tools

Michel Stempin

author

2013-04-04 09:41 UTC #0 lore raw

PATCH 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) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.chingistek.com/img/Product_Files/Pm25LQ032C%20datasheet%20v1.6.1.pdf) SPI Flash chips, used in various WiFi pocket routers.

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

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

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

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

index 5b6b072..5fd7500 100644
@@ -45,6 +45,7 @@
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
 #define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
 #define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
+#define	OPCODE_BE_4K_PMC	0xd7    /* Erase 4KiB block on PMC chips*/
 #define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
 #define	OPCODE_RDID		0x9f	/* Read JEDEC ID */

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

@@ -759,6 +761,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 -- pm25x "blocks" are 32K, sectors are 4K */
+	{ "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).
 	 */

@@ -1009,6 +1016,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;

Artem Bityutskiy

2013-05-10 14:16 UTC #1 lore raw

Quoted 4 lines · Michel Stempin

Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.chingistek.com/img/Product_Files/Pm25LQ032C%20datasheet%20v1.6.1.pdf) SPI Flash chips, used in various WiFi pocket routers.
Signed-off-by Gabor Juhos <[email protected]>
Signed-off-by Michel Stempin <[email protected]>

Would you please rebase against the latest kernel and re-send. Please, pay attention to this patch which is already in upstream:

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]>

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Michel Stempin

author

2013-05-10 21:06 UTC #2 lore raw

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

This patch is resent in order to take into account 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..4d45ce4 100644
@@ -45,6 +45,7 @@
 #define	OPCODE_BE_4K		0x20	/* Erase 4KiB block */
 #define	OPCODE_BE_32K		0x52	/* Erase 32KiB block */
 #define	OPCODE_CHIP_ERASE	0xc7	/* Erase whole flash chip */
+#define	OPCODE_BE_4K_PMC	0xd7	/* Erase 4KiB block on PMC chips*/
 #define	OPCODE_SE		0xd8	/* Sector erase (usually 64KiB) */
 #define	OPCODE_RDID		0x9f	/* Read JEDEC ID */

@@ -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 -- pm25x "blocks" are 32K, sectors are 4K */
+	{ "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

Artem Bityutskiy

2013-05-13 06:12 UTC #3 lore raw

Quoted 7 lines · Michel Stempin

    Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.

    This patch is resent in order to take into account this upstream patch:

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

I think it would make sense if Brien gook a glimpse on this patch.

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Artem Bityutskiy

2013-07-01 06:39 UTC #4 lore raw

Quoted 10 lines · Artem Bityutskiy

Quoted 7 lines · Michel Stempin

    Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.

    This patch is resent in order to take into account this upstream patch:

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

I think it would make sense if Brien gook a glimpse on this patch.

Oh My god, I meant "Brian (Norris) took a glimpse" here...

Signature · 2 lines

Best Regards,
Artem Bityutskiy

Brian Norris

2013-07-01 06:43 UTC #5 lore raw

Quoted 6 lines · Artem Bityutskiy

Quoted 5 lines · Artem Bityutskiy
Quoted 4 lines · Michel Stempin
Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.
This patch is resent in order to take into account this upstream patch:

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
     commit e534ee4f9ca29fdb38eea4b0c53f2154fbd8c1ee
     Author: Krzysztof Mazur <[email protected]>
     Date:   Fri Feb 22 15:51:05 2013 +0100
I think it would make sense if Brien gook a glimpse on this patch.
Oh My god, I meant "Brian (Norris) took a glimpse" here...

Hey, don't take it too hard :)

Thanks for the humorous reminder. I'll try to take a look at this one tomorrow.

Brian

Brian Norris

2013-07-01 17:26 UTC #6 lore raw

In reply to Michel Stempin

Hi Michael,

I gook a glimpse at this and have a few comments ;)

Quoted 1 line · Michel Stempin

    Add support for PMC (now Chingis, part of ISSI) Pm25LV512 (512 kBbit), Pm25LV010 (1 Mbit) (see datasheet http://www.geocities.jp/scottle556/pdf/Pm25LV512-010.pdf) and Pm25LQ032 (32 Mbit) (datasheet:http://www.

This line has no line breaks, and so it is hard to read. That may also explain why the sentence (and URL) is cut off. Please try to wrap it properly when you resend, so it is both complete and readable.

Are there any manufacturer links for old datasheets? That would be preferable to random Geocities links, but it looks like the manufacturer website only has new stuff.

A few other general notes:

It's worth documenting the quirks of these flash that you are supporting, probably just in the patch description. As I read it, you are supporting two different generations of PMC flash here:

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.

Pm25LQ032: a newer generation flash, with 4KB sectors and 32KB blocks. It uses the standard erase and JEDEC read-ID opcodes.
...

drivers/mtd/devices/m25p80.c line 48

Quoted 13 lines

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

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

index 2f3d2a5..4d45ce4 100644
@@ -45,6 +45,7 @@
 #define        OPCODE_BE_4K            0x20    /* Erase 4KiB block */
 #define        OPCODE_BE_32K           0x52    /* Erase 32KiB block */
 #define        OPCODE_CHIP_ERASE       0xc7    /* Erase whole flash chip */
+#define        OPCODE_BE_4K_PMC        0xd7    /* Erase 4KiB block on PMC chips*/

Missing a space after chips. (i.e., should be "... PMC chips */")

While you're at it, you might as well run scripts/checkpatch.pl on your patch.

I would also group OPCODE_BE_4K_PMC along with OPCODE_BE_4K_PMC (that is, by function) rather than just sorting by opcode.

Quoted 12 lines

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

 #define        OPCODE_SE               0xd8    /* Sector erase (usually 64KiB) */
 #define        OPCODE_RDID             0x9f    /* Read JEDEC ID */

@@ -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)     \

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

Quoted 4 lines

drivers/mtd/devices/m25p80.c +1 -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 -- pm25x "blocks" are 32K, sectors are 4K */

Is this true for all PMC flash that you know of? If not, then I'd just let the table speak for itself. But it's fine as-is, if there are no known flash without a 4KB sector, for example.

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

Quoted 3 lines

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

+       { "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) },

Use lower-case 0x7f9d46, to match the style of the rest of the table.

Quoted 5 lines

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

+
        /* Spansion -- single (large) sector size only, at least
         * for the chips listed here (without boot sectors).
         */

drivers/mtd/devices/m25p80.c static int m25p_probe(struct spi_device *spi) · line 1029

Quoted 9 lines

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

@@ -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;

Other than the suggestions for style and documentation improvements, it looks good to me. I gook forward to acking your revised patch.

Brian