Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751496AbdDCHF5 (ORCPT ); Mon, 3 Apr 2017 03:05:57 -0400 Received: from conssluserg-06.nifty.com ([210.131.2.91]:28964 "EHLO conssluserg-06.nifty.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751220AbdDCHF4 (ORCPT ); Mon, 3 Apr 2017 03:05:56 -0400 DKIM-Filter: OpenDKIM Filter v2.10.3 conssluserg-06.nifty.com v3375gQn023728 X-Nifty-SrcIP: [209.85.213.177] MIME-Version: 1.0 In-Reply-To: <20170330181623.5b595827@bbrezillon> References: <1490856383-31560-1-git-send-email-yamada.masahiro@socionext.com> <1490856383-31560-27-git-send-email-yamada.masahiro@socionext.com> <20170330181623.5b595827@bbrezillon> From: Masahiro Yamada Date: Mon, 3 Apr 2017 16:05:40 +0900 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH v3 26/37] mtd: nand: denali: fix bank reset function To: Boris Brezillon Cc: linux-mtd@lists.infradead.org, Enrico Jorns , Artem Bityutskiy , Dinh Nguyen , Marek Vasut , Graham Moore , David Woodhouse , Masami Hiramatsu , Chuanxiao Dong , Jassi Brar , Linux Kernel Mailing List , Brian Norris , Richard Weinberger , Cyrille Pitchen Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2807 Lines: 87 Hi Boris, 2017-03-31 1:16 GMT+09:00 Boris Brezillon : > On Thu, 30 Mar 2017 15:46:12 +0900 > Masahiro Yamada wrote: > >> The function denali_nand_reset() is called during the driver probe, >> and polls the INTR__RST_COMP and INTR__TIME_OUT bits. However, >> INTR__RST_COMP is set anyway even if no NAND device is connected to >> that bank. >> >> This can be a problem for ONFi devices. The nand_scan_ident() >> iterates over maxchips, and calls nand_reset() for each chip. > > Actually, maxchips is a bad name. What should be passed in argument to > nand_scan_ident() is not the maximum number of CS-line the controller > has, it's the expected number of CS-lines provided by a chip. > > If you're using DT, this information should be retrieved from the DT. If > you look at this binding doc [1] you'll see that each NAND chip has a > reg property encoding the CS line. When a chip exposes more than one > CS-line, the reg property should contain 2 entries describing which > controller-side CS lines are connected to the chip CS-lines. Actually, the Denali driver is much older than commit 2d472aba15ff169 ("mtd: nand: document the NAND controller/NAND chip DT representation"). So, I guess it is allowed to use the old binding, then you were kind to merge my commit 63757d463ea683b469c1976032054d46cecdef09 Author: Masahiro Yamada Date: Thu Mar 23 05:07:18 2017 +0900 mtd: nand: denali: call nand_set_flash_node() to set DT node Having said that, I have to admit the current implementation (not my fault) is not nice in a long run. In order to switch to the new binding, I have to de-couple the controller and chips (like you did for sunxi_nand) I am taking a close look for how much efforts are needed (because I am guessing you will recommend me to do this work :-) ) > For non-DT cases, this should be exposed by some other means (for > example pdata, but I'm not sure it works well with PCI where everything > is discoverable). > > So normally, you shouldn't have a timeout, or something is wrong with > the DT/board description. > > Note that you might have different NAND models connected to the same > NAND controller. If you call nand_scan_ident() only once and pass > controllers->max_cs_lines to it, you will only have one chip detected, > which is not what you expect. The Denali IP actually supports multiple chip selects, but has only a single set of parameter registers. For example, DEVICE_MAIN_AREA_SIZE must match to a chip's mtd->writesize. If denali_select_chip() updates all parameter registers every time, it would be theoretically possible to use different models. (And, looks like sunxi_select_chip does this.) -- Best Regards Masahiro Yamada