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

[Shivamurthy Shastri (sshivamurthy)]

Hi Emil,

> Hi,
> 
> Den mÃn 4 feb. 2019 kl 12:18 skrev Shivamurthy Shastri (sshivamurthy)
> <sshivamurthy@xxxxxxxxxx>:
> >
> > 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/