2013-03-02 14:19:07

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH V3 1/3] mtd: add new fields to nand_flash_dev{}

On Mon, 2013-01-28 at 12:57 +0800, Huang Shijie wrote:
> As time goes on, we begin to meet the situation that we can not get enough
> information from some nand chips's id data. Take some Toshiba's nand chips
> for example. I have 4 Toshiba's nand chips in my hand:
> TC58NVG2S0F, TC58NVG3S0F, TC58NVG5D2, TC58NVG6D2
>
> When we read these chips' datasheets, we will get the geometry of these chips:
> TC58NVG2S0F : 4096 + 224
> TC58NVG3S0F : 4096 + 232
> TC58NVG5D2 : 8192 + 640
> TC58NVG6D2 : 8192 + 640
>
> But we can not parse out the correct oob size for these chips from the id data.

Very good start of the commit message - you clearly defined the problem.

> So it is time to add some new fields to the nand_flash_dev{}, and update the
> detection mechanisms.

But continued with very poor description of how you address the problem.
Please, provide a better description.

> This patch just adds some new fields to the nand_flash_dev{}:
> @id[8] : the 8 bytes id data.

id[8] = 8 bytes id data, just like password[5] = 5 bytes of password
data. Please, provide a better commentary.

> @id_len: the valid length of the id data.

What does "valid" mean? Are "invalid" parts?

> @oobsize: the oob size.

Try to invent a better comment.

Huang, it is not that I am trying to be difficult, but I truly do not
understand how you are solving the issue.

--
Best Regards,
Artem Bityutskiy


2013-03-04 03:06:20

by Huang Shijie

[permalink] [raw]
Subject: Re: [PATCH V3 1/3] mtd: add new fields to nand_flash_dev{}

于 2013年03月02日 22:19, Artem Bityutskiy 写道:
> On Mon, 2013-01-28 at 12:57 +0800, Huang Shijie wrote:
>> As time goes on, we begin to meet the situation that we can not get enough
>> information from some nand chips's id data. Take some Toshiba's nand chips
>> for example. I have 4 Toshiba's nand chips in my hand:
>> TC58NVG2S0F, TC58NVG3S0F, TC58NVG5D2, TC58NVG6D2
>>
>> When we read these chips' datasheets, we will get the geometry of these chips:
>> TC58NVG2S0F : 4096 + 224
>> TC58NVG3S0F : 4096 + 232
>> TC58NVG5D2 : 8192 + 640
>> TC58NVG6D2 : 8192 + 640
>>
>> But we can not parse out the correct oob size for these chips from the id data.
> Very good start of the commit message - you clearly defined the problem.
>
>> So it is time to add some new fields to the nand_flash_dev{}, and update the
>> detection mechanisms.
> But continued with very poor description of how you address the problem.
> Please, provide a better description.

sorry. It's my fault.

I will add more description in the next version.
>> This patch just adds some new fields to the nand_flash_dev{}:
>> @id[8] : the 8 bytes id data.
> id[8] = 8 bytes id data, just like password[5] = 5 bytes of password
> data. Please, provide a better commentary.
>
okay.

>> @id_len: the valid length of the id data.
> What does "valid" mean? Are "invalid" parts?
yes.

some nand chips may only have 5 valid bytes in the 8 bytes id data
which is read
out by the READ ID(0x90) command. for example, the 8 bytes id data may
like this:
A1, A2, A3, A4, A5, A1, A2, A3

The last three bytes are just the repeat of the first three bytes.

Of course, we can remove this field, in other word, treat the last three
bytes as the
valid bytes too.
>> @oobsize: the oob size.
> Try to invent a better comment.
>
> Huang, it is not that I am trying to be difficult, but I truly do not
not at all, it's my fault.
> understand how you are solving the issue.
>
My method is:
Use the 8 bytes id data (which is read out by READ ID command) as the
keyword.
The 8bytes id data is unique for each nand chip. Do we meet two
different nand chips
have the same 8 bytes id data? I afraid not.

Since we can not parse out the oob size for these Toshiba nand chips,
we can add a
new field oob_size to store the right oob size for the nand chips.
that's why i add
the @oobsize field.

Are you clear now?

thanks
Huang Shijie