From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-iw0-f177.google.com ([209.85.214.177]) by canuck.infradead.org with esmtps (Exim 4.76 #1 (Red Hat Linux)) id 1Qbnwx-0004PJ-0e for linux-mtd@lists.infradead.org; Wed, 29 Jun 2011 06:08:36 +0000 Received: by iwn35 with SMTP id 35so968123iwn.36 for ; Tue, 28 Jun 2011 23:08:33 -0700 (PDT) Subject: Re: [PATCH 3/6] refactor mtd wait code From: Artem Bityutskiy To: Matthieu CASTET Date: Wed, 29 Jun 2011 09:09:24 +0300 In-Reply-To: <4E09EC77.10501@parrot.com> References: <1309105616-3609-1-git-send-email-matthieu.castet@parrot.com> <1309105616-3609-3-git-send-email-matthieu.castet@parrot.com> <1309248031.23597.45.camel@sauron> <1309248214.23597.47.camel@sauron> <4E09EC77.10501@parrot.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit Message-ID: <1309327769.23597.101.camel@sauron> Mime-Version: 1.0 Cc: "linux-mtd@lists.infradead.org" Reply-To: dedekind1@gmail.com List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, 2011-06-28 at 17:00 +0200, Matthieu CASTET wrote: > Artem Bityutskiy a écrit : > > On Tue, 2011-06-28 at 11:00 +0300, Artem Bityutskiy wrote: > >> On Sun, 2011-06-26 at 18:26 +0200, Matthieu CASTET wrote: > >>> +/** > >>> + * This is call after sending a read command, or for autoincrement > >>> + * chip that need it (!NAND_NO_READRDY). > >>> + * > >>> + * We can't call NAND_CMD_STATUS here, because the read command > >>> + * is not finished > >>> + */ > >>> +static void nand_wait_read(struct mtd_info *mtd, struct nand_chip *chip) > >>> +{ > >>> + /* > >>> + * If we don't have access to the busy pin, we apply the given > >>> + * command delay > >>> + */ > >>> + if (!chip->dev_ready) { > >>> + udelay(chip->chip_delay); > >>> + } > >>> + else { > >>> + /* Apply this short delay always to ensure that we do wait tWB in > >>> + * any case on any machine. */ > >>> + ndelay(100); > >> Please, all these hard-coded numbers should be hidden in the specific > >> driver. > > > > Or could you please explain a bit better why this delay has to be part > > of nand core? And why it is 100 and not 200? > > > This delay is already in the nand core. I only put it in a common function : OK, sorry, but would it be possible to take a look at how this patch could be split on smaller ones to make review a bit simpler? -- Best Regards, Artem Bityutskiy