Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.

[prahlad venkata]

On Mon, Jul 11, 2016 at 2:14 PM, Vignesh R <vigneshr@xxxxxx> wrote:
>
>
> On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
>> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr@xxxxxx> wrote:
>>> Hi Prahlad,
>>>
>>> On Sunday 10 July 2016 01:35 AM, Prahlad V wrote:
>>>> When a word length of 1 byte is selected and writing data of length
>>>> more than QSPI_WLEN_MAX_BYTES, first MAX_BYTES will be transfered
>>>> and remaining will be transfered byte by byte. In that case wlen
>>>> field should be cleared before setting.
>>>>
>>>> Signed-off-by: Prahlad V <prahlad.eee@xxxxxxxxx>
>>>> ---
>>>>  drivers/spi/spi-ti-qspi.c | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
>>>> index 29ea8d2..6c61f54 100644
>>>> --- a/drivers/spi/spi-ti-qspi.c
>>>> +++ b/drivers/spi/spi-ti-qspi.c
>>>> @@ -276,9 +276,9 @@ static int qspi_write_msg(struct ti_qspi *qspi, struct spi_transfer *t,
>>>>                               cmd |= QSPI_WLEN(QSPI_WLEN_MAX_BITS);
>>>>                       } else {
>>>>                               writeb(*txbuf, qspi->base + QSPI_SPI_DATA_REG);
>>>> -                             cmd = qspi->cmd | QSPI_WR_SNGL;
>
> This is wrong. Deleting this line means QSPI_WR_SNGL is not set and no
> data is sent out on the wire.
QSPI_WR_SNGL is already set as soon as we enter the function.
>
>>>
>>> qspi->cmd always has WLEN field cleared and set to WLEN = 1 byte (see
>>> ti_qspi_start_transfer_one()). And hence variable 'cmd' will also have
>>> WLEN set to 1 byte.
>> Even though WLEN=1 is set in the ti_qspi_transfer_one, if we ask for a
>> transfer of large data,
>> say 300 bytes in length, for attaining faster data rate WLEN 128 is
>> selected for the first two
>> transactions and remaining 44 bytes will be transmitted with WLEN 1.
>> During that case,
>> WLEN will be changed inside qspi_write_msg function itself and the
>> field should be cleared
>> first while doing that.
>
> In qspi_write_msg(), qspi->cmd will always have WLEN set to
> QSPI_WLEN(t->bits_per_word) and qspi->cmd is never changed within this
> function.
> It is the value of local variable 'cmd' that is changed to appropriate
> WLEN (128bit or 8bit) as necessary.
> So, as per you example, for the first 18 transactions(16*18 = 288bytes)
> 'cmd' variable will have WLEN field set to 0x7F(128bit). But for the
> last 12 bytes, cmd is reinitialized to (that's the line being deleted
> above):
exactly.
>
> cmd = qspi->cmd | QSPI_WR_SNGL;
This will select single wire write operation, which is already done.
WLEN field should be set back to 1 clearing 0x7f from the previous transaction.
>
> which means WLEN field is set 1 byte.
>
> So the below change won't be necessary. Or am I missing something?
>
>>>
>>>>                               xfer_len = wlen;
>>>> -                             cmd |= QSPI_WLEN(wlen);
>>>> +                             cmd = ((qspi->cmd & ~QSPI_WLEN_MASK) |
>>>> +                                          QSPI_WLEN(wlen));
>>>
>>> So, this won't be necessary.
>>>
>>> --
>>> Regards
>>> Vignesh
>>
>>
>>
>
> --
> Regards
> Vignesh



-- 
Regards,
Prahlad.
+91-9663742838