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

[Naga Sureshkumar Relli <nagasure@xilinx.com>]

Hi Boris,

> -----Original Message-----
> From: Boris Brezillon [mailto:boris.brezillon@bootlin.com]
> Sent: Tuesday, September 18, 2018 12:47 PM
> To: naga suresh kumar <nagasureshkumarrelli@gmail.com>
> Cc: Naga Sureshkumar Relli <nagasure@xilinx.com>; cyrille.pitchen@wedev4u.fr; Marek
> Vašut <marek.vasut@gmail.com>; David Woodhouse <dwmw2@infradead.org>; Brian
> Norris <computersforpeace@gmail.com>; boris.brezillon@free-electrons.com; linux-
> mtd@lists.infradead.org
> Subject: Re: [RFC PATCH 0/5] RFC for Zynq QSPI
> 
> 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.
Its working now.
> 
> 
> > > > 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.
Ok
> 
> > >
> > > 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, ...).
Ok
> 
> > > > 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.
Ok sure.

Thanks,
Naga Sureshkumar Relli.
> 
> Regards,
> 
> Boris