Re: [PATCH 11/15] tty: msm_serial: use dmaengine_prep_slave_sg()

[Marek Szyprowski]

On 19.04.2024 09:53, Jiri Slaby wrote:
> On 19. 04. 24, 9:43, Jiri Slaby wrote:
>> On 17. 04. 24, 14:45, Marek Szyprowski wrote:
>>> I alse tried to change the "if (dma->mapped)" check in 
>>> msm_start_tx() to:
>>>
>>> 1. if (dma->tx_sg.length)
>>>
>>> 2. if (dma->tx_sg.page_link & ~SG_PAGE_LINK_MASK)
>>>
>>> but none of the above worked what is really strange and 
>>> incomprehensible
>>> for me.
>>>
>>
>> Aaaah, nevermind, what about this?
>>
>> Two bugs:
>> 1) dma_map_sg() returns the number of mapped entries. Not zero!
>> 2) And I forgot to zero tx_sg in case of error.
>>
>> --- a/drivers/tty/serial/msm_serial.c
>> +++ b/drivers/tty/serial/msm_serial.c
>> @@ -506,8 +506,8 @@ static int msm_handle_tx_dma(struct msm_port 
>> *msm_port, unsigned int count)
>>          kfifo_dma_out_prepare(&tport->xmit_fifo, &dma->tx_sg, 1, 
>> count);
>>
>>          ret = dma_map_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>> -       if (ret)
>> -               return ret;
>> +       if (!ret)
>
> Still wrong, ret = -EIO missing in here.

"if (ret <= 0)" seems to be better here.

Indeed this fixed the issue. I checked the code many times, but I missed 
this dma_map_sg() return value issue.

Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>

>
>> +               goto zero_out;
>>
>>          dma->desc = dmaengine_prep_slave_sg(dma->chan, &dma->tx_sg, 1,
>>                                                  DMA_MEM_TO_DEV,
>> @@ -548,6 +548,7 @@ static int msm_handle_tx_dma(struct msm_port 
>> *msm_port, unsigned int count)
>>          return 0;
>>   unmap:
>>          dma_unmap_sg(port->dev, &dma->tx_sg, 1, dma->dir);
>> +zero_out:
>>          sg_init_table(&dma->tx_sg, 1);
>>          return ret;
>>   }
>>
>>
>> thanks,
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland