linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Miquel Raynal <miquel.raynal@bootlin.com>
To: "Lothar Waßmann" <LW@KARO-electronics.de>
Cc: Felix Matouschek <felix@matouschek.org>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] Bad ecc layout in drivers/mtd/nand/spi/xtx.c
Date: Tue, 4 Jul 2023 15:55:07 +0200	[thread overview]
Message-ID: <20230704155507.0643a856@xps-13> (raw)
In-Reply-To: <20230704123743.7f4c87af@ipc1.ka-ro>

Hi Lothar,

Thanks for the report.

LW@KARO-electronics.de wrote on Tue, 4 Jul 2023 12:37:43 +0200:

> 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?

These offsets do not reflect the protected area but the user area,
which can be used (at the users own risks). Somehow the only real user
is jffs2 upstream, and jffs2 was meant to be replaced a long time ago
by UBI which actually workaround-ed this limitation by not using the
OOB area at all. We do not have a real way to distinguish what is ECC
protected or not in the OOB area.

So unless you want to solve a real problem, I would advise to keep it
as it is.

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

      reply	other threads:[~2023-07-04 13:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-04 10:37 [RFC] Bad ecc layout in drivers/mtd/nand/spi/xtx.c Lothar Waßmann
2023-07-04 13:55 ` Miquel Raynal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230704155507.0643a856@xps-13 \
    --to=miquel.raynal@bootlin.com \
    --cc=LW@KARO-electronics.de \
    --cc=felix@matouschek.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).