Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 4 Jul 2023 12:37:43 +0200
From:   Lothar =?UTF-8?B?V2HDn21hbm4=?= <LW@KARO-electronics.de>
To:     Felix Matouschek <felix@matouschek.org>
Cc:     Miquel Raynal <miquel.raynal@bootlin.com>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: [RFC] Bad ecc layout in drivers/mtd/nand/spi/xtx.c
Message-ID: <20230704123743.7f4c87af@ipc1.ka-ro>
Organization: Ka-Ro electronics GmbH
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi,

while trying to add support for the 'XT26G01C' variant of the XTX
SPI-nand chip I noticed a flaw in the ECC layout of the existing
driver.
According to the  XT26G01A datasheet the first eight bytes of the OOB
area are not protected by ECC:
Offset      ECC  Bytes  Area               Description
800H 807H    No      8  Spare 4, Group E   This Area is not covered by internal ECC,
                                           800H is reserved for bad block mark        
808H 82FH   Yes 40 Spare 5 , Group F       User Meta Data Area covered by internal ECC.
830H 83FH    No 16 Spare 6 , Group G       ECC_EN=1: this area contains Internal ECC Data, Read-Only,
                                           Programming to this area will be ignored
                                           ECC_EN=0: this area is writable for user

But the driver defines bytes 1..47 as user OOB area:
|static int xt26g0xa_ooblayout_free(struct mtd_info *mtd, int section,
|				   struct mtd_oob_region *region)
|{
|	if (section)
|		return -ERANGE;
|
|	region->offset = 1;
|	region->length = 47;
|
|	return 0;
|}

IMO this should be:
|	region->offset = 8;
|	region->length = 40;
to have the whole user oob area protected by ECC.

Obviously this cannot simply be changed in the driver because it would
break access to flash that was programmed with the current parameters.
Since the data structures that provide the oob layout are completely
static and cannot be modified at runtime (e.g. depending on some DTB
property) the only way I see to provide a layout with ECC protection
for the whole user metadata would be a Kconfig option to select one or
the other layout at compile time.

Any comments or better ideas?


Lothar Waßmann
-- 
___________________________________________________________

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________