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

[Boris Brezillon <boris.brezillon@free-electrons.com>]

On Fri, 5 Jan 2018 14:38:48 +0100
Jonas Gorski <jonas.gorski@gmail.com> wrote:

> On 5 January 2018 at 11:21, Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com> wrote:
> > Thanks Boris for the encouragement.
> >  
> >> -----Original Message-----
> >> From: Boris Brezillon [mailto:boris.brezillon@free-electrons.com]
> >> Sent: Thursday, January 04, 2018 11:17 PM
> >> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> >> Cc: linux-mtd@lists.infradead.org; Cyrille Pitchen <cyrille.pitchen@wedev4u.fr>;
> >> Richard Weinberger <richard@nod.at>; Marek Vasut <marex@denx.de>; Brian
> >> Norris <computersforpeace@gmail.com>
> >> Subject: Re: mtd layer: support of hybrid flash(W25M161AW) having both NOR
> >> and NAND flash
> >>
> >> +MTD maintainers.
> >>
> >> On Thu, 4 Jan 2018 14:08:42 +0000
> >> Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com> wrote:
> >>  
> >> > Hi All,
> >> >
> >> > Winbond has come up with special flash i.e.  W25M161AW. It consist of Serial  
> >> NOR(Die #0) and Serial NAND(Die #1) flash.  
> >> > Means both NOR, NAND flashes are placed in W25M161AW controlled by  
> >> single chip-select.  
> >> >
> >> > "Software Die Select (C2h)" command is being used to switch die or flash.  
> >>
> >> Why are they so mean to us?! :-)
> >>  
> >> >
> >> > It looks to be quite unique chip and wondering if any kind framework or work in  
> >> progress available to handle it.  
> >> > I know that SPI-NAND framework discussions is still in progress.  
> >>
> >> Well, nothing impossible to handle, we just need to declare 2 MTD
> >> devices (one NAND and one NOR). This being said, it looks like we'll
> >> need this spi-flash abstraction we have been talking about with Marek
> >> and Cyrille to properly support these use cases: flash devices will be
> >> exposed through different sub-layers (spi-nor or spi-nand), but we need
> >> a common way to detect those spi-flash chips. I looked at a few SPI
> >> NAND and SPI NOR chips, and from what I've seen so far they were quite
> >> different (the opcodes and CMD+ADDR+DATA sequences were quite
> >> different) so I thought we were safe to start with a completely
> >> unconnected SPI NAND framework and merge some bits in a spi-flash layer
> >> afterwards, but this chip proves me wrong :-/.  
> >
> > I am thinking of following changes with fsl_qspi.c as controller
> >
> > &qspi {
> >         num-cs = <2>;
> >         bus-num = <0>;
> >         status = "okay";
> >         compatible = " fsl,ls1021a-qspi ", "fsl,ls1021a-qspi-nand"; <-- updated compatibility for drivers
> >         qflash0: w25q16fw @0 {
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >                 spi-max-frequency = <20000000>;
> >                 reg = <0>;
> >                 type = "serial-nor"   <-- Proposed New binding
> >                 is-hybrid <-- Proposed New binding
> >                 die-num <-- Proposed New binding
> >         };
> >
> >         qflash1: w25n01gw@1 {
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> >                 spi-max-frequency = <20000000>;
> >                 reg = <1>;
> >                 type = "serial-nand" <-- Proposed New binding
> >                 is-hybrid <-- Proposed New binding
> >                 die-num <-- Proposed New binding
> >         }
> > };  
> 
> assuming the NOR and NAND parts behave like "normal" SPI-NOR /
> SPI-NAND chips when selected, a more appropriate binding might be
> 
> &qspi {
>     ...
>         qflash0: dual-flash@0 {
>                 compatible = "winbond,w25q16fw";
>                 reg = <0>;
>                 spi-max-frequency = <20000000>;
>                 #address-cells = <1>;
>                 #size-cells = <0>;
> 
>                 nor@0 {
>                                 compatible = "jedec,spi-nor";
>                                 reg = <0>;
>                                 #address-cells = <1>;
>                                 #size-cells = <1>;
> 
>                                 partitions {
>                                                 ...
>                                 };
>                 };
> 
>                 nand@1 {
>                                 compatible = "jedec,spi-nand"; /* or
> whatever the correct nand-compatible would be */
>                                 reg = <1>;
>                                 #address-cells = <1>;
>                                 #size-cells = <1>;
> 
>                                 partitions {
>                                                 ...
>                                 };
>                 };
> 
>     };
> };
> 
> 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 ;-).

> 
> 
> Regards
> Jonas