linux-kernel - Re: [PATCH 3/3] mtd: rawnand/ingenic: JZ4740 needs 'oob

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]

Message-ID: <20211015081447.25836fc7@xps13>
Date:   Fri, 15 Oct 2021 08:14:47 +0200
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Harvey Hunt <harveyhuntnexus@...il.com>, list@...ndingux.net,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-mips@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH 3/3] mtd: rawnand/ingenic: JZ4740 needs 'oob_first' read
 page function

Hi Paul,

paul@...pouillou.net wrote on Sat,  9 Oct 2021 20:49:52 +0200:

> The ECC engine on the JZ4740 SoC requires the ECC data to be read before
> the page; using the default page reading function does not work. Indeed,
> the old JZ4740 NAND driver (removed in 5.4) did use the 'OOB first' flag
> that existed back then.
> 
> Use the newly created nand_read_page_hwecc_oob_first() to address this
> issue.
> 
> This issue was not found when the new ingenic-nand driver was developed,
> most likely because the Device Tree used had the nand-ecc-mode set to
> "hw_oob_first", which seems to not be supported anymore.

I would rename both the boolean and the helper "*_access_oob_first"
which seems more precise.

> 
> Cc: <stable@...r.kernel.org> # v5.2
> Fixes: a0ac778eb82c ("mtd: rawnand: ingenic: Add support for the JZ4740")
> Signed-off-by: Paul Cercueil <paul@...pouillou.net>
> ---
>  drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> index 0e9d426fe4f2..b18861bdcdc8 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> @@ -32,6 +32,7 @@ struct jz_soc_info {
>  	unsigned long addr_offset;
>  	unsigned long cmd_offset;
>  	const struct mtd_ooblayout_ops *oob_layout;
> +	bool oob_first;
>  };
>  
>  struct ingenic_nand_cs {
> @@ -240,6 +241,9 @@ static int ingenic_nand_attach_chip(struct nand_chip *chip)
>  	if (chip->bbt_options & NAND_BBT_USE_FLASH)
>  		chip->bbt_options |= NAND_BBT_NO_OOB;
>  
> +	if (nfc->soc_info->oob_first)
> +		chip->ecc.read_page = nand_read_page_hwecc_oob_first;
> +
>  	/* For legacy reasons we use a different layout on the qi,lb60 board. */
>  	if (of_machine_is_compatible("qi,lb60"))
>  		mtd_set_ooblayout(mtd, &qi_lb60_ooblayout_ops);
> @@ -534,6 +538,7 @@ static const struct jz_soc_info jz4740_soc_info = {
>  	.data_offset = 0x00000000,
>  	.cmd_offset = 0x00008000,
>  	.addr_offset = 0x00010000,
> +	.oob_first = true,
>  };
>  
>  static const struct jz_soc_info jz4725b_soc_info = {


Thanks,
Miquèl