linux-kernel - Re: [PATCH v2] mtd: rawnand: xway: No hardcoded ECC engine, use device tree setting

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]

Date:   Thu, 16 Sep 2021 21:38:26 +0200
From:   Jan Hoffmann <jan@....eu>
To:     Daniel Kestrel <kestrelseventyfour@...il.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Cc:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] mtd: rawnand: xway: No hardcoded ECC engine, use
 device tree setting

Hello,

Configuration of the ECC engine type using device tree has actually
worked before. I am using OpenWrt on a Fritzbox 7362 SL, which has a
Micron 29F1G08ABADA flash chip. The bootloader of the device uses on-die
ECC, so that has to be used for Linux as well. It is configured in DTS
using "nand-ecc-mode = "on-die";". This worked fine with kernel 5.4.
However, after switching to kernel 5.10 it is ignored and software ECC
is used instead.

If I understand this correctly, the situation is as follows:

Originally, xway-nand did set defaults for ECC mode and algorithm, but
different values could be configured using device tree.

Commit d7157ff49a5b ("mtd: rawnand: Use the ECC framework user input
parsing bits") broke these default values, as the ECC algorithm is now
unconditionally set from the user configuration in rawnand_dt_init.
Previously, the default value was only overwritten if the device tree
actually contained a value.

This is fixed in d525914b5bd8 ("mtd: rawnand: xway: Move the ECC
initialization to ->attach_chip()"). However, this makes it impossible
to configure the ECC engine type in the device tree, as it is now
overwritten by the default value in xway_attach_chip.

I am not sure if this patch is the best approach for fixing this, as it
would again cause breakage for anyone who relies on the existing
default value. And this kind of breakage seems to have been the reason
for moving the default values to attach_chip in the first place (see
https://lore.kernel.org/lkml/20201105084939.72ea6bfd@xps13/ ).

As similar changes were applied to other NAND drivers, the same issue
probably also exists there. Maybe it makes sense to add a proper fix
for all of them?

Thanks,
Jan