Hi Jean,
On 09/12/19 5:12 pm, Jean Pihet wrote:
> 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).
>
How did you get this impression? Word size is independent of number of
SPI I/O lines in use. There can be a slave of 32 bit word size on a
1 bit SPI bus (SPI_NBITS_SINGLE).
Flash devices have word size of 1 byte (IOW byte addressable)
but buswidth can be 1/2/4. A Quad IO flash does not mean word size of 32 bit
word size.
> 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).
>
But we have:
list_for_each_entry(t, &m->transfers, transfer_list)
frame_len_words += t->len / (t->bits_per_word >> 3);
/* Clamps max words to 4096 words */
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);
/* FLEN field is set to max of 4095 bytes as */
qspi->cmd |= QSPI_FLEN(frame_len_words);
So, without changes in patch 3/3, there is no need to set FLEN to be set to 0xFFF.
But FLEN needs to be set to even words in case of dual/quad mode as per AM437x TRM
> 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
>
--
Regards
Vignesh
