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

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

[Prahlad V]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 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;
 				xfer_len = wlen;
-				cmd |= QSPI_WLEN(wlen);
+				cmd = ((qspi->cmd & ~QSPI_WLEN_MASK) |
+					     QSPI_WLEN(wlen));
 			}
 			break;
 		case 2:
-- 
2.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Vignesh R]

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  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;

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.

>  				xfer_len = wlen;
> -				cmd |= QSPI_WLEN(wlen);
> +				cmd = ((qspi->cmd & ~QSPI_WLEN_MASK) |
> +					     QSPI_WLEN(wlen));

So, this won't be necessary.

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[prahlad venkata]

On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr@ti.com> 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@gmail.com>
>> ---
>>  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;
>
> 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.
>
>>                               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,
Prahlad.
+91-9663742838

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

[Vignesh R]

On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr@ti.com> 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@gmail.com>
>>> ---
>>>  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->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):

cmd = qspi->cmd | QSPI_WR_SNGL;

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

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-l0cyMroinI0@public.gmane.org> wrote:
>
>
> On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
>> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[prahlad venkata]

On Mon, Jul 11, 2016 at 2:36 PM, prahlad venkata <prahlad.eee@gmail.com> wrote:
> On Mon, Jul 11, 2016 at 2:14 PM, Vignesh R <vigneshr@ti.com> wrote:
>>
>>
>> On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
>>> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr@ti.com> 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@gmail.com>
>>>>> ---
>>>>>  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.
'cmd' is written back to qspi->cmd for every transaction.
>> 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



-- 
Regards,
Prahlad.
+91-9663742838

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

[Vignesh R]

On Monday 11 July 2016 02:39 PM, prahlad venkata wrote:
> On Mon, Jul 11, 2016 at 2:36 PM, prahlad venkata <prahlad.eee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Mon, Jul 11, 2016 at 2:14 PM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> wrote:
>>>
>>>
>>> On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
>>>> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>>  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.

> 'cmd' is written back to qspi->cmd for every transaction.

You mean qspi->cmd = cmd ?
I don't see this happening anywhere in the driver. Can you point me to
that line of code?

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[prahlad venkata]

On Mon, Jul 11, 2016 at 2:45 PM, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On Monday 11 July 2016 02:39 PM, prahlad venkata wrote:
>> On Mon, Jul 11, 2016 at 2:36 PM, prahlad venkata <prahlad.eee@gmail.com> wrote:
>>> On Mon, Jul 11, 2016 at 2:14 PM, Vignesh R <vigneshr@ti.com> wrote:
>>>>
>>>>
>>>> On Monday 11 July 2016 01:34 PM, prahlad venkata wrote:
>>>>> On Mon, Jul 11, 2016 at 10:37 AM, Vignesh R <vigneshr@ti.com> 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@gmail.com>
>>>>>>> ---
>>>>>>>  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.
>
>> 'cmd' is written back to qspi->cmd for every transaction.
>
> You mean qspi->cmd = cmd ?
> I don't see this happening anywhere in the driver. Can you point me to
> that line of code?
line 296:
ti_qspi_write(qspi, cmd, QSPI_SPI_CMD_REG);
if (ti_qspi_poll_wc(qspi)) {
    dev_err(qspi->dev, "write timed out\n");
    return -ETIMEDOUT;
 }


>
> --
> Regards
> Vignesh



-- 
Regards,
Prahlad.
+91-9663742838

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

[Vignesh R]

On Monday 11 July 2016 02:49 PM, prahlad venkata wrote:
> On Mon, Jul 11, 2016 at 2:45 PM, Vignesh R <vigneshr@ti.com> wrote:
[...]
>>>>>>>> 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.
>>
>>> 'cmd' is written back to qspi->cmd for every transaction.
>>
>> You mean qspi->cmd = cmd ?
>> I don't see this happening anywhere in the driver. Can you point me to
>> that line of code?
> line 296:
> ti_qspi_write(qspi, cmd, QSPI_SPI_CMD_REG);

Sorry, I don't understand. QSPI_SPI_CMD_REG and qspi->cmd are different.
qspi->cmd does not represent the QSPI_SPI_CMD_REG register. qspi->cmd is
just local driver data for book-keeping.

Please add some prints in driver to see how 'cmd' (and qspi->cmd)
variable changes in case of 128bit mode and 8bit mode.

Regards
Vignesh


-- 
Regards
Vignesh

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

[prahlad venkata]

On Mon, Jul 11, 2016 at 3:53 PM, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On Monday 11 July 2016 02:49 PM, prahlad venkata wrote:
>> On Mon, Jul 11, 2016 at 2:45 PM, Vignesh R <vigneshr@ti.com> wrote:
> [...]
>>>>>>>>> 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.
>>>
>>>> 'cmd' is written back to qspi->cmd for every transaction.
>>>
>>> You mean qspi->cmd = cmd ?
>>> I don't see this happening anywhere in the driver. Can you point me to
>>> that line of code?
>> line 296:
>> ti_qspi_write(qspi, cmd, QSPI_SPI_CMD_REG);
>
> Sorry, I don't understand. QSPI_SPI_CMD_REG and qspi->cmd are different.
> qspi->cmd does not represent the QSPI_SPI_CMD_REG register. qspi->cmd is
> just local driver data for book-keeping.
>
> Please add some prints in driver to see how 'cmd' (and qspi->cmd)
> variable changes in case of 128bit mode and 8bit mode.
I don't have hardware setup to verify this. Is there anyway to verify
this without hardware?
>
> Regards
> Vignesh
>
>
> --
> Regards
> Vignesh



-- 
Regards,
Prahlad.