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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 11 Jul 2016 14:39:54 +0530
From:	prahlad venkata <prahlad.eee@...il.com>
To:	Vignesh R <vigneshr@...com>
Cc:	"broonie@...nel.org" <broonie@...nel.org>,
	"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] spi: spi-ti-qspi: clear wlen field while setting word length.

On Mon, Jul 11, 2016 at 2:36 PM, prahlad venkata <prahlad.eee@...il.com> wrote:
> On Mon, Jul 11, 2016 at 2:14 PM, Vignesh R <vigneshr@...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@...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@...il.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