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:   Fri, 17 Sep 2021 19:01:54 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Jan Hoffmann <jan@....eu>
Cc:     Daniel Kestrel <kestrelseventyfour@...il.com>,
        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

Hi Jan,

jan@....eu wrote on Thu, 16 Sep 2021 21:38:26 +0200:

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

I am not sure to understand your message as answer to this thread.
There are two problems here:
1/ The DT values not being taken into account
2/ Kestrel's issue with two different integrations with no way to
distinguish between them.

1/ Has already been fixed (at least that is what I think)
2/ Cannot easily be fixed and I don't think there is anything we can do
besides the manufacturer "fixing" the board description.

Am I missing something?

Thanks,
Miquèl