Re: [PATCH] Revert "serial: 8250_dma: don't bother DMA with small transfers"

[Frederic Danis <frederic.danis-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

[ + Greg Kroah-Hartman ]

On 01/10/2015 10:55, Andy Shevchenko wrote:
> On Wed, 2015-09-30 at 16:11 -0400, Peter Hurley wrote:
>> [ + Heikko ]
>>
>> On 09/30/2015 10:12 AM, Andy Shevchenko wrote:
>>> On Wed, 2015-09-30 at 09:52 -0400, Peter Hurley wrote:
>>>> [ + Andy Shevchenko ]
>>>>
>>>> On 09/30/2015 08:46 AM, Frederic Danis wrote:
>>>>> This reverts commit 9119fba0cfeda6d415c9f068df66838a104b87cb.
>>>> Hi Frederic,
>>>>
>>>> It's customary to mail the commit author in patches which revert
>>>> their work.
>>>>
>>>>> This commit prevents from sending "big" file using Bluetooth.
>>>>> When sending a lot of data quickly through the Bluetooth
>>>>> interface,
>>>>> and
>>>>> after a variable amount of data sent, transfer fails with
>>>>> error:
>>>>>      kernel: [  415.247453] Bluetooth: hci0 hardware error 0x00
> Frederic, I forgot to ask how exactly the hw setup looks like?
>
> The message above is not present in the kernel (the closest what I
> found is
>
> drivers/bluetooth/btintel.c:98: BT_ERR("%s: Hardware error 0x%2.2x",
> hdev->name, code);
>
> which makes me think you have USB BT adapter attached to the same
> machine. Am I wrong?
>
>>>> Hmmm, was there any other information regarding why this
>>>> happened?
>>> Frederic informed us privately about this issue.
>> Ok, except no one else knows that.
>>
>> Least of all Greg, since the revert wasn't even addressed to him
>> either :/
> Agree, it's better to inform all parties about the problem.
>
>>> I tried to hunt the
>>> root cause but mostly (there was one observation but apparently
>>> seems
>>> it is not related to this issue) nothing looks wrong.
>>>
>>>> Eg., did the driver mistakenly re-order i/o (DMA occurred after
>>>> PIO)?
>>> I have few tracing dumps which Frederic provides to me, but I
>>> couldn't
>>> see such a problem. Maybe I missed it.
>> Ok.
>>
>>> My commit 9119fba0cfed introduces interleaved IO (DMA/PIO) which
>>> makes
>>> it like DMA -> PIO -> DMA -> ...
>> I realize that. However interleaving DMA and PIO _must_ be supported
>> by the platform to enable DMA for 8250 serial devices anway.
>>
>> When Heikki added DMA support to the 8250 driver, he left out
>> important
>> functionality which requires interleaved DMA and PIO.
>>
>> When I fix that omission, platforms using DMA that shouldn't be are
>> going
>> to get broken anyway.
>>
>> Regards,
>> Peter Hurley
>>
>>>> Or, might there be some race condition in the dmaengine driver on
>>>>
>>>> this
>>>> platform?
>>> There is [1] might be a clue, though I don't think it's directly
>>> related.
>>>
>>> [1] http://www.spinics.net/lists/linux-spi/msg01151.html
>>> The change prevents interleaved DMA/PIO IO.
>>>
>>>> Regards,
>>>> Peter Hurley
>>>>
>>>>> Found on T100TA.
>>>>>
>>>>> After reverting this commit, send works fine for any file size.
>>>>>
>>>>> Signed-off-by: Frederic Danis <frederic.danis-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>>>>> ---
>>>>>   drivers/tty/serial/8250/8250_dma.c | 4 ----
>>>>>   1 file changed, 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/tty/serial/8250/8250_dma.c
>>>>> b/drivers/tty/serial/8250/8250_dma.c
>>>>> index 21d01a4..e508939 100644
>>>>> --- a/drivers/tty/serial/8250/8250_dma.c
>>>>> +++ b/drivers/tty/serial/8250/8250_dma.c
>>>>> @@ -80,10 +80,6 @@ int serial8250_tx_dma(struct uart_8250_port
>>>>> *p)
>>>>>   		return 0;
>>>>>   
>>>>>   	dma->tx_size = CIRC_CNT_TO_END(xmit->head, xmit->tail,
>>>>>
>>>>> UART_XMIT_SIZE);
>>>>> -	if (dma->tx_size < p->port.fifosize) {
>>>>> -		ret = -EINVAL;
>>>>> -		goto err;
>>>>> -	}
>>>>>   
>>>>>   	desc = dmaengine_prep_slave_single(dma->txchan,
>>>>>   					   dma->tx_addr + xmit
>>>>> ->tail,
>>>>>


-- 
Frederic Danis                            Open Source Technology Center
frederic.danis-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org                              Intel Corporation