diff mbox series

[5/5] usb: gadget: uvc: stop the pump on more conditions

Message ID 20220402233914.3625405-6-m.grzeschik@pengutronix.de (mailing list archive)
State Superseded
Headers show
Series usb: gadget: uvc: fixes and improvements | expand

Commit Message

Michael Grzeschik April 2, 2022, 11:39 p.m. UTC
While looping in the pump, there are more conditions to stop handling
requests. The streamoff event that will disable the endpoint and
the vb2_queue is called early. We add the variables into account.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
 drivers/usb/gadget/function/uvc_video.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

kernel test robot April 4, 2022, 11:30 a.m. UTC | #1
Hi Michael,

I love your patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on v5.18-rc1 next-20220404]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Michael-Grzeschik/usb-gadget-uvc-fixes-and-improvements/20220404-165031
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm64-randconfig-r016-20220404 (https://download.01.org/0day-ci/archive/20220404/202204041903.wSoTM3yH-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/3577e91e5a0a9a94ee3d4b22240e7b143c31133c
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Michael-Grzeschik/usb-gadget-uvc-fixes-and-improvements/20220404-165031
        git checkout 3577e91e5a0a9a94ee3d4b22240e7b143c31133c
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/usb/gadget/function/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/gadget/function/uvc_video.c: In function 'uvcg_video_pump':
>> drivers/usb/gadget/function/uvc_video.c:371:74: error: 'struct uvc_device' has no member named 'streamon'
     371 |         while (video->ep->enabled || queue->queue.streaming || video->uvc->streamon) {
         |                                                                          ^~


vim +371 drivers/usb/gadget/function/uvc_video.c

   351	
   352	/* --------------------------------------------------------------------------
   353	 * Video streaming
   354	 */
   355	
   356	/*
   357	 * uvcg_video_pump - Pump video data into the USB requests
   358	 *
   359	 * This function fills the available USB requests (listed in req_free) with
   360	 * video data from the queued buffers.
   361	 */
   362	static void uvcg_video_pump(struct work_struct *work)
   363	{
   364		struct uvc_video *video = container_of(work, struct uvc_video, pump);
   365		struct uvc_video_queue *queue = &video->queue;
   366		struct usb_request *req = NULL;
   367		struct uvc_buffer *buf;
   368		unsigned long flags;
   369		int ret;
   370	
 > 371		while (video->ep->enabled || queue->queue.streaming || video->uvc->streamon) {
   372			/* Retrieve the first available USB request, protected by the
   373			 * request lock.
   374			 */
   375			spin_lock_irqsave(&video->req_lock, flags);
   376			if (list_empty(&video->req_free)) {
   377				spin_unlock_irqrestore(&video->req_lock, flags);
   378				return;
   379			}
   380			req = list_first_entry(&video->req_free, struct usb_request,
   381						list);
   382			list_del(&req->list);
   383			spin_unlock_irqrestore(&video->req_lock, flags);
   384	
   385			/* Retrieve the first available video buffer and fill the
   386			 * request, protected by the video queue irqlock.
   387			 */
   388			spin_lock_irqsave(&queue->irqlock, flags);
   389			buf = uvcg_queue_head(queue);
   390			if (buf == NULL) {
   391				spin_unlock_irqrestore(&queue->irqlock, flags);
   392				break;
   393			}
   394	
   395			video->encode(req, video, buf);
   396	
   397			/* With usb3 we have more requests. This will decrease the
   398			 * interrupt load to a quarter but also catches the corner
   399			 * cases, which needs to be handled */
   400			if (list_empty(&video->req_free) ||
   401			    buf->state == UVC_BUF_STATE_DONE ||
   402			    !(video->req_int_count %
   403			       DIV_ROUND_UP(video->uvc_num_requests, 4))) {
   404				video->req_int_count = 0;
   405				req->no_interrupt = 0;
   406			} else {
   407				req->no_interrupt = 1;
   408			}
   409	
   410			/* Queue the USB request */
   411			ret = uvcg_video_ep_queue(video, req);
   412			spin_unlock_irqrestore(&queue->irqlock, flags);
   413	
   414			if (ret < 0) {
   415				uvcg_queue_cancel(queue, 0);
   416				break;
   417			}
   418			video->req_int_count++;
   419		}
   420	
   421		if (!req)
   422			return;
   423	
   424		spin_lock_irqsave(&video->req_lock, flags);
   425		list_add_tail(&req->list, &video->req_free);
   426		spin_unlock_irqrestore(&video->req_lock, flags);
   427		return;
   428	}
   429
Michael Grzeschik April 4, 2022, 1:07 p.m. UTC | #2
I think we can skip this patch for now, since it is depending on this series:

https://lore.kernel.org/linux-usb/20220315143356.3919911-1-m.grzeschik@pengutronix.de/

The other Patches of this series have no dependencies.

Michael

On Sun, Apr 03, 2022 at 01:39:14AM +0200, Michael Grzeschik wrote:
>While looping in the pump, there are more conditions to stop handling
>requests. The streamoff event that will disable the endpoint and
>the vb2_queue is called early. We add the variables into account.
>
>Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>---
> drivers/usb/gadget/function/uvc_video.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
>index 8b3116d48d2bd8..b1d7083d07846e 100644
>--- a/drivers/usb/gadget/function/uvc_video.c
>+++ b/drivers/usb/gadget/function/uvc_video.c
>@@ -368,7 +368,7 @@ static void uvcg_video_pump(struct work_struct *work)
> 	unsigned long flags;
> 	int ret;
>
>-	while (video->ep->enabled) {
>+	while (video->ep->enabled || queue->queue.streaming || video->uvc->streamon) {
> 		/* Retrieve the first available USB request, protected by the
> 		 * request lock.
> 		 */
>-- 
>2.30.2
>
>
>
Greg Kroah-Hartman April 19, 2022, 2:21 p.m. UTC | #3
On Mon, Apr 04, 2022 at 03:07:58PM +0200, Michael Grzeschik wrote:
> I think we can skip this patch for now, since it is depending on this series:
> 
> https://lore.kernel.org/linux-usb/20220315143356.3919911-1-m.grzeschik@pengutronix.de/
> 
> The other Patches of this series have no dependencies.

Can you please fix up and resend then?  Our tools want to apply the
whole series at once, for obvious reasons.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 8b3116d48d2bd8..b1d7083d07846e 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -368,7 +368,7 @@  static void uvcg_video_pump(struct work_struct *work)
 	unsigned long flags;
 	int ret;
 
-	while (video->ep->enabled) {
+	while (video->ep->enabled || queue->queue.streaming || video->uvc->streamon) {
 		/* Retrieve the first available USB request, protected by the
 		 * request lock.
 		 */