From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C9BDFC282C2 for ; Thu, 7 Feb 2019 13:01:42 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id A383521904 for ; Thu, 7 Feb 2019 13:01:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727347AbfBGNBl convert rfc822-to-8bit (ORCPT ); Thu, 7 Feb 2019 08:01:41 -0500 Received: from relay11.mail.gandi.net ([217.70.178.231]:37423 "EHLO relay11.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726894AbfBGNBk (ORCPT ); Thu, 7 Feb 2019 08:01:40 -0500 Received: from xps13 (aaubervilliers-681-1-80-177.w90-88.abo.wanadoo.fr [90.88.22.177]) (Authenticated sender: miquel.raynal@bootlin.com) by relay11.mail.gandi.net (Postfix) with ESMTPSA id 587DC100002; Thu, 7 Feb 2019 13:01:36 +0000 (UTC) Date: Thu, 7 Feb 2019 14:01:36 +0100 From: Miquel Raynal To: Masahiro Yamada Cc: linux-mtd , Boris Brezillon , Brian Norris , Linux Kernel Mailing List , Marek Vasut , Richard Weinberger , David Woodhouse , Boris Brezillon Subject: Re: [PATCH] mtd: rawnand: call onfi_fill_data_interface() once again after nand_detect Message-ID: <20190207140136.2ae62982@xps13> In-Reply-To: References: <1549533476-3742-1-git-send-email-yamada.masahiro@socionext.com> <20190207111606.6b72dc34@xps13> Organization: Bootlin X-Mailer: Claws Mail 3.17.1 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Masahiro, Masahiro Yamada wrote on Thu, 7 Feb 2019 19:46:54 +0900: > On Thu, Feb 7, 2019 at 7:16 PM Miquel Raynal wrote: > > > > Hi Masahiro, > > > > Masahiro Yamada wrote on Thu, 7 Feb > > 2019 18:57:56 +0900: > > > > > nand_scan_ident() calls onfi_fill_data_interface() at its entry > > > to set up the initial timing parameters. > > > > > > The timing parameters are needed not only for ->setup_data_interface(), > > > but also for giving the correct delay to NAND_OP_WAIT_RDY, for example. > > > > > > If the driver sets the NAND_KEEP_TIMINGS flag, or does not support > > > ->setup_data_interface() hook, those parameters will never updated. > > > > ^ be > > Will fix (if v2 is welcome) > > > > > > > > Before nand_detect(), we never know whether the chip is ONFi or not. > > > So, onfi_fill_data_interface() has to assume the worst case, i.e. > > > non-ONFi. > > > > s/ONFi/ONFI/? > > Will fix. > > Looks like I was misunderstanding > maybe because the letter 'I' in the logo > (http://www.onfi.org/) > looks like a lowercase... > > Oh right. I don't know what's best. Pick your favorite :) > > > > > > > > > > After nand_detect(), if the chip turns out to be ONFi-compliant, > > > we can optimize tPROG_max, tBERS_max, etc. > > > > > > Call onfi_fill_data_interface() once again. > > > > Sorry but I don't get why this is needed as there is the same call at > > the top of this function. Can you be more specific on where/when the > > missing call produces a failure? > > > onfi_fill_data_interface() sets different values > for tPROG_max, tBER_max, tR_max, tCCS_min > depending on whether the chip is ONFI or not. > > For the first call, onfi_fill_data_interface() > chooses the else-part since we never know > the chip specification at this point. > > If we call onfi_fill_data_interface() once again > after nand_detect(), it may choose the if-part. > > > If a driver supports ->setup_data_interface(), > nand_init_data_interface() will set the optimal > timing parameters anyway. > > But, if a driver does not support ->setup_data_interface(), > it will not happen since nand_has_setup_data_iface() returns false. And I think this is the expected behavior. Calling again onfi_fill_data_interface() would probably enhance a bit the timings. The effect is that later exchanges with the NAND chip would be just a bit faster. But if you care about performance, then why not implementing ->setup_data_interface()? Even a dummy implementation would do the trick: only accept timing mode 0 without any changes on the controller side. Unless you give me a use case where this is not possible, I don't think it is worth changing this path. Thanks, Miquèl