linux-kernel - RE: [EXT] Re: [PATCH 3/4] mtd: spinand: Add M70A series Micron SPI NAND devices

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: <MN2PR08MB6397062A37D39287E820A0D8B80D0@MN2PR08MB6397.namprd08.prod.outlook.com>
Date:   Tue, 21 Jan 2020 12:23:20 +0000
From:   "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@...ron.com>
To:     Miquel Raynal <miquel.raynal@...tlin.com>
CC:     Richard Weinberger <richard@....at>,
        Vignesh Raghavendra <vigneshr@...com>,
        Boris Brezillon <bbrezillon@...nel.org>,
        Frieder Schrempf <frieder.schrempf@...tron.de>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Shivamurthy Shastri <shiva.linuxworks@...il.com>
Subject: RE: [EXT] Re: [PATCH 3/4] mtd: spinand: Add M70A series Micron SPI
 NAND devices

Hi Miquel,

> 
> Hi Shiva,
> 
> This is remark common to the four patches: you miss the 'v2' prefix in
> the object.
> 

Sorry for this mistake.
I recognized this after sending out the patches.

> shiva.linuxworks@...il.com wrote on Sun, 19 Jan 2020 15:54:31 +0100:
> 
> > From: Shivamurthy Shastri <sshivamurthy@...ron.com>
> >
> > Add device table for M70A series Micron SPI NAND devices.
> >
> > While at it, disable the Continuous Read feature which is enabled by
> > default.
> 
> Can you please give us more detail on why this is an issue?

"Continuous Read" is the new feature added by the Micron for 
M70A series devices. If this feature is enabled, the READ command 
doesn't output the OOB area. The following short description
describes this feature.

Description:
If the Continuous Read feature is enabled, the device provides 
the capability to read the whole block with a single command.
However, the read command doesn't output the OOB area.

Read command behavior (if Continuous Read enabled):
The READ CACHE command doesn't require the starting column address.
The device always output the data starting from the first column of the
cache register, and once the end of the cache register reached, the data
output continues through the next page. With the continuous read mode,
it is possible to read out the entire block using a single READ command, and
once the end of the block reached, the output pins become High-Z state.

> 
> Shall we backport it to stable?

This is not a bug fix and applicable only to M70A series devices, there is no
need to backport.
(FYI, the previously enabled device was M79A series)

> 
> As a rule of thumb, when you start a sentence by "while at it" in a
> commit message and this is not a trivial change : split the patch,
> please. Unless this is really related and in this case explain how and
> why in the commit message.

Okay, I will explain in my next version.

> 
> >
> > Signed-off-by: Shivamurthy Shastri <sshivamurthy@...ron.com>
> > ---
> >  drivers/mtd/nand/spi/micron.c | 31
> +++++++++++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/spi/micron.c
> b/drivers/mtd/nand/spi/micron.c
> > index 5fd1f921ef12..45fc37c58f8a 100644
> > --- a/drivers/mtd/nand/spi/micron.c
> > +++ b/drivers/mtd/nand/spi/micron.c
> > @@ -131,6 +131,26 @@ static const struct spinand_info
> micron_spinand_table[] = {
> >  		     0,
> >  		     SPINAND_ECCINFO(&micron_8_ooblayout,
> >  				     micron_8_ecc_get_status)),
> > +	/* M70A 4Gb 3.3V */
> > +	SPINAND_INFO("MT29F4G01ABAFD", 0x34,
> > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > +		     NAND_ECCREQ(8, 512),
> > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > +					      &write_cache_variants,
> > +					      &update_cache_variants),
> > +		     0,
> > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > +				     micron_8_ecc_get_status)),
> > +	/* M70A 4Gb 1.8V */
> > +	SPINAND_INFO("MT29F4G01ABBFD", 0x35,
> > +		     NAND_MEMORG(1, 4096, 256, 64, 2048, 40, 1, 1, 1),
> > +		     NAND_ECCREQ(8, 512),
> > +		     SPINAND_INFO_OP_VARIANTS(&read_cache_variants,
> > +					      &write_cache_variants,
> > +					      &update_cache_variants),
> > +		     0,
> > +		     SPINAND_ECCINFO(&micron_8_ooblayout,
> > +				     micron_8_ecc_get_status)),
> >  };
> >
> >  static int micron_spinand_detect(struct spinand_device *spinand)
> > @@ -153,8 +173,19 @@ static int micron_spinand_detect(struct
> spinand_device *spinand)
> >  	return 1;
> >  }
> >
> > +static int micron_spinand_init(struct spinand_device *spinand)
> > +{
> > +	/*
> > +	 * M70A device series enable Continuous Read feature at Power-up,
> > +	 * which is not supported. Disable this bit to avoid any possible
> > +	 * failure.
> > +	 */
> > +	return spinand_upd_cfg(spinand, CFG_QUAD_ENABLE, 0);
> > +}
> > +
> >  static const struct spinand_manufacturer_ops
> micron_spinand_manuf_ops = {
> >  	.detect = micron_spinand_detect,
> > +	.init = micron_spinand_init,
> >  };
> >
> >  const struct spinand_manufacturer micron_spinand_manufacturer = {
> 
> Thanks,
> Miquèl

Thanks,
Shiva