From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758752AbbLCEpu (ORCPT ); Wed, 2 Dec 2015 23:45:50 -0500 Received: from mail-pa0-f47.google.com ([209.85.220.47]:35645 "EHLO mail-pa0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758149AbbLCEps (ORCPT ); Wed, 2 Dec 2015 23:45:48 -0500 Date: Wed, 2 Dec 2015 20:45:44 -0800 From: Brian Norris To: Boris Brezillon Cc: Roger Quadros , tony@atomide.com, dwmw2@infradead.org, ezequiel@vanguardiasur.com.ar, javier@dowhile0.org, fcooper@ti.com, nsekhar@ti.com, linux-mtd@lists.infradead.org, linux-omap@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Alex Smith , Harvey Hunt Subject: Re: [PATCH v3 18/27] mtd: nand: omap2: Implement NAND ready using gpiolib Message-ID: <20151203044544.GC120110@google.com> References: <1442588029-13769-1-git-send-email-rogerq@ti.com> <1442588029-13769-19-git-send-email-rogerq@ti.com> <20151026204900.GI13239@google.com> <20151027092832.4d7bb114@bbrezillon> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151027092832.4d7bb114@bbrezillon> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org (to be clear, this branch of discussion isn't directly regarding the TI changes; we can handle any generic handling afterward, as long as we get the DT binding right now) On Tue, Oct 27, 2015 at 09:28:32AM +0100, Boris Brezillon wrote: > On Mon, 26 Oct 2015 13:49:00 -0700 > Brian Norris wrote: > > On Fri, Sep 18, 2015 at 05:53:40PM +0300, Roger Quadros wrote: > > > @@ -1782,7 +1780,9 @@ static int omap_nand_probe(struct platform_device *pdev) > > > info->reg = pdata->reg; > > > info->of_node = pdata->of_node; > > > info->ecc_opt = pdata->ecc_opt; > > > - info->dev_ready = pdata->dev_ready; > > > + if (pdata->dev_ready) > > > + dev_info(&pdev->dev, "pdata->dev_ready is deprecated\n"); > > > + > > > info->xfer_type = pdata->xfer_type; > > > info->devsize = pdata->devsize; > > > info->elm_of_node = pdata->elm_of_node; > > > @@ -1815,6 +1815,13 @@ static int omap_nand_probe(struct platform_device *pdev) > > > nand_chip->IO_ADDR_W = nand_chip->IO_ADDR_R; > > > nand_chip->cmd_ctrl = omap_hwcontrol; > > > > > > + info->ready_gpiod = devm_gpiod_get_optional(&pdev->dev, "ready", > > > + GPIOD_IN); > > > > Others have been looking at using GPIOs for the ready/busy pin too. At a > > minimum, we need an updated DT binding doc for this, since I see you're > > adding this via device tree in a later patch (I don't see any DT binding > > patch for this; but I could just be overlooking it). It'd also be great > > if this support was moved to nand_dt_init() so other platforms can > > benefit, but I won't require that. > > Actually I started to work on a generic solution parsing the DT and > creating CS, WP and RB gpios when they are provided, but I think it's a > bit more complicated than just moving the rb-gpios parsing into > nand_dt_init(). > First you'll need something to store your gpio_desc pointers, which > means you'll have to allocate a table of gpio_desc pointers (or a table > of struct embedding a gpio_desc pointer). I'm not sure what you mean by table. It seems like we would just have a few gpio_desc pointers in struct nand_chip, no? > The other blocking point is that when nand_scan_ident() is called, the > caller is supposed to have filled the ->dev_ready() or ->waitfunc() > fields, and to choose how to implement it he may need to know > which kind of RB handler should be used (this is the case in the sunxi > driver, where the user can either use a GPIO or native R/B pin directly > connected to the controller). Right, I was thinking about this one. > All this makes me think that maybe nand_dt_init() should be called > separately or in a different helper (nand_init() ?) taking care of the > basic nand_chip initializations/allocations without interacting with > the NAND itself. Yeah, I feel like there have been other good reasons for something like this before, and we just have worked around them so far. Maybe something more like the alloc/add split in many other subsystems? e.g., platform_device_{alloc,add}, spi_{alloc,add}_device. Now we might want nand_alloc()? On a slight tangent, it seems silly that nand_scan_tail() doesn't register the MTD, but nand_release() unregisters it. That shouldn't be. > Another solution would be to add an ->init() function to nand_chip > and call it after the generic initialization has been done (but before > NAND chip detection). This way the NAND controller driver could adapt > some fields and parse controller specific properties. That could work too, I guess. > What do you think? Brian