Patchwork mtd: OneNAND: Detect the correct NOP when 4KiB pagesize

login
register
mail settings
Submitter Kyungmin Park
Date June 8, 2011, 1:56 a.m.
Message ID <20110608015625.GA27537@july>
Download mbox | patch
Permalink /patch/99352/
State New
Headers show

Comments

Kyungmin Park - June 8, 2011, 1:56 a.m.
From: Kyungmin Park <kyungmin.park@samsung.com>

There are two different 4KiB pagesize chips
KFM4G16Q4M series have NOP 4 with version ID 0x0131
But KFM4G16Q5M has NOP 1 with versoin ID 0x013e

Note that Q5M means that it has NOP 1.

Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Artem Bityutskiy - June 8, 2011, 11:03 a.m.
On Wed, 2011-06-08 at 10:56 +0900, Kyungmin Park wrote:
> From: Kyungmin Park <kyungmin.park@samsung.com>
> 
> There are two different 4KiB pagesize chips
> KFM4G16Q4M series have NOP 4 with version ID 0x0131
> But KFM4G16Q5M has NOP 1 with versoin ID 0x013e
> 
> Note that Q5M means that it has NOP 1.
> 
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
> index ac9e959..2adca63 100644
> --- a/drivers/mtd/onenand/onenand_base.c
> +++ b/drivers/mtd/onenand/onenand_base.c
> @@ -3429,6 +3429,13 @@ static void onenand_check_features(struct mtd_info *mtd)
>  		else if (numbufs == 1) {
>  			this->options |= ONENAND_HAS_4KB_PAGE;
>  			this->options |= ONENAND_HAS_CACHE_PROGRAM;
> +			/*
> +			 * There are two different 4KiB pagesize chips
> +			 * KFM4G16Q4M has NOP 4 with version ID 0x0131
> +			 * KFM4G16Q5M has NOP 1 with versoin ID 0x013e
> +			 */

If you confirm that it is impossible to detect NOP and distinguish
between KFM4G16Q4M and KFM4G16Q5M, then can we take the following steps:

1. Add a comment that this is a hacky quirk to work around HW
   identification flaw, to make it clear for the people who read the
   code.
2. Make sure the default is NOP4 (you already do).
3. Compare _both_ device ID and version ID. This means that we'll end up
   with NOP1 only for your device, all the other will be NOP4. If then
   someone has issues, they'll have to reporte about them and add their
   device ID to this hack.
Artem Bityutskiy - June 8, 2011, 1:19 p.m.
On Wed, 2011-06-08 at 22:21 +0900, Kyungmin Park wrote:
> > 1. Add a comment that this is a hacky quirk to work around HW
> >   identification flaw, to make it clear for the people who read the
> >   code.
> 
> For the record, chip maker (memory) made different chips for each
> vendor (Samsung Mobile, and Nokia)
> with the design flaw, it made a workaround, not hacky quirk.

OK :-) Would you please send an updated patch ?
Kyungmin Park - June 8, 2011, 1:21 p.m.
On Wed, Jun 8, 2011 at 8:03 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2011-06-08 at 10:56 +0900, Kyungmin Park wrote:
>> From: Kyungmin Park <kyungmin.park@samsung.com>
>>
>> There are two different 4KiB pagesize chips
>> KFM4G16Q4M series have NOP 4 with version ID 0x0131
>> But KFM4G16Q5M has NOP 1 with versoin ID 0x013e
>>
>> Note that Q5M means that it has NOP 1.
>>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> ---
>> diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
>> index ac9e959..2adca63 100644
>> --- a/drivers/mtd/onenand/onenand_base.c
>> +++ b/drivers/mtd/onenand/onenand_base.c
>> @@ -3429,6 +3429,13 @@ static void onenand_check_features(struct mtd_info *mtd)
>>               else if (numbufs == 1) {
>>                       this->options |= ONENAND_HAS_4KB_PAGE;
>>                       this->options |= ONENAND_HAS_CACHE_PROGRAM;
>> +                     /*
>> +                      * There are two different 4KiB pagesize chips
>> +                      * KFM4G16Q4M has NOP 4 with version ID 0x0131
>> +                      * KFM4G16Q5M has NOP 1 with versoin ID 0x013e
>> +                      */
>
> If you confirm that it is impossible to detect NOP and distinguish
> between KFM4G16Q4M and KFM4G16Q5M, then can we take the following steps:
>
> 1. Add a comment that this is a hacky quirk to work around HW
>   identification flaw, to make it clear for the people who read the
>   code.

For the record, chip maker (memory) made different chips for each
vendor (Samsung Mobile, and Nokia)
with the design flaw, it made a workaround, not hacky quirk.

> 2. Make sure the default is NOP4 (you already do).
> 3. Compare _both_ device ID and version ID. This means that we'll end up
>   with NOP1 only for your device, all the other will be NOP4. If then
>   someone has issues, they'll have to reporte about them and add their
>   device ID to this hack.

Okay no problem to compare the device ID.

Thank you,
Kyungmin Park
>
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
Kyungmin Park - June 9, 2011, 6:12 a.m.
On Wed, Jun 8, 2011 at 10:19 PM, Artem Bityutskiy <dedekind1@gmail.com> wrote:
> On Wed, 2011-06-08 at 22:21 +0900, Kyungmin Park wrote:
>> > 1. Add a comment that this is a hacky quirk to work around HW
>> >   identification flaw, to make it clear for the people who read the
>> >   code.
>>
>> For the record, chip maker (memory) made different chips for each
>> vendor (Samsung Mobile, and Nokia)
>> with the design flaw, it made a workaround, not hacky quirk.
>
> OK :-) Would you please send an updated patch ?

BTW, as you mentioned, it's already checked. DeviceID is just
capacity, I mean size is already checked and 4KiB pagesize is also
checked by numbufs... So check the version ID is enough in this case?

How do you think?

>
> --
> Best Regards,
> Artem Bityutskiy (Артём Битюцкий)
>
>
Artem Bityutskiy - June 9, 2011, 8:16 a.m.
On Thu, 2011-06-09 at 15:12 +0900, Kyungmin Park wrote:
> BTW, as you mentioned, it's already checked. DeviceID is just
> capacity, I mean size is already checked and 4KiB pagesize is also
> checked by numbufs... So check the version ID is enough in this case?

Probably yes.

Patch

diff --git a/drivers/mtd/onenand/onenand_base.c b/drivers/mtd/onenand/onenand_base.c
index ac9e959..2adca63 100644
--- a/drivers/mtd/onenand/onenand_base.c
+++ b/drivers/mtd/onenand/onenand_base.c
@@ -3429,6 +3429,13 @@  static void onenand_check_features(struct mtd_info *mtd)
 		else if (numbufs == 1) {
 			this->options |= ONENAND_HAS_4KB_PAGE;
 			this->options |= ONENAND_HAS_CACHE_PROGRAM;
+			/*
+			 * There are two different 4KiB pagesize chips
+			 * KFM4G16Q4M has NOP 4 with version ID 0x0131
+			 * KFM4G16Q5M has NOP 1 with versoin ID 0x013e
+			 */
+			if ((this->version_id & 0xf) == 0xe)
+				this->options |= ONENAND_HAS_NOP_1;
 		}
 
 	case ONENAND_DEVICE_DENSITY_2Gb:
@@ -4054,6 +4061,8 @@  int onenand_scan(struct mtd_info *mtd, int maxchips)
 			this->ecclayout = &onenand_oob_128;
 			mtd->subpage_sft = 2;
 		}
+		if (ONENAND_IS_NOP_1(this))
+			mtd->subpage_sft = 0;
 		break;
 	case 64:
 		this->ecclayout = &onenand_oob_64;
diff --git a/include/linux/mtd/onenand.h b/include/linux/mtd/onenand.h
index 52b6f18..4596503 100644
--- a/include/linux/mtd/onenand.h
+++ b/include/linux/mtd/onenand.h
@@ -184,6 +184,9 @@  struct onenand_chip {
 #define ONENAND_IS_CACHE_PROGRAM(this)					\
 	(this->options & ONENAND_HAS_CACHE_PROGRAM)
 
+#define ONENAND_IS_NOP_1(this)						\
+	(this->options & ONENAND_HAS_NOP_1)
+
 /* Check byte access in OneNAND */
 #define ONENAND_CHECK_BYTE_ACCESS(addr)		(addr & 0x1)
 
@@ -195,6 +198,7 @@  struct onenand_chip {
 #define ONENAND_HAS_2PLANE		(0x0004)
 #define ONENAND_HAS_4KB_PAGE		(0x0008)
 #define ONENAND_HAS_CACHE_PROGRAM	(0x0010)
+#define ONENAND_HAS_NOP_1		(0x0020)
 #define ONENAND_SKIP_UNLOCK_CHECK	(0x0100)
 #define ONENAND_PAGEBUF_ALLOC		(0x1000)
 #define ONENAND_OOBBUF_ALLOC		(0x2000)