Re: SPI regression with today's build

[kernel@xxxxxxxxxxxxxxxx]

> On 08.05.2019, at 19:33, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote:
> 
> [cc:Martin]
> 
> Den 08.05.2019 19.07, skrev Nicolas Saenz Julienne:
>> Small follow-up on this, and CCing Noralf as I forgot to add him on the last
>> e-mail.
>> 
>> On Wed, 2019-05-08 at 17:01 +0200, Nicolas Saenz Julienne wrote:
...
>> It seems the SPI controller thread is racing with the device's thread.
>> Something like this:
>> 
>>     SPI Controller Thread                       SPI Device Thread
>> 
>> 					    -> spi_sync_transfer() creates
>> 					       spi_message on stack then
>> 					       sleeps until finished
>> 
>> 			[SPI transfer happens...]
>> 
>> -> spi_finalize_current_message()
>>   which wakes up SPI Device Thread
>> 
>>                                            -> spi_sync_transfer() returns, the
>> 					       message disapears from the stack
>> 
>> -> spi_res_release() there is no more
>>   spi_message and the memory is
>>   potentialy used for something else
>> 
>> I've been looking at the spi_split_transfers_maxsize() code and can't think of
>> a reason why spi_res_release() couldn't be placed before
>> spi_finalize_current_message(). Which would solve the issue, but I guess Noralf
>> has a better perspective on the topic.
>> 
> 
> The problem was that spi_res_release() restored the original transfers
> before spi_unmap_msg() is called in spi_finalize_current_message() thus
> dma unmapping the original transfers, not the split ones that was mapped.
> 
> This is the accompanying change to the driver that hasn't been applied:
> [v5,3/4] spi/spi-bcm2835: Split transfers that exceed DLEN
> https://patchwork.kernel.org/patch/10899587/
> 
> Unless Martin Sperl, who wrote spi_split_transfers_maxsize(), has other
> suggestions, I think we should just revert this patch.

As per (intended) api finalize_current_message should be called before
finalize current message, as all sorts of reordering may occur and data
may get moved arround.

For example you could even transform a spi_write_then_read into a single
spi_transfer using a buffer and then copy the data back to the original
place, whioch would not be supported as is.

In the end it may even make sense to make the dma-mapping also a
spi resource transformation at the right place and move spi_res_release
before the finalize current message.

But obviously that is a bigger change to core we may not be able to
get into the current release window.

Martin