| Message ID | 518D6138.2090009@wanadoo.fr |
|---|---|
| State | New, archived |
| Headers | show |
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.
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...
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
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
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;
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(+)