linux-kernel - Re: [PATCH 2/2] mtd: spinand: micron: Support for all Micron SPI NAND flashes

Open Source and information security mailing list archives

Message-ID: <MN2PR08MB5951CEE5E54676FDA619490BB86D0@MN2PR08MB5951.namprd08.prod.outlook.com>
Date:   Mon, 4 Feb 2019 18:02:56 +0000
From:   "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@...ron.com>
To:     Emil Lenngren <emil.lenngren@...il.com>
CC:     Boris Brezillon <bbrezillon@...nel.org>,
        Richard Weinberger <richard@....at>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Schrempf Frieder <frieder.schrempf@...tron.de>,
        Marek Vasut <marek.vasut@...il.com>,
        "Frieder Schrempf" <frieder.schrempf@...eet.de>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Brian Norris <computersforpeace@...il.com>,
        Chuanhong Guo <gch981213@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        "Bean Huo (beanhuo)" <beanhuo@...ron.com>
Subject: Re: [PATCH 2/2] mtd: spinand: micron: Support for all Micron SPI NAND
 flashes

Hi Emil,

> Hi,
> 
> Den mån 4 feb. 2019 kl 12:18 skrev Shivamurthy Shastri (sshivamurthy)
> <sshivamurthy@...ron.com>:
> >
> > Driver is redesigned using parameter page to support all the Micron
> > SPI NAND flashes.
> >
> > Parameter page of Micron flashes is similar to ONFI parameter table
> > and functionality is same, so copied some of the common functions like
> > crc16 and bit_wise_majority from nand_onfi.c.
> >
> > This driver is tested using MT29F2G01ABXGD, MT29F4G01ABXFD,
> > MT29F8G01ADXFD, MT29F1G01ABXFD.
> >
> 
> > -static const struct spinand_info micron_spinand_table[] = {
> > -       SPINAND_INFO("MT29F2G01ABAGD", 0x24,
> > -                    NAND_MEMORG(1, 2048, 128, 64, 2048, 2, 1, 1),
> 
> > +       deviceinfo.memorg.eraseblocks_per_lun =
> > +               params->blocks_per_lun * params->lun_count;
> > +       deviceinfo.memorg.planes_per_lun = params->lun_count;
> > +       deviceinfo.memorg.luns_per_target = 1;
> > +       deviceinfo.memorg.ntargets = 1;
> 
> > +       __le32 blocks_per_lun;
> > +       u8 lun_count;
> > +       u8 addr_cycles;
> > +       u8 bits_per_cell;
> > +       __le16 bb_per_lun;
> 
> I have a question about the lun_count. As it is now, the planes_per_lun
> parameter is initialized to 2 in NAND_MEMORG. In your patch, it is instead
> initialized from the "lun_count" property from the parameter table. But I
> looked at a datasheet I found by a simple Google search
> (https://www.google.se/search?q=micron+nand+spi+datasheet),
> the first hit is to the 1 Gb flash MT29F1G01AAADD. That device clearly has
> two planes per lun (you need the "plane select" bit in the requests), but still,
> according to the parameter page data structure, byte 100, Number of logical
> units is set to 01h. Also, the "blocks per lun" count, which is called "blocks per
> unit" is 1024, which should be
> 512 if this parameter really meant "blocks per plane" and the calculation in
> the patch was correct.

You are right, I somehow didn't send the correct patch for this.
This is my first time sending patch to mainline, lost in nervousness.

In micron flashes we can identify number of planes in LUN using 
parameter page Byte 166 -> vendor_specific[0].
Byte 166 will be "1", if LUN has more than one plane. 
I will resend the correct patch.

However, the device you mentioned is obsolete, very old and not in production.
This code will not support that.
Please refer new datasheets mentioned in the commit messages.

> 
> As a reference, the 2 Gb version of the Macronix flash
> (http://www.macronix.com/Lists/Datasheet/Attachments/6866/MX35LF2GE
> 4AB,%203V,%202Gb,%20v1.5.pdf),
> also has two planes per lun. It also sets byte 100, Number of logical units to
> 01h.
> 
> So what I'm wondering is of course if this parameter is the correct one to use
> for planes_per_lun. I tried to locate the correct "planes per lun" parameter in
> the table, but didn't find anyone. Maybe it's the unfortunate fact that
> "planes per lun" isn't exposed in the parameter table?
> 
> /Emil
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

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