[PATCH v5 1/9] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members

[PATCH v5 1/9] mtd: cfi_probe: enter Auto Select Mode after filling cfi->cfiq members

[Guillaume LECERF]

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>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mtd/chips/cfi_probe.c |   47 +++++++++++++++++++++--------------------
 1 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index e63e674..f657d16 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,30 @@ 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);

[PATCH v5 2/9] mtd: cfi_probe: make the addresses used to enter Auto Select Mode variable

[Guillaume LECERF]

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>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 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 f657d16..045dc10 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
@@ -214,9 +215,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);
 

[PATCH v5 3/9] mtd: cfi_probe: add support for SST 0x0701 vendorname

[Guillaume LECERF]

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>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mtd/chips/cfi_probe.c |    8 ++++++++
 include/linux/mtd/cfi.h       |    1 +
 2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/chips/cfi_probe.c b/drivers/mtd/chips/cfi_probe.c
index 045dc10..b2acd32 100644
--- a/drivers/mtd/chips/cfi_probe.c
+++ b/drivers/mtd/chips/cfi_probe.c
@@ -206,6 +206,11 @@ static int __xipram cfi_chip_setup(struct map_info *map,
 #endif
 	}
 
+	if (cfi->cfiq->P_ID == P_ID_SST_OLD) {
+		addr_unlock1 = 0x5555;
+		addr_unlock2 = 0x2AAA;
+	}
+
 	/*
 	 * Note we put the device back into Read Mode BEFORE going into Auto
 	 * Select Mode, as some devices support nesting of modes, others
@@ -271,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

[PATCH v5 4/9] mtd: cfi_probe: use P_ID_* definitions instead of hardcoded values

[Guillaume LECERF]

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>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 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:

[PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Guillaume LECERF]

From: Uwe Kleine-König <Uwe.Kleine-Koenigi@digi.com>

After the deleted block bootloc is only used once as follows:

	if (bootloc == 3 && something_else) {
		...

So setting bootloc = 2 doesn't change anything.  Taking that the warning is
wrong and missleading.

Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
Acked-by: Christopher Moore <moore@free.fr>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index ea2a7f6..8da8655 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -391,11 +391,6 @@ struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 #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;
-		}
 
 		if (bootloc == 3 && cfi->cfiq->NumEraseRegions > 1) {
 			printk(KERN_WARNING "%s: Swapping erase regions for broken CFI table.\n", map->name);

[PATCH v5 6/9] mtd: cfi_cmdset_0002: do not fail on no extended query table as they are both optional

[Guillaume LECERF]

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>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |   79 ++++++++++++++++++-----------------
 1 files changed, 40 insertions(+), 39 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 8da8655..23114ab 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -353,60 +353,61 @@ 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;
+			bootloc = extp->TopBottom;
 
-		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) {

[PATCH v5 7/9] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{16, 32}xx chips

[Guillaume LECERF]

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.

Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |   35 +++++++++++++++++++++++++++++++++++
 drivers/mtd/chips/gen_probe.c       |    1 +
 2 files changed, 36 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c b/drivers/mtd/chips/cfi_cmdset_0002.c
index 23114ab..c5837d0 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -256,6 +256,31 @@ static void fixup_use_atmel_lock(struct mtd_info *mtd, void *param)
 	mtd->flags |= MTD_POWERUP_LOCK;
 }
 
+static void fixup_old_sst_eraseregion(struct mtd_info *mtd)
+{
+	struct map_info *map = mtd->priv;
+	struct cfi_private *cfi = map->fldrv_priv;
+
+	/*
+	 * 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.
+	 */
+	cfi->cfiq->NumEraseRegions = 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 +303,15 @@ 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
@@ -403,6 +437,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

[PATCH v5 8/9] mtd: cfi_cmdset_0002: add CFI detection for SST 39VF{32, 64}xxB chips

[Guillaume LECERF]

This patch adds support for detecting SST 39VF32xxB and 39VF64xxB
chips in CFI mode.

Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 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 c5837d0..ef64080 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -281,6 +281,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;
@@ -309,6 +320,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 }
 };
 

[PATCH v5 9/9] mtd: cfi_util: do not printk if no extended query table

[Guillaume LECERF]

Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
---
 drivers/mtd/chips/cfi_util.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c
index ca584d0..d7c2c67 100644
--- a/drivers/mtd/chips/cfi_util.c
+++ b/drivers/mtd/chips/cfi_util.c
@@ -104,10 +104,11 @@ __xipram cfi_read_pri(struct map_info *map, __u16 adr, __u16 size, const char* n
 	int i;
 	struct cfi_extquery *extp = NULL;
 
-	printk(" %s Extended Query Table at 0x%4.4X\n", name, adr);
 	if (!adr)
 		goto out;
 
+	printk(KERN_INFO "%s Extended Query Table at 0x%4.4X\n", name, adr);
+
 	extp = kmalloc(size, GFP_KERNEL);
 	if (!extp) {
 		printk(KERN_ERR "Failed to allocate memory\n");

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Artem Bityutskiy]

On Sat, 2010-04-24 at 17:58 +0200, Guillaume LECERF wrote:
> From: Uwe Kleine-König <Uwe.Kleine-Koenigi@digi.com>
> 
> After the deleted block bootloc is only used once as follows:
> 
> 	if (bootloc == 3 && something_else) {
> 		...
> 
> So setting bootloc = 2 doesn't change anything.  Taking that the warning is
> wrong and missleading.
> 
> Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> Acked-by: Christopher Moore <moore@free.fr>

Pushed this v5 series to l2-mtd-2.6 / dunno. But please, if you re-send
the series, use consistent "mtd: cfi_cmdset_0002:" prefix for this
patch. Then I will be able to do git-am without manual work. Now I have
to tweak this patch because git am removes [MTD].

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Guillaume LECERF]

2010/4/29 Artem Bityutskiy <dedekind1@gmail.com>:
> Pushed this v5 series to l2-mtd-2.6 / dunno. But please, if you re-send
> the series, use consistent "mtd: cfi_cmdset_0002:" prefix for this
> patch. Then I will be able to do git-am without manual work. Now I have
> to tweak this patch because git am removes [MTD].

Thanks for pushing it.
And I take your advice into consideration for my next series.

-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Guillaume LECERF]

Hi.

2010/4/29 Artem Bityutskiy <dedekind1@gmail.com>:
> Pushed this v5 series to l2-mtd-2.6 / dunno. But please, if you re-send
> the series, use consistent "mtd: cfi_cmdset_0002:" prefix for this
> patch. Then I will be able to do git-am without manual work. Now I have
> to tweak this patch because git am removes [MTD].

David, could you please pick this series and push it to mtd-2.6.git
before the merge window ?
Thanks.


-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[David Woodhouse]

On Sat, 2010-04-24 at 17:58 +0200, Guillaume LECERF wrote:
> From: Uwe Kleine-König <Uwe.Kleine-Koenigi@digi.com>
> 
> After the deleted block bootloc is only used once as follows:
> 
>         if (bootloc == 3 && something_else) {
>                 ...
> 
> So setting bootloc = 2 doesn't change anything.  Taking that the
> warning is wrong and missleading.
> 
> Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@digi.com>
> Signed-off-by: Guillaume LECERF <glecerf@gmail.com>
> Acked-by: Christopher Moore <moore@free.fr> 

The warning should be preserved for invalid values, or values we don't
support. I don't think it's correct to just remove it.

Looking at the latest datasheet, it looks like values of 0x04 or 0x05
are acceptable, meaning "Uniform, bottom WP protect" and "Uniform, top
WP protect" respectively.

Zero is acceptable only for pre-CFI 1.1 devices, and in that case it
says "refer to device ID code for Top/Bottom boot version of AM29LV160
or AM29LV116". But perhaps we should just probe such devices using the
JEDEC mode anyway.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Chris Moore]

Le 14/05/2010 02:19, David Woodhouse a écrit :
> On Sat, 2010-04-24 at 17:58 +0200, Guillaume LECERF wrote:
>    
>> From: Uwe Kleine-König<Uwe.Kleine-Koenigi@digi.com>
>>
>> After the deleted block bootloc is only used once as follows:
>>
>>          if (bootloc == 3&&  something_else) {
>>                  ...
>>
>> So setting bootloc = 2 doesn't change anything.  Taking that the
>> warning is wrong and missleading.
>>
>> Signed-off-by: Uwe Kleine-König<Uwe.Kleine-Koenig@digi.com>
>> Signed-off-by: Guillaume LECERF<glecerf@gmail.com>
>> Acked-by: Christopher Moore<moore@free.fr>
>>      
> The warning should be preserved for invalid values, or values we don't
> support. I don't think it's correct to just remove it.
>
>    

But, David, the only effect of this code is to print an *erroneous* message.
The message says "Assuming top" but sets bootloc to bottom (2) :(
See this thread: http://thread.gmane.org/gmane.linux.drivers.mtd/22176
Remark: this would be even clearer if the bootloc values were macros 
instead of 2, 3, ...

If you really want to keep the message then it must at least be changed 
to "Assuming bottom".

Cheers,
Chris

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[David Woodhouse]

On Fri, 2010-05-14 at 08:17 +0200, Chris Moore wrote:
> Le 14/05/2010 02:19, David Woodhouse a écrit :
> > On Sat, 2010-04-24 at 17:58 +0200, Guillaume LECERF wrote:
> >    
> >> From: Uwe Kleine-König<Uwe.Kleine-Koenigi@digi.com>
> >>
> >> After the deleted block bootloc is only used once as follows:
> >>
> >>          if (bootloc == 3&&  something_else) {
> >>                  ...
> >>
> >> So setting bootloc = 2 doesn't change anything.  Taking that the
> >> warning is wrong and missleading.
> >>
> >> Signed-off-by: Uwe Kleine-König<Uwe.Kleine-Koenig@digi.com>
> >> Signed-off-by: Guillaume LECERF<glecerf@gmail.com>
> >> Acked-by: Christopher Moore<moore@free.fr>
> >>      
> > The warning should be preserved for invalid values, or values we don't
> > support. I don't think it's correct to just remove it.
> >
> >    
> 
> But, David, the only effect of this code is to print an *erroneous* message.
> The message says "Assuming top" but sets bootloc to bottom (2) :(
> See this thread: http://thread.gmane.org/gmane.linux.drivers.mtd/22176
> Remark: this would be even clearer if the bootloc values were macros 
> instead of 2, 3, ...
> 
> If you really want to keep the message then it must at least be changed 
> to "Assuming bottom".

http://git.infradead.org/mtd-2.6.git/commitdiff/412da2f6

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[Guillaume LECERF]

2010/5/14 David Woodhouse <dwmw2@infradead.org>:
> http://git.infradead.org/mtd-2.6.git/commitdiff/412da2f6

Thanks David.

One thing I noted :

+                       if ((bootloc < 2) || (bootloc > 5)) {
+                               printk(KERN_WARNING "%s: CFI contains
unrecognised boot "
+                                      "bank location (%d). Assuming bottom.\n",
+                                      bootloc, map->name);

I think bootloc and map->name need to be swapped to match the printk
pattern order (%s first then %d).

-- 
Guillaume LECERF
GeeXboX developer - www.geexbox.org

Re: [PATCH v5 5/9] [MTD] remove bogus warning about missing boot bank location

[David Woodhouse]

On Fri, 2010-05-14 at 10:09 +0200, Guillaume LECERF wrote:
> I think bootloc and map->name need to be swapped to match the printk
> pattern order (%s first then %d). 

Doh. Fixed; thanks. 

-- 
dwmw2