linux-kernel - Re: [PATCH] spi: Add FSI-attached SPI controller driver

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:   Fri, 7 Feb 2020 14:59:31 -0600
From:   Eddie James <eajames@...ux.vnet.ibm.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Eddie James <eajames@...ux.ibm.com>,
        linux-spi <linux-spi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>, Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>
Subject: Re: [PATCH] spi: Add FSI-attached SPI controller driver


On 2/7/20 2:34 PM, Andy Shevchenko wrote:
> On Fri, Feb 7, 2020 at 10:04 PM Eddie James <eajames@...ux.vnet.ibm.com> wrote:
>> On 2/7/20 1:39 PM, Andy Shevchenko wrote:
>>> On Fri, Feb 7, 2020 at 9:28 PM Eddie James <eajames@...ux.vnet.ibm.com> wrote:
>>>> On 2/5/20 9:51 AM, Andy Shevchenko wrote:
>>>>> On Tue, Feb 4, 2020 at 6:06 PM Eddie James <eajames@...ux.ibm.com> wrote:
>>>>>> On 2/4/20 5:02 AM, Andy Shevchenko wrote:
>>>>>>> On Mon, Feb 3, 2020 at 10:33 PM Eddie James <eajames@...ux.vnet.ibm.com> wrote:
>>>>>>>> On 1/30/20 10:37 AM, Andy Shevchenko wrote:
>
>>>>>>>>>> +       for (i = 0; i < num_bytes; ++i)
>>>>>>>>>> +               rx[i] = (u8)((in >> (8 * ((num_bytes - 1) - i))) & 0xffULL);
>>>>>>>>> Redundant & 0xffULL part.
>>>>> For me it looks like
>>>>>
>>>>>      u8 tmp[8];
>>>>>
>>>>>      put_unaligned_be64(in, tmp);
>>>>>      memcpy(rx, tmp, num_bytes);
>>>>>
>>>>> put_unaligned*() is just a method to unroll the value to the u8 buffer.
>>>>> See, for example, linux/unaligned/be_byteshift.h implementation.
>>>> Unforunately it is not the same. put_unaligned_be64 will take the
>>>> highest 8 bits (0xff00000000000000) and move it into tmp[0]. Then
>>>> 0x00ff000000000000 into tmp[1], etc. This is only correct for this
>>>> driver IF my transfer is 8 bytes. If, for example, I transfer 5 bytes,
>>>> then I need 0x000000ff00000000 into tmp[0], 0x00000000ff000000 into
>>>> tmp[1], etc. So I think my current implementation is correct.
>>> Yes, I missed correction of the start address in memcpy(). Otherwise
>>> it's still the same what I was talking about.
>>
>> I see now, yes, thanks.
>>
>> Do you think this is worth a v3? Perhaps put_unaligned is slightly more
>> optimized than the loop but there is more memory copy with that way too.
> I already forgot the entire context when this has been called. Can you
> summarize what the sequence(s) of num_bytes are expected usually.
>
> IIUC if packets small, less than 8 bytes, than num_bytes will be that value.
> Otherwise it will be something like 8 + 8 + 8 ... + tail. Is it
> correct assumption?


Yes, it will typically be 8 + 8 +... remainder. Basically, on any RX, 
the driver polls for the rx register full. Once full, it will read 
however much data is left to be transferred. Since we use min(len, 8) 
then we read 8 usually, until we get to the end.


>
>>>>>>>>>> +       return num_bytes;
>>>>>>>>>> +}