Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751540AbbHAAbZ (ORCPT ); Fri, 31 Jul 2015 20:31:25 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:47393 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751370AbbHAAbX (ORCPT ); Fri, 31 Jul 2015 20:31:23 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Sat, 01 Aug 2015 02:28:06 +0200 From: Stefan Agner To: Brian Norris Cc: dwmw2@infradead.org, sebastian@breakpoint.cc, robh+dt@kernel.org, pawel.moll@arm.com, mark.rutland@arm.com, ijc+devicetree@hellion.org.uk, galak@codeaurora.org, shawn.guo@linaro.org, kernel@pengutronix.de, boris.brezillon@free-electrons.com, marb@ixxat.de, aaron@tastycactus.com, bpringlemeir@gmail.com, linux-mtd@lists.infradead.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, albert.aribaud@3adev.fr, Bill Pringlemeir Subject: Re: [PATCH v9 2/5] mtd: nand: vf610_nfc: add hardware BCH-ECC support In-Reply-To: <20150731234718.GO10676@google.com> References: <1438361581-2702-1-git-send-email-stefan@agner.ch> <1438361581-2702-3-git-send-email-stefan@agner.ch> <20150731230901.GK10676@google.com> <9dd7975072cf16dd6ea1947bd4ae830a@agner.ch> <20150731234718.GO10676@google.com> Message-ID: User-Agent: Roundcube Webmail/1.1.2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3354 Lines: 86 On 2015-08-01 01:47, Brian Norris wrote: > On Sat, Aug 01, 2015 at 01:35:52AM +0200, Stefan Agner wrote: >> On 2015-08-01 01:09, Brian Norris wrote: > >> >> +static int vf610_nfc_read_page(struct mtd_info *mtd, struct nand_chip *chip, >> >> + uint8_t *buf, int oob_required, int page) >> >> +{ >> >> + int eccsize = chip->ecc.size; >> >> + int stat; >> >> + >> >> + vf610_nfc_read_buf(mtd, buf, eccsize); >> >> + >> >> + if (oob_required) >> >> + vf610_nfc_read_buf(mtd, chip->oob_poi, mtd->oobsize); >> > >> > To fix the bitflips issue above, you'll just want to unconditionally >> > read the OOB (it's fine to ignore 'oob_required') and... >> > >> >> + >> >> + stat = vf610_nfc_correct_data(mtd, buf); >> > >> > ...pass in chip->oob_poi as a third argument. >> > >> >> Hm, this probably will have an effect on performance, since we usually >> omit the OOB if not requested. > > You could test :) I don't really like performance claims without tests. > (I say this because I added the oob_required flag myself, but just for > functional purposes, not performance. Many drivers got by just fine by > always copying the OOB data.) Did the measurement: As is: ... [ 30.955675] mtd_speedtest: testing eraseblock write speed [ 143.349572] mtd_speedtest: eraseblock write speed is 4641 KiB/s [ 143.355606] mtd_speedtest: testing eraseblock read speed [ 183.816690] mtd_speedtest: eraseblock read speed is 12893 KiB/s [ 185.874702] mtd_speedtest: testing page write speed [ 302.608719] mtd_speedtest: page write speed is 4468 KiB/s [ 302.614229] mtd_speedtest: testing page read speed [ 343.831663] mtd_speedtest: page read speed is 12656 KiB/s ... Unconditionally read OOB: ... [ 29.076983] mtd_speedtest: testing eraseblock write speed [ 140.829920] mtd_speedtest: eraseblock write speed is 4667 KiB/s [ 140.835960] mtd_speedtest: testing eraseblock read speed [ 181.594498] mtd_speedtest: eraseblock read speed is 12798 KiB/s [ 183.652793] mtd_speedtest: testing page write speed [ 299.772069] mtd_speedtest: page write speed is 4492 KiB/s [ 299.777583] mtd_speedtest: testing page read speed [ 341.283668] mtd_speedtest: page read speed is 12568 KiB/s ... And with conditional OOB again, reading OOB if required in vf610_nfc_correct_data. ... [ 29.907147] mtd_speedtest: testing eraseblock write speed [ 141.146171] mtd_speedtest: eraseblock write speed is 4689 KiB/s [ 141.152185] mtd_speedtest: testing eraseblock read speed [ 181.644380] mtd_speedtest: eraseblock read speed is 12883 KiB/s [ 183.703198] mtd_speedtest: testing page write speed [ 299.423179] mtd_speedtest: page write speed is 4507 KiB/s [ 299.428671] mtd_speedtest: testing page read speed [ 340.695925] mtd_speedtest: page read speed is 12640 KiB/s [ 342.747510] mtd_speedtest: testing 2 page write speed ... The last test is probably pointless since we never read a empty page in the speedtest. So performance hit is measurable but small (somewhat below 100KiB/s). This is with 64 bytes OOB. Since OOB sizes are only getting bigger, I would rather still consider it... What do you think? -- Stefan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/