Received: by 2002:ac0:a582:0:0:0:0:0 with SMTP id m2-v6csp885029imm; Wed, 10 Oct 2018 05:59:10 -0700 (PDT) X-Google-Smtp-Source: ACcGV63H3uOXiKVijq9mznFPUdX2p23BFEsAtUcHFbfHbvqttP5QpPw3llv1yt8xFFb1NSunKsNL X-Received: by 2002:a62:221c:: with SMTP id i28-v6mr34904270pfi.196.1539176350190; Wed, 10 Oct 2018 05:59:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1539176350; cv=none; d=google.com; s=arc-20160816; b=szYwv/k+ngHcd1932iJoudu5+OCbJbSIeSlllvRaI7DGRsNTjtbuYWIL6M4k1B6DJg fa8HTSTE/e9hVralUfj0bXoCrBqnAdlmEWVA4XyregOP7cWIEllan/hIEs70VTbR15As KfTHc0E20FnOpThCRoND2uYDPxFOnKwCknu1EtQGxOFqv7ymrInOgrQAc5cuMTKBSvNN WOhmuBVHoKkbZGAc+oVuVrHd4Mn4SVGdPDvjiNmWYNNnedG2bQr6oT7G5jxM47kG9Juz sRawE9f6o34hQrcarHsQFpcs4HsMolsEJlWkrAS6R9CXyaga4emtGwfHwcW4Uv5J4ZmV Sq9Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:organization:message-id:date:subject:cc:to :from:dkim-signature; bh=7hFEH0MVVeUbbgSULxXKgTyvqrAgZVHdaaTWQ4A5rng=; b=xRtJg7DPmSesAW61a6uU5tC5NMi1gEn480DNs+swnmPAxNh0/Lt9uufUUe+sNh9OW1 FJRNCEAy++k5Cw53Ec6C0eaHKCltzVdXuNjBC1i3ivtJroJjXZQ14k6/J0UAZsjOg8+P rUgEZPZXZ6RbNAret3seflnz77OEbWftqoRMHHPEH84p7v4ZTTFjtxQvqPh9Vbun3Zk0 jeDdqCW4msW5Djx9ncH/kZDafNJLfBcwwrB4BhWpTbcfc5To3lww04++SbHVFr0XTXPC yPSTtX5Aila5SYcNbu/c/f9RK7ngU/yu/xOXsSv0c36CM1bkQ+ydvolELK7o6btktjvE 1Phw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=s2C55WD9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id c5-v6si21090410pgq.226.2018.10.10.05.58.25; Wed, 10 Oct 2018 05:59:10 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass (test mode) header.i=@ideasonboard.com header.s=mail header.b=s2C55WD9; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726873AbeJJUTU (ORCPT + 99 others); Wed, 10 Oct 2018 16:19:20 -0400 Received: from perceval.ideasonboard.com ([213.167.242.64]:53880 "EHLO perceval.ideasonboard.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726573AbeJJUTU (ORCPT ); Wed, 10 Oct 2018 16:19:20 -0400 Received: from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 58D186F; Wed, 10 Oct 2018 14:57:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1539176234; bh=pmL6XteOp5TSXZrDx0tQJzlbmvJlIufybbRNtvZiBAI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=s2C55WD9nv1CuWeAJOytNISTlPplrIY1XrN+PzQuK0PTldQrfKQNLsQu4Y/My1B4F 5wsTb8Wt5svmvDHM6ItBU1f5csuY9RFAXZ9ce1apGbprBebfWIiOdgeGjUmgIk3mFM Yh2F9jSSlzi07GPV2ov2crHfDYQDLuGWI6Zzq4ac= From: Laurent Pinchart To: Paul Elder Cc: kieran.bingham@ideasonboard.com, b-liu@ti.com, gregkh@linuxfoundation.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, balbi@kernel.org, stern@rowland.harvard.edu, rogerq@ti.com Subject: Re: [PATCH 0/6] usb: gadget: add mechanism to asynchronously validate data stage of ctrl out request Date: Wed, 10 Oct 2018 15:57:15 +0300 Message-ID: <64135795.5y2tiWKxvC@avalon> Organization: Ideas on Board Oy In-Reply-To: <20181010024903.1633-1-paul.elder@ideasonboard.com> References: <20181010024903.1633-1-paul.elder@ideasonboard.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Paul, Thank you for the patches. On Wednesday, 10 October 2018 05:48:57 EEST Paul Elder wrote: > This patch series adds a mechanism to allow asynchronously validating > the data stage of a control out request, and for stalling or suceeding > the request accordingly. This mechanism is implemented for MUSB, and is > used by UVC. At the same time, UVC packages the setup stage and data > stage data together to send to userspace to save on a pair of context > switches per control out request. > > This patch series does change the userspace API. We however believe that > it is justified because the current API is broken, and because it isn't > being used (because it's broken). I'd like to point out for the reviewers that it changes the API of the UVC gadget driver only, not any other USB gadget userspace API. > The current API is broken such that it is subject to race conditions > that cause fatal errors with a high frequency. This is actually what > motivated this patch series in the first place. In the current API, not > only is there no way to asynchronously validate the data stage of a > control OUT request, but an empty buffer is expected to be provided to > hold the data stage data -- which is more likely than not to be late. > There is even a warning in musb_g_ep0_queue: > > /* else for sequence #2 (OUT), caller provides a buffer > * before the next packet arrives. deferred responses > * (after SETUP is acked) are racey. > */ > > This problem has never been reported in years, which is a sign that the > API isn't used. Furthermore, the vendor kernels that we have seen using > the UVC gadget driver (such as QC and Huawei) are heavily patched with > local changes to the API. This corroborates the suspicion that the > current mainline API is not being used. > > Additionally, this API isn't meant to be used by generic applications, > but by a dedicated userspace helper. uvc-gadget is one such example, but > it has bitrotten and isn't compatible with the current kernel API. The > fact that nobody has submitted patches nor complained for a long time > again shows that it isn't being used. > > The conclusion is that since the API hasn't been used for a long time, > it is safe to fix it. > > Paul Elder (6): > usb: uvc: include videodev2.h in g_uvc.h > usb: gadget: uvc: enqueue usb request in setup handler for control OUT > usb: gadget: uvc: package setup and data for control OUT requests > usb: gadget: add functions to signal udc driver to delay status stage > usb: musb: gadget: implement send_response > usb: gadget: uvc: allow ioctl to send response in status stage > > drivers/usb/gadget/function/f_uvc.c | 33 ++++++++++++++++----- > drivers/usb/gadget/function/uvc.h | 1 + > drivers/usb/gadget/function/uvc_v4l2.c | 21 +++++++++++++ > drivers/usb/gadget/udc/core.c | 40 +++++++++++++++++++++++++ > drivers/usb/musb/musb_gadget_ep0.c | 41 ++++++++++++++++++++++++++ > include/linux/usb/gadget.h | 11 +++++++ > include/uapi/linux/usb/g_uvc.h | 4 ++- > 7 files changed, 142 insertions(+), 9 deletions(-) -- Regards, Laurent Pinchart