Re: [PATCH 00/11] mtd: add datasheet's ECC information to nand_chip{}

[Huang Shijie <b32955@freescale.com>]

于 2013年04月23日 05:35, Brian Norris 写道:
>> > Initial comments:
>> >
>> > What are the semantics for these new ecc_strength and ecc_size fields
>> > for chips which aren't encoded in nand_ids.c (or aren't
>> > ONFI-compliant)? We can't necessarily say that '0' means "no data
>> > provided" - it is perfectly valid to have a value of 0, e.g., for EZ
>> > NAND (or something like that) which manages its own ECC on-die. I
>> > think there are even a few words in the ONFI spec about it. Basically,
>> > I'm trying to point out that there is a difference between those chips
>> > which we can't/don't detect their ecc_{strength,size} and those which
>> > need no ECC.
> We need to straighten out these semantics now. Perhaps the fields in
> struct nand_chip can be signed, where -1 is "unknown" and 0 is "no ECC
> required."
>
IMHO, use -1 as "unknown" makes the things a little complexed.
You have to add a patch to fix the nand_ids or init these two fields in
nand_base.c.

For the ECC on-die case(sorry, i never meet this type of nand), I prefer
to add a
new flags to distinguish them.

For the "unknown" case, use the default 0.


>> > Also, I might consider different names for these fields so that you
>> > don't just rely on the in-code comments to differentiate the nand_chip
>> > and nand_ecc_ctrl values. Perhaps min_ecc_strength and ecc_size?
>> > (can't think of a great distinguishing name for the second one)
> The naming can be changed after your series, if it seems worth it.
If we use the min_ecc_strength, we'd better use the min_ecc_size too.


thanks
Huang Shijie