Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1439980 > unrolled thread

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

Started byPrahlad V <prahlad.eee@gmail.com>
First post2016-07-09 22:10 +0200
Last post2016-07-11 19:10 +0200
Articles 10 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. Prahlad V <prahlad.eee@gmail.com> - 2016-07-09 22:10 +0200
    Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word  length. Vignesh R <vigneshr@ti.com> - 2016-07-11 07:10 +0200
      Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. prahlad venkata <prahlad.eee@gmail.com> - 2016-07-11 10:10 +0200
        Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word  length. Vignesh R <vigneshr@ti.com> - 2016-07-11 10:50 +0200
          Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. prahlad venkata <prahlad.eee@gmail.com> - 2016-07-11 11:10 +0200
            Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. prahlad venkata <prahlad.eee@gmail.com> - 2016-07-11 11:20 +0200
              Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word  length. Vignesh R <vigneshr@ti.com> - 2016-07-11 11:20 +0200
                Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. prahlad venkata <prahlad.eee@gmail.com> - 2016-07-11 11:20 +0200
                  Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word  length. Vignesh R <vigneshr@ti.com> - 2016-07-11 12:30 +0200
                    Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length. prahlad venkata <prahlad.eee@gmail.com> - 2016-07-11 19:10 +0200

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

FromPrahlad V <prahlad.eee@gmail.com>
Date2016-07-09 22:10 +0200
Subject[PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.
Message-ID<rT6JA-7AO-9@gated-at.bofh.it>
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;
 				xfer_len = wlen;
-				cmd |= QSPI_WLEN(wlen);
+				cmd = ((qspi->cmd & ~QSPI_WLEN_MASK) |
+					     QSPI_WLEN(wlen));
 			}
 			break;
 		case 2:
-- 
2.5.5

[toc] | [next] | [standalone]

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

FromVignesh R <vigneshr@ti.com>
Date2016-07-11 07:10 +0200
SubjectRe: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.
Message-ID<rTBDH-2Ct-9@gated-at.bofh.it>
In reply to#1439980
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.

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

So, this won't be necessary.

-- 
Regards
Vignesh

[toc] | [prev] | [next] | [standalone]

#1440341

Fromprahlad venkata <prahlad.eee@gmail.com>
Date2016-07-11 10:10 +0200
Message-ID<rTErU-4nS-21@gated-at.bofh.it>
In reply to#1440251
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

[toc] | [prev] | [next] | [standalone]

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

FromVignesh R <vigneshr@ti.com>
Date2016-07-11 10:50 +0200
SubjectRe: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.
Message-ID<rTF4D-4Da-45@gated-at.bofh.it>
In reply to#1440341

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

[toc] | [prev] | [next] | [standalone]

#1440381

Fromprahlad venkata <prahlad.eee@gmail.com>
Date2016-07-11 11:10 +0200
Message-ID<rTFnX-4Z1-3@gated-at.bofh.it>
In reply to#1440373
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.
> 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

[toc] | [prev] | [next] | [standalone]

#1440387

Fromprahlad venkata <prahlad.eee@gmail.com>
Date2016-07-11 11:20 +0200
Message-ID<rTFxE-52u-7@gated-at.bofh.it>
In reply to#1440381
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

[toc] | [prev] | [next] | [standalone]

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

FromVignesh R <vigneshr@ti.com>
Date2016-07-11 11:20 +0200
SubjectRe: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.
Message-ID<rTFxE-52u-11@gated-at.bofh.it>
In reply to#1440387

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?

-- 
Regards
Vignesh

[toc] | [prev] | [next] | [standalone]

#1440389

Fromprahlad venkata <prahlad.eee@gmail.com>
Date2016-07-11 11:20 +0200
Message-ID<rTFxE-52u-9@gated-at.bofh.it>
In reply to#1440388
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

[toc] | [prev] | [next] | [standalone]

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

FromVignesh R <vigneshr@ti.com>
Date2016-07-11 12:30 +0200
SubjectRe: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.
Message-ID<rTGDo-5Kn-17@gated-at.bofh.it>
In reply to#1440389

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

[toc] | [prev] | [next] | [standalone]

#1440774

Fromprahlad venkata <prahlad.eee@gmail.com>
Date2016-07-11 19:10 +0200
Message-ID<rTMSt-1yI-7@gated-at.bofh.it>
In reply to#1440442
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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web