| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANiDSCt98rP2RtUfC2EFL+H2U4TMV3BKEhAfpPK1Nm+tU=4j4Q@mail.gmail.com>
Date: Thu, 3 Jul 2025 13:32:02 +0200
From: Ricardo Ribalda <ribalda@...omium.org>
To: Xu Yang <xu.yang_2@....com>
Cc: ezequiel@...guardiasur.com.ar, mchehab@...nel.org,
laurent.pinchart@...asonboard.com, hdegoede@...hat.com,
gregkh@...uxfoundation.org, mingo@...nel.org, tglx@...utronix.de,
andriy.shevchenko@...ux.intel.com, viro@...iv.linux.org.uk,
thomas.weissschuh@...utronix.de, dafna.hirschfeld@...labora.com,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org, imx@...ts.linux.dev, jun.li@....com
Subject: Re: [PATCH v3 2/3] media: uvcvideo: use usb_alloc_noncoherent/usb_free_noncoherent()
Hi Xu
On Thu, 3 Jul 2025 at 10:51, Xu Yang <xu.yang_2@....com> wrote:
>
> Hi Ricardo,
>
> On Wed, Jul 02, 2025 at 02:30:45PM +0200, Ricardo Ribalda wrote:
> > Hi Xu
> >
> > The code looks much cleaner :)
> >
> > It seems that the hcd.c uses the urb transfer_flags to know the
> > direction of the transfer.
> > But the uvc driver is not setting it, you probably want to set it.
>
> The USB HCD will get transfer direction from endpoint capability. So
> we needn't add such info to transfer_flags.
For future me...
usb_submit_urb() set that flag. :)
Thanks!
>
> >
> > On Wed, 2 Jul 2025 at 13:01, Xu Yang <xu.yang_2@....com> wrote:
> > >
> > > This will use USB noncoherent API to alloc/free urb buffers, then
> > > uvc driver needn't to do dma sync operations by itself.
> > >
> > > Signed-off-by: Xu Yang <xu.yang_2@....com>
> > >
> > > ---
> > > Changes in v3:
> > > - no changes
> > > ---
> > > drivers/media/usb/uvc/uvc_video.c | 56 ++++++++-----------------------
> > > 1 file changed, 14 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > > index e3567aeb0007..614cf4781221 100644
> > > --- a/drivers/media/usb/uvc/uvc_video.c
> > > +++ b/drivers/media/usb/uvc/uvc_video.c
> > > @@ -1280,15 +1280,6 @@ static inline struct device *uvc_stream_to_dmadev(struct uvc_streaming *stream)
> > > return bus_to_hcd(stream->dev->udev->bus)->self.sysdev;
> > > }
> >
> > The uvc_stream_to_dmadev() function is not used anymore, please drop it.
>
> Sure.
>
> Thanks,
> Xu Yang
>
> >
> >
> >
> > >
> > > -static int uvc_submit_urb(struct uvc_urb *uvc_urb, gfp_t mem_flags)
> > > -{
> > > - /* Sync DMA. */
> > > - dma_sync_sgtable_for_device(uvc_stream_to_dmadev(uvc_urb->stream),
> > > - uvc_urb->sgt,
> > > - uvc_stream_dir(uvc_urb->stream));
> > > - return usb_submit_urb(uvc_urb->urb, mem_flags);
> > > -}
> > > -
> > > /*
> > > * uvc_video_decode_data_work: Asynchronous memcpy processing
> > > *
> > > @@ -1310,7 +1301,7 @@ static void uvc_video_copy_data_work(struct work_struct *work)
> > > uvc_queue_buffer_release(op->buf);
> > > }
> > >
> > > - ret = uvc_submit_urb(uvc_urb, GFP_KERNEL);
> > > + ret = usb_submit_urb(uvc_urb->urb, GFP_KERNEL);
> > > if (ret < 0)
> > > dev_err(&uvc_urb->stream->intf->dev,
> > > "Failed to resubmit video URB (%d).\n", ret);
> > > @@ -1736,12 +1727,6 @@ static void uvc_video_complete(struct urb *urb)
> > > /* Re-initialise the URB async work. */
> > > uvc_urb->async_operations = 0;
> > >
> > > - /* Sync DMA and invalidate vmap range. */
> > > - dma_sync_sgtable_for_cpu(uvc_stream_to_dmadev(uvc_urb->stream),
> > > - uvc_urb->sgt, uvc_stream_dir(stream));
> > > - invalidate_kernel_vmap_range(uvc_urb->buffer,
> > > - uvc_urb->stream->urb_size);
> > > -
> > > /*
> > > * Process the URB headers, and optionally queue expensive memcpy tasks
> > > * to be deferred to a work queue.
> > > @@ -1750,7 +1735,7 @@ static void uvc_video_complete(struct urb *urb)
> > >
> > > /* If no async work is needed, resubmit the URB immediately. */
> > > if (!uvc_urb->async_operations) {
> > > - ret = uvc_submit_urb(uvc_urb, GFP_ATOMIC);
> > > + ret = usb_submit_urb(uvc_urb->urb, GFP_ATOMIC);
> > > if (ret < 0)
> > > dev_err(&stream->intf->dev,
> > > "Failed to resubmit video URB (%d).\n", ret);
> > > @@ -1765,17 +1750,15 @@ static void uvc_video_complete(struct urb *urb)
> > > */
> > > static void uvc_free_urb_buffers(struct uvc_streaming *stream)
> > > {
> > > - struct device *dma_dev = uvc_stream_to_dmadev(stream);
> > > + struct usb_device *udev = stream->dev->udev;
> > > struct uvc_urb *uvc_urb;
> > >
> > > for_each_uvc_urb(uvc_urb, stream) {
> > > if (!uvc_urb->buffer)
> > > continue;
> > >
> > > - dma_vunmap_noncontiguous(dma_dev, uvc_urb->buffer);
> > > - dma_free_noncontiguous(dma_dev, stream->urb_size, uvc_urb->sgt,
> > > - uvc_stream_dir(stream));
> > > -
> > > + usb_free_noncoherent(udev, stream->urb_size, uvc_urb->buffer,
> > > + uvc_stream_dir(stream), uvc_urb->sgt);
> > > uvc_urb->buffer = NULL;
> > > uvc_urb->sgt = NULL;
> > > }
> > > @@ -1786,26 +1769,13 @@ static void uvc_free_urb_buffers(struct uvc_streaming *stream)
> > > static bool uvc_alloc_urb_buffer(struct uvc_streaming *stream,
> > > struct uvc_urb *uvc_urb, gfp_t gfp_flags)
> > > {
> > > - struct device *dma_dev = uvc_stream_to_dmadev(stream);
> > > -
> > > - uvc_urb->sgt = dma_alloc_noncontiguous(dma_dev, stream->urb_size,
> > > - uvc_stream_dir(stream),
> > > - gfp_flags, 0);
> > > - if (!uvc_urb->sgt)
> > > - return false;
> > > - uvc_urb->dma = uvc_urb->sgt->sgl->dma_address;
> > > -
> > > - uvc_urb->buffer = dma_vmap_noncontiguous(dma_dev, stream->urb_size,
> > > - uvc_urb->sgt);
> > > - if (!uvc_urb->buffer) {
> > > - dma_free_noncontiguous(dma_dev, stream->urb_size,
> > > - uvc_urb->sgt,
> > > - uvc_stream_dir(stream));
> > > - uvc_urb->sgt = NULL;
> > > - return false;
> > > - }
> > > + struct usb_device *udev = stream->dev->udev;
> > >
> > > - return true;
> > > + uvc_urb->buffer = usb_alloc_noncoherent(udev, stream->urb_size,
> > > + gfp_flags, &uvc_urb->dma,
> > > + uvc_stream_dir(stream),
> > > + &uvc_urb->sgt);
> > > + return !!uvc_urb->buffer;
> > > }
> > >
> > > /*
> > > @@ -1953,6 +1923,7 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
> > > urb->complete = uvc_video_complete;
> > > urb->number_of_packets = npackets;
> > > urb->transfer_buffer_length = size;
> > > + urb->sgt = uvc_urb->sgt;
> > >
> > > for (i = 0; i < npackets; ++i) {
> > > urb->iso_frame_desc[i].offset = i * psize;
> > > @@ -2009,6 +1980,7 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream,
> > > size, uvc_video_complete, uvc_urb);
> > > urb->transfer_flags = URB_NO_TRANSFER_DMA_MAP;
> > > urb->transfer_dma = uvc_urb->dma;
> > > + urb->sgt = uvc_urb->sgt;
> > >
> > > uvc_urb->urb = urb;
> > > }
> > > @@ -2120,7 +2092,7 @@ static int uvc_video_start_transfer(struct uvc_streaming *stream,
> > >
> > > /* Submit the URBs. */
> > > for_each_uvc_urb(uvc_urb, stream) {
> > > - ret = uvc_submit_urb(uvc_urb, gfp_flags);
> > > + ret = usb_submit_urb(uvc_urb->urb, gfp_flags);
> > > if (ret < 0) {
> > > dev_err(&stream->intf->dev,
> > > "Failed to submit URB %u (%d).\n",
> > > --
> > > 2.34.1
> > >
> > >
> >
> >
> > --
> > Ricardo Ribalda
--
Ricardo Ribalda
Powered by blists - more mailing lists