From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.free-electrons.com ([62.4.15.54]) by bombadil.infradead.org with esmtp (Exim 4.89 #1 (Red Hat Linux)) id 1eSIhH-0006PE-Oz for linux-mtd@lists.infradead.org; Fri, 22 Dec 2017 08:28:57 +0000 Date: Fri, 22 Dec 2017 09:28:35 +0100 From: Boris Brezillon To: Peter Pan Cc: Frieder Schrempf , "Peter Pan =?UTF-8?B?5r2Y5qCL?= (peterpandong)" , "linux-mtd@lists.infradead.org" Subject: Re: [PATCH v6 00/15] A SPI NAND framework under generic NAND framework Message-ID: <20171222092835.5ba7dfb3@bbrezillon> In-Reply-To: References: <20170529225931.226da943@bbrezillon> <90bcb366-c494-2dee-3bb7-d16b77e347c6@exceet.de> <20171204150514.1f17671d@bbrezillon> <20171205135804.478902dc@bbrezillon> <20171205140313.40f3f17d@bbrezillon> <256b4ab6-c104-4580-87cd-527178bde460@exceet.de> <20171213222710.32d12514@bbrezillon> <20171214085036.436b4fcf@bbrezillon> <23d17416-e555-4b92-cc47-f644fbe676c6@exceet.de> <20171214163848.41f6d277@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, 22 Dec 2017 14:37:06 +0800 Peter Pan wrote: > Hi Boris and Frieder > > On Fri, Dec 22, 2017 at 8:49 AM, Peter Pan wrote: > > Hi Frieder, > > > > On Thu, Dec 21, 2017 at 7:48 PM, Frieder Schrempf > > wrote: > >> Hello Boris, > >> > >>>>>> So shouldn't there be an spinand_die_select_op in the SPI NAND core > >>>>>> that > >>>>>> is executed when necessary and skipped if there's only one die. > >>>>> > >>>>> > >>>>> Sure, I was arguing against a ->select_chip() at the generic NAND > >>>>> level. That's something you can add in the SPI NAND framework. > >> > >> > >> I added an op to send the die selection command and call it, where I think > >> it is needed: [1] > > > > I think we should add die_select_op in vendor's file and call a hook in core.c > > since the die select command implementation is different by vendors. > > > > Thanks > > Peter Pan > > > > > >> > >> Accessing both dies on the Winbond SPI NAND works fine like this. > >> > >> While running tests I came across some problems: > >> > >> * On accessing the BBT in RAM via nanddev_bbt_[set/get]_block_status, the > >> calculation of position and offset seems to be wrong. > >> My fix is here: [2] > >> > >> * On calculating the row address for read/program/erase via > >> nanddev_pos_to_row, it seems like the eraseblock_addr_shift is wrong. > >> > >> * Also, I'm not sure if the LUN should be taken into account when > >> calculating the row address. At least when you select the LUN by issuing a > >> separate command, the row address sent to the chip should only contain the > >> page address. > >> But I'm not sure if that's the case in general, or if some code is needed to > >> differentiate. > >> > >> See my changes of nanddev_pos_to_row here: [3] > >> > >> * I run into a mutex deadlock, when spinand_write_page fails (e.g. because > >> of a bad block) as the lock is not released in such cases. See my fix here: > >> [4] > >> > >> With these fixes applied and as far as I can tell everything seems to work > >> fine. I'll do some tests with UBI next and look into the ECC topic. > > UBI attach failed since missing mtd->writebufsize assignment in nanddev_init() > After fixing it and with Frieder's fixing, UBIFS can be mounted successfully on > Zedboard with generic spi controller and Micron SPI NAND 2Gb device. I'll squash your fix, thanks. > > Also, the code now can pass mtd read and page test. 1 error found in oob test > since we don't have "past end of partition" check in part_write_oob() which I > mentioned in earlier mail. Still not sure about that one, but I'll reply in the other thread. > Since we don't support ECC right now, I didn't try > nandbiterror test. > > > @@ -195,6 +195,7 @@ int nanddev_init(struct nand_device *nand, const > struct nand_ops *ops, > mtd->flags = MTD_CAP_NANDFLASH; > mtd->erasesize = memorg->pagesize * memorg->pages_per_eraseblock; > mtd->writesize = memorg->pagesize; > + mtd->writebufsize = mtd->writesize; > mtd->oobsize = memorg->oobsize; > mtd->size = nanddev_size(nand); > mtd->owner = owner; > > > Thanks > Peter Pan > > >> > >> Regards, > >> > >> Frieder > >> > >> [1]: > >> https://github.com/fschrempf/linux-0day/compare/4f8472ea05f27d55ebb2e42eadc9ed59d7036f4c...fschrempf:2c5471c1f1e04fa91ff80c95276b4828b99c4f94 > >> > >> [2]: > >> https://github.com/fschrempf/linux-0day/commit/937a16248570368fe05e15b1f70e753e202b6ae6 > >> > >> [3]: > >> https://github.com/fschrempf/linux-0day/commit/8013696e0394960bfc36625d277f0d2425262939 > >> > >> [4]: > >> https://github.com/fschrempf/linux-0day/commit/28ffc7211745dd124702358c24ddccff8e6f2d95