[PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Huang Shijie]

From: Huang Shijie <shijie8@gmail.com>

The H27UBG8T2A is not supported by the current code.
Its full-id is : 0xad, 0xd7, 0x94, 0x9a, 0x74, 0x42.

Now, add the parsing code for it.

Signed-off-by: Huang Shijie <shijie8@gmail.com>
Signed-off-by: Huang Shijie <b32955@freescale.com>
---
 drivers/mtd/nand/nand_base.c |   43 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 6315b94..8997023 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -3028,6 +3028,49 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
 			mtd->erasesize = (128 * 1024) <<
 				(((extid >> 1) & 0x04) | (extid & 0x03));
 			busw = 0;
+		} else if (id_data[0] == NAND_MFR_HYNIX && id_data[1] == 0xd7) {
+			/* Calc pagesize */
+			mtd->writesize = 2048 << (extid & 0x03);
+			extid >>= 2;
+
+			/* Calc oobsize */
+			switch (extid & 0x03) {
+			case 0:
+				mtd->oobsize = 128;
+				break;
+			case 1:
+				mtd->oobsize = 224;
+				break;
+			case 2:
+				mtd->oobsize = 448;
+				break;
+			default:
+				pr_info("Cannot parse out the oobsize.\n");
+				break;
+			}
+			extid >>= 2;
+
+			/* Calc blocksize */
+			extid = (((extid >> 1) & 0x04) | (extid & 0x03));
+
+			switch (extid) {
+			case 0:
+			case 1:
+			case 2:
+				mtd->erasesize = (128 * 1024) << extid;
+				break;
+			case 3:
+				mtd->erasesize = (128 * 1024) * 6;
+				break;
+			case 4:
+			case 5:
+				mtd->erasesize = (128 * 1024) << (extid - 1);
+				break;
+			default:
+				pr_info("Cannot parse out the blocksize.\n");
+				break;
+			}
+			busw = 0;
 		} else {
 			/* Calc pagesize */
 			mtd->writesize = 1024 << (extid & 0x03);
-- 
1.7.0.4

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

Hi Huang,

On Mon, Apr 9, 2012 at 7:56 PM, Huang Shijie <b32955@freescale.com> wrote:
>  drivers/mtd/nand/nand_base.c |   43 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index 6315b94..8997023 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -3028,6 +3028,49 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
>                        mtd->erasesize = (128 * 1024) <<
>                                (((extid >> 1) & 0x04) | (extid & 0x03));
>                        busw = 0;
> +               } else if (id_data[0] == NAND_MFR_HYNIX && id_data[1] == 0xd7) {
> +                       /* Calc pagesize */

I'm not sure this is the beset heuristic. Hynix could easily make a
larger chip that has the same decoding table, so we probably shouldn't
compare dev_id == 0xD7. It actually seems like the decoding table
might be intended for all Hynix MLC. What do you think about the
following condition?

+       } else if ((chip->cellinfo & NAND_CI_CELLTYPE_MSK) &&
id_data[0] == NAND_MFR_HYNIX) {

> +                       /* Calc oobsize */
> +                       switch (extid & 0x03) {

The table actually includes a third column (bit 6) that corresponds
with the OOB field that, in future revs could have an expanded OOB
size. Personally, I wrote this condition as:

+               switch (((extid >> 2) & 0x04) | (extid & 0x03)) {

That way, we can catch the "default" case as an unknown OOB size.

> +                       default:
> +                               pr_info("Cannot parse out the oobsize.\n");
> +                               break;
> +                       }

I'm not sure if we should just print a message for the default case;
the warning message could easily be missed, and the uninitialized
oobsize would cause problems. Either we should throw an error somehow
or just default to the highest known OOB for this chip type (448
bytes). Personally, I just chose the latter...

BTW, I actually have some patches queued up that I need to complete
properly, where I'm trying to clean up and update NAND detection a
little. So I basically have this same patch, but rebased on top of
some other changes I have ready... I don't mind taking a revised
version of yours, or I can try to clean up my series and send it out
soon.

Either way, please remember to copy me on these type of NAND detection
patches! I've been studying this for a while and have a lot of stuff
queued up to work on sometime, so it's nice to know when others are
touching these areas.

Thanks,
Brian

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Huang Shijie]

Hi Brian:
> Hi Huang,
>
> On Mon, Apr 9, 2012 at 7:56 PM, Huang Shijie<b32955@freescale.com>  wrote:
>>   drivers/mtd/nand/nand_base.c |   43 ++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 43 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 6315b94..8997023 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -3028,6 +3028,49 @@ static struct nand_flash_dev *nand_get_flash_type(struct mtd_info *mtd,
>>                         mtd->erasesize = (128 * 1024)<<
>>                                 (((extid>>  1)&  0x04) | (extid&  0x03));
>>                         busw = 0;
>> +               } else if (id_data[0] == NAND_MFR_HYNIX&&  id_data[1] == 0xd7) {
>> +                       /* Calc pagesize */
> I'm not sure this is the beset heuristic. Hynix could easily make a
> larger chip that has the same decoding table, so we probably shouldn't
> compare dev_id == 0xD7. It actually seems like the decoding table
> might be intended for all Hynix MLC. What do you think about the
> following condition?
>
> +       } else if ((chip->cellinfo&  NAND_CI_CELLTYPE_MSK)&&
> id_data[0] == NAND_MFR_HYNIX) {
I am afraid this can not solve the problem.
The HY27UT088G2M whose id_data[2] is 0x14 DOES not follow your code.
Please check the datasheet of HY27UT088G2M. ( I ever emailed to you.)

The same issue exits in H627UW08CGFM too.

>> +                       /* Calc oobsize */
>> +                       switch (extid&  0x03) {
> The table actually includes a third column (bit 6) that corresponds
> with the OOB field that, in future revs could have an expanded OOB
> size. Personally, I wrote this condition as:
>
> +               switch (((extid>>  2)&  0x04) | (extid&  0x03)) {
yes, we can add this.
> That way, we can catch the "default" case as an unknown OOB size.
>
>> +                       default:
>> +                               pr_info("Cannot parse out the oobsize.\n");
>> +                               break;
>> +                       }
> I'm not sure if we should just print a message for the default case;
> the warning message could easily be missed, and the uninitialized
> oobsize would cause problems. Either we should throw an error somehow
> or just default to the highest known OOB for this chip type (448
> bytes). Personally, I just chose the latter...
ok. I can use the 448 as the default.
> BTW, I actually have some patches queued up that I need to complete
> properly, where I'm trying to clean up and update NAND detection a
> little. So I basically have this same patch, but rebased on top of
> some other changes I have ready... I don't mind taking a revised
> version of yours, or I can try to clean up my series and send it out
> soon.
>
I hope you send it out as soon as possible. :)

> Either way, please remember to copy me on these type of NAND detection
> patches! I've been studying this for a while and have a lot of stuff
> queued up to work on sometime, so it's nice to know when others are
> touching these areas.
ok, no problem.

thanks
Huang Shijie

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

On Wed, Apr 11, 2012 at 7:43 PM, Huang Shijie <b32955@freescale.com> wrote:
>> On Mon, Apr 9, 2012 at 7:56 PM, Huang Shijie<b32955@freescale.com>  wrote:
>>>
>>>  drivers/mtd/nand/nand_base.c |   43
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>  1 files changed, 43 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index 6315b94..8997023 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -3028,6 +3028,49 @@ static struct nand_flash_dev
>>> *nand_get_flash_type(struct mtd_info *mtd,
>>>                        mtd->erasesize = (128 * 1024)<<
>>>                                (((extid>>  1)&  0x04) | (extid&  0x03));
>>>                        busw = 0;
>>> +               } else if (id_data[0] == NAND_MFR_HYNIX&&  id_data[1] ==
>>> 0xd7) {
>>>
>>> +                       /* Calc pagesize */
>>
>> I'm not sure this is the beset heuristic. Hynix could easily make a
>> larger chip that has the same decoding table, so we probably shouldn't
>> compare dev_id == 0xD7. It actually seems like the decoding table
>> might be intended for all Hynix MLC. What do you think about the
>> following condition?
>>
>> +       } else if ((chip->cellinfo&  NAND_CI_CELLTYPE_MSK)&&
>> id_data[0] == NAND_MFR_HYNIX) {
>
> I am afraid this can not solve the problem.
> The HY27UT088G2M whose id_data[2] is 0x14 DOES not follow your code.
> Please check the datasheet of HY27UT088G2M. ( I ever emailed to you.)
>
> The same issue exits in H627UW08CGFM too.

Ah yes, I overlooked this. My proposal was a bit off-the-cuff. Thanks
for the data sheets, BTW. I need to integrate them into my master
table (and submit them to mtd-www) so that I check them whenever I'm
looking at new Hynix support.

Now that I've given them all a second read, it really looks like this
new Hynix chip actually follows no discernible pattern, which seems to
suggest that we have to know all the chip parameters in order to know
which ID decode table to use...but if we know all the parameters, then
why do we need the table?? (Just being devil's advocate) If I had
samples of all the chips, perhaps I could test some to determine other
patterns, like ID wraparound behavior, but I doubt it...

So really, I'd be interested in asking Hynix (and Toshiba and probably
Samsung, for that matter): what do you expect software developers to
do when you provide datasheets with useless ID decode tables? Are we
supposed to read your mind to determine which chips are supposed to
follow a given decode table?

But seriously: let me know if you have better insight into how to
associate this table with a set of chip IDs. Otherwise, I might as
well finish adding an 8-byte ID table and adding this one chip
manually.

>> BTW, I actually have some patches queued up that I need to complete
>> properly, where I'm trying to clean up and update NAND detection a
>> little. So I basically have this same patch, but rebased on top of
>> some other changes I have ready... I don't mind taking a revised
>> version of yours, or I can try to clean up my series and send it out
>> soon.
>>
> I hope you send it out as soon as possible. :)

I'm trying, but I get very busy with other things :)

Anyway, I'm not very comfortable with the heuristics for this Hynix
decode method, so I don't want to post another patch for it yet. If it
helps, I can submit the other patches soon; they amount to mostly
cleanup (IMO). But at least that way, whoever works on NAND detection
in the near future has the same code base.

Brian

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Huang Shijie]

于 2012年04月18日 13:00, Brian Norris 写道:
> Now that I've given them all a second read, it really looks like this
> new Hynix chip actually follows no discernible pattern, which seems to
> suggest that we have to know all the chip parameters in order to know
> which ID decode table to use...but if we know all the parameters, then
> why do we need the table?? (Just being devil's advocate) If I had
This makes me confused too.
It looks like hardcode when i use the 0xd7 to disdinguish this nand chip.

> samples of all the chips, perhaps I could test some to determine other
> patterns, like ID wraparound behavior, but I doubt it...
>
> So really, I'd be interested in asking Hynix (and Toshiba and probably
> Samsung, for that matter): what do you expect software developers to
> do when you provide datasheets with useless ID decode tables? Are we
> supposed to read your mind to determine which chips are supposed to
> follow a given decode table?
>
> But seriously: let me know if you have better insight into how to
> associate this table with a set of chip IDs. Otherwise, I might as
I only have the H27UBG8T2A nand chip in my hand, and test it in the 
imx28 board.
I do not have other nand chips which may also follow the same parsing 
code as the H27UBG8T2A.


Best Regards
Huang Shijie

> well finish adding an 8-byte ID table and adding this one chip
> manually.

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

Hi Huang,

Sorry (again) that this is sitting for a while. I did some more work
this evening on it, and I think I could have a solution. But I still
don't have any real samples, so I'd like you help.

On Wed, Apr 18, 2012 at 12:52 AM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2012年04月18日 13:00, Brian Norris 写道:
> I only have the H27UBG8T2A nand chip in my hand, and test it in the imx28
> board.
> I do not have other nand chips which may also follow the same parsing code
> as the H27UBG8T2A.

Can you capture a full ID string for any Hynix parts you have?
Especially of interest are this H27UBG8T2A and other MLC.

I want to check the ID length and wraparound behavior, since I think
that we can write a rule such that we use this data table for all
Hynix MLC with 6-byte ID strings. I have info for 3 type of Hynix MLC
that follow the 6-byte pattern, and a few 5-byte Hynix MLC that follow
existing patterns.

I appreciate the help and the patience.

Thanks,
Brian

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

On Wed, Apr 11, 2012 at 7:43 PM, Huang Shijie <b32955@freescale.com> wrote:
>> On Mon, Apr 9, 2012 at 7:56 PM, Huang Shijie<b32955@freescale.com>  wrote:
>>>
>>>  drivers/mtd/nand/nand_base.c |   43
>>> ++++++++++++++++++++++++++++++++++++++++++
>>>  1 files changed, 43 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index 6315b94..8997023 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -3028,6 +3028,49 @@ static struct nand_flash_dev
>>> *nand_get_flash_type(struct mtd_info *mtd,
>>>                        mtd->erasesize = (128 * 1024)<<
>>>                                (((extid>>  1)&  0x04) | (extid&  0x03));
>>>                        busw = 0;
>>> +               } else if (id_data[0] == NAND_MFR_HYNIX&&  id_data[1] ==
>>> 0xd7) {
>>>
>>> +                       /* Calc pagesize */
>>
>> I'm not sure this is the beset heuristic. Hynix could easily make a
>> larger chip that has the same decoding table, so we probably shouldn't
>> compare dev_id == 0xD7. It actually seems like the decoding table
>> might be intended for all Hynix MLC. What do you think about the
>> following condition?
>>
>> +       } else if ((chip->cellinfo&  NAND_CI_CELLTYPE_MSK)&&
>> id_data[0] == NAND_MFR_HYNIX) {
>
> I am afraid this can not solve the problem.
> The HY27UT088G2M whose id_data[2] is 0x14 DOES not follow your code.
> Please check the datasheet of HY27UT088G2M. ( I ever emailed to you.)
>
> The same issue exits in H627UW08CGFM too.

Ah yes, I overlooked this. My proposal was a bit off-the-cuff. Thanks
for the data sheets, BTW. I need to integrate them into my master
table (and submit them to mtd-www) so that I check them whenever I'm
looking at new Hynix support.

Now that I've given them all a second read, it really looks like this
new Hynix chip actually follows no discernible pattern, which seems to
suggest that we have to know all the chip parameters in order to know
which ID decode table to use...but if we know all the parameters, then
why do we need the table?? (Just being devil's advocate) If I had
samples of all the chips, perhaps I could test some to determine other
patterns, like ID wraparound behavior, but I doubt it...

So really, I'd be interested in asking Hynix (and Toshiba and probably
Samsung, for that matter): what do you expect software developers to
do when you provide datasheets with useless ID decode tables? Are we
supposed to read your mind to determine which chips are supposed to
follow a given decode table?

But seriously: let me know if you have better insight into how to
associate this table with a set of chip IDs. Otherwise, I might as
well finish adding an 8-byte ID table and adding this one chip
manually.

>> BTW, I actually have some patches queued up that I need to complete
>> properly, where I'm trying to clean up and update NAND detection a
>> little. So I basically have this same patch, but rebased on top of
>> some other changes I have ready... I don't mind taking a revised
>> version of yours, or I can try to clean up my series and send it out
>> soon.
>>
> I hope you send it out as soon as possible. :)

I'm trying, but I get very busy with other things :)

Anyway, I'm not very comfortable with the heuristics for this Hynix
decode method, so I don't want to post another patch for it yet. If it
helps, I can submit the other patches soon; they amount to mostly
cleanup (IMO). But at least that way, whoever works on NAND detection
in the near future has the same code base.

Brian

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Huang Shijie]

Hi Brian:
> BTW, I actually have some patches queued up that I need to complete
> properly, where I'm trying to clean up and update NAND detection a
> little. So I basically have this same patch, but rebased on top of
Did you finish your nand detection patches?
I really hope this Hynix's nand chip could be supported by the mtd code.
If your plan is blocked,  could i send out a new version of this patch?

Best Regards
Huang Shijie

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

On Fri, Aug 17, 2012 at 1:43 AM, Huang Shijie <b32955@freescale.com> wrote:
>> BTW, I actually have some patches queued up that I need to complete
>> properly, where I'm trying to clean up and update NAND detection a
>> little. So I basically have this same patch, but rebased on top of
>
> Did you finish your nand detection patches?
> I really hope this Hynix's nand chip could be supported by the mtd code.
> If your plan is blocked,  could i send out a new version of this patch?

Well, I have some working patches, but they aren't entirely ready for
submission. Other things have taken priority. Sorry about the delay,
since you were probably waiting...

Yes, please go ahead if you have a good submission; but I thought we
agreed there was no good way to detect these chips without practically
hard-coding the ID + parameter configuration. If that's the case, then
maybe I should speed up my patches (sitting around here as well...)
regarding a full 8-byte ID detection table. I think this kind of table
will be necessary to support some chips anyway.

Brian

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Huang Shijie]

于 2012年08月18日 10:51, Brian Norris 写道:
> agreed there was no good way to detect these chips without practically
> hard-coding the ID + parameter configuration. If that's the case, then
Yes, we have to hard-coding the ID for Hynix. But there is already a 
hard-coding for SAMSUNG.

so I think the hard-coding is accecpted, aren't we?

thanks
Huang Shijie

Re: [PATCH] mtd : add parsing code for one kind of Hynix's nand chip

[Brian Norris]

On Sun, Aug 19, 2012 at 11:01 PM, Huang Shijie <b32955@freescale.com> wrote:
> 于 2012年08月18日 10:51, Brian Norris 写道:
>
>> agreed there was no good way to detect these chips without practically
>> hard-coding the ID + parameter configuration. If that's the case, then
>
> Yes, we have to hard-coding the ID for Hynix. But there is already a
> hard-coding for SAMSUNG.

It's not exactly hard-coding, as there is a recognizable pattern that
covers a class of different chips: all Samsung MLC with 6-byte ID
strings.

> so I think the hard-coding is accecpted, aren't we?

We try to do as little hard-coding as possible, as it can blow up as a
larger maintainability problem. But as we see, we may need real
hard-coding.

Brian