From patchwork Thu Jun 20 04:36:17 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Heiko Schocher X-Patchwork-Id: 252745 X-Patchwork-Delegate: marek.vasut@gmail.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from theia.denx.de (theia.denx.de [85.214.87.163]) by ozlabs.org (Postfix) with ESMTP id 109DD2C0AAF for ; Thu, 20 Jun 2013 15:36:38 +1000 (EST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 46E8E4A094; Thu, 20 Jun 2013 07:36:36 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id AaLR3nXr07a8; Thu, 20 Jun 2013 07:36:36 +0200 (CEST) Received: from theia.denx.de (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 14ED24A0FC; Thu, 20 Jun 2013 07:36:33 +0200 (CEST) Received: from localhost (localhost [127.0.0.1]) by theia.denx.de (Postfix) with ESMTP id 299934A0FC for ; Thu, 20 Jun 2013 07:36:28 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at theia.denx.de Received: from theia.denx.de ([127.0.0.1]) by localhost (theia.denx.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id S4TIzjVv179y for ; Thu, 20 Jun 2013 07:36:23 +0200 (CEST) X-policyd-weight: NOT_IN_SBL_XBL_SPAMHAUS=-1.5 NOT_IN_SPAMCOP=-1.5 NOT_IN_BL_NJABL=-1.5 (only DNSBL check requested) Received: from mail-out.m-online.net (mail-out.m-online.net [212.18.0.9]) by theia.denx.de (Postfix) with ESMTPS id 2CE614A094 for ; Thu, 20 Jun 2013 07:36:17 +0200 (CEST) Received: from frontend1.mail.m-online.net (unknown [192.168.8.180]) by mail-out.m-online.net (Postfix) with ESMTP id 3bbWwb6Yktz4KK2G; Thu, 20 Jun 2013 07:36:15 +0200 (CEST) X-Auth-Info: cuMyzc6tA70LKjzIHQnHC+gTQdbYZNsjA38VAo3vtZk= Received: from linux-lenovo.site (82.131.228.45.pool.invitel.hu [82.131.228.45]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by smtp-auth.mnet-online.de (Postfix) with ESMTPSA id 3bbWwb0Vyzzbbnd; Thu, 20 Jun 2013 07:36:15 +0200 (CEST) Message-ID: <51C286C1.2030101@denx.de> Date: Thu, 20 Jun 2013 06:36:17 +0200 From: Heiko Schocher Organization: DENX Software Engineering User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Scott Wood References: <1371516686.9073.17@snotra> In-Reply-To: <1371516686.9073.17@snotra> Cc: Marek Vasut , Pantelis Antoniou , u-boot@lists.denx.de, Kyungmin Park , Tom Rini Subject: Re: [U-Boot] [PATCH v2] dfu, nand: before write a buffer to nand, erase the nand sectors X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.11 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: u-boot-bounces@lists.denx.de Errors-To: u-boot-bounces@lists.denx.de Hello Scott, Am 18.06.2013 02:51, schrieb Scott Wood: > On 06/17/2013 12:01:01 AM, Heiko Schocher wrote: >> before writing the received buffer to nand, erase the nand >> sectors. If not doing this, nand write fails. See for >> more info here: >> >> http://lists.denx.de/pipermail/u-boot/2013-June/156361.html >> >> Signed-off-by: Heiko Schocher >> Cc: Scott Wood >> Cc: Pantelis Antoniou >> Cc: Lukasz Majewski >> Cc: Kyungmin Park >> Cc: Marek Vasut >> Cc: Tom Rini >> >> --- >> - changes for v2: >> - use opts.spread as Scott Wood suggested >> >> drivers/dfu/dfu_nand.c | 17 +++++++++++++++-- >> 1 Datei geändert, 15 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) >> >> diff --git a/drivers/dfu/dfu_nand.c b/drivers/dfu/dfu_nand.c >> index 7dc89b2..93db9bd 100644 >> --- a/drivers/dfu/dfu_nand.c >> +++ b/drivers/dfu/dfu_nand.c >> @@ -63,12 +63,25 @@ static int nand_block_op(enum dfu_nand_op op, >> struct dfu_entity *dfu, >> >> nand = &nand_info[nand_curr_device]; >> >> - if (op == DFU_OP_READ) >> + if (op == DFU_OP_READ) { >> ret = nand_read_skip_bad(nand, start, &count, &actual, >> lim, buf); >> - else >> + } else { >> + nand_erase_options_t opts; >> + >> + memset(&opts, 0, sizeof(opts)); >> + opts.offset = start; >> + opts.length = count; >> + opts.spread = 1; >> + opts.quiet = 1; >> + /* first erase */ >> + ret = nand_erase_opts(nand, &opts); >> + if (ret) >> + return ret; >> + /* then write */ >> ret = nand_write_skip_bad(nand, start, &count, &actual, >> lim, buf, 0); > > BTW, I notice you are currently using the limit functionality of > nand_read/write_skip_bad... opts.spread currently does not have this > support (as I noted before), which means that if there's an error you'd > erase too much and then refuse to write. > > Maybe we need an opts.limit? Yes, I think so ... whats with the following proposal: I checked for all calls from nand_erase_opts, that the nand_erase_options_t parameters are initialized with 0 ... so this patch should not change current behaviour. Should I do this in a seperate patch, or add it to the "dfu, nand: before write a buffer to nand, erase the nand sectors" patch, so it adds no dead code ... > adjust_size_for_badblocks, OTOH, is probably the opposite of what you > wanted -- it subtracts from the size in order to get the number of good > blocks within an interval, rather than adding the number of bad blocks > to turn a data size into an interval. It's meant to produce an input > to be used with skipping/spreading operations. Yes, thats not what I wanted ... > Which makes me think we have a bug in cmd_nand.c -- we should be > setting .spread in erase cases where we call adjust_size_for_badblocks. Yes, seems so ... bye, Heiko diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index d81972c..b877c7d 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -120,6 +120,10 @@ int nand_erase_opts(nand_info_t *meminfo, const nand_erase_options_t *opts) WATCHDOG_RESET(); + if ((opts->limit) && (erase.addr > opts->limit)) { + puts("Size of write exceeds partition or device limit\n"); + return -EFBIG; + } if (!opts->scrub && bbtest) { int ret = mtd_block_isbad(meminfo, erase.addr); if (ret > 0) { diff --git a/include/nand.h b/include/nand.h index 26190e4..d799df3 100644 --- a/include/nand.h +++ b/include/nand.h @@ -125,6 +125,8 @@ struct nand_erase_options { /* Don't include skipped bad blocks in size to be erased */ int spread; + /* maximum size that actual may be in order to not exceed the buf */ + loff_t limit; }; typedef struct nand_erase_options nand_erase_options_t;