2011-05-11 16:25:28

by Nitin Garg

[permalink] [raw]
Subject: Bug in MTD NAND ONFI chipsize detection

Hi All,

The nand_flash_detect_onfi function in mtd/nand detects the NAND flash
device size using the ONFI parameters:
chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;

The lun_count is not taken into consideration due to which we detect
wrong size for Micron MT29F8G08ADADAH4 as it has 2 logical units.

We should change the chipsize calculation to:
chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
(uint64_t)le32_to_cpu(p->lun_count) * mtd->erasesize;

Pls suggest.

Regards,
Nitin Garg


2011-05-12 01:26:49

by Andrew Morton

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

On Wed, 11 May 2011 11:25:22 -0500 Nitin Garg <[email protected]> wrote:

> Hi All,
>
> The nand_flash_detect_onfi function in mtd/nand detects the NAND flash
> device size using the ONFI parameters:
> chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
>
> The lun_count is not taken into consideration due to which we detect
> wrong size for Micron MT29F8G08ADADAH4 as it has 2 logical units.
>
> We should change the chipsize calculation to:
> chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
> (uint64_t)le32_to_cpu(p->lun_count) * mtd->erasesize;
>
> Pls suggest.
>

Please send a tested, changelogged patch to fix it. Be sure to cc the
relevant maintainer and mailing list.

Thanks.

2011-05-12 06:54:39

by Nitin Garg

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

Here is the tested patch, pls apply.

Regards,
Nitin Garg

>From 1a73f1c3d066a491d0c806883788ab9abdc736f3 Mon Sep 17 00:00:00 2001
From: Nitin Garg <[email protected]>
Date: Thu, 12 May 2011 01:31:53 -0500
Subject: [PATCH] Fix ONFI NAND flash size detection by using number of
Logical Units in device


Signed-off-by: Nitin Garg <[email protected]>
---
drivers/mtd/nand/nand_base.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c54a4cb..cdf6015 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
mtd_info *mtd, struct nand_chip *chip,
mtd->writesize = le32_to_cpu(p->byte_per_page);
mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
- chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
+ chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
le32_to_cpu(p->lun_count) * mtd->erasesize;
busw = 0;
if (le16_to_cpu(p->features) & 1)
busw = NAND_BUSWIDTH_16;
--
1.5.5.6



On Wed, May 11, 2011 at 8:33 PM, Andrew Morton
<[email protected]> wrote:
> On Wed, 11 May 2011 11:25:22 -0500 Nitin Garg <[email protected]> wrote:
>
>> Hi All,
>>
>> The nand_flash_detect_onfi function in mtd/nand detects the NAND flash
>> device size using the ONFI parameters:
>> ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
>>
>> The lun_count is not taken into consideration due to which we detect
>> wrong size for Micron MT29F8G08ADADAH4 as it has 2 logical units.
>>
>> We should change the chipsize calculation to:
>> ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
>> (uint64_t)le32_to_cpu(p->lun_count) * mtd->erasesize;
>>
>> Pls suggest.
>>
>
> Please send a tested, changelogged patch to fix it. ?Be sure to cc the
> relevant maintainer and mailing list.
>
> Thanks.
>


Attachments:
0001-Fix-ONFI-NAND-flash-size-detection-by-using-number-o.patch (1.09 kB)

2011-05-12 07:05:39

by Nitin Garg

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

Here is the tested patch, pls apply.

>From 1a73f1c3d066a491d0c806883788ab9abdc736f3 Mon Sep 17 00:00:00 2001
From: Nitin Garg <[email protected]>
Date: Thu, 12 May 2011 01:31:53 -0500
Subject: [PATCH] Fix ONFI NAND flash size detection by using number of
Logical Units in device

Signed-off-by: Nitin Garg <[email protected]>
---
?drivers/mtd/nand/nand_base.c | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c54a4cb..cdf6015 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
mtd_info *mtd, struct nand_chip *chip,
? ? ? ?mtd->writesize = le32_to_cpu(p->byte_per_page);
? ? ? ?mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
? ? ? ?mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
- ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
mtd->erasesize;
+ ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
le32_to_cpu(p->lun_count) * mtd->erasesize;
? ? ? ?busw = 0;
? ? ? ?if (le16_to_cpu(p->features) & 1)
? ? ? ? ? ? ? ?busw = NAND_BUSWIDTH_16;
--
1.5.5.6

2011-05-12 07:16:09

by Nitin Garg

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

Why isn't my patch going through? re-sending again,

Signed-off-by: Nitin Garg <nitingarg98@xxxxxxxx>
---
drivers/mtd/nand/nand_base.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c54a4cb..cdf6015 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
mtd_info *mtd, struct nand_chip *chip,
mtd->writesize = le32_to_cpu(p->byte_per_page);
mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
- chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
+ chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
le32_to_cpu(p->lun_count) * mtd->erasesize;
busw = 0;
if (le16_to_cpu(p->features) & 1)
busw = NAND_BUSWIDTH_16;
--
1.5.5.6



On Thu, May 12, 2011 at 2:05 AM, Nitin Garg <[email protected]> wrote:
> Here is the tested patch, pls apply.
>
> From 1a73f1c3d066a491d0c806883788ab9abdc736f3 Mon Sep 17 00:00:00 2001
> From: Nitin Garg <[email protected]>
> Date: Thu, 12 May 2011 01:31:53 -0500
> Subject: [PATCH] Fix ONFI NAND flash size detection by using number of
> Logical Units in device
>
> Signed-off-by: Nitin Garg <[email protected]>
> ---
> ?drivers/mtd/nand/nand_base.c | ? ?2 +-
> ?1 files changed, 1 insertions(+), 1 deletions(-)
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index c54a4cb..cdf6015 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
> mtd_info *mtd, struct nand_chip *chip,
> ? ? ? ?mtd->writesize = le32_to_cpu(p->byte_per_page);
> ? ? ? ?mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
> ? ? ? ?mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> - ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
> mtd->erasesize;
> + ? ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
> le32_to_cpu(p->lun_count) * mtd->erasesize;
> ? ? ? ?busw = 0;
> ? ? ? ?if (le16_to_cpu(p->features) & 1)
> ? ? ? ? ? ? ? ?busw = NAND_BUSWIDTH_16;
> --
> 1.5.5.6
>

2011-05-12 07:54:32

by Matthieu CASTET

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

Hi,


What's the difference between one lun and multiple lun for mtd ?

Aren't any command to select the current lun ?

Matthieu


Nitin Garg a ?crit :
> Why isn't my patch going through? re-sending again,
>
> Signed-off-by: Nitin Garg <nitingarg98@xxxxxxxx>
> ---
> drivers/mtd/nand/nand_base.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
> index c54a4cb..cdf6015 100644
> --- a/drivers/mtd/nand/nand_base.c
> +++ b/drivers/mtd/nand/nand_base.c
> @@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
> mtd_info *mtd, struct nand_chip *chip,
> mtd->writesize = le32_to_cpu(p->byte_per_page);
> mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
> mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
> - chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
> + chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
> le32_to_cpu(p->lun_count) * mtd->erasesize;
> busw = 0;
> if (le16_to_cpu(p->features) & 1)
> busw = NAND_BUSWIDTH_16;

2011-05-12 12:34:29

by Nitin Garg

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

I do not see any diff for mtd.

Regards,
Nitin

On Thu, May 12, 2011 at 2:47 AM, Matthieu CASTET
<[email protected]> wrote:
> Hi,
>
>
> What's the difference between one lun and multiple lun for mtd ?
>
> Aren't any command to select the current lun ?
>
> Matthieu
>
>
> Nitin Garg a ?crit :
>> Why isn't my patch going through? re-sending again,
>>
>> Signed-off-by: Nitin Garg <nitingarg98@xxxxxxxx>
>> ---
>> ?drivers/mtd/nand/nand_base.c | ? ?2 +-
>> ?1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index c54a4cb..cdf6015 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
>> mtd_info *mtd, struct nand_chip *chip,
>> ? ? ? mtd->writesize = le32_to_cpu(p->byte_per_page);
>> ? ? ? mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
>> ? ? ? mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
>> - ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
>> + ? ? chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
>> le32_to_cpu(p->lun_count) * mtd->erasesize;
>> ? ? ? busw = 0;
>> ? ? ? if (le16_to_cpu(p->features) & 1)
>> ? ? ? ? ? ? ? busw = NAND_BUSWIDTH_16;
>

2011-05-12 13:56:40

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

On Thu, 2011-05-12 at 07:34 -0500, Nitin Garg wrote:
> I do not see any diff for mtd.

Please, send a patch with a better commit message clearly explaining
which problem the bug fixes, and make sure the commit message is
understandable even by people who have not read the ONFI standard.

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

2011-05-12 13:57:12

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

On Thu, 2011-05-12 at 16:53 +0300, Artem Bityutskiy wrote:
> which problem the bug fixes, and make sure the commit message is

Sorry, s/the bug fixes/the patch fixes/

--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

2011-05-12 15:19:45

by Nitin Garg

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

I hope this is better and acceptable, pls review.

As per ONFI standard, the size of NAND Flash device is Number of
data bytes per page * Number of pages per block * Number of blocks
per logical unit * Number of logical units. The nand_flash_detect_onfi
function is missing the Number of logical units due to which the kernel
detects wrong size of Micron MT29F8G08ADADAH4 NAND Flash as
it has 2 logical units. This patch fixes this issue by multiplying
p->lun_count for chipsize.

Signed-off-by: Nitin Garg <[email protected]>
---
drivers/mtd/nand/nand_base.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index c54a4cb..cdf6015 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
mtd_info *mtd, struct nand_chip *chip,
mtd->writesize = le32_to_cpu(p->byte_per_page);
mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
- chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
mtd->erasesize;
+ chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
le32_to_cpu(p->lun_count) * mtd->erasesize;
busw = 0;
if (le16_to_cpu(p->features) & 1)
busw = NAND_BUSWIDTH_16;
--
1.5.5.6


Attachments:
onfi_nand_flash_chipsize.patch (1.43 kB)

2011-05-12 15:40:33

by Matthieu CASTET

[permalink] [raw]
Subject: Re: Bug in MTD NAND ONFI chipsize detection

Nitin Garg a ?crit :
> I do not see any diff for mtd.
>
ONFI spec say [1].

Why this doesn't apply to mtd ?


[1]
3.1.2. Logical Unit Selection
Logical units within one target share a single data bus with the host. The host
shall ensure that
only one LUN is selected for data output to the host at any particular point in
time to avoid bus
contention.
The host selects a LUN for future data output by issuing a Read Status Enhanced
command to
that LUN. The Read Status Enhanced command shall deselect the output path for
all LUNs that
are not addressed by the command. The page register selected for output within
the LUN is
determined by the previous Read (Cache) commands issued, and is not impacted by Read
Status Enhanced.



> Regards,
> Nitin
>
> On Thu, May 12, 2011 at 2:47 AM, Matthieu CASTET
> <[email protected]> wrote:
>> Hi,
>>
>>
>> What's the difference between one lun and multiple lun for mtd ?
>>
>> Aren't any command to select the current lun ?
>>
>> Matthieu
>>
>>
>> Nitin Garg a ?crit :
>>> Why isn't my patch going through? re-sending again,
>>>
>>> Signed-off-by: Nitin Garg <nitingarg98@xxxxxxxx>
>>> ---
>>> drivers/mtd/nand/nand_base.c | 2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>>> index c54a4cb..cdf6015 100644
>>> --- a/drivers/mtd/nand/nand_base.c
>>> +++ b/drivers/mtd/nand/nand_base.c
>>> @@ -2892,7 +2892,7 @@ static int nand_flash_detect_onfi(struct
>>> mtd_info *mtd, struct nand_chip *chip,
>>> mtd->writesize = le32_to_cpu(p->byte_per_page);
>>> mtd->erasesize = le32_to_cpu(p->pages_per_block) * mtd->writesize;
>>> mtd->oobsize = le16_to_cpu(p->spare_bytes_per_page);
>>> - chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) * mtd->erasesize;
>>> + chip->chipsize = (uint64_t)le32_to_cpu(p->blocks_per_lun) *
>>> le32_to_cpu(p->lun_count) * mtd->erasesize;
>>> busw = 0;
>>> if (le16_to_cpu(p->features) & 1)
>>> busw = NAND_BUSWIDTH_16;
>