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

Boris Brezillon
2018-09-18T07:17:29Z

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