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.87 #1 (Red Hat Linux)) id 1ctYXW-00016O-5Q for linux-mtd@lists.infradead.org; Thu, 30 Mar 2017 11:46:57 +0000 Date: Thu, 30 Mar 2017 13:46:22 +0200 From: Boris Brezillon To: Sekhar Nori Cc: Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen , , , Muralidharan Karicheri , Franklin S Cooper Jr Subject: Re: [PATCH] Revert "mtd: davinci-nand: disable subpage write for keystone-nand" Message-ID: <20170330134622.22ffa45a@bbrezillon> In-Reply-To: <0c648fbb-bb73-30e3-21bb-2875fa8c281d@ti.com> References: <9d21903d38de6685c9bf45d6eb8e32ab0e8c65c5.1490609415.git.nsekhar@ti.com> <20170329170352.2d6c0721@bbrezillon> <7e06e416-8c4a-46dd-5223-2da522daba08@ti.com> <20170330120012.081310fa@bbrezillon> <0c648fbb-bb73-30e3-21bb-2875fa8c281d@ti.com> 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 Thu, 30 Mar 2017 15:38:54 +0530 Sekhar Nori wrote: > On Thursday 30 March 2017 03:30 PM, Boris Brezillon wrote: > > On Thu, 30 Mar 2017 13:13:58 +0530 > > Sekhar Nori wrote: > > > >> Hi Boris, > >> > >> On Wednesday 29 March 2017 08:33 PM, Boris Brezillon wrote: > >>> On Mon, 27 Mar 2017 15:56:55 +0530 > >>> Sekhar Nori wrote: > >>> > >>>> This partly reverts commit 28c015a9daabe4ed3aeb0ccf669a3f1c2b8b81d5. > >>>> > >>>> Since f6d7c1b5598b ("mtd: nand: davinci: Reinitialize the HW ECC > >>>> engine in 4bit hwctl") using subpage and 4bit HWECC works. > >>>> > >>>> Revert the original workaround made for this. Tested with > >>>> mtd_subpagetest and UBIFS on K2E, K2HK, K2L EVMs. > >>>> > >>>> The addition of keystone specific compatible to davinci_nand_of_match[] > >>>> contained in original commit is retained. > >>> > >>> Hm, it sounds like a bad idea to re-enable sub-page writes afterwards. > >>> You might break existing systems using UBI, because UBI detects the > >>> sub-page write feature and decides to place the VID header at a > >>> different position depending on that. > >>> To sum-up, you're likely to break users that have already flashed a UBI > >>> image with the old driver. > >> > >> Okay, I didn't realize that. > >> > >>> If you want to re-enable sub-page writes, just switch back to the > >>> "ti,davinci-nand" compatible. > >> > >> How about a comment on top of the disable subpage write code explaining > >> that its not really needed anymore ? Something like this patch: > >> > >> ---8<--- > >> From: Sekhar Nori > >> Date: Thu, 30 Mar 2017 13:08:30 +0530 > >> Subject: [PATCH] mtd: nand: davinci: add comment on NAND subpage write status > >> on keystone > >> > >> Add a comment clarifying that NAND subpage write on keystone works, > >> but is not being enabled in the interest of backward compatibility. > >> > >> Signed-off-by: Sekhar Nori > >> --- > >> drivers/mtd/nand/davinci_nand.c | 11 +++++++++++ > >> 1 file changed, 11 insertions(+) > >> > >> diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c > >> index 27fa8b87cd5f..5bf8be1cfc8c 100644 > >> --- a/drivers/mtd/nand/davinci_nand.c > >> +++ b/drivers/mtd/nand/davinci_nand.c > >> @@ -581,6 +581,17 @@ static struct davinci_nand_pdata > >> "ti,davinci-nand-use-bbt")) > >> pdata->bbt_options = NAND_BBT_USE_FLASH; > >> > >> + /* > >> + * Since kernel v4.8, this driver has been fixed to enable > >> + * use of 4-bit hardware ECC with subpages and verified on > >> + * TI's keystone EVMs (K2L, K2HK and K2E). > >> + * However, in the interest of not breaking systems using > >> + * existing UBI partitions, sub-page writes are not being > >> + * (re)enabled. If you want to use subpage writes on Keystone > >> + * platforms (i.e. do not have any existing UBI partitions), > >> + * then remove the if() block below or pass "ti,davinci-nand" > >> + * as the compatible in your device-tree file. > > > > Hm, I'm not sure we should suggest to remove these lines from the code. > > Just suggest to use "ti,davinci-nand" compatible. Can you send a separate patch so that it appears on its own in patchwork?