USB: DMA: mapping existing buffer is not supported?

USB: DMA: mapping existing buffer is not supported?

[Randy Li]

Hello

I was trying to understand why USB webcams (UVC) have to copy video data 
through the CPU (uvc_video_complete() schedules 
uvc_video_copy_data_work() for this purpose). During my investigation, I 
noticed that functions like|usb_sg_*() and |usb_buffer_*() are not 
available since kernel version 2.6.12.

I can comprehend that USB host controllers such as xHCI, commonly found 
in embedded implementations like dwc3, do not support IOMMU. However, 
this should not prevent them from operating on a contiguous buffer.

If the USB subsystem can no longer work with existing buffers, I propose 
that we consider removing the remaining documentation in the "Working 
with existing buffers" section of Documentation/driver-api/usb/dma.rst.

Sincerely

Randy

Re: USB: DMA: mapping existing buffer is not supported?

[Greg KH]

On Thu, Sep 14, 2023 at 03:06:58AM +0800, Randy Li wrote:
> Hello
> 
> I was trying to understand why USB webcams (UVC) have to copy video data
> through the CPU (uvc_video_complete() schedules uvc_video_copy_data_work()
> for this purpose). During my investigation, I noticed that functions
> like|usb_sg_*() and |usb_buffer_*() are not available since kernel version
> 2.6.12.

What do you mean by "not available"?  I see them in the tree today, why
do you think they are not present?

> I can comprehend that USB host controllers such as xHCI, commonly found in
> embedded implementations like dwc3, do not support IOMMU. However, this
> should not prevent them from operating on a contiguous buffer.

Are you sure that the protocol for these devices allow this?

> If the USB subsystem can no longer work with existing buffers, I propose
> that we consider removing the remaining documentation in the "Working with
> existing buffers" section of Documentation/driver-api/usb/dma.rst.

I don't understand, what is wrong with the information there exactly?
Have you tried following the suggestions there?

thanks,

greg k-h

Re: USB: DMA: mapping existing buffer is not supported?

[Randy Li]

On 2023/9/14 03:19, Greg KH wrote:
> On Thu, Sep 14, 2023 at 03:06:58AM +0800, Randy Li wrote:
>> Hello
>>
>> I was trying to understand why USB webcams (UVC) have to copy video data
>> through the CPU (uvc_video_complete() schedules uvc_video_copy_data_work()
>> for this purpose). During my investigation, I noticed that functions
>> like|usb_sg_*() and |usb_buffer_*() are not available since kernel version
>> 2.6.12.
> What do you mean by "not available"?  I see them in the tree today, why
> do you think they are not present?
>
usb_buffer_dmasync_sg(), usb_buffer_map(), usb_buffer_dmasync() and usb_buffer_unmap() are all disabled
by #if 0 in include/usb/usb.h

usb_buffer_map_sg() and usb_buffer_unmap_sg() are just declaration without definition.

>> I can comprehend that USB host controllers such as xHCI, commonly found in
>> embedded implementations like dwc3, do not support IOMMU. However, this
>> should not prevent them from operating on a contiguous buffer.
> Are you sure that the protocol for these devices allow this?
I thought that is what hcd_buffer_alloc() does.
>
>> If the USB subsystem can no longer work with existing buffers, I propose
>> that we consider removing the remaining documentation in the "Working with
>> existing buffers" section of Documentation/driver-api/usb/dma.rst.
> I don't understand, what is wrong with the information there exactly?
> Have you tried following the suggestions there?
Besides my answer to first question, I found no code use them today.
>
> thanks,
>
> greg k-h

Re: USB: DMA: mapping existing buffer is not supported?

[Greg KH]

On Thu, Sep 14, 2023 at 04:09:06AM +0800, Randy Li wrote:
> 
> On 2023/9/14 03:19, Greg KH wrote:
> > On Thu, Sep 14, 2023 at 03:06:58AM +0800, Randy Li wrote:
> > > Hello
> > > 
> > > I was trying to understand why USB webcams (UVC) have to copy video data
> > > through the CPU (uvc_video_complete() schedules uvc_video_copy_data_work()
> > > for this purpose). During my investigation, I noticed that functions
> > > like|usb_sg_*() and |usb_buffer_*() are not available since kernel version
> > > 2.6.12.
> > What do you mean by "not available"?  I see them in the tree today, why
> > do you think they are not present?
> > 
> usb_buffer_dmasync_sg(), usb_buffer_map(), usb_buffer_dmasync() and usb_buffer_unmap() are all disabled
> by #if 0 in include/usb/usb.h
> 
> usb_buffer_map_sg() and usb_buffer_unmap_sg() are just declaration without definition.

Sorry, I was looking at the usb_sg_* calls, those are there if you want
to use them.

But again, why not just use the normal sg field in the urb itself for
the scatter-gather pointer?  Will that not work for you?

> > > If the USB subsystem can no longer work with existing buffers, I propose
> > > that we consider removing the remaining documentation in the "Working with
> > > existing buffers" section of Documentation/driver-api/usb/dma.rst.
> > I don't understand, what is wrong with the information there exactly?
> > Have you tried following the suggestions there?
> Besides my answer to first question, I found no code use them today.

The old-style usb-storage driver uses the usb_sg_* calls, and the uas.c
driver uses the sg fields and provides line-speed transfers (the speed
limit is in the device, not the kernel).

thanks,

greg k-h

Re: USB: DMA: mapping existing buffer is not supported?

[Alan Stern]

On Wed, Sep 13, 2023 at 10:32:12PM +0200, Greg KH wrote:
> On Thu, Sep 14, 2023 at 04:09:06AM +0800, Randy Li wrote:
> > 
> > On 2023/9/14 03:19, Greg KH wrote:
> > > On Thu, Sep 14, 2023 at 03:06:58AM +0800, Randy Li wrote:
> > > > Hello
> > > > 
> > > > I was trying to understand why USB webcams (UVC) have to copy video data
> > > > through the CPU (uvc_video_complete() schedules uvc_video_copy_data_work()
> > > > for this purpose). During my investigation, I noticed that functions
> > > > like|usb_sg_*() and |usb_buffer_*() are not available since kernel version
> > > > 2.6.12.
> > > What do you mean by "not available"?  I see them in the tree today, why
> > > do you think they are not present?
> > > 
> > usb_buffer_dmasync_sg(), usb_buffer_map(), usb_buffer_dmasync() and usb_buffer_unmap() are all disabled
> > by #if 0 in include/usb/usb.h
> > 
> > usb_buffer_map_sg() and usb_buffer_unmap_sg() are just declaration without definition.
> 
> Sorry, I was looking at the usb_sg_* calls, those are there if you want
> to use them.
> 
> But again, why not just use the normal sg field in the urb itself for
> the scatter-gather pointer?  Will that not work for you?
> 
> > > > If the USB subsystem can no longer work with existing buffers, I propose
> > > > that we consider removing the remaining documentation in the "Working with
> > > > existing buffers" section of Documentation/driver-api/usb/dma.rst.
> > > I don't understand, what is wrong with the information there exactly?
> > > Have you tried following the suggestions there?
> > Besides my answer to first question, I found no code use them today.

That's exactly what it says at the end of the file:

----------------------------------------------------------------------------
Note that several of those interfaces are currently commented out, since
they don't have current users.  See the source code.  Other than the dmasync
calls (where the underlying DMA primitives have changed), most of them can
easily be commented back in if you want to use them.
----------------------------------------------------------------------------

So Randy, the answer to your question is that those routines are not 
available any more because nothing needs to use them.  If you would like 
to submit a patch removing the "Working with existing buffers" section 
from the documentation file, feel free to do so.

Alan Stern