All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@free-electrons.com>
To: Jonas Gorski <jonas.gorski@gmail.com>
Cc: Frieder Schrempf <frieder.schrempf@exceet.de>,
	MTD Maling List <linux-mtd@lists.infradead.org>,
	Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>,
	Marek Vasut <marex@denx.de>, Richard Weinberger <richard@nod.at>,
	Brian Norris <computersforpeace@gmail.com>,
	Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>,
	Suresh Gupta <suresh.gupta@nxp.com>,
	Poonam Aggrwal <poonam.aggrwal@nxp.com>
Subject: Re: mtd layer: support of hybrid flash(W25M161AW) having both NOR and NAND flash
Date: Mon, 8 Jan 2018 15:32:49 +0100	[thread overview]
Message-ID: <20180108153249.24d05341@bbrezillon> (raw)
In-Reply-To: <CAOiHx=kw=L-H1yCwnzhfJhg3vUeXiTz_39n4_Wq3qbKMdDCitA@mail.gmail.com>

On Mon, 8 Jan 2018 14:43:56 +0100
Jonas Gorski <jonas.gorski@gmail.com> wrote:

> On 8 January 2018 at 13:31, Boris Brezillon
> <boris.brezillon@free-electrons.com> wrote:
> > Hi Frieder,
> >
> > On Mon, 8 Jan 2018 12:02:19 +0100
> > Frieder Schrempf <frieder.schrempf@exceet.de> wrote:
> >  
> >> >> > >> with the "windbond,w25q16fw" driver modeled as a simple
> >> >> > >> "spi-multiplexer" that registers its own virtual spi-bus. Then when
> >> >> > >> spi-nor or spi-nand tries to communicate with their appropriate die,
> >> >> > >> it sends the Software Die Select command if needed and then passes on
> >> >> > >> the message to its parent bus.
> >> >> > >>
> >> >> > >> That way there should be no changes needed for spi-nor / spi-nand
> >> >> > >> themselves. (The devil is probably in the details ;-)  
> >> >> > >
> >> >> > > Yep, I thought about this approach, and it's indeed quite elegant, but
> >> >> > > we're missing the lock I was mentioning in my previous reply. We need
> >> >> > > to prevent die selection not only for the time we're sending a single
> >> >> > > SPI message, but for the whole operation (which can be formed of
> >> >> > > several SPI messages). Or maybe I'm wrong, and operations can actually
> >> >> > > be interleaved, but I wouldn't bet on that ;-).  
> >>
> >> With operations, that consist of several SPI messages, do you mean
> >> something like NAND page program?  
> >
> > Yep.
> >  
> >>
> >> Because I'm quite sure something like this should be possible (and all
> >> of these commands consist only of one spi message, don't they?):
> >> 1. Select second (NAND) die
> >> 2. Send data to the NAND page buffer (PROGRAM)
> >> 3. Select first (NOR) die
> >> 4. Program data to a NOR block
> >> 5. Select second (NAND) die
> >> 6. Send command to transfer page buffer to flash (PROGRAM EXECUTE)  
> >
> > Yes, and that's the problem, if you don't have a lock, the sequence you
> > describe above could be re-ordered like this:
> >
> > 1. Select second (NAND) die
> > 2. Select first (NOR) die
> > 3. Send data to the NAND page buffer (PROGRAM)
> > 4. Program data to a NOR block
> > ...  
> 
> At least with what I was thinking of (with the spi-multiplexer) this
> should be impossible, as the (virtual) spi bus message pump should
> ensure that any messages from spi-nand and spi-nor are properly
> serialized so they could only end up as in Frieder's example, unless
> I'm overlooking something.
> 
> So my virtual's spi implementation would look like this:
> 
> struct W25M161AW {
>         struct spi_device *spi; /* we at the parent bus */
> };
> 
> int W25M161AW_transfer_one_message(master, message)
> {
>         W25M161AW *w = spi_master_get_devdata(master);
>         spi_device *spi = message->spi;
> 
>         W25M161AW_send_die_select(w->spi, spi->chip_select);
> 
>         /* pass on the message to the parent bus */
>         spi_sync(w->spi, msg);
>         spi_finalize_current_message(master);
> 
>         return 0;
> }

I wish all QSPI controllers were exposed as regular SPI controllers with
QSPI capabilities, unfortunately most of them are standalone drivers
that simply does not implement the generic SPI interface and instead
directly register to their flash devices to SPI NOR layer, thus
preventing the generic logic you're proposing here :-(.

> 
> (this is a very simplified version, of course we can't pass on the
> message directly to the parent bus but need to clone it)
> 
> This way AFAICT there should be a guarantee that there can be no other
> message to the flash scheduled between the die select and the message
> to the die.
> 
> Of course if we need to ensure that there is no die change between the
> PROGRAM and the PROGRAM EXECUTE we need the extra bus lock. Without a
> public datasheet I can't tell though.

I found this one [1] if you want to have a look.

Regards,

Boris

[1]http://www.winbond.com.tw/resource-files/w25m161av%20combo%20reva%20091317%20mod%20final.pdf

  reply	other threads:[~2018-01-08 14:33 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-04 14:08 mtd layer: support of hybrid flash(W25M161AW) having both NOR and NAND flash Prabhakar Kushwaha
2018-01-04 17:47 ` Boris Brezillon
2018-01-05 10:21   ` Prabhakar Kushwaha
2018-01-05 13:35     ` Boris Brezillon
2018-01-05 13:38     ` Jonas Gorski
2018-01-05 13:44       ` Boris Brezillon
2018-01-05 13:58         ` Jonas Gorski
2018-01-08  8:42           ` Prabhakar Kushwaha
2018-01-08  9:14             ` Boris Brezillon
2018-01-08  9:39               ` Jonas Gorski
2018-01-08  9:47                 ` Boris Brezillon
2018-01-08 11:02               ` Frieder Schrempf
2018-01-08 12:31                 ` Boris Brezillon
2018-01-08 13:14                   ` Frieder Schrempf
2018-01-08 13:43                   ` Jonas Gorski
2018-01-08 14:32                     ` Boris Brezillon [this message]
2018-01-08 15:18                       ` Jonas Gorski
2018-01-08 15:45                         ` Boris Brezillon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180108153249.24d05341@bbrezillon \
    --to=boris.brezillon@free-electrons.com \
    --cc=computersforpeace@gmail.com \
    --cc=cyrille.pitchen@wedev4u.fr \
    --cc=frieder.schrempf@exceet.de \
    --cc=jonas.gorski@gmail.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=poonam.aggrwal@nxp.com \
    --cc=prabhakar.kushwaha@nxp.com \
    --cc=richard@nod.at \
    --cc=suresh.gupta@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.