Messages in this thread Patch in this message | | | From | Chris Packham <> | Subject | Re: [RFC PATCH 0/2] mtd: rawnand: support MT29F1G08ABAFAWP-ITE:F | Date | Tue, 19 Jun 2018 01:44:24 +0000 |
| |
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@alliedtelesis.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.
| |