Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751882AbdFFVLX (ORCPT ); Tue, 6 Jun 2017 17:11:23 -0400 Received: from mail.free-electrons.com ([62.4.15.54]:52796 "EHLO mail.free-electrons.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751562AbdFFVLW (ORCPT ); Tue, 6 Jun 2017 17:11:22 -0400 Date: Tue, 6 Jun 2017 23:11:19 +0200 From: Boris Brezillon To: KOBAYASHI Yoshitake Cc: richard@nod.at, dwmw2@infradead.org, computersforpeace@gmail.com, marek.vasut@gmail.com, cyrille.pitchen@wedev4u.fr, linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH -next] mtd: nand: Add support for Toshiba BENAND (Built-in ECC NAND) Message-ID: <20170606231119.5f0fcf85@bbrezillon> In-Reply-To: References: <1495761335-20535-1-git-send-email-yoshitake.kobayashi@toshiba.co.jp> <20170526182254.6a4f98c8@bbrezillon> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1878 Lines: 51 On Mon, 5 Jun 2017 14:36:23 +0900 KOBAYASHI Yoshitake wrote: > Hi Boris, > > Thank you very much for your detailed review. > I just comming back from busy week for conference and started to look it. > > I have a question regarding to the following comment. > > >> static int toshiba_nand_init(struct nand_chip *chip) > >> { > >> + struct mtd_info *mtd = nand_to_mtd(chip); > >> + > >> if (nand_is_slc(chip)) > >> chip->bbt_options |= NAND_BBT_SCAN2NDPAGE; > >> > >> + if (chip->ecc.mode == NAND_ECC_BENAND) { > >> + chip->ecc.options = NAND_ECC_CUSTOM_PAGE_ACCESS; > >> + chip->ecc.bytes = 0; > > > > It should be set to 16 according to the datasheet. But I guess this is > > not exactly true. I'm pretty sure we can use some of these bytes to > > store real data. Assuming you're using BCH, only 13bytes are needed for > > 8bits/512bytes strength, and I guess the BBM region is also left > > untouched (first 2 bytes of the OOB region). > > On BENAND, all OOB reginon can be used by user (driver). The calculated > ECC data stored into other isolated area which is ubable to access from user. > This is why chip->ecc.bytes = 0. Oh, nice! > To make sure this specification, I also checked the BENAND datasheet > but unfortunatelly it was undocumented. Sorry for this confusion. No problem. Do you think you can update the datasheet (or ask someone who can) to clarify this aspect? As you said, it's really not clear that these ECC bytes are actually stored in a dedicated region that is invisible to users. > > I think I can add comment here or add definition somewhere to describe this > specification. Yes please, add a comment in the code, just above the ->ecc.bytes = 0 assignment. > If you have any preference or suggestion to describe this, please let me know. Nope. Just put the explanation you give here.