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: Mon, 6 May 2024 17:51:06 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Sascha Hauer <s.hauer@...gutronix.de>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
 <vigneshr@...com>, linux-mtd@...ts.infradead.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] mtd: nand: mxc_nand: support software ECC

Hi Miquel,

miquel.raynal@...tlin.com wrote on Mon, 6 May 2024 16:05:08 +0200:

> Hi Sascha,
> 
> s.hauer@...gutronix.de wrote on Wed, 17 Apr 2024 09:13:30 +0200:
> 
> > To support software ECC we still need the driver provided read_oob,
> > read_page_raw and write_page_raw ops, so set them unconditionally
> > no matter which engine_type we use. The OOB layout on the other hand
> > represents the layout the i.MX ECC hardware uses, so set this only
> > when NAND_ECC_ENGINE_TYPE_ON_HOST is in use.
> > 
> > With these changes the driver can be used with software BCH ECC which
> > is useful for NAND chips that require a stronger ECC than the i.MX
> > hardware supports.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> > ---
> >  drivers/mtd/nand/raw/mxc_nand.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> > index fc70c65dea268..f44c130dca18d 100644
> > --- a/drivers/mtd/nand/raw/mxc_nand.c
> > +++ b/drivers/mtd/nand/raw/mxc_nand.c
> > @@ -1394,15 +1394,16 @@ static int mxcnd_attach_chip(struct nand_chip *chip)
> >  	chip->ecc.bytes = host->devtype_data->eccbytes;
> >  	host->eccsize = host->devtype_data->eccsize;
> >  	chip->ecc.size = 512;
> > -	mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
> > +
> > +	chip->ecc.read_oob = mxc_nand_read_oob;
> > +	chip->ecc.read_page_raw = mxc_nand_read_page_raw;
> > +	chip->ecc.write_page_raw = mxc_nand_write_page_raw;

A second thought on this. Maybe you should consider keeping these for
on-host operations only.

The read/write_page_raw operations are supposed to detangle the data
organization to show a proper [all data][all oob] organization to the
user. But of course if the data is stored differently when using
software ECC, you'll expect the implementation to be different (and the
core provides such helpers, even though in your case they use RNDOUT
which is not yet supported).

> >  
> >  	switch (chip->ecc.engine_type) {
> >  	case NAND_ECC_ENGINE_TYPE_ON_HOST:
> > +		mtd_set_ooblayout(mtd, host->devtype_data->ooblayout);
> >  		chip->ecc.read_page = mxc_nand_read_page;
> > -		chip->ecc.read_page_raw = mxc_nand_read_page_raw;
> > -		chip->ecc.read_oob = mxc_nand_read_oob;
> >  		chip->ecc.write_page = mxc_nand_write_page_ecc;
> > -		chip->ecc.write_page_raw = mxc_nand_write_page_raw;
> >  		chip->ecc.write_oob = mxc_nand_write_oob;
> >  		break;  
> 
> You also need to disable the ECC engine by default (and then you're
> free to use the raw page helpers).
> 
> I thought patch 4 was needed for this patch to work, do you confirm?
> Otherwise with the little change requested I might also merge this one.
> 
> Thanks, Miquèl


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ