[PATCH] mtd: nand: Fix Spansion sparearea size detection

[PATCH] mtd: nand: Fix Spansion sparearea size detection

[Nikolay Martynov]

According to datasheet S34ML02G2 and S34ML04G2 have
larger sparea area size than was detected.

Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
---
 drivers/mtd/nand/nand_base.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index ceb68ca..2c01f9e 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3415,6 +3415,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 	/*
 	 * Field definitions are in the following datasheets:
 	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
+	 *                          Spansion S34ML02G2 (p.33)
 	 * New Samsung (6 byte ID): Samsung K9GAG08U0F (p.44)
 	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
 	 *
@@ -3512,6 +3513,14 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
 		*busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
 
 		/*
+		 * Spansion S34ML0[24]G2 have oobsize twice as large
+		 * as S34ML01G2 encoded in the same bit. We
+		 * differentiate them by their ID length
+		 */
+		if (id_len == 5 && id_data[0] == NAND_MFR_AMD)
+			mtd->oobsize *= 2;
+
+		/*
 		 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
 		 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
 		 * follows:
-- 
2.6.3

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Brian Norris]

Hi Nikolay,

On Thu, Nov 26, 2015 at 01:25:22AM -0500, Nikolay Martynov wrote:
> According to datasheet S34ML02G2 and S34ML04G2 have
> larger sparea area size than was detected.
> 
> Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
> ---
>  drivers/mtd/nand/nand_base.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index ceb68ca..2c01f9e 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3415,6 +3415,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
>  	/*
>  	 * Field definitions are in the following datasheets:
>  	 * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
> +	 *                          Spansion S34ML02G2 (p.33)
>  	 * New Samsung (6 byte ID): Samsung K9GAG08U0F (p.44)
>  	 * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
>  	 *
> @@ -3512,6 +3513,14 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
>  		*busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
>  
>  		/*
> +		 * Spansion S34ML0[24]G2 have oobsize twice as large
> +		 * as S34ML01G2 encoded in the same bit. We
> +		 * differentiate them by their ID length

Hmm, are you sure the ID length heuristic works correctly for these
particular flash? i.e., did you actually test both of these flash?
Sometimes it's hard to tell if a device has a 4-byte or 5-byte ID just
from reading out bytes, since a "4-byte ID" NAND may just have a "don't
care" byte for the 5th one, and so we might still compute id_len == 5.

Also, why do you even need nand_decode_ext_id() for these flash?
According to the datasheet, these flash support ONFI.

Brian

> +		 */
> +		if (id_len == 5 && id_data[0] == NAND_MFR_AMD)
> +			mtd->oobsize *= 2;
> +
> +		/*
>  		 * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
>  		 * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
>  		 * follows:
> -- 
> 2.6.3
> 

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Nikolay Martynov]

Hi.

  Thanks for your response!
  I'm sorry, I only have board with S34ML02G2 and I've tested by
change on it - it works fine. But unfortunately I do not have any way
to test S34ML01G2.
  It looks like that logic can be simplified if just check id_data[1]
is one of 0xda, 0xdc, 0xca, 0xcc - this should be safer option.

  I'm sorry, I'm very new to all this. This patch made kernel boot on
a board that I have. The kernel was 'oldish' - 3.18. So I'm not sure
why ONFI is not used here, I will see if I can figure it out.

Thanks!


2015-11-30 15:35 GMT-05:00 Brian Norris <computersforpeace@gmail.com>:
> Hi Nikolay,
>
> On Thu, Nov 26, 2015 at 01:25:22AM -0500, Nikolay Martynov wrote:
>> According to datasheet S34ML02G2 and S34ML04G2 have
>> larger sparea area size than was detected.
>>
>> Signed-off-by: Nikolay Martynov <mar.kolya@gmail.com>
>> ---
>>  drivers/mtd/nand/nand_base.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index ceb68ca..2c01f9e 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -3415,6 +3415,7 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
>>       /*
>>        * Field definitions are in the following datasheets:
>>        * Old style (4,5 byte ID): Samsung K9GAG08U0M (p.32)
>> +      *                          Spansion S34ML02G2 (p.33)
>>        * New Samsung (6 byte ID): Samsung K9GAG08U0F (p.44)
>>        * Hynix MLC   (6 byte ID): Hynix H27UBG8T2B (p.22)
>>        *
>> @@ -3512,6 +3513,14 @@ static void nand_decode_ext_id(struct mtd_info *mtd, struct nand_chip *chip,
>>               *busw = (extid & 0x01) ? NAND_BUSWIDTH_16 : 0;
>>
>>               /*
>> +              * Spansion S34ML0[24]G2 have oobsize twice as large
>> +              * as S34ML01G2 encoded in the same bit. We
>> +              * differentiate them by their ID length
>
> Hmm, are you sure the ID length heuristic works correctly for these
> particular flash? i.e., did you actually test both of these flash?
> Sometimes it's hard to tell if a device has a 4-byte or 5-byte ID just
> from reading out bytes, since a "4-byte ID" NAND may just have a "don't
> care" byte for the 5th one, and so we might still compute id_len == 5.
>
> Also, why do you even need nand_decode_ext_id() for these flash?
> According to the datasheet, these flash support ONFI.
>
> Brian
>
>> +              */
>> +             if (id_len == 5 && id_data[0] == NAND_MFR_AMD)
>> +                     mtd->oobsize *= 2;
>> +
>> +             /*
>>                * Toshiba 24nm raw SLC (i.e., not BENAND) have 32B OOB per
>>                * 512B page. For Toshiba SLC, we decode the 5th/6th byte as
>>                * follows:
>> --
>> 2.6.3
>>



-- 
Martynov Nikolay.
Email: mar.kolya@gmail.com

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Brian Norris]

Hi Nikolay,

On Mon, Nov 30, 2015 at 06:59:32PM -0500, Nikolay Martynov wrote:
>   I'm sorry, I only have board with S34ML02G2 and I've tested by
> change on it - it works fine. But unfortunately I do not have any way
> to test S34ML01G2.
>   It looks like that logic can be simplified if just check id_data[1]
> is one of 0xda, 0xdc, 0xca, 0xcc - this should be safer option.

Possibly safer, but it's uglier, and I'd like not to have to make this
change at all, if ONFI can help it.

>   I'm sorry, I'm very new to all this. This patch made kernel boot on
> a board that I have. The kernel was 'oldish' - 3.18. So I'm not sure
> why ONFI is not used here, I will see if I can figure it out.

What NAND driver are you using? It's possible that it doesn't support
the PARAMETER READ command properly. If so, it'd be better to fix that
than to clutter the NAND core.

Regards,
Brian

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Nikolay Martynov]

Hi.

  I'm using mt7621 board, so this would be mtk_nand with matches from openwrt.

2015-11-30 19:09 GMT-05:00 Brian Norris <computersforpeace@gmail.com>:
> Hi Nikolay,
>
> On Mon, Nov 30, 2015 at 06:59:32PM -0500, Nikolay Martynov wrote:
>>   I'm sorry, I only have board with S34ML02G2 and I've tested by
>> change on it - it works fine. But unfortunately I do not have any way
>> to test S34ML01G2.
>>   It looks like that logic can be simplified if just check id_data[1]
>> is one of 0xda, 0xdc, 0xca, 0xcc - this should be safer option.
>
> Possibly safer, but it's uglier, and I'd like not to have to make this
> change at all, if ONFI can help it.
>
>>   I'm sorry, I'm very new to all this. This patch made kernel boot on
>> a board that I have. The kernel was 'oldish' - 3.18. So I'm not sure
>> why ONFI is not used here, I will see if I can figure it out.
>
> What NAND driver are you using? It's possible that it doesn't support
> the PARAMETER READ command properly. If so, it'd be better to fix that
> than to clutter the NAND core.
>
> Regards,
> Brian



-- 
Martynov Nikolay.
Email: mar.kolya@gmail.com

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Brian Norris]

On Mon, Nov 30, 2015 at 07:19:22PM -0500, Nikolay Martynov wrote:
> Hi.
> 
>   I'm using mt7621 board, so this would be mtk_nand with matches from openwrt.

Sorry, you'll have to address this to the OpenWrt team then. I see the
OpenWrt NAND driver for your board is pretty crappy, but it might be
fixable to support ONFI properly.

Cc'ing John, who checked in mtk_nand.c into OpenWrt, I think.

Regards,
Brian

> 2015-11-30 19:09 GMT-05:00 Brian Norris <computersforpeace@gmail.com>:
> > Hi Nikolay,
> >
> > On Mon, Nov 30, 2015 at 06:59:32PM -0500, Nikolay Martynov wrote:
> >>   I'm sorry, I only have board with S34ML02G2 and I've tested by
> >> change on it - it works fine. But unfortunately I do not have any way
> >> to test S34ML01G2.
> >>   It looks like that logic can be simplified if just check id_data[1]
> >> is one of 0xda, 0xdc, 0xca, 0xcc - this should be safer option.
> >
> > Possibly safer, but it's uglier, and I'd like not to have to make this
> > change at all, if ONFI can help it.
> >
> >>   I'm sorry, I'm very new to all this. This patch made kernel boot on
> >> a board that I have. The kernel was 'oldish' - 3.18. So I'm not sure
> >> why ONFI is not used here, I will see if I can figure it out.
> >
> > What NAND driver are you using? It's possible that it doesn't support
> > the PARAMETER READ command properly. If so, it'd be better to fix that
> > than to clutter the NAND core.
> >
> > Regards,
> > Brian
> 
> 
> 
> -- 
> Martynov Nikolay.
> Email: mar.kolya@gmail.com

Re: [PATCH] mtd: nand: Fix Spansion sparearea size detection

[Nikolay Martynov]

Brian, thank you for detailed explanation!

Please disregard this patch then, since this problem is openwrt specific.
Thanks!

2015-11-30 20:05 GMT-05:00 Brian Norris <computersforpeace@gmail.com>:
> On Mon, Nov 30, 2015 at 07:19:22PM -0500, Nikolay Martynov wrote:
>> Hi.
>>
>>   I'm using mt7621 board, so this would be mtk_nand with matches from openwrt.
>
> Sorry, you'll have to address this to the OpenWrt team then. I see the
> OpenWrt NAND driver for your board is pretty crappy, but it might be
> fixable to support ONFI properly.
>
> Cc'ing John, who checked in mtk_nand.c into OpenWrt, I think.
>
> Regards,
> Brian
>
>> 2015-11-30 19:09 GMT-05:00 Brian Norris <computersforpeace@gmail.com>:
>> > Hi Nikolay,
>> >
>> > On Mon, Nov 30, 2015 at 06:59:32PM -0500, Nikolay Martynov wrote:
>> >>   I'm sorry, I only have board with S34ML02G2 and I've tested by
>> >> change on it - it works fine. But unfortunately I do not have any way
>> >> to test S34ML01G2.
>> >>   It looks like that logic can be simplified if just check id_data[1]
>> >> is one of 0xda, 0xdc, 0xca, 0xcc - this should be safer option.
>> >
>> > Possibly safer, but it's uglier, and I'd like not to have to make this
>> > change at all, if ONFI can help it.
>> >
>> >>   I'm sorry, I'm very new to all this. This patch made kernel boot on
>> >> a board that I have. The kernel was 'oldish' - 3.18. So I'm not sure
>> >> why ONFI is not used here, I will see if I can figure it out.
>> >
>> > What NAND driver are you using? It's possible that it doesn't support
>> > the PARAMETER READ command properly. If so, it'd be better to fix that
>> > than to clutter the NAND core.
>> >
>> > Regards,
>> > Brian
>>
>>
>>
>> --
>> Martynov Nikolay.
>> Email: mar.kolya@gmail.com



-- 
Martynov Nikolay.
Email: mar.kolya@gmail.com