lkml.org 
[lkml]   [2015]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 18/27] mtd: nand: omap2: Implement NAND ready using gpiolib
(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@gmail.com> 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


\
 
 \ /
  Last update: 2015-12-03 06:01    [W:0.151 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site