Messages in this thread | | | Date | Thu, 30 Mar 2017 13:46:22 +0200 | From | Boris Brezillon <> | Subject | Re: [PATCH] Revert "mtd: davinci-nand: disable subpage write for keystone-nand" |
| |
On Thu, 30 Mar 2017 15:38:54 +0530 Sekhar Nori <nsekhar@ti.com> wrote:
> On Thursday 30 March 2017 03:30 PM, Boris Brezillon wrote: > > On Thu, 30 Mar 2017 13:13:58 +0530 > > Sekhar Nori <nsekhar@ti.com> 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 <nsekhar@ti.com> 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 <nsekhar@ti.com> > >> 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 <nsekhar@ti.com> > >> --- > >> 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?
| |