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

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

[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 <juhosg@openwrt.org>
Signed-off-by: Michel Stempin <michel.stempin@wanadoo.fr>
--
 b/drivers/mtd/devices/m25p80.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 5b6b072..5fd7500 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -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;

Re: [PATCH] mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

[Artem Bityutskiy]

On Thu, 2013-04-04 at 11:40 +0200, Michel Stempin wrote:
> 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 <juhosg@openwrt.org>
> Signed-off-by: Michel Stempin <michel.stempin@wanadoo.fr>

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 <krzysiek@podlesie.net>
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 <krzysiek@podlesie.net>
    Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

-- 
Best Regards,
Artem Bityutskiy

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

[Michel Stempin]

[-- Attachment #1: Type: text/plain, Size: 3053 bytes --]

    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 <krzysiek@podlesie.net>
    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 <krzysiek@podlesie.net>
        Signed-off-by: Artem Bityutskiy <artem.bityutskiy@linux.intel.com>

    Signed-off-by: Gabor Juhos <juhosg@openwrt.org>
    Signed-off-by: Michel Stempin <michel.stempin@wanadoo.fr>
---
 drivers/mtd/devices/m25p80.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
index 2f3d2a5..4d45ce4 100644
--- a/drivers/mtd/devices/m25p80.c
+++ b/drivers/mtd/devices/m25p80.c
@@ -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;
-- 
1.7.10.4


[-- Attachment #2: Portion de message joint --]
[-- Type: text/plain, Size: 0 bytes --]

Re: [PATCH] mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

[Artem Bityutskiy]

On Fri, 2013-05-10 at 23:06 +0200, Michel Stempin wrote:
>     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 <krzysiek@podlesie.net>
>     Date:   Fri Feb 22 15:51:05 2013 +0100

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

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

[Artem Bityutskiy]

On Mon, 2013-05-13 at 09:14 +0300, Artem Bityutskiy wrote:
> On Fri, 2013-05-10 at 23:06 +0200, Michel Stempin wrote:
> >     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 <krzysiek@podlesie.net>
> >     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...

-- 
Best Regards,
Artem Bityutskiy

Re: [PATCH] mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

[Brian Norris]

On 06/30/2013 11:42 PM, Artem Bityutskiy wrote:
> On Mon, 2013-05-13 at 09:14 +0300, Artem Bityutskiy wrote:
>> On Fri, 2013-05-10 at 23:06 +0200, Michel Stempin wrote:
>>>      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 <krzysiek@podlesie.net>
>>>      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

Re: [PATCH] mtd: chips: Add support for PMC SPI Flash chips in m25p80.c

[Brian Norris]

Hi Michael,

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

On Fri, May 10, 2013 at 2:06 PM, Michel Stempin
<michel.stempin@wanadoo.fr> wrote:
>     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 |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 2f3d2a5..4d45ce4 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -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.

>  #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 */

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.

> +       { "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.

> +
>         /* 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;

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

Brian