From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755067AbcEDWCi (ORCPT ); Wed, 4 May 2016 18:02:38 -0400 Received: from mail-bl2on0133.outbound.protection.outlook.com ([65.55.169.133]:22336 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1754971AbcEDWCf (ORCPT ); Wed, 4 May 2016 18:02:35 -0400 Authentication-Results: free-electrons.com; dkim=none (message not signed) header.d=none;free-electrons.com; dmarc=none action=none header.from=ni.com; Date: Wed, 4 May 2016 17:02:43 -0500 From: Kyle Roeschley To: Boris Brezillon CC: , , , , , , , , Peter Pan Subject: Re: [PATCH v3] mtd: nand_bbt: scan for next free bbt block if writing bbt fails Message-ID: <20160504220236.GA29977@senary> References: <1458945076-18305-1-git-send-email-kyle.roeschley@ni.com> <20160330151351.323a5333@bbrezillon> <20160330151623.7c1e4241@bbrezillon> <20160429173417.GA18490@senary> <20160429211631.35bf48d2@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20160429211631.35bf48d2@bbrezillon> User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [130.164.62.218] X-ClientProxiedBy: BLUPR05CA0066.namprd05.prod.outlook.com (10.141.20.36) To BN1PR0401MB0931.namprd04.prod.outlook.com (10.160.78.24) X-MS-Office365-Filtering-Correlation-Id: 4d1f9943-21c8-4ba8-fe53-08d37467cac1 X-Microsoft-Exchange-Diagnostics: 1;BN1PR0401MB0931;2:kHzw8WYA1tnctGlgaiDtsbEiLKHeeNyVSnS8zFg3MAl3/WNhS7vZnR4R3BYcHuy8sXuRijEfEtqYidD6Wu1BpbtUTUKmjgVLtKPDxEi+NiJUdsak86J1d5kHksQs8C/ao+fBZi3KaAVeHX90hPG0BTbVOXcOxLriCWgXoy5E1sBthmIeGy+J5ILP8eIYl65I;3:4506rKmrEyJqsSpTwoyp9+vDRMKL/lQ0+1CnSydFBHkC0wR9oK0b575ZRmoj5wXl6U9vymsXzxD8QerSrPpl6CjaSwo2Z1hgaYV676nwDT2BtQ9/L6Br2TYoy7NVgb7u X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BN1PR0401MB0931; X-Microsoft-Exchange-Diagnostics: 1;BN1PR0401MB0931;25:l4JW6hURwm+0lT1bdqWWvcqCZ7/Xu/XCFFpqrd5GOdh5aGgWMOyW94l5W7sT3KneqnwpCXz/Lm192gJ37NstnnxCgqTMOEPYVhyhbR8MelyiudfrN/geoiHTgdi7GCxw473fATwVh6dtwXiQdHCYudZns/KcQMH1GurrMRxrCSAIQRtRMEu4+fqwShA1YD3MH2RldLGxHjd3brBXkElhF/T46zx1nadyi83cFpbDEN8/VI3vWUmXQ1E6mk+erAXIfEAXuQP8SszsAoCDn2l81Ffn4EoYwtMRilZriBhwoI13WVU55Arg+E6QIJkk/Td4jpWVw3fSbEUepsuyIkyGYENY16q4PppARToW7DO60zm3ccCndR9W8ynkFokWFhs5LWuLnUYUubh1oC7qAzh6mFwevupNWPahvgq4Yzop4+hw5OtxNXIPxT4xoiKklDtvjfl4uwVCuRvmc5F5uU8nwHdxjQv5jlb142p3EU/a2AEs/UCyfQE9FTNtD4SNjUgnbHa2cTNGnaK/pHZKozNF8+zrQkBG5BE5bhO8ITep9q45Txv3w7RZM43px6rswTUNj3ODbmG8RLAyb8ixUdAerY9/8Za5VWbi0MR/y7iHW/CL4zsp8ttx2MHMw3K0Ld6lo7j0I4RALuOucKd6ZcxgVH3XgZj7MYpyxJ41uSMQ1ds7W5Ex9rvCn2sajpohas0APHisVCr7qAUYjwb9E2FTZjK3nx6fX47CXmAkFn1od9E= X-Microsoft-Exchange-Diagnostics: 1;BN1PR0401MB0931;20:8MtchGbV+ApVz+H8+UYwhEHk9Y3mrw0DDEJIexIJY2o+phcpHt6UJtFBvNsXpxUvm64SIQ4L1HGbwzXNpIVoSBRBrLZpJ0ogsEL14M+DFoj1vPKGyQpmzGTZFdQMxNOTYTeEPWPRn9pkxxvun5mo9x3D6tN8LCxxInagGUFcvUxSD/XU3VymfAqaLMFZMz1m9Uue4i9cip746IOa4sPOERBMA2Du/6Q9PBFLibpTuk9JcN5dtaL9B4rBL7qu3mv4QvKiOrRsyEC5cb17LyT355aVUvuOywTecS6EP9V/tE/E/C6fW3XOJBO0uhW1hDGhptiboXN/fdtrGrIyDzeStNZFwklk3NXWIHUg3NpoRSeaZqExcP3r+Dcd4uH7LoRxbJQNAh6ey8iR+Rk4ekjdCQP2HpzjLk/pEXXACBYLVB1ilDn4NFFdKok/ouAfHZTtNpg1uUVDBeesjHq0XiVcNabMQsRE4TEAGXzy20DnRO1uXEqC1KSkxr5Z8ABTnwZQ+QdsUnw8K6X+DWwM4AuFBPKdz3A53e0VEfIV9DJI+4C3w9VWTfeHpzlqOahJGQpOvrdZ4ilgcU/LwQgRiWeM6TdlZQ7OpuIW5ZtlJzXzqaw= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(9101521098)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:BN1PR0401MB0931;BCL:0;PCL:0;RULEID:;SRVR:BN1PR0401MB0931; X-Microsoft-Exchange-Diagnostics: 1;BN1PR0401MB0931;4:K3EZXU20xHi6mENcwfYpLZ0HSiOsXsbaXB9oHpiQVAZuwmxJZefVB93NjrkFNstHTHaxIXDu+CZnjal4cKY9AEVVL/8ijIejHOcgXnyjZPSHrgCwrd0xQ1UimMAqyENPR8fmqjZTYFeeGlvRf3KOJRn7Bepr2BESRTOVUaNh55UnCvSngHQMIiOIJPqECY4eB+/qGeTapvVA5XK3UrFwFKRJbzo8TJGTnQAffDESTx0w8QnAUreL0LDtzNSGnUDGQhRaRavPT+t/bBa3j7C7q5G65TcZhi2pTpvSPK8WJkO4NsvQHxqpi1yGuzHwTIvEXM/xHPCFOK/uvlt9dKQGK5isH4mz3dq9NlKb7WsZ6J8Yi4RQiGEBQAZ+1bSagQPiFpMrnPnfn7GWCF42KbGcJg== X-Forefront-PRVS: 093290AD39 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(24454002)(52314003)(43544003)(92566002)(81166005)(15975445007)(77096005)(97756001)(19580395003)(5004730100002)(86362001)(19580405001)(83506001)(50986999)(4326007)(189998001)(33656002)(2950100001)(54356999)(46406003)(93886004)(76176999)(23726003)(33716001)(110136002)(6116002)(3846002)(9686002)(2906002)(5008740100001)(1076002)(47776003)(586003)(50466002)(42186005);DIR:OUT;SFP:1102;SCL:1;SRVR:BN1PR0401MB0931;H:senary;FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;BN1PR0401MB0931;23:XpUQBqsEd/PXzebsqceWzUljGVQD6/d2JST5x9g?= =?us-ascii?Q?BqPagwqNDsYtZBki0+ukY5j7qDl612pgHJe7BgITKNLN+NPs5PNDOZTg8oTK?= =?us-ascii?Q?CHCoVIpyNbdJc/F/B+hHD082DdEfq67nTk9/o0aCuFIpW4XYNkJlLFQWm1JT?= =?us-ascii?Q?pol9u8fdmBZUk0JDOm1tpcfz6lzmYjf6iWOsnFQUfKNBXOcteVT/OedPnnby?= =?us-ascii?Q?9qexWAlwwUdyiy1bgpTu8xHxkxUb1DFzklNhaZl8qS9PJJRgVZoG/xJeHPP9?= =?us-ascii?Q?r9BrEaJ/Fkumnac5ENZjIfx2g/hjMyFlfWcghOCpkXYNUaJ2Pfh7HpdwZUix?= =?us-ascii?Q?yskvfAdrT+AJMK3wr0T+D4jtFQ3lbcxkU2RJJdm0zLeJqoDbZVRBlgxXaKx1?= =?us-ascii?Q?AsH2iXTPIE/aM+fby4ImCa80DMYIUoHFpXUYBRdOmREv4iiINJ2LPsUaMIc0?= =?us-ascii?Q?EtgYZBH3I0tgO46N4idgfBFAdX0x9ef7+n+tMJReAIoONqTfECdowTob2jZC?= =?us-ascii?Q?bClvsndOXN7Ttorb4DQDEmFTZwPykcap06nmtvZQT25ncIKuQ+mnwkm9taJd?= =?us-ascii?Q?aevqjHe1Bk+IMVpoNsU/CdlwDFiUlonz8lnXc1TK1pDWEB705McqD9LXcO7J?= =?us-ascii?Q?KtGbl7Zvq8C2FnGMU30BAgz+ZWUmuVOJ/TiDohFuBsecqIaDj+WFAMXusIy0?= =?us-ascii?Q?/4XMg1xmvZ6ZNMfYZsevJRuM4nBiqzN0zkJJYm+prec4KBkWq1m8UvE97OvN?= =?us-ascii?Q?iNiq8xk1c4PQZeVijhs++2gY2nF8XRs9kmNi83pbRO/ud/+Rx1eHLVpcigva?= =?us-ascii?Q?gjac9vIYNn1vqDBur8bwSqxRAQucVo5hzkEeMFiyAANxtwsN0188REec7guo?= =?us-ascii?Q?lmGC0UwEhfu3wLkKXdplzxqVSYj7aftE3j5AUHJMnMVpDWkaXCoCCaihWlj6?= =?us-ascii?Q?m8UQdLRFthKCzg+WRR8B2cM6sQfFRP9EDtTIVwTR3t0/jGdfc8bfpmhicvOY?= =?us-ascii?Q?a8lA=3D?= X-Microsoft-Exchange-Diagnostics: 1;BN1PR0401MB0931;5:JYvJAsnWZ0JRSiJB5a1nK79Tkp1fMukpTlqt0j9Eplm2GZZqJ58VT3v71HCMrdm0X6dtyXUrKfrXAfAuXoWQsebwuuVWxstKDRSqSwJmUrF72CGfxJ+cvtEYTmkxd5faG0IZ1rDyapnCXGfFIi398A==;24:ZLBbDH4jhwUCiM+EoYtXu5D2lwiuUDplPlkaVr3Z8oJ8mmlhgNMPzwMDKs6S8phP7izkIJUbKLbFnblCBYh24nbGIcmxVxJDtUvJVUxmTNo=;7:E7V5Bw6RY+6sKu3JhCeouRbkPLm/elEF66Dsw1TyvTm8z0p3sexklJ3xn97mWGnjMznqn2aHpRWujgFvOmbVKfl3elHQdO3sHJv5SyrhTNGd3kUyx0iY574ejCWjl/vIGFRlIbf0PRWR4NK+WXMMcgmUS1KWoXfu4//S4SdWR+08e0t1+251mtezGkQzfQY8 SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: ni.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 May 2016 22:02:31.4973 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN1PR0401MB0931 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 29, 2016 at 09:16:31PM +0200, Boris Brezillon wrote: > On Fri, 29 Apr 2016 12:34:18 -0500 > Kyle Roeschley wrote: > > > Hi Boris, > > > > On Wed, Mar 30, 2016 at 03:16:23PM +0200, Boris Brezillon wrote: > > > +Peter, who's currently reworking the NAND BBT code. > > > > > > On Wed, 30 Mar 2016 15:13:51 +0200 > > > Boris Brezillon wrote: > > > > > > > Hi Kyle, > > > > > > > > On Fri, 25 Mar 2016 17:31:16 -0500 > > > > Kyle Roeschley wrote: > > > > > > > > > If erasing or writing the BBT fails, we should mark the current BBT > > > > > block as bad and use the BBT descriptor to scan for the next available > > > > > unused block in the BBT. We should only return a failure if there isn't > > > > > any space left. > > > > > > > > > > Based on original code implemented by Jeff Westfahl > > > > > . > > > > > > > > > > Signed-off-by: Kyle Roeschley > > > > > Suggested-by: Jeff Westfahl > > > > > --- > > > > > This v3 is in response to comments from Brian Norris and Bean Ho on 8/26/15: > > > > > http://lists.infradead.org/pipermail/linux-mtd/2015-August/061411.html > > > > > > > > > > v3: Don't overload mtd->priv > > > > > Keep nand_erase_nand from erroring on protected BBT blocks > > > > > > > > > > v2: Mark OOB area in each block as well as BBT > > > > > Avoid marking read-only, bad address, or known bad blocks as bad > > > > > --- > > > > > drivers/mtd/nand/nand_base.c | 4 ++-- > > > > > drivers/mtd/nand/nand_bbt.c | 37 +++++++++++++++++++++++++++++++++++-- > > > > > 2 files changed, 37 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c > > > > > index b6facac..9ad8a86 100644 > > > > > --- a/drivers/mtd/nand/nand_base.c > > > > > +++ b/drivers/mtd/nand/nand_base.c > > > > > @@ -2916,8 +2916,8 @@ int nand_erase_nand(struct mtd_info *mtd, struct erase_info *instr, > > > > > /* Select the NAND device */ > > > > > chip->select_chip(mtd, chipnr); > > > > > > > > > > - /* Check, if it is write protected */ > > > > > - if (nand_check_wp(mtd)) { > > > > > + /* Check if it is write protected, unless we're erasing BBT */ > > > > > + if (nand_check_wp(mtd) && !allowbbt) { > > > > > > > > Hm, will this really work. Can a write-protected device accept erase > > > > commands? > > > > > > > > Having looked into this more, no. Since v2, we called block_markbad in > > write_bbt incorrectly and caused the chip to report that it was write > > protected. Fixing that makes this unnecessary. > > > > > > > pr_debug("%s: device is write protected!\n", > > > > > __func__); > > > > > instr->state = MTD_ERASE_FAILED; > > > > > diff --git a/drivers/mtd/nand/nand_bbt.c b/drivers/mtd/nand/nand_bbt.c > > > > > index 2fbb523..01526e5 100644 > > > > > --- a/drivers/mtd/nand/nand_bbt.c > > > > > +++ b/drivers/mtd/nand/nand_bbt.c > > > > > @@ -662,6 +662,7 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > > > > > page = td->pages[chip]; > > > > > goto write; > > > > > } > > > > > + next: > > > > > > > > Please put this label at the beginning of the line and fix all the other > > > > issues reported by checkpatch (I know we already have a 'write' label > > > > which does not follow this rule, but let's try to avoid adding new > > > > ones). > > > > > > > > Will do. > > > > > > > > > > > > /* > > > > > * Automatic placement of the bad block table. Search direction > > > > > @@ -787,14 +788,46 @@ static int write_bbt(struct mtd_info *mtd, uint8_t *buf, > > > > > einfo.addr = to; > > > > > einfo.len = 1 << this->bbt_erase_shift; > > > > > res = nand_erase_nand(mtd, &einfo, 1); > > > > > - if (res < 0) > > > > > + if (res == -EIO) { > > > > > + /* This block is bad. Mark it as such and see if > > > > > + * there's another block available in the BBT area. */ > > > > > + int block = page >> > > > > > + (this->bbt_erase_shift - this->page_shift); > > > > > + pr_info("nand_bbt: failed to erase block %d when writing BBT\n", > > > > > + block); > > > > > + bbt_mark_entry(this, block, BBT_BLOCK_WORN); > > > > > + > > > > > + res = this->block_markbad(mtd, block); > > > > > > > > Not sure we should mark the block bad until we managed to write a new > > > > BBT. ITOH, if we do so and the new BBT write is interrupted, it > > > > will trigger a full BBM scan, which should be harmless on most > > > > platforms (except those overwriting BBM with real data :-/) > > > > > > > > So is your suggestion here just to swap the order of block_markbad and > > bbt_mark_entry? > > No, my suggestion was to move this->block_markbad() call after > scan_write_bbt(), but this leads to another problem: if the BBT content > is still valid after the erasure and you move this->block_markbad(), > you might have a power-cut in the middle and the BBT detection code > will pick the first valid one BBT (i.e. the one you were about to mark > as bad). > Again, this is all hypothetical, and anyway, the current BBT > implementation is not so robust, so maybe we shouldn't care and rely on > full bad block scan in this case (too bad for controllers that did not > take care of keeping valid bad block markers :-/). Sounds like a plan, I'll work on this. -- Kyle Roeschley Software Engineer National Instruments