Hi,
On 5/20/2020 11:14 PM, Alan Stern wrote:
> On Wed, May 20, 2020 at 05:00:50PM +0000, Tejas Joglekar wrote:
>> Hi,
>> On 5/20/2020 10:20 PM, Alan Stern wrote:
>>> On Wed, May 20, 2020 at 06:41:16AM +0000, Tejas Joglekar wrote:
>>>>>> + urb->transfer_buffer = temp;
>>>>>> + urb->transfer_dma = dma_map_single(hcd->self.sysdev,
>>>>>> + urb->transfer_buffer,
>>>>>> + urb->transfer_buffer_length,
>>>>>> + dir);
>>>>>> +
>>>>>> + if (dma_mapping_error(hcd->self.sysdev,
>>>>>> + urb->transfer_dma)) {
>>>>>> + ret = -EAGAIN;
>>>>>> + kfree(temp);
>>>>>> + } else {
>>>>>> + urb->transfer_flags |= URB_DMA_MAP_SINGLE;
>>>>>
>>>>> Not sure if using URB_DMA_MAP_SINGLE to flag that this urb is boucebuffered is
>>>>> appropriate.
>>>>>
>>>>> If Greg or Alan don't object then it's fine for me as well.
>>>>>
>>>>>
>>>>>
>>>> @Greg/Alan do you suggest any change for the flag here?
>>>
>>> This requires care, because the core will already have set other flags
>>> for this URB. I don't remember the details and I don't have time to
>>> check them now. But it wouldn't be at all surprising if the core
>>> doesn't expect an URB both to use SG and to use DMA_MAP_SINGLE.
>>>
>> When this quirk is enabled and it is required to consolidate SG list to
>> single buffer no other flags are set for the URB.
>
> I don't believe that. What about URB_DMA_MAP_SG or
> URB_DMA_MAP_PAGE? The USB core certainly would have set one of them
> already. Not only would you need to clear that flag, you want also need
> to undo the mapping.
>
URB_DMA_MAP_SG or URB_DMA_MAP_PAGE flags are set for an URB in core within
function usb_hcd_map_urb_for_dma(), with my patch the xhci_map_urb_for_dma()
does not call usb_hcd_map_urb_for_dma() when SG list consolidation is required
but it calls the newly added function xhci_map_temp_buffer() within that function
only dma_map_single() is called to map consolidated SG list.
>> Only question here is
>> if it is good to use DMA_MAP_SINGLE flag for the consolidated SG list
>> into a single temporary buffer.
>
> If you call dma_map_single() then USB_DMA_MAP_SINGLE is the appropriate
> flag to sest.
>
Yes, within xhci_map_temp_buffer(), after consolidating the SG list in a temp
buffer I call dma_map_single() to map the temp buffer.
> Alan Stern
>
Thanks & Regards,
Tejas Joglekar
