lkml.org 
[lkml]   [2017]   [Oct]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH -next v2] mtd: nand: Add support for Toshiba BENAND (Built-in ECC NAND)
Date
On 2017/09/27 6:15, Boris Brezillon wrote:
> On Tue, 26 Sep 2017 18:18:04 +0900
> KOBAYASHI Yoshitake <yoshitake.kobayashi@toshiba.co.jp> wrote:
>
>> On 2017/09/21 16:28, Boris Brezillon wrote:
>>> On Thu, 21 Sep 2017 14:32:02 +0900
>>> KOBAYASHI Yoshitake <yoshitake.kobayashi@toshiba.co.jp> wrote:
>>>
>>>> This patch enables support for Toshiba BENAND.
>>>> The current implementation does not support vondor specific command
>>>
>>> ^ vendor
>>>
>>>> TOSHIBA_NAND_CMD_ECC_STATUS. I would like to add the command, when
>>>> the exec_op() [1] infrastructure is implemented.
>>>
>>> It's not a good idea to reference a branch that is likely to disappear
>>> in a commit message. Just say that you can't properly support the
>>> TOSHIBA_NAND_CMD_ECC_STATUS operation right and that it might be
>>> addressed in the future.
>>
>> Thanks. I'll change the comment.
>>
>>>> + */
>>>> + if (status & NAND_STATUS_FAIL) {
>>>> + /* uncorrectable */
>>>> + mtd->ecc_stats.failed++;
>>>> + } else if (status & TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED) {
>>>> + /* correctable */
>>>> + max_bitflips = mtd->bitflip_threshold;
>>>> + mtd->ecc_stats.corrected += max_bitflips;
>>>> + }
>>>
>>> Is this working correctly when you read more than one ECC chunk? The
>>> ECC step size is 512 bytes and the page is bigger than that, which means
>>> you have more than one ECC chunk per page. What happens to the
>>> NAND_STATUS_FAIL flag if the first chunk is uncorrectable but
>>> following ones are correctable (or do not contain bitflips at all)?
>>
>> As you mentioned, the ECC step size is 512 byte. But when 0x70 command is used
>> a simplified ECC status per page is generated.
>
> I'm fine with that as long as uncorrectable errors are detected
> correctly and TOSHIBA_NAND_STATUS_REWRITE_RECOMMENDED is set as soon as
> one of the ECC chunk exposes too many bitflips.
>
>> In case the first chunk is uncorrectable but the following ones are correctable,
>> the 0x70 command can only check the status of the uncorrectable one.
>
> Sounds good.
>
>> Each ECC chunk status can be checked by using the 0x7A command.
>>
>>>> @@ -39,9 +105,43 @@ static void toshiba_nand_decode_id(struct nand_chip *chip)
>>>>
>>>> static int toshiba_nand_init(struct nand_chip *chip)
>>>> {
>>>> + struct mtd_info *mtd = nand_to_mtd(chip);
>>>> +
>>>> if (nand_is_slc(chip))
>>>> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE;
>>>>
>>>> + if (nand_is_slc(chip) && (chip->id.data[4] & 0x80)) {
>>>> + /* BENAND */
>>>> +
>>>> + /*
>>>> + * We can't disable the internal ECC engine, the user
>>>> + * has to use on-die ECC, there is no alternative.
>>>> + */
>>>> + if (chip->ecc.mode != NAND_ECC_ON_DIE) {
>>>> + pr_err("On-die ECC should be selected.\n");
>>>> + return -EINVAL;
>>>> + }
>>>
>>> According to your previous explanation that's not exactly true. Since
>>> ECC bytes are stored in a separate area, the user can decide to use
>>> another mode without trouble. Just skip the BENAND initialization when
>>> mode != NAND_ECC_ON_DIE and we should be good, or am I missing something?
>>
>> I am asking to product department to confirm it.
>
> I'm almost sure this is the case ;-).

According to the command sequence written in BENAND's datasheet, the status
of the internal ECC must be checked after reading. To do that, ecc.mode has been
set to NAND_ECC_ON_DIE and the status of the internal ECC is checked through
the 0x70 or 0x7A command. That's the reason we are returning EINVAL here.

-- Yoshi

\
 
 \ /
  Last update: 2017-10-05 09:25    [W:0.055 / U:42.536 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site