Re: mtd layer: support of hybrid flash(W25M161AW) having both NOR and NAND flash

Boris Brezillon
2018-01-08T12:31:43Z

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
...

>
> >> >
> >> > Ah, I missed that. I thought about it, and then tried to hand wave it
> >> > away with the "if they behave like normal chips" ;-)
> >> >
> >> > The mdio-bus supports nested locking, so you can do something like this:
> >> >
> >> > mutex_lock_nested(bus->mdio_lock, MDIO_BUS_NESTED);
> >> > bus->write();
> >> > bus->read();
> >> > mutex_unlock(bus->mdio_lock);
> >> >
> >> > without worrying someone else using the bus in between. [1] for an example
> >> > user.
> >> >
> >> > So going a similar approach with flagging the appropriate chips in
> >> > spi-nor/spi-nand as needing nested locking and then doing it for the
> >> > appropriate commands should solve that issue.
> >> >
> >> >
> >>
> >> Device tree update:-
> >>
> >> &qspi {
> >> ...
> >> qflash0: dual-flash at 0 {
> >> compatible = "winbond,w25q16fw", "hybrid"; <-- new compatibility value
> >
> > "hybrid" is not needed, you know that the flash is hybrid with the
> > "winbond,w25q16fw" string.
> >
> >> reg = <0>;
> >> spi-max-frequency = <20000000>;
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> nor at 0 {
> >> compatible = "jedec,spi-nor";
> >> reg = <0>;
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> partitions {
> >> ...
> >> };
> >> };
> >>
> >> nand at 1 {
> >> compatible = "jedec,spi-nand"; /* or
> >> whatever the correct nand-compatible would be */
> >> reg = <1>;
> >> #address-cells = <1>;
> >> #size-cells = <1>;
> >>
> >> partitions {
> >> ...
> >> };
> >> };
> >
> > Not sure exposing the dies in the DT is such a good idea. You should
> > have a specific handling for "winbond,w25q16fw" which registers one
> > NAND and one NOR.
> >
> >>
> >> };
> >> };
> >
> >
> >
> >>
> >>
> >> There will be only one file i.e. fsl_qspi.c handing NOR and NAND. QSPI controller will have SPI NOR and a SPI NAND controller embedded.
> >> Question: What should be the location of this file? driver/mtd/spi-nor definitely is not right place??
> >
> > It stay in driver/mtd/spi-nor until we create a spi-flash layer
> > (driver/mtd/spi-flash).
>
> Can it really stay there even if it would have NAND support implemented
> and be used by the SPI NAND framework?

Yes, as long as this is a temporary situation.

>
> How does the longterm plan for implementing SPI NAND, FSL QSPI NAND/NOR
> and spi-flash layer look like?
>
> I would propose something like this, but I'm not sure if this is
> appropriate:
>
> 1. Adding SPI NAND framework (with Micron and generic SPI)

Yep, that's the first step, and I think we should move on with what we
have and work on improving the situation (to share more code between
SPI NOR and SPI NAND) in parallel.

> 2. Adding FSL QSPI as a SPI NAND controller in mtd/nand/spi/controllers
> 3. Merging FSL QSPI NAND driver from mtd/nand/spi/controllers with NOR
> driver at mtd/spi-nor/fsl_quadspi.c

I'd really prefer to have a single driver supporting both NOR and NAND
devices. What's the problem with adding SPI NAND support to the
mtd/spi-nor/fsl_quadspi.c?

> 4. Creating spi-flash layer and move FSL QSPI NOR/NAND driver to
> mtd/spi-flash

Yep, that's the long term goal.

>
> The support for hybrid NAND/NOR chips would not be available until the
> spi-flash layer is done, right?

Exactly.

Regards,

Boris