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

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

[Wolfgang Grandegger]

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.

Signed-off-by: Wolfgang Grandegger <wg@grandegger.com>
---
 drivers/mtd/chips/cfi_cmdset_0002.c |   10 ----------
 1 file changed, 10 deletions(-)

Index: linux-2.6/drivers/mtd/chips/cfi_cmdset_0002.c
===================================================================
--- linux-2.6.orig/drivers/mtd/chips/cfi_cmdset_0002.c
+++ linux-2.6/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -363,16 +363,6 @@ struct mtd_info *cfi_cmdset_0002(struct 
 			return NULL;
 		}
 
-		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;
 

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Nicolas Pitre]

On Thu, 8 Jan 2009, Wolfgang Grandegger wrote:

> 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.


Nicolas

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[David Woodhouse]

On Thu, 2009-01-08 at 11:47 -0500, Nicolas Pitre wrote:
> > 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?

-- 
dwmw2

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Wolfgang Grandegger]

David Woodhouse wrote:
> On Thu, 2009-01-08 at 11:47 -0500, Nicolas Pitre wrote:
>>> 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?

The major/minor version number is not used anywhere else and therefore a
quirk seems to be overkill too me. But we may want to keep the message:

  Unknown Amd/Fujitsu Extended Query version 0.0.

Wolfgang.

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Nicolas Pitre]

On Thu, 8 Jan 2009, Wolfgang Grandegger wrote:

> David Woodhouse wrote:
> > On Thu, 2009-01-08 at 11:47 -0500, Nicolas Pitre wrote:
> >>> 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?
> 
> 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.


Nicolas

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Wolfgang Grandegger]

Nicolas Pitre wrote:
> On Thu, 8 Jan 2009, Wolfgang Grandegger wrote:
> 
>> David Woodhouse wrote:
>>> On Thu, 2009-01-08 at 11:47 -0500, Nicolas Pitre wrote:
>>>>> 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?
>> 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.

I see.

> 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
===================================================================
--- linux-2.6.orig/drivers/mtd/chips/cfi_cmdset_0002.c
+++ linux-2.6/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -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
===================================================================
--- linux-2.6.orig/include/linux/mtd/cfi.h
+++ linux-2.6/include/linux/mtd/cfi.h
@@ -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);

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Nicolas Pitre]

On Thu, 8 Jan 2009, Wolfgang Grandegger wrote:

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

Looks fine to me.


> 
> 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
> ===================================================================
> --- linux-2.6.orig/drivers/mtd/chips/cfi_cmdset_0002.c
> +++ linux-2.6/drivers/mtd/chips/cfi_cmdset_0002.c
> @@ -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
> ===================================================================
> --- linux-2.6.orig/include/linux/mtd/cfi.h
> +++ linux-2.6/include/linux/mtd/cfi.h
> @@ -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);
> 

Re: [PATCH] CFI: remove major/minor version check for command set 0x0002

[Wolfgang Grandegger]

Nicolas Pitre wrote:
> On Thu, 8 Jan 2009, Wolfgang Grandegger wrote:
> 
>> Yep, the patch below introduces cfi_fixup_major_minor() to do the quirk. 
>> What do you think now?
> 
> Looks fine to me.

I just resent the patch with a proper description.

Thanks,

Wolfgang.