CFI: remove major/minor version check for command set 0x0002

The NOR Flash memory K8P2815UQB from Samsung uses the major version number '0', which seems to be unusual but not illegal, IIUC. This patch removes the major-minor version check to get this Flash memory supported as well.

Quoted 4 lines

The NOR Flash memory K8P2815UQB from Samsung uses the major version number '0', which seems to be unusual but not illegal, IIUC. This patch removes the major-minor version check to get this Flash memory supported as well.

Instead of removing the test entirely, I'd suggest you augment it with the manufacturer ID (cfi->mfr and cfi->id). Or better yet, just use that information to fix up the version number to something sensible.

Quoted 7 lines · Nicolas Pitre

Quoted 4 lines

The NOR Flash memory K8P2815UQB from Samsung uses the major version number '0', which seems to be unusual but not illegal, IIUC. This patch removes the major-minor version check to get this Flash memory supported as well.

Instead of removing the test entirely, I'd suggest you augment it with the manufacturer ID (cfi->mfr and cfi->id). Or better yet, just use that information to fix up the version number to something sensible.

Couldn't you use a quirk to do the latter?

Quoted 5 lines · David Woodhouse

Quoted 4 lines · Nicolas Pitre

Quoted 4 lines

The NOR Flash memory K8P2815UQB from Samsung uses the major version number '0', which seems to be unusual but not illegal, IIUC. This patch removes the major-minor version check to get this Flash memory supported as well.

Quoted 5 lines

Quoted 3 lines

Instead of removing the test entirely, I'd suggest you augment it with the manufacturer ID (cfi->mfr and cfi->id). Or better yet, just use that information to fix up the version number to something sensible.

Couldn't you use a quirk to do the latter?

The major/minor version number is not used anywhere else and therefore a quirk seems to be overkill too me.

Quoted 10 lines · Wolfgang Grandegger

Quoted 9 lines · David Woodhouse

Quoted 7 lines · Nicolas Pitre

Quoted 4 lines

The NOR Flash memory K8P2815UQB from Samsung uses the major version number '0', which seems to be unusual but not illegal, IIUC. This patch removes the major-minor version check to get this Flash memory supported as well.

Instead of removing the test entirely, I'd suggest you augment it with the manufacturer ID (cfi->mfr and cfi->id). Or better yet, just use that information to fix up the version number to something sensible.

Couldn't you use a quirk to do the latter?

Quoted 2 lines

The major/minor version number is not used anywhere else and therefore a quirk seems to be overkill too me.

Please look again. Searching for "MinorVersion" turns up a couple places.

A quirk is not overkill at all. To the contrary it's the whole purpose of quirks to fix up things like this.

Although, in this case, I think quirks are evaluated only after the CFI version is tested, meaning that the fixup might have to be done inline.

Yep, the patch below introduces cfi_fixup_major_minor() to do the quirk. What do you think now?

Wolfgang.

---
 drivers/mtd/chips/cfi_cmdset_0002.c |   10 ++++++++++
 include/linux/mtd/cfi.h             |    1 +
 2 files changed, 11 insertions(+)

Index: linux-2.6/drivers/mtd/chips/cfi_cmdset_0002.c
===================================================================

drivers/mtd/chips/cfi_cmdset_0002.c +10 -0

@@ -322,6 +322,14 @@ static struct cfi_fixup fixup_table[] = 
 };
 
 
+static void cfi_fixup_major_minor(struct cfi_private *cfi, 
+				  struct cfi_pri_amdstd *extp)
+{
+	if (cfi->mfr == CFI_MFR_SAMSUNG && cfi->id == 0x257e &&
+	    extp->MajorVersion == '0')
+		extp->MajorVersion = '1';
+}
+
 struct mtd_info *cfi_cmdset_0002(struct map_info *map, int primary)
 {
 	struct cfi_private *cfi = map->fldrv_priv;

@@ -363,6 +371,8 @@ struct mtd_info *cfi_cmdset_0002(struct 
 			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 "

Index: linux-2.6/include/linux/mtd/cfi.h
===================================================================

include/linux/mtd/cfi.h +1 -0

@@ -520,6 +520,7 @@ struct cfi_fixup {
 
 #define CFI_MFR_AMD 0x0001
 #define CFI_MFR_ATMEL 0x001F
+#define CFI_MFR_SAMSUNG 0x00EC
 #define CFI_MFR_ST  0x0020 	/* STMicroelectronics */
 
 void cfi_fixup(struct mtd_info *mtd, struct cfi_fixup* fixups);

Quoted 2 lines · Wolfgang Grandegger

Yep, the patch below introduces cfi_fixup_major_minor() to do the quirk. What do you think now?

Looks fine to me.