* [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members
@ 2010-03-30 13:34 Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 2/7] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable Guillaume LECERF
` (7 more replies)
0 siblings, 8 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:34 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
members to add support for chips using alternative sequence / unlock
addresses.
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_probe.c | 46 +++++++++++++++++++++--------------------
1 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index e63e674..9b511ef 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -181,29 +181,6 @@ static int __xipram cfi_chip_setup(struct map_info *map,
for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
((unsigned char *)cfi->cfiq)[i] = cfi_read_query(map,base + (0x10 + i)*ofs_factor);
- /* Note we put the device back into Read Mode BEFORE going into Auto
- * Select Mode, as some devices support nesting of modes, others
- * don't. This way should always work.
- * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
- * so should be treated as nops or illegal (and so put the device
- * back into Read Mode, which is a nop in this case).
- */
- cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
- cfi->mfr = cfi_read_query16(map, base);
- cfi->id = cfi_read_query16(map, base + ofs_factor);
-
- /* Get AMD/Spansion extended JEDEC ID */
- if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
- cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
- cfi_read_query(map, base + 0xf * ofs_factor);
-
- /* Put it back into Read Mode */
- cfi_qry_mode_off(base, map, cfi);
- xip_allowed(base, map);
-
/* Do any necessary byteswapping */
cfi->cfiq->P_ID = le16_to_cpu(cfi->cfiq->P_ID);
@@ -228,6 +205,29 @@ static int __xipram cfi_chip_setup(struct map_info *map,
#endif
}
+ /* Note we put the device back into Read Mode BEFORE going into Auto
+ * Select Mode, as some devices support nesting of modes, others
+ * don't. This way should always work.
+ * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
+ * so should be treated as nops or illegal (and so put the device
+ * back into Read Mode, which is a nop in this case).
+ */
+ cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
+ cfi->mfr = cfi_read_query16(map, base);
+ cfi->id = cfi_read_query16(map, base + ofs_factor);
+
+ /* Get AMD/Spansion extended JEDEC ID */
+ if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
+ cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
+ cfi_read_query(map, base + 0xf * ofs_factor);
+
+ /* Put it back into Read Mode */
+ cfi_qry_mode_off(base, map, cfi);
+ xip_allowed(base, map);
+
printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
map->name, cfi->interleave, cfi->device_type*8, base,
map->bankwidth*8);
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 2/7] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
@ 2010-03-30 13:34 ` Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname Guillaume LECERF
` (6 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:34 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
Make the addresses used to enter Auto Select Mode variable to leave place
for handling chips using non-standard addresses.
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_probe.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index 9b511ef..a28659d 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -158,6 +158,7 @@ static int __xipram cfi_chip_setup(struct map_info *map,
__u32 base = 0;
int num_erase_regions = cfi_read_query(map, base + (0x10 + 28)*ofs_factor);
int i;
+ int addr_unlock1 = 0x555, addr_unlock2 = 0x2AA;
xip_enable(base, map, cfi);
#ifdef DEBUG_CFI
@@ -213,9 +214,9 @@ static int __xipram cfi_chip_setup(struct map_info *map,
* back into Read Mode, which is a nop in this case).
*/
cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
- cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0xaa, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x55, addr_unlock2, base, map, cfi, cfi->device_type, NULL);
+ cfi_send_gen_cmd(0x90, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
cfi->mfr = cfi_read_query16(map, base);
cfi->id = cfi_read_query16(map, base + ofs_factor);
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 2/7] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable Guillaume LECERF
@ 2010-03-30 13:34 ` Guillaume LECERF
2010-04-08 9:05 ` Wolfram Sang
2010-03-30 13:35 ` [PATCH v2 4/7] mtd: cfi_probe: use P_ID_* definitions instead of hardcoded values Guillaume LECERF
` (5 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:34 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
SST 39VF160x and 39VF320x chips use vendorname id 0x0701 and alternative
unlock addresses. Add support for them in cfi_probe.c.
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_probe.c | 9 +++++++++
include/linux/mtd/cfi.h | 1 +
2 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index a28659d..f931e94 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -214,6 +214,12 @@ static int __xipram cfi_chip_setup(struct map_info *map,
* back into Read Mode, which is a nop in this case).
*/
cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
+
+ if (cfi->cfiq->P_ID == P_ID_SST_OLD) {
+ addr_unlock1 = 0x5555;
+ addr_unlock2 = 0x2AAA;
+ }
+
cfi_send_gen_cmd(0xaa, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
cfi_send_gen_cmd(0x55, addr_unlock2, base, map, cfi, cfi->device_type, NULL);
cfi_send_gen_cmd(0x90, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
@@ -270,6 +276,9 @@ static char *vendorname(__u16 vendor)
case P_ID_SST_PAGE:
return "SST Page Write";
+ case P_ID_SST_OLD:
+ return "SST 39VF160x/39VF320x";
+
case P_ID_INTEL_PERFORMANCE:
return "Intel Performance Code";
diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
index cee05b1..5716fc7 100644
--- a/include/linux/mtd/cfi.h
+++ b/include/linux/mtd/cfi.h
@@ -253,6 +253,7 @@ struct cfi_bri_query {
#define P_ID_MITSUBISHI_STD 0x0100
#define P_ID_MITSUBISHI_EXT 0x0101
#define P_ID_SST_PAGE 0x0102
+#define P_ID_SST_OLD 0x0701
#define P_ID_INTEL_PERFORMANCE 0x0200
#define P_ID_INTEL_DATA 0x0210
#define P_ID_RESERVED 0xffff
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 4/7] mtd: cfi_probe: use P_ID_* definitions instead of hardcoded values
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 2/7] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname Guillaume LECERF
@ 2010-03-30 13:35 ` Guillaume LECERF
2010-03-30 13:35 ` [PATCH v2 5/7] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional Guillaume LECERF
` (4 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:35 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
Use P_ID_* definitions already in include/linux/mtd/cfi.h instead of the
hardcoded values. Make the code more readable.
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/gen_probe.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index e2dc964..991c457 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -242,17 +242,17 @@ static struct mtd_info *check_cmd_set(struct map_info *map, int primary)
/* We need these for the !CONFIG_MODULES case,
because symbol_get() doesn't work there */
#ifdef CONFIG_MTD_CFI_INTELEXT
- case 0x0001:
- case 0x0003:
- case 0x0200:
+ case P_ID_INTEL_EXT:
+ case P_ID_INTEL_STD:
+ case P_ID_INTEL_PERFORMANCE:
return cfi_cmdset_0001(map, primary);
#endif
#ifdef CONFIG_MTD_CFI_AMDSTD
- case 0x0002:
+ case P_ID_AMD_STD:
return cfi_cmdset_0002(map, primary);
#endif
#ifdef CONFIG_MTD_CFI_STAA
- case 0x0020:
+ case P_ID_ST_ADV:
return cfi_cmdset_0020(map, primary);
#endif
default:
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 5/7] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
` (2 preceding siblings ...)
2010-03-30 13:35 ` [PATCH v2 4/7] mtd: cfi_probe: use P_ID_* definitions instead of hardcoded values Guillaume LECERF
@ 2010-03-30 13:35 ` Guillaume LECERF
2010-03-30 13:35 ` [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips Guillaume LECERF
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:35 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
After looking at AMD's CFI specification [1], both of the extended query
tables are optional. Thus, it looks like relying that at least one of
those tables exist is a bug in cfi_cmdset_0002.
This patch inverts the logic and checks for unlock function pointers before
exiting on error. This approach leaves place to add a call to a fixup
function to try to handle chips compatible with the early AMD specification
from 1995 [2].
[1] http://www.amd.com/us-en/assets/content_type/DownloadableAssets/cfi_r20.pdf
[2] http://noel.feld.cvut.cz/hw/amd/20158a.pdf
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_cmdset_0002.c | 89 ++++++++++++++++++-----------------
1 files changed, 45 insertions(+), 44 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index ea2a7f6..de1b4ba 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -353,65 +353,66 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
if (cfi->cfi_mode==CFI_MODE_CFI){
unsigned char bootloc;
- /*
- * It's a real CFI chip, not one for which the probe
- * routine faked a CFI structure. So we read the feature
- * table from it.
- */
__u16 adr = primary?cfi->cfiq->P_ADR:cfi->cfiq->A_ADR;
struct cfi_pri_amdstd *extp;
extp = (struct cfi_pri_amdstd*)cfi_read_pri(map, adr, sizeof(*extp), "Amd/Fujitsu");
- if (!extp) {
- kfree(mtd);
- return NULL;
- }
-
- cfi_fixup_major_minor(cfi, extp);
-
- if (extp->MajorVersion != '1' ||
- (extp->MinorVersion < '0' || extp->MinorVersion > '4')) {
- printk(KERN_ERR " Unknown Amd/Fujitsu Extended Query "
- "version %c.%c.\n", extp->MajorVersion,
- extp->MinorVersion);
- kfree(extp);
- kfree(mtd);
- return NULL;
- }
+ if (extp) {
+ /*
+ * It's a real CFI chip, not one for which the probe
+ * routine faked a CFI structure.
+ */
+ cfi_fixup_major_minor(cfi, extp);
+
+ if (extp->MajorVersion != '1' ||
+ (extp->MinorVersion < '0' || extp->MinorVersion > '4')) {
+ printk(KERN_ERR " Unknown Amd/Fujitsu Extended Query "
+ "version %c.%c.\n", extp->MajorVersion,
+ extp->MinorVersion);
+ kfree(extp);
+ kfree(mtd);
+ return NULL;
+ }
- /* Install our own private info structure */
- cfi->cmdset_priv = extp;
+ /* Install our own private info structure */
+ cfi->cmdset_priv = extp;
- /* Apply cfi device specific fixups */
- cfi_fixup(mtd, cfi_fixup_table);
+ /* Apply cfi device specific fixups */
+ cfi_fixup(mtd, cfi_fixup_table);
#ifdef DEBUG_CFI_FEATURES
- /* Tell the user about it in lots of lovely detail */
- cfi_tell_features(extp);
+ /* Tell the user about it in lots of lovely detail */
+ cfi_tell_features(extp);
#endif
- bootloc = extp->TopBottom;
- if ((bootloc != 2) && (bootloc != 3)) {
- printk(KERN_WARNING "%s: CFI does not contain boot "
- "bank location. Assuming top.\n", map->name);
- bootloc = 2;
- }
+ bootloc = extp->TopBottom;
+ if ((bootloc != 2) && (bootloc != 3)) {
+ printk(KERN_WARNING "%s: CFI does not contain boot "
+ "bank location. Assuming top.\n", map->name);
+ bootloc = 2;
+ }
- if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) {
- printk(KERN_WARNING "%s: Swapping erase regions for broken CFI table.\n", map->name);
+ if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) {
+ printk(KERN_WARNING "%s: Swapping erase regions for broken CFI table.\n", map->name);
- for (i=0; i<cfi->cfiq->NumEraseRegions / 2; i++) {
- int j = (cfi->cfiq->NumEraseRegions-1)-i;
- __u32 swap;
+ for (i=0; i<cfi->cfiq->NumEraseRegions / 2; i++) {
+ int j = (cfi->cfiq->NumEraseRegions-1)-i;
+ __u32 swap;
- swap = cfi->cfiq->EraseRegionInfo[i];
- cfi->cfiq->EraseRegionInfo[i] = cfi->cfiq->EraseRegionInfo[j];
- cfi->cfiq->EraseRegionInfo[j] = swap;
+ swap = cfi->cfiq->EraseRegionInfo[i];
+ cfi->cfiq->EraseRegionInfo[i] = cfi->cfiq->EraseRegionInfo[j];
+ cfi->cfiq->EraseRegionInfo[j] = swap;
+ }
}
+ /* Set the default CFI lock/unlock addresses */
+ cfi->addr_unlock1 = 0x555;
+ cfi->addr_unlock2 = 0x2aa;
+ }
+
+ if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
+ kfree(mtd);
+ return NULL;
}
- /* Set the default CFI lock/unlock addresses */
- cfi->addr_unlock1 = 0x555;
- cfi->addr_unlock2 = 0x2aa;
} /* CFI mode */
else if (cfi->cfi_mode == CFI_MODE_JEDEC) {
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
` (3 preceding siblings ...)
2010-03-30 13:35 ` [PATCH v2 5/7] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional Guillaume LECERF
@ 2010-03-30 13:35 ` Guillaume LECERF
2010-04-08 9:12 ` Wolfram Sang
2010-04-08 15:21 ` Wolfram Sang
2010-03-30 13:35 ` [PATCH v2 7/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{32, 64}xxB chips Guillaume LECERF
` (2 subsequent siblings)
7 siblings, 2 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:35 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
SST 39VF{16,32}xx chips use the 0x0701 command set, fully compatible
with the AMD one. This patch adds support for detecting them in CFI
mode.
Based on a patch by Peter Turczaki [1].
[1] http://www.mail-archive.com/uclinux-dev@uclinux.org/msg05737.html
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_cmdset_0002.c | 41 +++++++++++++++++++++++++++++++++++
drivers/mtd/chips/gen_probe.c | 1 +
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index de1b4ba..e3e4a94 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -256,6 +256,36 @@ static void fixup_use_atmel_lock(struct mtd_info *mtd, void *param)
mtd->flags |= MTD_POWERUP_LOCK;
}
+/** While reporting a mostly-correct CFI-Information block
+ * the eraseblock-region information is severely damaged in SST
+ * parts at least those of the 39VF{16,32,64}xxB series.
+ **/
+static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
+{
+ struct map_info *map = mtd->priv;
+ struct cfi_private *cfi = map->fldrv_priv;
+
+ /** Although the part claims to have two eraseblock-regions
+ * these refer to the same region within the flash-array.
+ * Because a really CFI-Compliant part may only return
+ * one eraseblock-length per physical memory region
+ * we pretend the part said it had just one region ;)
+ **/
+ cfi->cfiq->NumEraseRegions = 1;
+ cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
+}
+
+static void fixup_sst39vf(struct mtd_info *mtd, void *param)
+{
+ struct map_info *map = mtd->priv;
+ struct cfi_private *cfi = map->fldrv_priv;
+
+ fixup_old_sst_eraseregion(mtd);
+
+ cfi->addr_unlock1 = 0x5555;
+ cfi->addr_unlock2 = 0x2AAA;
+}
+
static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
{
struct map_info *map = mtd->priv;
@@ -278,6 +308,16 @@ static void fixup_s29gl032n_sectors(struct mtd_info *mtd, void *param)
}
}
+/* Used to fix CFI-Tables of chips without Extended Query Tables
+ */
+static struct cfi_fixup cfi_nopri_fixup_table[] = {
+ { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
+ { CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
+ { CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
+ { CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
+ { 0, 0, NULL, NULL }
+};
+
static struct cfi_fixup cfi_fixup_table[] = {
{ CFI_MFR_ATMEL, CFI_ID_ANY, fixup_convert_atmel_pri, NULL },
#ifdef AMD_BOOTLOC_BUG
@@ -408,6 +448,7 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
cfi->addr_unlock1 = 0x555;
cfi->addr_unlock2 = 0x2aa;
}
+ cfi_fixup(mtd, cfi_nopri_fixup_table);
if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
kfree(mtd);
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index 991c457..599c259 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -249,6 +249,7 @@ static struct mtd_info *check_cmd_set(struct map_info *map, int primary)
#endif
#ifdef CONFIG_MTD_CFI_AMDSTD
case P_ID_AMD_STD:
+ case P_ID_SST_OLD:
return cfi_cmdset_0002(map, primary);
#endif
#ifdef CONFIG_MTD_CFI_STAA
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2 7/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{32, 64}xxB chips
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
` (4 preceding siblings ...)
2010-03-30 13:35 ` [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips Guillaume LECERF
@ 2010-03-30 13:35 ` Guillaume LECERF
2010-03-31 13:28 ` [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Artem Bityutskiy
2010-04-08 8:59 ` Wolfram Sang
7 siblings, 0 replies; 24+ messages in thread
From: Guillaume LECERF @ 2010-03-30 13:35 UTC (permalink / raw)
To: linux-mtd; +Cc: David Woodhouse, Artem Bityutskiy
This patch adds support for detecting SST 39VF32xxB and 39VF64xxB
chips in CFI mode.
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
---
drivers/mtd/chips/cfi_cmdset_0002.c | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index e3e4a94..b7846ba 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -286,6 +286,17 @@ static void fixup_sst39vf(struct mtd_info *mtd, void *param)
cfi->addr_unlock2 = 0x2AAA;
}
+static void fixup_sst39vf_rev_b(struct mtd_info *mtd, void *param)
+{
+ struct map_info *map = mtd->priv;
+ struct cfi_private *cfi = map->fldrv_priv;
+
+ fixup_old_sst_eraseregion(mtd);
+
+ cfi->addr_unlock1 = 0x555;
+ cfi->addr_unlock2 = 0x2AA;
+}
+
static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
{
struct map_info *map = mtd->priv;
@@ -315,6 +326,10 @@ static struct cfi_fixup cfi_nopri_fixup_table[] = {
{ CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
{ CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
{ CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
+ { CFI_MFR_SST, 0x235C, fixup_sst39vf_rev_b, NULL, }, // SST39VF3202B
+ { CFI_MFR_SST, 0x235D, fixup_sst39vf_rev_b, NULL, }, // SST39VF3201B
+ { CFI_MFR_SST, 0x236C, fixup_sst39vf_rev_b, NULL, }, // SST39VF6402B
+ { CFI_MFR_SST, 0x236D, fixup_sst39vf_rev_b, NULL, }, // SST39VF6401B
{ 0, 0, NULL, NULL }
};
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
` (5 preceding siblings ...)
2010-03-30 13:35 ` [PATCH v2 7/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{32, 64}xxB chips Guillaume LECERF
@ 2010-03-31 13:28 ` Artem Bityutskiy
2010-04-02 11:46 ` Guillaume LECERF
2010-04-08 8:59 ` Wolfram Sang
7 siblings, 1 reply; 24+ messages in thread
From: Artem Bityutskiy @ 2010-03-31 13:28 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd
On Tue, 2010-03-30 at 15:34 +0200, Guillaume LECERF wrote:
> Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
> members to add support for chips using alternative sequence / unlock
> addresses.
>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
thanks.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members
2010-03-31 13:28 ` [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Artem Bityutskiy
@ 2010-04-02 11:46 ` Guillaume LECERF
2010-04-02 12:11 ` Artem Bityutskiy
0 siblings, 1 reply; 24+ messages in thread
From: Guillaume LECERF @ 2010-04-02 11:46 UTC (permalink / raw)
To: dedekind1; +Cc: David Woodhouse, linux-mtd
2010/3/31 Artem Bityutskiy <dedekind1@gmail.com>:
> Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
> thanks.
Thanks.
May I ask you what is the status of this branch ? Do you plan to merge
it to mtd-2.6 (and finally to the official kernel.git) ?
--
Guillaume LECERF
GeeXboX developer - www.geexbox.org
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members
2010-04-02 11:46 ` Guillaume LECERF
@ 2010-04-02 12:11 ` Artem Bityutskiy
0 siblings, 0 replies; 24+ messages in thread
From: Artem Bityutskiy @ 2010-04-02 12:11 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd
On Fri, 2010-04-02 at 13:46 +0200, Guillaume LECERF wrote:
> 2010/3/31 Artem Bityutskiy <dedekind1@gmail.com>:
> > Pushed the series to the "dunno" branch of my l2-mtd-2.6.git tree,
> > thanks.
>
> Thanks.
> May I ask you what is the status of this branch ? Do you plan to merge
> it to mtd-2.6 (and finally to the official kernel.git) ?
dwwm2 should at some point look at that branch and take your patches,
and then merge them upstream, unless he does not like something, in
which case I guess he should let you know what is wrong.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
` (6 preceding siblings ...)
2010-03-31 13:28 ` [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Artem Bityutskiy
@ 2010-04-08 8:59 ` Wolfram Sang
7 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-08 8:59 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 4132 bytes --]
Hi Guillaume,
thanks for this patch series! I will test it right away.
A few comments/questions after having a look at the code:
On Tue, Mar 30, 2010 at 03:34:48PM +0200, Guillaume LECERF wrote:
> Move the code to enter Auto Select Mode down to be able to use cfi->cfiq
> members to add support for chips using alternative sequence / unlock
> addresses.
>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> ---
> drivers/mtd/chips/cfi_probe.c | 46 +++++++++++++++++++++--------------------
> 1 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> index e63e674..9b511ef 100644
> --- a/drivers/mtd/chips/cfi_probe.c
> +++ b/drivers/mtd/chips/cfi_probe.c
> @@ -181,29 +181,6 @@ static int __xipram cfi_chip_setup(struct map_info *map,
> for (i=0; i<(sizeof(struct cfi_ident) + num_erase_regions * 4); i++)
> ((unsigned char *)cfi->cfiq)[i] = cfi_read_query(map,base + (0x10 + i)*ofs_factor);
>
> - /* Note we put the device back into Read Mode BEFORE going into Auto
> - * Select Mode, as some devices support nesting of modes, others
> - * don't. This way should always work.
> - * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
> - * so should be treated as nops or illegal (and so put the device
> - * back into Read Mode, which is a nop in this case).
> - */
> - cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
> - cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
> - cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
> - cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
> - cfi->mfr = cfi_read_query16(map, base);
> - cfi->id = cfi_read_query16(map, base + ofs_factor);
> -
> - /* Get AMD/Spansion extended JEDEC ID */
> - if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
> - cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
> - cfi_read_query(map, base + 0xf * ofs_factor);
> -
> - /* Put it back into Read Mode */
> - cfi_qry_mode_off(base, map, cfi);
> - xip_allowed(base, map);
> -
> /* Do any necessary byteswapping */
> cfi->cfiq->P_ID = le16_to_cpu(cfi->cfiq->P_ID);
>
> @@ -228,6 +205,29 @@ static int __xipram cfi_chip_setup(struct map_info *map,
> #endif
> }
>
> + /* Note we put the device back into Read Mode BEFORE going into Auto
> + * Select Mode, as some devices support nesting of modes, others
> + * don't. This way should always work.
> + * On cmdset 0001 the writes of 0xaa and 0x55 are not needed, and
> + * so should be treated as nops or illegal (and so put the device
> + * back into Read Mode, which is a nop in this case).
> + */
Very minor: If you touch this code anyhow, maybe you could adapt it to
standard kernel-multiline comments?
> + cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
> + cfi_send_gen_cmd(0xaa, 0x555, base, map, cfi, cfi->device_type, NULL);
> + cfi_send_gen_cmd(0x55, 0x2aa, base, map, cfi, cfi->device_type, NULL);
> + cfi_send_gen_cmd(0x90, 0x555, base, map, cfi, cfi->device_type, NULL);
> + cfi->mfr = cfi_read_query16(map, base);
> + cfi->id = cfi_read_query16(map, base + ofs_factor);
> +
> + /* Get AMD/Spansion extended JEDEC ID */
> + if (cfi->mfr == CFI_MFR_AMD && (cfi->id & 0xff) == 0x7e)
> + cfi->id = cfi_read_query(map, base + 0xe * ofs_factor) << 8 |
> + cfi_read_query(map, base + 0xf * ofs_factor);
> +
> + /* Put it back into Read Mode */
> + cfi_qry_mode_off(base, map, cfi);
> + xip_allowed(base, map);
> +
> printk(KERN_INFO "%s: Found %d x%d devices at 0x%x in %d-bit bank\n",
> map->name, cfi->interleave, cfi->device_type*8, base,
> map->bankwidth*8);
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname
2010-03-30 13:34 ` [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname Guillaume LECERF
@ 2010-04-08 9:05 ` Wolfram Sang
2010-04-12 3:36 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2010-04-08 9:05 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 2433 bytes --]
On Tue, Mar 30, 2010 at 03:34:58PM +0200, Guillaume LECERF wrote:
> SST 39VF160x and 39VF320x chips use vendorname id 0x0701 and alternative
> unlock addresses. Add support for them in cfi_probe.c.
>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> ---
> drivers/mtd/chips/cfi_probe.c | 9 +++++++++
> include/linux/mtd/cfi.h | 1 +
> 2 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> index a28659d..f931e94 100644
> --- a/drivers/mtd/chips/cfi_probe.c
> +++ b/drivers/mtd/chips/cfi_probe.c
> @@ -214,6 +214,12 @@ static int __xipram cfi_chip_setup(struct map_info *map,
> * back into Read Mode, which is a nop in this case).
> */
> cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
> +
> + if (cfi->cfiq->P_ID == P_ID_SST_OLD) {
> + addr_unlock1 = 0x5555;
> + addr_unlock2 = 0x2AAA;
> + }
Can't we set cfi->addr_unlock[12] here already? This way, we could later have
just one fixup function for all old SST flashes.
> +
> cfi_send_gen_cmd(0xaa, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
> cfi_send_gen_cmd(0x55, addr_unlock2, base, map, cfi, cfi->device_type, NULL);
> cfi_send_gen_cmd(0x90, addr_unlock1, base, map, cfi, cfi->device_type, NULL);
> @@ -270,6 +276,9 @@ static char *vendorname(__u16 vendor)
> case P_ID_SST_PAGE:
> return "SST Page Write";
>
> + case P_ID_SST_OLD:
> + return "SST 39VF160x/39VF320x";
> +
> case P_ID_INTEL_PERFORMANCE:
> return "Intel Performance Code";
>
> diff --git a/include/linux/mtd/cfi.h b/include/linux/mtd/cfi.h
> index cee05b1..5716fc7 100644
> --- a/include/linux/mtd/cfi.h
> +++ b/include/linux/mtd/cfi.h
> @@ -253,6 +253,7 @@ struct cfi_bri_query {
> #define P_ID_MITSUBISHI_STD 0x0100
> #define P_ID_MITSUBISHI_EXT 0x0101
> #define P_ID_SST_PAGE 0x0102
> +#define P_ID_SST_OLD 0x0701
> #define P_ID_INTEL_PERFORMANCE 0x0200
> #define P_ID_INTEL_DATA 0x0210
> #define P_ID_RESERVED 0xffff
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-03-30 13:35 ` [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips Guillaume LECERF
@ 2010-04-08 9:12 ` Wolfram Sang
2010-04-12 2:24 ` Wolfram Sang
2010-04-08 15:21 ` Wolfram Sang
1 sibling, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2010-04-08 9:12 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 4065 bytes --]
On Tue, Mar 30, 2010 at 03:35:13PM +0200, Guillaume LECERF wrote:
> SST 39VF{16,32}xx chips use the 0x0701 command set, fully compatible
> with the AMD one. This patch adds support for detecting them in CFI
> mode.
>
> Based on a patch by Peter Turczaki [1].
>
> [1] http://www.mail-archive.com/uclinux-dev@uclinux.org/msg05737.html
>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> ---
> drivers/mtd/chips/cfi_cmdset_0002.c | 41 +++++++++++++++++++++++++++++++++++
> drivers/mtd/chips/gen_probe.c | 1 +
> 2 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index de1b4ba..e3e4a94 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -256,6 +256,36 @@ static void fixup_use_atmel_lock(struct mtd_info *mtd, void *param)
> mtd->flags |= MTD_POWERUP_LOCK;
> }
>
> +/** While reporting a mostly-correct CFI-Information block
> + * the eraseblock-region information is severely damaged in SST
> + * parts at least those of the 39VF{16,32,64}xxB series.
> + **/
Kernel mulit line comments, please.
> +static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
> +{
> + struct map_info *map = mtd->priv;
> + struct cfi_private *cfi = map->fldrv_priv;
> +
> + /** Although the part claims to have two eraseblock-regions
> + * these refer to the same region within the flash-array.
> + * Because a really CFI-Compliant part may only return
s/Compliant/compliant/?
> + * one eraseblock-length per physical memory region
> + * we pretend the part said it had just one region ;)
> + **/
> + cfi->cfiq->NumEraseRegions = 1;
> + cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
Why is this last line needed? The comment says they are the same?
> +}
> +
> +static void fixup_sst39vf(struct mtd_info *mtd, void *param)
> +{
> + struct map_info *map = mtd->priv;
> + struct cfi_private *cfi = map->fldrv_priv;
> +
> + fixup_old_sst_eraseregion(mtd);
> +
> + cfi->addr_unlock1 = 0x5555;
> + cfi->addr_unlock2 = 0x2AAA;
> +}
> +
> static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
> {
> struct map_info *map = mtd->priv;
> @@ -278,6 +308,16 @@ static void fixup_s29gl032n_sectors(struct mtd_info *mtd, void *param)
> }
> }
>
> +/* Used to fix CFI-Tables of chips without Extended Query Tables
> + */
> +static struct cfi_fixup cfi_nopri_fixup_table[] = {
> + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
> + { CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
> + { CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
> + { CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
> + { 0, 0, NULL, NULL }
> +};
> +
> static struct cfi_fixup cfi_fixup_table[] = {
> { CFI_MFR_ATMEL, CFI_ID_ANY, fixup_convert_atmel_pri, NULL },
> #ifdef AMD_BOOTLOC_BUG
> @@ -408,6 +448,7 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> cfi->addr_unlock1 = 0x555;
> cfi->addr_unlock2 = 0x2aa;
> }
> + cfi_fixup(mtd, cfi_nopri_fixup_table);
>
> if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
> kfree(mtd);
> diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
> index 991c457..599c259 100644
> --- a/drivers/mtd/chips/gen_probe.c
> +++ b/drivers/mtd/chips/gen_probe.c
> @@ -249,6 +249,7 @@ static struct mtd_info *check_cmd_set(struct map_info *map, int primary)
> #endif
> #ifdef CONFIG_MTD_CFI_AMDSTD
> case P_ID_AMD_STD:
> + case P_ID_SST_OLD:
> return cfi_cmdset_0002(map, primary);
> #endif
> #ifdef CONFIG_MTD_CFI_STAA
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-03-30 13:35 ` [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips Guillaume LECERF
2010-04-08 9:12 ` Wolfram Sang
@ 2010-04-08 15:21 ` Wolfram Sang
2010-04-08 15:32 ` Guillaume LECERF
1 sibling, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2010-04-08 15:21 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 3974 bytes --]
On Tue, Mar 30, 2010 at 03:35:13PM +0200, Guillaume LECERF wrote:
> SST 39VF{16,32}xx chips use the 0x0701 command set, fully compatible
> with the AMD one. This patch adds support for detecting them in CFI
> mode.
>
> Based on a patch by Peter Turczaki [1].
>
> [1] http://www.mail-archive.com/uclinux-dev@uclinux.org/msg05737.html
>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> ---
> drivers/mtd/chips/cfi_cmdset_0002.c | 41 +++++++++++++++++++++++++++++++++++
> drivers/mtd/chips/gen_probe.c | 1 +
> 2 files changed, 42 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> index de1b4ba..e3e4a94 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -256,6 +256,36 @@ static void fixup_use_atmel_lock(struct mtd_info *mtd, void *param)
> mtd->flags |= MTD_POWERUP_LOCK;
> }
>
> +/** While reporting a mostly-correct CFI-Information block
> + * the eraseblock-region information is severely damaged in SST
> + * parts at least those of the 39VF{16,32,64}xxB series.
> + **/
> +static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
> +{
> + struct map_info *map = mtd->priv;
> + struct cfi_private *cfi = map->fldrv_priv;
> +
> + /** Although the part claims to have two eraseblock-regions
> + * these refer to the same region within the flash-array.
> + * Because a really CFI-Compliant part may only return
> + * one eraseblock-length per physical memory region
> + * we pretend the part said it had just one region ;)
> + **/
> + cfi->cfiq->NumEraseRegions = 1;
> + cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
> +}
> +
> +static void fixup_sst39vf(struct mtd_info *mtd, void *param)
> +{
> + struct map_info *map = mtd->priv;
> + struct cfi_private *cfi = map->fldrv_priv;
> +
> + fixup_old_sst_eraseregion(mtd);
> +
> + cfi->addr_unlock1 = 0x5555;
> + cfi->addr_unlock2 = 0x2AAA;
> +}
> +
> static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
> {
> struct map_info *map = mtd->priv;
> @@ -278,6 +308,16 @@ static void fixup_s29gl032n_sectors(struct mtd_info *mtd, void *param)
> }
> }
>
> +/* Used to fix CFI-Tables of chips without Extended Query Tables
> + */
> +static struct cfi_fixup cfi_nopri_fixup_table[] = {
> + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
I got a build error. CFI_MFR_SST is missing.
> + { CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
> + { CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
> + { CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
> + { 0, 0, NULL, NULL }
> +};
> +
> static struct cfi_fixup cfi_fixup_table[] = {
> { CFI_MFR_ATMEL, CFI_ID_ANY, fixup_convert_atmel_pri, NULL },
> #ifdef AMD_BOOTLOC_BUG
> @@ -408,6 +448,7 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> cfi->addr_unlock1 = 0x555;
> cfi->addr_unlock2 = 0x2aa;
> }
> + cfi_fixup(mtd, cfi_nopri_fixup_table);
>
> if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
> kfree(mtd);
> diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
> index 991c457..599c259 100644
> --- a/drivers/mtd/chips/gen_probe.c
> +++ b/drivers/mtd/chips/gen_probe.c
> @@ -249,6 +249,7 @@ static struct mtd_info *check_cmd_set(struct map_info *map, int primary)
> #endif
> #ifdef CONFIG_MTD_CFI_AMDSTD
> case P_ID_AMD_STD:
> + case P_ID_SST_OLD:
> return cfi_cmdset_0002(map, primary);
> #endif
> #ifdef CONFIG_MTD_CFI_STAA
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-08 15:21 ` Wolfram Sang
@ 2010-04-08 15:32 ` Guillaume LECERF
2010-04-09 8:55 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Guillaume LECERF @ 2010-04-08 15:32 UTC (permalink / raw)
To: Wolfram Sang; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
2010/4/8 Wolfram Sang <w.sang@pengutronix.de>:
>> +/* Used to fix CFI-Tables of chips without Extended Query Tables
>> + */
>> +static struct cfi_fixup cfi_nopri_fixup_table[] = {
>> + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
>
> I got a build error. CFI_MFR_SST is missing.
This patch depends on this commit
http://git.infradead.org/mtd-2.6.git/commit/f3e69c6584be2db1ccd5292d6a1d7c566d265701.
Your personnal repo must be outdated.
--
Guillaume LECERF
GeeXboX developer - www.geexbox.org
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-08 15:32 ` Guillaume LECERF
@ 2010-04-09 8:55 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-09 8:55 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
On Thu, Apr 08, 2010 at 05:32:08PM +0200, Guillaume LECERF wrote:
> 2010/4/8 Wolfram Sang <w.sang@pengutronix.de>:
> >> +/* Used to fix CFI-Tables of chips without Extended Query Tables
> >> + */
> >> +static struct cfi_fixup cfi_nopri_fixup_table[] = {
> >> + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
> >
> > I got a build error. CFI_MFR_SST is missing.
>
> This patch depends on this commit
> http://git.infradead.org/mtd-2.6.git/commit/f3e69c6584be2db1ccd5292d6a1d7c566d265701.
> Your personnal repo must be outdated.
Thanks. I was unaware that there are pending patches below the v2.6.34-rc2 tag.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-08 9:12 ` Wolfram Sang
@ 2010-04-12 2:24 ` Wolfram Sang
2010-04-12 16:28 ` Fabio Giovagnini
2010-04-16 10:17 ` Guillaume LECERF
0 siblings, 2 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-12 2:24 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 5564 bytes --]
On Thu, Apr 08, 2010 at 11:12:15AM +0200, Wolfram Sang wrote:
> On Tue, Mar 30, 2010 at 03:35:13PM +0200, Guillaume LECERF wrote:
> > SST 39VF{16,32}xx chips use the 0x0701 command set, fully compatible
> > with the AMD one. This patch adds support for detecting them in CFI
> > mode.
> >
> > Based on a patch by Peter Turczaki [1].
> >
> > [1] http://www.mail-archive.com/uclinux-dev@uclinux.org/msg05737.html
> >
> > Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> > ---
> > drivers/mtd/chips/cfi_cmdset_0002.c | 41 +++++++++++++++++++++++++++++++++++
> > drivers/mtd/chips/gen_probe.c | 1 +
> > 2 files changed, 42 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
> > index de1b4ba..e3e4a94 100644
> > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > @@ -256,6 +256,36 @@ static void fixup_use_atmel_lock(struct mtd_info *mtd, void *param)
> > mtd->flags |= MTD_POWERUP_LOCK;
> > }
> >
> > +/** While reporting a mostly-correct CFI-Information block
> > + * the eraseblock-region information is severely damaged in SST
> > + * parts at least those of the 39VF{16,32,64}xxB series.
> > + **/
>
> Kernel mulit line comments, please.
>
> > +static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
> > +{
> > + struct map_info *map = mtd->priv;
> > + struct cfi_private *cfi = map->fldrv_priv;
> > +
> > + /** Although the part claims to have two eraseblock-regions
> > + * these refer to the same region within the flash-array.
> > + * Because a really CFI-Compliant part may only return
>
> s/Compliant/compliant/?
>
> > + * one eraseblock-length per physical memory region
> > + * we pretend the part said it had just one region ;)
> > + **/
> > + cfi->cfiq->NumEraseRegions = 1;
> > + cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
>
> Why is this last line needed? The comment says they are the same?
Okay, my remark was rubbish, yet the comment in the source was a bit confusing,
too. It is correct, though maybe the term 'region' is a bit overloaded. What
about replacing both comments with something a bit simpler like this:
/*
* These flashes report two seperate eraseblock regions based on the
* sector_erase-size and block_erase-size, although they both operate on the
* same memory. This is not allowed according to CFI, so we just pick the
* sector_erase-size.
*/
This is according to the datasheets. You pick the block-data size here
(ususally using command 0x50). Why is that? I tried sector_size on a
SST39WF1601 and it works fine so far, testing with mtd_stresstest. (Sidenote: I
have to use JEDEC-probing though, as my flashes don't report 0x701 but 0x002
(AMD standard) as their primary command set. But they still need their custom
unlock address :( )
>
> > +}
> > +
> > +static void fixup_sst39vf(struct mtd_info *mtd, void *param)
> > +{
> > + struct map_info *map = mtd->priv;
> > + struct cfi_private *cfi = map->fldrv_priv;
> > +
> > + fixup_old_sst_eraseregion(mtd);
> > +
> > + cfi->addr_unlock1 = 0x5555;
> > + cfi->addr_unlock2 = 0x2AAA;
> > +}
> > +
> > static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
> > {
> > struct map_info *map = mtd->priv;
> > @@ -278,6 +308,16 @@ static void fixup_s29gl032n_sectors(struct mtd_info *mtd, void *param)
> > }
> > }
> >
> > +/* Used to fix CFI-Tables of chips without Extended Query Tables
> > + */
> > +static struct cfi_fixup cfi_nopri_fixup_table[] = {
> > + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
> > + { CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
> > + { CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
> > + { CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
> > + { 0, 0, NULL, NULL }
> > +};
> > +
> > static struct cfi_fixup cfi_fixup_table[] = {
> > { CFI_MFR_ATMEL, CFI_ID_ANY, fixup_convert_atmel_pri, NULL },
> > #ifdef AMD_BOOTLOC_BUG
> > @@ -408,6 +448,7 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
> > cfi->addr_unlock1 = 0x555;
> > cfi->addr_unlock2 = 0x2aa;
> > }
> > + cfi_fixup(mtd, cfi_nopri_fixup_table);
> >
> > if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
> > kfree(mtd);
> > diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
> > index 991c457..599c259 100644
> > --- a/drivers/mtd/chips/gen_probe.c
> > +++ b/drivers/mtd/chips/gen_probe.c
> > @@ -249,6 +249,7 @@ static struct mtd_info *check_cmd_set(struct map_info *map, int primary)
> > #endif
> > #ifdef CONFIG_MTD_CFI_AMDSTD
> > case P_ID_AMD_STD:
> > + case P_ID_SST_OLD:
> > return cfi_cmdset_0002(map, primary);
> > #endif
> > #ifdef CONFIG_MTD_CFI_STAA
> >
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
> --
> Pengutronix e.K. | Wolfram Sang |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname
2010-04-08 9:05 ` Wolfram Sang
@ 2010-04-12 3:36 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-12 3:36 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]
On Thu, Apr 08, 2010 at 11:05:53AM +0200, Wolfram Sang wrote:
> On Tue, Mar 30, 2010 at 03:34:58PM +0200, Guillaume LECERF wrote:
> > SST 39VF160x and 39VF320x chips use vendorname id 0x0701 and alternative
> > unlock addresses. Add support for them in cfi_probe.c.
> >
> > Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> > ---
> > drivers/mtd/chips/cfi_probe.c | 9 +++++++++
> > include/linux/mtd/cfi.h | 1 +
> > 2 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
> > index a28659d..f931e94 100644
> > --- a/drivers/mtd/chips/cfi_probe.c
> > +++ b/drivers/mtd/chips/cfi_probe.c
> > @@ -214,6 +214,12 @@ static int __xipram cfi_chip_setup(struct map_info *map,
> > * back into Read Mode, which is a nop in this case).
> > */
> > cfi_send_gen_cmd(0xf0, 0, base, map, cfi, cfi->device_type, NULL);
> > +
> > + if (cfi->cfiq->P_ID == P_ID_SST_OLD) {
> > + addr_unlock1 = 0x5555;
> > + addr_unlock2 = 0x2AAA;
> > + }
>
> Can't we set cfi->addr_unlock[12] here already? This way, we could later have
> just one fixup function for all old SST flashes.
Well, even if there is a way to do it (by replacing the NULL-checks of
addr_unlock[12] in cfi_cmdset_0002() with something else), it can be applied
later and on top of this series.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-12 2:24 ` Wolfram Sang
@ 2010-04-12 16:28 ` Fabio Giovagnini
2010-04-13 0:27 ` Wolfram Sang
2010-04-16 10:17 ` Guillaume LECERF
1 sibling, 1 reply; 24+ messages in thread
From: Fabio Giovagnini @ 2010-04-12 16:28 UTC (permalink / raw)
To: linux-mtd
Cc: David Woodhouse, Guillaume LECERF, Wolfram Sang, Artem Bityutskiy
Hi guys,
I'm very interested to use such a patch; which kernel version Do I need to use
for appling it successfully?
Regards
In data lunedì 12 aprile 2010 04:24:58, Wolfram Sang ha scritto:
: > On Thu, Apr 08, 2010 at 11:12:15AM +0200, Wolfram Sang wrote:
> > On Tue, Mar 30, 2010 at 03:35:13PM +0200, Guillaume LECERF wrote:
> > > SST 39VF{16,32}xx chips use the 0x0701 command set, fully compatible
> > > with the AMD one. This patch adds support for detecting them in CFI
> > > mode.
> > >
> > > Based on a patch by Peter Turczaki [1].
> > >
> > > [1] http://www.mail-archive.com/uclinux-dev@uclinux.org/msg05737.html
> > >
> > > Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> > > ---
> > > drivers/mtd/chips/cfi_cmdset_0002.c | 41
> > > +++++++++++++++++++++++++++++++++++ drivers/mtd/chips/gen_probe.c
> > > | 1 +
> > > 2 files changed, 42 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c
> > > b/drivers/mtd/chips/cfi_cmdset_0002.c index de1b4ba..e3e4a94 100644
> > > --- a/drivers/mtd/chips/cfi_cmdset_0002.c
> > > +++ b/drivers/mtd/chips/cfi_cmdset_0002.c
> > > @@ -256,6 +256,36 @@ static void fixup_use_atmel_lock(struct mtd_info
> > > *mtd, void *param) mtd->flags |= MTD_POWERUP_LOCK;
> > > }
> > >
> > > +/** While reporting a mostly-correct CFI-Information block
> > > + * the eraseblock-region information is severely damaged in SST
> > > + * parts at least those of the 39VF{16,32,64}xxB series.
> > > + **/
> >
> > Kernel mulit line comments, please.
> >
> > > +static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
> > > +{
> > > + struct map_info *map = mtd->priv;
> > > + struct cfi_private *cfi = map->fldrv_priv;
> > > +
> > > + /** Although the part claims to have two eraseblock-regions
> > > + * these refer to the same region within the flash-array.
> > > + * Because a really CFI-Compliant part may only return
> >
> > s/Compliant/compliant/?
> >
> > > + * one eraseblock-length per physical memory region
> > > + * we pretend the part said it had just one region ;)
> > > + **/
> > > + cfi->cfiq->NumEraseRegions = 1;
> > > + cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
> >
> > Why is this last line needed? The comment says they are the same?
>
> Okay, my remark was rubbish, yet the comment in the source was a bit
> confusing, too. It is correct, though maybe the term 'region' is a bit
> overloaded. What about replacing both comments with something a bit
> simpler like this:
>
> /*
> * These flashes report two seperate eraseblock regions based on the
> * sector_erase-size and block_erase-size, although they both operate on
> the * same memory. This is not allowed according to CFI, so we just pick
> the * sector_erase-size.
> */
>
> This is according to the datasheets. You pick the block-data size here
> (ususally using command 0x50). Why is that? I tried sector_size on a
> SST39WF1601 and it works fine so far, testing with mtd_stresstest.
> (Sidenote: I have to use JEDEC-probing though, as my flashes don't report
> 0x701 but 0x002 (AMD standard) as their primary command set. But they
> still need their custom unlock address :( )
>
> > > +}
> > > +
> > > +static void fixup_sst39vf(struct mtd_info *mtd, void *param)
> > > +{
> > > + struct map_info *map = mtd->priv;
> > > + struct cfi_private *cfi = map->fldrv_priv;
> > > +
> > > + fixup_old_sst_eraseregion(mtd);
> > > +
> > > + cfi->addr_unlock1 = 0x5555;
> > > + cfi->addr_unlock2 = 0x2AAA;
> > > +}
> > > +
> > > static void fixup_s29gl064n_sectors(struct mtd_info *mtd, void *param)
> > > {
> > > struct map_info *map = mtd->priv;
> > > @@ -278,6 +308,16 @@ static void fixup_s29gl032n_sectors(struct
> > > mtd_info *mtd, void *param) }
> > > }
> > >
> > > +/* Used to fix CFI-Tables of chips without Extended Query Tables
> > > + */
> > > +static struct cfi_fixup cfi_nopri_fixup_table[] = {
> > > + { CFI_MFR_SST, 0x234A, fixup_sst39vf, NULL, }, // SST39VF1602
> > > + { CFI_MFR_SST, 0x234B, fixup_sst39vf, NULL, }, // SST39VF1601
> > > + { CFI_MFR_SST, 0x235A, fixup_sst39vf, NULL, }, // SST39VF3202
> > > + { CFI_MFR_SST, 0x235B, fixup_sst39vf, NULL, }, // SST39VF3201
> > > + { 0, 0, NULL, NULL }
> > > +};
> > > +
> > > static struct cfi_fixup cfi_fixup_table[] = {
> > > { CFI_MFR_ATMEL, CFI_ID_ANY, fixup_convert_atmel_pri, NULL },
> > > #ifdef AMD_BOOTLOC_BUG
> > > @@ -408,6 +448,7 @@ struct mtd_info *cfi_cmdset_0002(struct map_info
> > > *map, int primary) cfi->addr_unlock1 = 0x555;
> > > cfi->addr_unlock2 = 0x2aa;
> > > }
> > > + cfi_fixup(mtd, cfi_nopri_fixup_table);
> > >
> > > if (!cfi->addr_unlock1 || !cfi->addr_unlock2) {
> > > kfree(mtd);
> > > diff --git a/drivers/mtd/chips/gen_probe.c
> > > b/drivers/mtd/chips/gen_probe.c index 991c457..599c259 100644
> > > --- a/drivers/mtd/chips/gen_probe.c
> > > +++ b/drivers/mtd/chips/gen_probe.c
> > > @@ -249,6 +249,7 @@ static struct mtd_info *check_cmd_set(struct
> > > map_info *map, int primary) #endif
> > > #ifdef CONFIG_MTD_CFI_AMDSTD
> > > case P_ID_AMD_STD:
> > > + case P_ID_SST_OLD:
> > > return cfi_cmdset_0002(map, primary);
> > > #endif
> > > #ifdef CONFIG_MTD_CFI_STAA
> > >
> > >
> > > ______________________________________________________
> > > Linux MTD discussion mailing list
> > > http://lists.infradead.org/mailman/listinfo/linux-mtd/
> >
> > ______________________________________________________
> > Linux MTD discussion mailing list
> > http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
--
Fabio Giovagnini
Aurion s.r.l.
P.I e C.F.
00885711200
Tel. +39.051.594.78.24
Cell. +39.335.83.50.919
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-12 16:28 ` Fabio Giovagnini
@ 2010-04-13 0:27 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-13 0:27 UTC (permalink / raw)
To: Fabio Giovagnini
Cc: David Woodhouse, Guillaume LECERF, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 460 bytes --]
On Mon, Apr 12, 2010 at 06:28:38PM +0200, Fabio Giovagnini wrote:
> I'm very interested to use such a patch; which kernel version Do I need to use
> for appling it successfully?
I tried the mtd-master-tree and 2.6.33.2 with commit
f3e69c6584be2db1ccd5292d6a1d7c566d265701. Worked with both.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-12 2:24 ` Wolfram Sang
2010-04-12 16:28 ` Fabio Giovagnini
@ 2010-04-16 10:17 ` Guillaume LECERF
2010-04-18 23:29 ` Wolfram Sang
1 sibling, 1 reply; 24+ messages in thread
From: Guillaume LECERF @ 2010-04-16 10:17 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
2010/4/12 Wolfram Sang <w.sang@pengutronix.de>:
>> > + * one eraseblock-length per physical memory region
>> > + * we pretend the part said it had just one region ;)
>> > + **/
>> > + cfi->cfiq->NumEraseRegions = 1;
>> > + cfi->cfiq->EraseRegionInfo[0] = cfi->cfiq->EraseRegionInfo[1];
>>
>> Why is this last line needed? The comment says they are the same?
>
> Okay, my remark was rubbish, yet the comment in the source was a bit confusing,
> too. It is correct, though maybe the term 'region' is a bit overloaded. What
> about replacing both comments with something a bit simpler like this:
>
> /*
> * These flashes report two seperate eraseblock regions based on the
> * sector_erase-size and block_erase-size, although they both operate on the
> * same memory. This is not allowed according to CFI, so we just pick the
> * sector_erase-size.
> */
>
> This is according to the datasheets. You pick the block-data size here
> (ususally using command 0x50). Why is that? I tried sector_size on a
> SST39WF1601 and it works fine so far, testing with mtd_stresstest. (Sidenote: I
> have to use JEDEC-probing though, as my flashes don't report 0x701 but 0x002
> (AMD standard) as their primary command set. But they still need their custom
> unlock address :( )
The 39VF3201 chip I use is on a brcm63xx board, running OpenWRT. It
reports 2 erase regions (as indicated in the datasheet):
Number of Erase Block Regions: 2
Erase Region #0: BlockSize 0x1000 bytes, 1024 blocks
Erase Region #1: BlockSize 0x10000 bytes, 64 blocks
But the function parse_cfe_partitions() in
drivers/mtd/maps/bcm963xx-flash.c [1] tries to read the partition
table at $(master->erasesize) (cf. line 67).
If I use the sector_erase-size, parse_cfe_partitions() tries to read
at 0x1000, and fails because the partition table is actually at
0x10000 on my flash.
[1] https://dev.openwrt.org/browser/trunk/target/linux/brcm63xx/patches-2.6.32/040-bcm963xx_flashmap.patch
--
Guillaume LECERF
GeeXboX developer - www.geexbox.org
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-16 10:17 ` Guillaume LECERF
@ 2010-04-18 23:29 ` Wolfram Sang
2010-04-20 9:44 ` Guillaume LECERF
0 siblings, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2010-04-18 23:29 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]
Hi Guillaume,
> > Okay, my remark was rubbish, yet the comment in the source was a bit confusing,
> > too. It is correct, though maybe the term 'region' is a bit overloaded. What
> > about replacing both comments with something a bit simpler like this:
> >
> > /*
> > * These flashes report two seperate eraseblock regions based on the
> > * sector_erase-size and block_erase-size, although they both operate on the
> > * same memory. This is not allowed according to CFI, so we just pick the
> > * sector_erase-size.
> > */
> >
Any opinion about this change?
> > This is according to the datasheets. You pick the block-data size here
> > (ususally using command 0x50). Why is that? I tried sector_size on a
> > SST39WF1601 and it works fine so far, testing with mtd_stresstest. (Sidenote: I
> > have to use JEDEC-probing though, as my flashes don't report 0x701 but 0x002
> > (AMD standard) as their primary command set. But they still need their custom
> > unlock address :( )
>
> The 39VF3201 chip I use is on a brcm63xx board, running OpenWRT. It
> reports 2 erase regions (as indicated in the datasheet):
>
> Number of Erase Block Regions: 2
> Erase Region #0: BlockSize 0x1000 bytes, 1024 blocks
> Erase Region #1: BlockSize 0x10000 bytes, 64 blocks
Yes, same here.
> But the function parse_cfe_partitions() in
> drivers/mtd/maps/bcm963xx-flash.c [1] tries to read the partition
> table at $(master->erasesize) (cf. line 67).
> If I use the sector_erase-size, parse_cfe_partitions() tries to read
> at 0x1000, and fails because the partition table is actually at
> 0x10000 on my flash.
I see the problem, but think it should be fixed differently (in the driver?).
cfi_cmdset_0002.c uses CMD 0x30 for erasing (line 1604 in mtd/master). The SST
datasheet now tells that the chip then expects the sector address (table 6 or
figure 11). Given that I wonder if erasing truly works unless I missed
something (at least it fails with mtd_stresstest for me if I change my
jedec_probe to use 0x10000 instead of 0x1000).
Kind regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-18 23:29 ` Wolfram Sang
@ 2010-04-20 9:44 ` Guillaume LECERF
2010-04-22 6:13 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Guillaume LECERF @ 2010-04-20 9:44 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mtd, David Woodhouse, Artem Bityutskiy
Hi Wolfram,
>> > Okay, my remark was rubbish, yet the comment in the source was a bit confusing,
>> > too. It is correct, though maybe the term 'region' is a bit overloaded. What
>> > about replacing both comments with something a bit simpler like this:
>> >
>> > /*
>> > * These flashes report two seperate eraseblock regions based on the
>> > * sector_erase-size and block_erase-size, although they both operate on the
>> > * same memory. This is not allowed according to CFI, so we just pick the
>> > * sector_erase-size.
>> > */
>> >
>
> Any opinion about this change?
I'm of course ok with this change, I integrated it in my patch.
> > This is according to the datasheets. You pick the block-data size here
>> > (ususally using command 0x50). Why is that? I tried sector_size on a
>> > SST39WF1601 and it works fine so far, testing with mtd_stresstest. (Sidenote: I
>> > have to use JEDEC-probing though, as my flashes don't report 0x701 but 0x002
>> > (AMD standard) as their primary command set. But they still need their custom
>> > unlock address :( )
>>
>> The 39VF3201 chip I use is on a brcm63xx board, running OpenWRT. It
>> reports 2 erase regions (as indicated in the datasheet):
>>
>> Number of Erase Block Regions: 2
>> Erase Region #0: BlockSize 0x1000 bytes, 1024 blocks
>> Erase Region #1: BlockSize 0x10000 bytes, 64 blocks
>
> Yes, same here.
>
>> But the function parse_cfe_partitions() in
>> drivers/mtd/maps/bcm963xx-flash.c [1] tries to read the partition
>> table at $(master->erasesize) (cf. line 67).
>> If I use the sector_erase-size, parse_cfe_partitions() tries to read
>> at 0x1000, and fails because the partition table is actually at
>> 0x10000 on my flash.
>
> I see the problem, but think it should be fixed differently (in the driver?).
> cfi_cmdset_0002.c uses CMD 0x30 for erasing (line 1604 in mtd/master). The SST
> datasheet now tells that the chip then expects the sector address (table 6 or
> figure 11). Given that I wonder if erasing truly works unless I missed
> something (at least it fails with mtd_stresstest for me if I change my
> jedec_probe to use 0x10000 instead of 0x1000).
Ok, you were right, the problem is in the driver.
I tried to use the hardcoded value 0x10000 instead of mtd->erasesize,
and use the sector_erase-size.
mtd_stresstest now runs smoothly, so I need to ask bcm963xx-flash.c
authors why they use this variable.
I continue my tests and discussions with the bcm963xx-flash.c authors
and get back when I've got a clean solution.
Thanks for your investigations !
Regards.
--
Guillaume LECERF
GeeXboX developer - www.geexbox.org
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips
2010-04-20 9:44 ` Guillaume LECERF
@ 2010-04-22 6:13 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2010-04-22 6:13 UTC (permalink / raw)
To: Guillaume LECERF; +Cc: David Woodhouse, linux-mtd, Artem Bityutskiy
[-- Attachment #1: Type: text/plain, Size: 394 bytes --]
> I continue my tests and discussions with the bcm963xx-flash.c authors
> and get back when I've got a clean solution.
>
> Thanks for your investigations !
You are welcome. Please CC me on the next round, so I can add my tags.
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2010-04-22 6:13 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-30 13:34 [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 2/7] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable Guillaume LECERF
2010-03-30 13:34 ` [PATCH v2 3/7] mtd: cfi_probe: add support for SST 0x0701 vendorname Guillaume LECERF
2010-04-08 9:05 ` Wolfram Sang
2010-04-12 3:36 ` Wolfram Sang
2010-03-30 13:35 ` [PATCH v2 4/7] mtd: cfi_probe: use P_ID_* definitions instead of hardcoded values Guillaume LECERF
2010-03-30 13:35 ` [PATCH v2 5/7] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional Guillaume LECERF
2010-03-30 13:35 ` [PATCH v2 6/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips Guillaume LECERF
2010-04-08 9:12 ` Wolfram Sang
2010-04-12 2:24 ` Wolfram Sang
2010-04-12 16:28 ` Fabio Giovagnini
2010-04-13 0:27 ` Wolfram Sang
2010-04-16 10:17 ` Guillaume LECERF
2010-04-18 23:29 ` Wolfram Sang
2010-04-20 9:44 ` Guillaume LECERF
2010-04-22 6:13 ` Wolfram Sang
2010-04-08 15:21 ` Wolfram Sang
2010-04-08 15:32 ` Guillaume LECERF
2010-04-09 8:55 ` Wolfram Sang
2010-03-30 13:35 ` [PATCH v2 7/7] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{32, 64}xxB chips Guillaume LECERF
2010-03-31 13:28 ` [PATCH v2 1/7] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members Artem Bityutskiy
2010-04-02 11:46 ` Guillaume LECERF
2010-04-02 12:11 ` Artem Bityutskiy
2010-04-08 8:59 ` Wolfram Sang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.