From patchwork Mon Aug 28 02:10:30 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: andrey.konovalov@linux.dev X-Patchwork-Id: 718173 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3A524C83F11 for ; Mon, 28 Aug 2023 02:18:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229608AbjH1CSJ (ORCPT ); Sun, 27 Aug 2023 22:18:09 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229656AbjH1CRt (ORCPT ); Sun, 27 Aug 2023 22:17:49 -0400 Received: from out-252.mta1.migadu.com (out-252.mta1.migadu.com [95.215.58.252]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 82068DD for ; Sun, 27 Aug 2023 19:17:46 -0700 (PDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1693188642; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=s3mKfn7GAN06ou/8BCH2fRTTl02YY6jrfUlWZqgBA1E=; b=S5Jlqj9UFLejHmRcgZ4/6G80Kbyo2zYmVbxS5xaHtfuC0RMPPFDzZNvT4YJIcxF8JuS/2i euoJ3VV4tyyadjGnrNlWL/5ZmHIXs/uPodvi63Kf6bFOecUoGVmWpg7R12NAXrOud5dBkO ekAL9kWUDhgU02qM4vhBQz5ZD3xFLVY= From: andrey.konovalov@linux.dev To: Greg Kroah-Hartman , Alan Stern Cc: Andrey Konovalov , Felipe Balbi , Thinh Nguyen , Pawel Laszczak , Chunfeng Yun , Minas Harutyunyan , Justin Chen , Al Cooper , Herve Codina , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/3] usb: gadget: clarify usage of USB_GADGET_DELAYED_STATUS Date: Mon, 28 Aug 2023 04:10:30 +0200 Message-Id: <5c2913d70556b03c9bb1893c6941e8ece04934b0.1693188390.git.andreyknvl@gmail.com> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org From: Andrey Konovalov USB_GADGET_DELAYED_STATUS was introduced in commit 1b9ba000177e ("usb: gadget: composite: Allow function drivers to pause control transfers"). It was initially intended for the composite framework to allow delaying completing the status stage of a SET_CONFIGURATION request until all functions are ready. Unfortunately, that commit had an unintended side-effect of returning USB_GADGET_DELAYED_STATUS from the ->setup() call of the composite framework gadget driver. As a result of this and the incomplete documentation, some UDC drivers started relying on USB_GADGET_DELAYED_STATUS to decide when to avoid autocompleting the status stage for 0-length control transfers. dwc3 was the first in commit 5bdb1dcc6330 ("usb: dwc3: ep0: handle delayed_status again"). And a number of other UDC drivers followed later, probably relying on the dwc3 behavior as a reference. Unfortunately, this violated the interface between the UDC and the gadget driver for 0-length control transfers: the UDC driver must only proceed with the status stage for a 0-length control transfer once the gadget driver queued a response to EP0. As a result, a few gadget drivers are partially broken when used with a UDC that only delays the status stage for 0-length transfers when USB_GADGET_DELAYED_STATUS is returned from the setup() callback. This includes Raw Gadget and GadgetFS. For FunctionFS, a workaround was added in commit 946ef68ad4e4 ("usb: gadget: ffs: Let setup() return USB_GADGET_DELAYED_STATUS") and commit 4d644abf2569 ("usb: gadget: f_fs: Only return delayed status when len is 0"). The proper solution to this issue would be to contain USB_GADGET_DELAYED_STATUS within the composite framework and make all UDC drivers to not complete the status stage for 0-length requests on their own. Unfortunately, there is quite a few UDC drivers that need to get fixed and the required changes for some of them are not trivial. For now, update the comments to clarify that USB_GADGET_DELAYED_STATUS must not be used by the UDC drivers. The following two commits also add workarounds to Raw Gadget and GadgetFS to make them compatible with the broken UDC drivers until they are fixed. Signed-off-by: Andrey Konovalov --- include/linux/usb/composite.h | 8 ++++++++ include/linux/usb/gadget.h | 9 +++++++++ 2 files changed, 17 insertions(+) diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h index 07531c4f4350..1d2cf6a070ac 100644 --- a/include/linux/usb/composite.h +++ b/include/linux/usb/composite.h @@ -35,6 +35,14 @@ * are ready. The control transfer will then be kept from completing till * all the function drivers that requested for USB_GADGET_DELAYED_STAUS * invoke usb_composite_setup_continue(). + * + * NOTE: USB_GADGET_DELAYED_STATUS must not be used in UDC drivers: they + * must delay completing the status stage for 0-length control transfers + * regardless of the whether USB_GADGET_DELAYED_STATUS is returned from + * the gadget driver's setup() callback. + * Currently, a number of UDC drivers rely on USB_GADGET_DELAYED_STATUS, + * which is a bug. These drivers must be fixed and USB_GADGET_DELAYED_STATUS + * must be contained within the composite framework. */ #define USB_GADGET_DELAYED_STATUS 0x7fff /* Impossibly large value */ diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h index 75bda0783395..6532beb587b1 100644 --- a/include/linux/usb/gadget.h +++ b/include/linux/usb/gadget.h @@ -711,6 +711,15 @@ static inline int usb_gadget_check_config(struct usb_gadget *gadget) * get_interface. Setting a configuration (or interface) is where * endpoints should be activated or (config 0) shut down. * + * The gadget driver's setup() callback does not have to queue a response to + * ep0 within the setup() call, the driver can do it after setup() returns. + * The UDC driver must wait until such a response is queued before proceeding + * with the data/status stages of the control transfer. + * + * NOTE: Currently, a number of UDC drivers rely on USB_GADGET_DELAYED_STATUS + * being returned from the setup() callback, which is a bug. See the comment + * next to USB_GADGET_DELAYED_STATUS for details. + * * (Note that only the default control endpoint is supported. Neither * hosts nor devices generally support control traffic except to ep0.) * From patchwork Mon Aug 28 02:10:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: andrey.konovalov@linux.dev X-Patchwork-Id: 719383 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5B9D0C83F10 for ; Mon, 28 Aug 2023 02:16:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229490AbjH1CQB (ORCPT ); Sun, 27 Aug 2023 22:16:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51502 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230087AbjH1CPr (ORCPT ); Sun, 27 Aug 2023 22:15:47 -0400 X-Greylist: delayed 255 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Sun, 27 Aug 2023 19:15:44 PDT Received: from out-245.mta1.migadu.com (out-245.mta1.migadu.com [IPv6:2001:41d0:203:375::f5]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id F1293EB for ; Sun, 27 Aug 2023 19:15:44 -0700 (PDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1693188643; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=nn8j1xlZiIaYyk41MeUAjHTR8kw6djY6y6n4AYyo8oQ=; b=wrlwc8wbRnTKtb9ELzWKJpvmI0XJrjleVVIVz4kg8r56fEO8pnD9pOR8/XkGaubISLoUnW ATTS31e3wIVCX+E2p0KnOUt86TtX4co61EeXkuz3++9khWiCWdVMNDgso3pF50iZ8U5MEF 8EWqbJR3cVBaT5RYABn4KkAxXabW1ds= From: andrey.konovalov@linux.dev To: Greg Kroah-Hartman , Alan Stern Cc: Andrey Konovalov , Felipe Balbi , Thinh Nguyen , Pawel Laszczak , Chunfeng Yun , Minas Harutyunyan , Justin Chen , Al Cooper , Herve Codina , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/3] usb: raw-gadget: return USB_GADGET_DELAYED_STATUS from setup() Date: Mon, 28 Aug 2023 04:10:31 +0200 Message-Id: <739253b1efa3469e421c869c4520284e854dc48f.1693188390.git.andreyknvl@gmail.com> In-Reply-To: <5c2913d70556b03c9bb1893c6941e8ece04934b0.1693188390.git.andreyknvl@gmail.com> References: <5c2913d70556b03c9bb1893c6941e8ece04934b0.1693188390.git.andreyknvl@gmail.com> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org From: Andrey Konovalov Return USB_GADGET_DELAYED_STATUS from the setup() callback for 0-length transfers as a workaround to stop some UDC drivers (e.g. dwc3) from automatically proceeding with the status stage. This workaround should be removed once all UDC drivers are fixed to always delay the status stage until a response is queued to EP0. Signed-off-by: Andrey Konovalov --- drivers/usb/gadget/legacy/raw_gadget.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/gadget/legacy/raw_gadget.c b/drivers/usb/gadget/legacy/raw_gadget.c index e549022642e5..b9ecc55a2ce2 100644 --- a/drivers/usb/gadget/legacy/raw_gadget.c +++ b/drivers/usb/gadget/legacy/raw_gadget.c @@ -25,6 +25,7 @@ #include #include #include +#include #include @@ -363,6 +364,16 @@ static int gadget_setup(struct usb_gadget *gadget, out_unlock: spin_unlock_irqrestore(&dev->lock, flags); out: + if (ret == 0 && ctrl->wLength == 0) { + /* + * Return USB_GADGET_DELAYED_STATUS as a workaround to stop + * some UDC drivers (e.g. dwc3) from automatically proceeding + * with the status stage for 0-length transfers. + * Should be removed once all UDC drivers are fixed to always + * delay the status stage until a response is queued to EP0. + */ + return USB_GADGET_DELAYED_STATUS; + } return ret; } From patchwork Mon Aug 28 02:10:32 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: andrey.konovalov@linux.dev X-Patchwork-Id: 719382 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6D51AC83F10 for ; Mon, 28 Aug 2023 02:18:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229660AbjH1CSK (ORCPT ); Sun, 27 Aug 2023 22:18:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56224 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229654AbjH1CRs (ORCPT ); Sun, 27 Aug 2023 22:17:48 -0400 X-Greylist: delayed 349 seconds by postgrey-1.37 at lindbergh.monkeyblade.net; Sun, 27 Aug 2023 19:17:46 PDT Received: from out-246.mta1.migadu.com (out-246.mta1.migadu.com [95.215.58.246]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 79EB9DA for ; Sun, 27 Aug 2023 19:17:46 -0700 (PDT) X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1693188644; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=rtKvrVKkVQRnt5532V2eBTwRFEQ3kLH/lmwR/1FmH1s=; b=nX51DTd6k2pYWpSmnb5tXrNcL7PyuZHU/VuPVXFkQycEyI73p4Ck5Zd3i6mIhYwxplI1OU 5x2GgSy80mXEG3edYDZ16LNYljPSqkJtJ54EuxeSCHqLUlv08XpVsouQghCSaIzq81AqLn 39nwhdXuhAOArDezqY66niYxhT+Ljmw= From: andrey.konovalov@linux.dev To: Greg Kroah-Hartman , Alan Stern Cc: Andrey Konovalov , Felipe Balbi , Thinh Nguyen , Pawel Laszczak , Chunfeng Yun , Minas Harutyunyan , Justin Chen , Al Cooper , Herve Codina , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/3] usb: gadgetfs: return USB_GADGET_DELAYED_STATUS from setup() Date: Mon, 28 Aug 2023 04:10:32 +0200 Message-Id: <35c01a524b2eb6c3f01bc08f16bdff2d72256a1f.1693188390.git.andreyknvl@gmail.com> In-Reply-To: <5c2913d70556b03c9bb1893c6941e8ece04934b0.1693188390.git.andreyknvl@gmail.com> References: <5c2913d70556b03c9bb1893c6941e8ece04934b0.1693188390.git.andreyknvl@gmail.com> MIME-Version: 1.0 X-Migadu-Flow: FLOW_OUT Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org From: Andrey Konovalov Return USB_GADGET_DELAYED_STATUS from the setup() callback for 0-length transfers as a workaround to stop some UDC drivers (e.g. dwc3) from automatically proceeding with the status stage. This workaround should be removed once all UDC drivers are fixed to always delay the status stage until a response is queued to EP0. Signed-off-by: Andrey Konovalov --- drivers/usb/gadget/legacy/inode.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c index 28249d0bf062..154bbf578ba2 100644 --- a/drivers/usb/gadget/legacy/inode.c +++ b/drivers/usb/gadget/legacy/inode.c @@ -31,6 +31,7 @@ #include #include +#include /* @@ -241,6 +242,7 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ #define xprintk(d,level,fmt,args...) \ printk(level "%s: " fmt , shortname , ## args) +#undef DBG #ifdef DEBUG #define DBG(dev,fmt,args...) \ xprintk(dev , KERN_DEBUG , fmt , ## args) @@ -256,8 +258,10 @@ static DEFINE_MUTEX(sb_mutex); /* Serialize superblock operations */ do { } while (0) #endif /* DEBUG */ +#undef ERROR #define ERROR(dev,fmt,args...) \ xprintk(dev , KERN_ERR , fmt , ## args) +#undef INFO #define INFO(dev,fmt,args...) \ xprintk(dev , KERN_INFO , fmt , ## args) @@ -1511,7 +1515,16 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) event->u.setup = *ctrl; ep0_readable (dev); spin_unlock (&dev->lock); - return 0; + /* + * Return USB_GADGET_DELAYED_STATUS as a workaround to + * stop some UDC drivers (e.g. dwc3) from automatically + * proceeding with the status stage for 0-length + * transfers. + * Should be removed once all UDC drivers are fixed to + * always delay the status stage until a response is + * queued to EP0. + */ + return w_length == 0 ? USB_GADGET_DELAYED_STATUS : 0; } }