diff mbox

[U-Boot,NEXT,v1,2/7] NAND: added NAND type to nand_ids

Message ID 1346918700-32429-3-git-send-email-sbabic@denx.de
State Deferred
Delegated to: Scott Wood
Headers show

Commit Message

Stefano Babic Sept. 6, 2012, 8:04 a.m. UTC
Signed-off-by: Stefano Babic <sbabic@denx.de>
---
 drivers/mtd/nand/nand_ids.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Scott Wood Sept. 6, 2012, 11:19 p.m. UTC | #1
On 09/06/2012 03:04 AM, Stefano Babic wrote:
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> ---
>  drivers/mtd/nand/nand_ids.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index 3953549..fe75686 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -131,6 +131,8 @@ const struct nand_flash_dev nand_flash_ids[] = {
>  	/* 128 Gigabit */
>  	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
>  	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
> +	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
> +		LP_OPTIONS},
>  	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
>  	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
>  
> 

Why does this NAND chip need things specified that are zeroes for other
chips?

-Scott
Stefano Babic Sept. 7, 2012, 9:12 a.m. UTC | #2
On 07/09/2012 01:19, Scott Wood wrote:
> On 09/06/2012 03:04 AM, Stefano Babic wrote:
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> ---

Hi Scott,

>>  drivers/mtd/nand/nand_ids.c |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
>> index 3953549..fe75686 100644
>> --- a/drivers/mtd/nand/nand_ids.c
>> +++ b/drivers/mtd/nand/nand_ids.c
>> @@ -131,6 +131,8 @@ const struct nand_flash_dev nand_flash_ids[] = {
>>  	/* 128 Gigabit */
>>  	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
>>  	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
>> +	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
>> +		LP_OPTIONS},
>>  	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
>>  	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
>>  
>>
> 
> Why does this NAND chip need things specified that are zeroes for other
> chips?

At least on this board with MX35, the chip cannot be recognized.
Manufacturer ID and device ID are read flawlessly, but then u-boot fails
to get the correct geometry. Setting explicitely the values, I can then
read / write into the NAND without any problem. It can be more a problem
related to the specific MXC NAND driver (mxc_nand.c).

Regards,
Stefano
Stefano Babic Sept. 7, 2012, 3:23 p.m. UTC | #3
On 07/09/2012 11:12, Stefano Babic wrote:
> On 07/09/2012 01:19, Scott Wood wrote:
>> On 09/06/2012 03:04 AM, Stefano Babic wrote:
>>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>>> ---
> 
> Hi Scott,
> 
>>>  drivers/mtd/nand/nand_ids.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
>>> index 3953549..fe75686 100644
>>> --- a/drivers/mtd/nand/nand_ids.c
>>> +++ b/drivers/mtd/nand/nand_ids.c
>>> @@ -131,6 +131,8 @@ const struct nand_flash_dev nand_flash_ids[] = {
>>>  	/* 128 Gigabit */
>>>  	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
>>>  	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
>>> +	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
>>> +		LP_OPTIONS},
>>>  	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
>>>  	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
>>>  
>>>
>>
>> Why does this NAND chip need things specified that are zeroes for other
>> chips?
> 
> At least on this board with MX35, the chip cannot be recognized.
> Manufacturer ID and device ID are read flawlessly, but then u-boot fails
> to get the correct geometry. Setting explicitely the values, I can then
> read / write into the NAND without any problem. It can be more a problem
> related to the specific MXC NAND driver (mxc_nand.c).

It seems to me that the values returned by this flash cannot be
interpreted in nand_get_flash_type().

The values returned from a READ-ID command with address 0x00 are:

	0x2C 0x48 0x04 0x4A 0xA5,

I can really get these values from the flash, so the MXC controller gets
the correct data.

However, the code in nand_base.c (lines from 2718, so not-Samsung case)
parses the answer setting the NAND as a 16-bit device, but this is
really a 8bit device. I do not know the meaning of the answer, it is not
described in the datasheet.

Cheers,
Stefano
Scott Wood Sept. 7, 2012, 6:56 p.m. UTC | #4
On 09/07/2012 10:23 AM, Stefano Babic wrote:
> On 07/09/2012 11:12, Stefano Babic wrote:
>> On 07/09/2012 01:19, Scott Wood wrote:
>>> On 09/06/2012 03:04 AM, Stefano Babic wrote:
>>>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>>>> ---
>>
>> Hi Scott,
>>
>>>>  drivers/mtd/nand/nand_ids.c |    2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
>>>> index 3953549..fe75686 100644
>>>> --- a/drivers/mtd/nand/nand_ids.c
>>>> +++ b/drivers/mtd/nand/nand_ids.c
>>>> @@ -131,6 +131,8 @@ const struct nand_flash_dev nand_flash_ids[] = {
>>>>  	/* 128 Gigabit */
>>>>  	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
>>>>  	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
>>>> +	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
>>>> +		LP_OPTIONS},
>>>>  	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
>>>>  	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
>>>>  
>>>>
>>>
>>> Why does this NAND chip need things specified that are zeroes for other
>>> chips?
>>
>> At least on this board with MX35, the chip cannot be recognized.
>> Manufacturer ID and device ID are read flawlessly, but then u-boot fails
>> to get the correct geometry. Setting explicitely the values, I can then
>> read / write into the NAND without any problem. It can be more a problem
>> related to the specific MXC NAND driver (mxc_nand.c).
> 
> It seems to me that the values returned by this flash cannot be
> interpreted in nand_get_flash_type().
> 
> The values returned from a READ-ID command with address 0x00 are:
> 
> 	0x2C 0x48 0x04 0x4A 0xA5,
> 
> I can really get these values from the flash, so the MXC controller gets
> the correct data.
>
> However, the code in nand_base.c (lines from 2718, so not-Samsung case)
> parses the answer setting the NAND as a 16-bit device, but this is
> really a 8bit device. I do not know the meaning of the answer, it is not
> described in the datasheet.

Does the datasheet say anything about what the ID data is supposed to
look like and how to interpret it?  I tried to download it and Micron
shoved an NDA in my face.

What kind of chip is this?  Is the datasheet publicly available?

These threads seem relevant:
http://patchwork.ozlabs.org/patch/60042/
http://old.nabble.com/-U-Boot--Add-new-NAND-flash-td29858370.html

Does the chip support ONFI?

-Scott
Stefano Babic Sept. 10, 2012, 12:09 p.m. UTC | #5
On 07/09/2012 20:56, Scott Wood wrote:

Hi Scott,

>>>
>>>>>  drivers/mtd/nand/nand_ids.c |    2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
>>>>> index 3953549..fe75686 100644
>>>>> --- a/drivers/mtd/nand/nand_ids.c
>>>>> +++ b/drivers/mtd/nand/nand_ids.c
>>>>> @@ -131,6 +131,8 @@ const struct nand_flash_dev nand_flash_ids[] = {
>>>>>  	/* 128 Gigabit */
>>>>>  	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
>>>>>  	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
>>>>> +	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
>>>>> +		LP_OPTIONS},
>>>>>  	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
>>>>>  	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},
>>>>>  
>>>>>
>>>>
>>>> Why does this NAND chip need things specified that are zeroes for other
>>>> chips?
>>>
>>> At least on this board with MX35, the chip cannot be recognized.
>>> Manufacturer ID and device ID are read flawlessly, but then u-boot fails
>>> to get the correct geometry. Setting explicitely the values, I can then
>>> read / write into the NAND without any problem. It can be more a problem
>>> related to the specific MXC NAND driver (mxc_nand.c).
>>
>> It seems to me that the values returned by this flash cannot be
>> interpreted in nand_get_flash_type().
>>
>> The values returned from a READ-ID command with address 0x00 are:
>>
>> 	0x2C 0x48 0x04 0x4A 0xA5,
>>
>> I can really get these values from the flash, so the MXC controller gets
>> the correct data.
>>
>> However, the code in nand_base.c (lines from 2718, so not-Samsung case)
>> parses the answer setting the NAND as a 16-bit device, but this is
>> really a 8bit device. I do not know the meaning of the answer, it is not
>> described in the datasheet.
> 
> Does the datasheet say anything about what the ID data is supposed to
> look like and how to interpret it?  I tried to download it and Micron
> shoved an NDA in my face.

I find no description in the datasheet.

> 
> What kind of chip is this?  Is the datasheet publicly available?
> 
> These threads seem relevant:
> http://patchwork.ozlabs.org/patch/60042/
> http://old.nabble.com/-U-Boot--Add-new-NAND-flash-td29858370.html

It is the same case,  as I can see, with the same chip.

> 
> Does the chip support ONFI?

The chip supports ONFI, but it seems the i.MX driver does not. Quite as
described in http://patchwork.ozlabs.org/patch/60042/. READ-ID is always
sent with address 0, I do not know if we can convince the driver to send
the address.

Regards,
Stefano
Scott Wood Sept. 10, 2012, 11:18 p.m. UTC | #6
On 09/10/2012 07:09 AM, Stefano Babic wrote:
> On 07/09/2012 20:56, Scott Wood wrote:
>> What kind of chip is this?  Is the datasheet publicly available?
>>
>> These threads seem relevant:
>> http://patchwork.ozlabs.org/patch/60042/
>> http://old.nabble.com/-U-Boot--Add-new-NAND-flash-td29858370.html
> 
> It is the same case,  as I can see, with the same chip.
> 
>>
>> Does the chip support ONFI?
> 
> The chip supports ONFI, but it seems the i.MX driver does not. Quite as
> described in http://patchwork.ozlabs.org/patch/60042/. READ-ID is always
> sent with address 0, I do not know if we can convince the driver to send
> the address.

How did Linux end up resolving this?  I think it'd be better to have the
Micron decoding logic from that patch, than to introduce a special
addition to the ID table for this one chip, which might not be correct
for all chips with that ID byte.

Or, we could treat it as information to be supplied by platform code (or
the device tree).

-Scott
Eric Benard Sept. 23, 2012, 7:01 p.m. UTC | #7
Hi Stefano,

Le Mon, 10 Sep 2012 14:09:21 +0200,
Stefano Babic <sbabic@denx.de> a écrit :
> The chip supports ONFI, but it seems the i.MX driver does not. Quite as
> described in http://patchwork.ozlabs.org/patch/60042/. READ-ID is always
> sent with address 0, I do not know if we can convince the driver to send
> the address.

to add ONFI support to i.MX's driver, you can check this patch (in
barebox, a similar patch for Linux is cooking, actually tested on
i.MX53 and i.MX25) :
http://git.pengutronix.de/?p=barebox.git;a=commit;h=632c45795065e6a7471ab82be38e808eb6204341

Eric
Stefano Babic Sept. 24, 2012, 8:29 a.m. UTC | #8
On 23/09/2012 21:01, Eric Bénard wrote:
> Hi Stefano,
> 
> Le Mon, 10 Sep 2012 14:09:21 +0200,
> Stefano Babic <sbabic@denx.de> a écrit :
>> The chip supports ONFI, but it seems the i.MX driver does not. Quite as
>> described in http://patchwork.ozlabs.org/patch/60042/. READ-ID is always
>> sent with address 0, I do not know if we can convince the driver to send
>> the address.
> 
> to add ONFI support to i.MX's driver, you can check this patch (in
> barebox, a similar patch for Linux is cooking, actually tested on
> i.MX53 and i.MX25) :
> http://git.pengutronix.de/?p=barebox.git;a=commit;h=632c45795065e6a7471ab82be38e808eb6204341

Thanks Eric, I look at it

Regards,
Stefano
Stefano Babic Oct. 1, 2012, 7:02 a.m. UTC | #9
On 11/09/2012 01:18, Scott Wood wrote:
> On 09/10/2012 07:09 AM, Stefano Babic wrote:
>> On 07/09/2012 20:56, Scott Wood wrote:
>>> What kind of chip is this?  Is the datasheet publicly available?
>>>
>>> These threads seem relevant:
>>> http://patchwork.ozlabs.org/patch/60042/
>>> http://old.nabble.com/-U-Boot--Add-new-NAND-flash-td29858370.html
>>
>> It is the same case,  as I can see, with the same chip.
>>
>>>

Hi Scott,

>>> Does the chip support ONFI?
>>
>> The chip supports ONFI, but it seems the i.MX driver does not. Quite as
>> described in http://patchwork.ozlabs.org/patch/60042/. READ-ID is always
>> sent with address 0, I do not know if we can convince the driver to send
>> the address.
> 
> How did Linux end up resolving this?

I could take a closer look. The issue is not solved on linux, too. In
fact, the MXC driver in current kernel supports ONFI, but it should work
for newer version of the controller - I think with MX5.

On MX35, the NFC controller is V1.1, exactly as on MX25, and I have the
same issue reported by Matthias that you pointed out in the past
threads: ONFI command does not work.

>  I think it'd be better to have the
> Micron decoding logic from that patch, than to introduce a special
> addition to the ID table for this one chip, which might not be correct
> for all chips with that ID byte.

Agree. I am trying to contact Micron to get some information about it. I
have tried to decode the bytes on basis of other Micron's NAND, but then
bit 6 in byte 4 says that the NAND is a 16 bit device, and that is
wrong. Exactly the same wrong value that Matthias found some times ago.

Stefano
diff mbox

Patch

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index 3953549..fe75686 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -131,6 +131,8 @@  const struct nand_flash_dev nand_flash_ids[] = {
 	/* 128 Gigabit */
 	{"NAND 16GiB 1,8V 8-bit",	0x1A, 0, 16384, 0, LP_OPTIONS},
 	{"NAND 16GiB 3,3V 8-bit",	0x3A, 0, 16384, 0, LP_OPTIONS},
+	{"NAND 16GiB 3,3V 8-bit",	0x48, 4096, 16384, 0x100000,
+		LP_OPTIONS},
 	{"NAND 16GiB 1,8V 16-bit",	0x2A, 0, 16384, 0, LP_OPTIONS16},
 	{"NAND 16GiB 3,3V 16-bit",	0x4A, 0, 16384, 0, LP_OPTIONS16},