linux-kernel - Re: [RFC PATCH 0/2] mtd: rawnand: support MT29F1G08ABAFAWP-ITE:F

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0caaea1b4f5a4b45b436833424c4c8c8@svr-chch-ex1.atlnz.lc>
Date:   Tue, 19 Jun 2018 01:44:24 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
CC:     "boris.brezillon@...tlin.com" <boris.brezillon@...tlin.com>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "computersforpeace@...il.com" <computersforpeace@...il.com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 0/2] mtd: rawnand: support MT29F1G08ABAFAWP-ITE:F

On 19/06/18 12:35, Chris Packham wrote:
> On 19/06/18 01:15, Miquel Raynal wrote:
>> Hi Chris,
>>
>> On Mon, 18 Jun 2018 16:52:53 +1200, Chris Packham
>> <chris.packham@...iedtelesis.co.nz> wrote:
>>
>>> Hi,
>>>
>>> I'm looking at adding support for the Micron MT29F1G08ABAFAWP-ITE:F chip
>>> to one of our boards which uses the Marvell NFCv2 controller.
>>>
>>> This particular chip is a bit odd in that the datasheet states support
>>> for ONFI 1.0 but the revision number field is 00 00. It also is marked
>>> ABAFA but reports internally as ABAGA. Finally it has internal 8-bit ECC
>>> which cannot be disabled.
>>
>> Boris and I agree that in this case, the chip should not be probed if
>> ecc->type != ON_DIE (and eventually NONE).
>>
>> This should be handled in the Micron driver.
>>
>> Also, what is the returned value of micron_supports_on_die_ecc() (with
>> patch 1/2)?
> 
> micron_supports_on_die_ecc() returns MICRON_ON_DIE_UNSUPPORTED.
> Technically this chip should be MICRON_ON_DIE_MANDATORY since it can't
> be disabled but that wouldn't be much help since that would still result
> in -EINVAL. I'll dig into micron_supports_on_die_ecc() and see if I can
> find something in the datasheet to use.
> 

Some further debugging. Nothing (in 4.17) calls 
set_bit(ONFI_FEATURE_ON_DIE_ECC) so I don't think 
micron_supports_on_die_ecc() can return anything other than 
MICRON_ON_DIE_UNSUPPORTED, unless I'm missing something for how the 
{get,set}_feature_list is populated.

With the onfi.version fix and the following

--- a/drivers/mtd/nand/raw/nand_micron.c
+++ b/drivers/mtd/nand/raw/nand_micron.c
@@ -66,7 +66,9 @@ static int micron_nand_onfi_init(struct nand_chip *chip)

         if (p->supports_set_get_features) {
                 set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->set_feature_list);
+               set_bit(ONFI_FEATURE_ON_DIE_ECC, p->set_feature_list);
                 set_bit(ONFI_FEATURE_ADDR_READ_RETRY, p->get_feature_list);
+               set_bit(ONFI_FEATURE_ON_DIE_ECC, p->get_feature_list);
         }
@@ -240,7 +246,7 @@ static int micron_supports_on_die_ecc(struct 
nand_chip *chip)
          * Some Micron NANDs have an on-die ECC of 4/512, some other
-         * 8/512. We only support the former.
+         * 8/512.
          */
-       if (chip->ecc_strength_ds != 4)
+       if (chip->ecc_strength_ds != 4 && chip->ecc_strength_ds != 8)
                 return MICRON_ON_DIE_UNSUPPORTED;

I can get micron_supports_on_die_ecc() to return MICRON_ON_DIE_SUPPORTED.

Then I run into a problem with the marvell_nand.c which currently 
doesn't handle NAND_ECC_ON_DIE which is easily fixed.

But then I have the issue that I need to handle systems with either type 
of ECC scheme ("on-die" or "hw") which I'm not sure is even possible 
within the dts.

I'll re-base against 4.18-rc1 and send what I have so-far.