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

login
register
mail settings
Submitter Wolfgang Grandegger
Date Jan. 8, 2009, 6:21 p.m.
Message ID <49664427.7060801@grandegger.com>
Download mbox | patch
Permalink /patch/17394/
State New, archived
Headers show

Comments

Wolfgang Grandegger - Jan. 8, 2009, 6:21 p.m.
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(+)
Nicolas Pitre - Jan. 8, 2009, 6:33 p.m.
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);
>
Wolfgang Grandegger - Jan. 9, 2009, 1:04 p.m.
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.

Patch

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);