Re: [PATCH 2/3] TI QSPI: support large flash devices

[Jean Pihet <jean.pihet@xxxxxxxxxxxxxx>]

Hi Vignesh,

On Mon, Dec 9, 2019 at 11:27 AM Vignesh Raghavendra <vigneshr@xxxxxx> wrote:
>
>
>
> On 06/12/19 9:30 pm, Jean Pihet wrote:
> > The TI QSPI IP has limitations:
> > - the MMIO region is 64MB in size
> > - in non-MMIO mode, the transfer can handle 4096 words max.
> >
> > Add support for bigger devices.
> > Use MMIO and DMA transfers below the 64MB boundary, use
> > software generated transfers above.
>
> Could you change the $subject prefix to be "spi: spi-ti-qspi:"

Yes sure.

>
>
> [...]
>
> > -574,6 +601,7 @@ static int ti_qspi_exec_mem_op(struct spi_mem *mem,
> >
> >  static const struct spi_controller_mem_ops ti_qspi_mem_ops = {
> >       .exec_op = ti_qspi_exec_mem_op,
> > +     .adjust_op_size = ti_qspi_adjust_op_size,
> >  };
> >
> >  static int ti_qspi_start_transfer_one(struct spi_master *master,
> > @@ -599,12 +627,11 @@ static int ti_qspi_start_transfer_one(struct spi_master *master,
> >       frame_len_words = 0;
> >       list_for_each_entry(t, &m->transfers, transfer_list)
> >               frame_len_words += t->len / (t->bits_per_word >> 3);
> > -     frame_len_words = min_t(unsigned int, frame_len_words, QSPI_FRAME);
> >
> >       /* setup command reg */
> >       qspi->cmd = 0;
> >       qspi->cmd |= QSPI_EN_CS(spi->chip_select);
> > -     qspi->cmd |= QSPI_FLEN(frame_len_words);
> > +     qspi->cmd |= QSPI_FLEN(QSPI_FRAME);
> >
>
> Hmm, change itself is harmless. But what is the motivation behind the
> change?
Indeed this change does not hurt but is required to prevent an invalid
FLEN value.

Here are the details:
- t->len is in bytes,
- the length passed to qspi_transfer_msg is in bytes,
- frame_len_words is the number of words in the SPI model,
- the words as used by the TI QSPI IP is not the same as
frame_len_words. In TI QSPI the word size depends on the number of
I/Os i in use (SPI_NBITS_xxx and op->data.buswidth).

For example a quad I/O transfer with t->bits_per_word=8 generates 4
bytes of data every 8 clock cycles. In this case frame_len_words =
16384. The maximum of bytes transferred by TI QSPI is 4096 * 4 =
16384. Setting FLEN to frame_len_words leads to an invalid value (max
value is 0xFFF + 1).

So this changes takes the I/O mode into account by limiting the
maximum number of bytes per frame in ti_qspi_adjust_op_size, by
programming FLEN with the maximum allowed value and by stopping the
transfer when the data is transferred.

Does this make sense?

Regards,
Jean