Re: [RFC PATCH 0/5] RFC for Zynq QSPI

[Boris Brezillon <boris.brezillon@bootlin.com>]

Hi Naga,

On Tue, 18 Sep 2018 11:26:05 +0530
naga suresh kumar <nagasureshkumarrelli@gmail.com> wrote:

> Hi,
> 
> Please see my reply inline by prefacing my name, currently i am facing
> issues with my outlook. the reply looks not good but please don't mind.

I do mind. Please find a way to fix that, 'cause I'm pretty sure others
will soon complain about that too.


> > > As i said, it needs tweaking the mtd->size and spi read/write addresses.  
> >
> > Can you elaborate a bit on why you think this is needed? Did you look
> > at [1]? Maybe it will prevent us from exposing the flash size at the
> > spi-mem level.
> > [naga]: some example snippet below
> >
> > spi_nor_read () {
> >
> >    if (nor->isparallel == 1)
> >
> >    {
> >
> >       offset /= 2; //byte stripping will happen here
> >
> >        nor->spi->master->flags |= SPI_DATA_STRIPE;
> >
> >        //By setting this flag even bits of data word located in lower
> > memory and odd are in upper memory
> >
> >    }
> >
> >    if (nor->isstacked == 1) {
> >
> >      stack_shift = 1;
> >
> >      if (offset >= (mtd->size / 2)) {
> >
> >      offset = offset - (mtd->size / 2);
> >
> >      nor->spi->master->flags |= SPI_MASTER_U_PAGE;
> >
> >      //We can access Upper memory or lower memory, by setting this flag in
> > controller.
> >
> >   } else {
> >
> >         nor->spi->master->flags &= ~SPI_MASTER_U_PAGE;
> >
> >   }
> >
> >  }
> >
> > same for spi_nor_write() also.

Let's put the stacked and parallel support on the side for now and
focus on single die/chip support. Will find a clean way to handle that
afterwards.

> >
> > Also in both dual parallel and stacked mode mtd->size should be sum of
> > both flash device sizes

Yes, but this information should stay at the spi-nor/mtd level.

> >
> > also status register read will change, it needs status from both devices.
> > like that some changes
> >
> > will be needed in core.  

Which is why this needs to be handled in spi-nor.c. The spi-nor core
needs to know about the parallel/stack setup and handle it differently
(read both status bytes, read both IDs, ...).

> > > Can somebody share your thoughts on this(Adding Zynq QSPI Dual parallel  
> > and  
> > > stacked)?  
> >
> > I always have a hard time with this naming. I guess stacked is when you
> > have 2 chips sharing the same I/O bus, and parallel is when you have 2
> > chips with one taking all of the I/O and the other taking the other
> > half. Is that correct?
> > [naga]: Yes, you are correct, i have attached the diagrams for these mode.
> >
> > In parallel mode, with the help of controller STRIPE feature, even bits of
> >
> > data words are located in lower memory and odd bits are located in upper
> > memory.
> >
> > Data management will be taken care by controller in both modes

Ok. I need to think about it a bit more. In the meantime, I recommend
that you submit a version of the driver that does not support parallel
and stacked modes.

Regards,

Boris