(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 <computersforpeace@xxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html