Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <MN2PR08MB5951FD3B8244ABBA72FF7430B86D0@MN2PR08MB5951.namprd08.prod.outlook.com>
In-Reply-To: <MN2PR08MB5951FD3B8244ABBA72FF7430B86D0@MN2PR08MB5951.namprd08.prod.outlook.com>
From:   Emil Lenngren <emil.lenngren@gmail.com>
Date:   Mon, 4 Feb 2019 13:32:50 +0100
Message-ID: <CAO1O6sejw4zYeh7iewT3gRZz=C5K3iOXhY-kO6FgSJNRHOBuKA@mail.gmail.com>
Subject: Re: [PATCH 2/2] mtd: spinand: micron: Support for all Micron SPI NAND flashes
To:     "Shivamurthy Shastri (sshivamurthy)" <sshivamurthy@micron.com>
Cc:     Boris Brezillon <bbrezillon@kernel.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>,
        "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Chuanhong Guo <gch981213@gmail.com>,
        Richard Weinberger <richard@nod.at>,
        Schrempf Frieder <frieder.schrempf@kontron.de>,
        Marek Vasut <marek.vasut@gmail.com>,
        Frieder Schrempf <frieder.schrempf@exceet.de>,
        Brian Norris <computersforpeace@gmail.com>,
        David Woodhouse <dwmw2@infradead.org>,
        "Bean Huo (beanhuo)" <beanhuo@micron.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

Den m=C3=A5n 4 feb. 2019 kl 12:18 skrev Shivamurthy Shastri (sshivamurthy)
<sshivamurthy@micron.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, MT29F8G01ADXF=
D,
> MT29F1G01ABXFD.
>

> -static const struct spinand_info micron_spinand_table[] =3D {
> -       SPINAND_INFO("MT29F2G01ABAGD", 0x24,
> -                    NAND_MEMORG(1, 2048, 128, 64, 2048, 2, 1, 1),

> +       deviceinfo.memorg.eraseblocks_per_lun =3D
> +               params->blocks_per_lun * params->lun_count;
> +       deviceinfo.memorg.planes_per_lun =3D params->lun_count;
> +       deviceinfo.memorg.luns_per_target =3D 1;
> +       deviceinfo.memorg.ntargets =3D 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=3Dmicron+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.

As a reference, the 2 Gb version of the Macronix flash
(http://www.macronix.com/Lists/Datasheet/Attachments/6866/MX35LF2GE4AB,%203=
V,%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