From: claus.stovgaard@gmail.com
To: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Felipe Balbi <balbi@kernel.org>
Cc: linux-usb@vger.kernel.org, v.anuragkumar@gmail.com,
Rob Weber <rob@gnarbox.com>
Subject: [0/3] usb: gadget: Add support for disabling U1 and U2 entries
Date: Thu, 02 May 2019 23:36:26 +0200 [thread overview]
Message-ID: <1556832986.22007.15.camel@gmail.com> (raw)
Hi
On tor, 2019-05-02 at 15:50 +0530, Anurag Kumar Vulisha wrote:
> Gadget applications may have a requirement to disable the U1 and U2
> entry based on the usecase. For example, when performing performance
> benchmarking on mass storage gadget the U1 and U2 entries can be
> disabled.
> Another example is when periodic transfers like ISOC transfers are
> used
> with bInterval of 1 which doesn't require the link to enter into U1
> or U2
> state (since ping is issued from host for every uframe interval). In
> this
> case the U1 and U2 entry can be disabled. This can be done by setting
> U1DevExitLat and U2DevExitLat values to 0 in the BOS descriptor. Host
> on
> seeing 0 value for U1DevExitLat and U2DevExitLat, it doesn't send
> SET_SEL
> commands to the gadget. Thus entry of U1 and U2 states can be
> avioded.
> This patch updates the same.
>
Will just vote for this feature, as I will also be able to use it for
solving Rob Webers and my issue [1]
Just today I was making another solution for this feature, using the
configfs instead of the devicetree. Though thinks your solution is
better, as it uses the U1DevExitLat and U2DevExitLat instead. I just
added my solution to the bottem of the mail for reference.
[1] https://www.spinics.net/lists/linux-usb/msg179393.html
---
From 798ea2f5f365ecdf2dbcf436a2a0302e208c6c73 Mon Sep 17 00:00:00 2001
From: "Claus H. Stovgaard" <cst@phaseone.com>
Date: Thu, 2 May 2019 17:54:45 +0200
Subject: [PATCH] usb: gadget: configfs: Add lpm_Ux_disable
When combining dwc3 with an redriver for a USB Type-C device solution,
it
sometimes have problems with leaving U1/U2 for certain hosts, resulting
in
link training errors and reconnects. This patch create an interface via
configfs for disabling U1/U2, enabling a workaround for devices based
on
dwc3.
Signed-off-by: Claus H. Stovgaard <cst@phaseone.com>
---
drivers/usb/dwc3/ep0.c | 9 ++++++-
drivers/usb/gadget/configfs.c | 56
+++++++++++++++++++++++++++++++++++++++++++
include/linux/usb/gadget.h | 6 ++++-
3 files changed, 69 insertions(+), 2 deletions(-)
*
* Devices are disabled till a gadget driver successfully bind()s,
which
* means the driver will handle setup() requests needed to enumerate
(and
@@ -684,6 +686,8 @@ struct usb_gadget_driver {
char *udc_name;
struct list_head pending;
unsigned match_existing_only:1;
+ unsigned lpm_U1_disable:1;
+ unsigned lpm_U2_disable:1;
};
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 8efde17..5b2d26b 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -379,6 +379,8 @@ static int dwc3_ep0_handle_u1(struct dwc3 *dwc,
enum usb_device_state state,
if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
(dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
return -EINVAL;
+ if (dwc->gadget_driver->lpm_U1_disable)
+ return -EINVAL;
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (set)
@@ -401,6 +403,8 @@ static int dwc3_ep0_handle_u2(struct dwc3 *dwc,
enum usb_device_state state,
if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
(dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
return -EINVAL;
+ if (dwc->gadget_driver->lpm_U2_disable)
+ return -EINVAL;
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (set)
@@ -626,7 +630,10 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc,
struct usb_ctrlrequest *ctrl)
* nothing is pending from application.
*/
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
- reg |= (DWC3_DCTL_ACCEPTU1ENA |
DWC3_DCTL_ACCEPTU2ENA);
+ if (!dwc->gadget_driver->lpm_U1_disable)
+ reg |= DWC3_DCTL_ACCEPTU1ENA;
+ if (!dwc->gadget_driver->lpm_U2_disable)
+ reg |= DWC3_DCTL_ACCEPTU2ENA;
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
}
break;
diff --git a/drivers/usb/gadget/configfs.c
b/drivers/usb/gadget/configfs.c
index 0251299..2ee9d10 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -229,6 +229,56 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct
config_item *item,
return len;
}
+static ssize_t gadget_dev_desc_lpm_U1_disable_show(struct config_item
*item,
+ char *page)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+
+ return sprintf(page, "%d\n",
+ gi->composite.gadget_driver.lpm_U1_disable);
+}
+
+static ssize_t gadget_dev_desc_lpm_U1_disable_store(struct config_item
*item,
+ const char *page, size_t len)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+ bool disable;
+ int ret;
+
+ ret = strtobool(page, &disable);
+ if (!ret) {
+ gi->composite.gadget_driver.lpm_U1_disable = disable;
+ ret = len;
+ }
+
+ return ret;
+}
+
+static ssize_t gadget_dev_desc_lpm_U2_disable_show(struct config_item
*item,
+ char *page)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+
+ return sprintf(page, "%d\n",
+ gi->composite.gadget_driver.lpm_U2_disable);
+}
+
+static ssize_t gadget_dev_desc_lpm_U2_disable_store(struct config_item
*item,
+ const char *page, size_t len)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+ bool disable;
+ int ret;
+
+ ret = strtobool(page, &disable);
+ if (!ret) {
+ gi->composite.gadget_driver.lpm_U2_disable = disable;
+ ret = len;
+ }
+
+ return ret;
+}
+
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char
*page)
{
char *udc_name = to_gadget_info(item)-
>composite.gadget_driver.udc_name;
@@ -299,6 +349,8 @@ CONFIGFS_ATTR(gadget_dev_desc_, idVendor);
CONFIGFS_ATTR(gadget_dev_desc_, idProduct);
CONFIGFS_ATTR(gadget_dev_desc_, bcdDevice);
CONFIGFS_ATTR(gadget_dev_desc_, bcdUSB);
+CONFIGFS_ATTR(gadget_dev_desc_, lpm_U1_disable);
+CONFIGFS_ATTR(gadget_dev_desc_, lpm_U2_disable);
CONFIGFS_ATTR(gadget_dev_desc_, UDC);
static struct configfs_attribute *gadget_root_attrs[] = {
@@ -310,6 +362,8 @@ static struct configfs_attribute
*gadget_root_attrs[] = {
&gadget_dev_desc_attr_idProduct,
&gadget_dev_desc_attr_bcdDevice,
&gadget_dev_desc_attr_bcdUSB,
+ &gadget_dev_desc_attr_lpm_U1_disable,
+ &gadget_dev_desc_attr_lpm_U2_disable,
&gadget_dev_desc_attr_UDC,
NULL,
};
@@ -1408,6 +1462,8 @@ static const struct usb_gadget_driver
configfs_driver_template = {
.name = "configfs-gadget",
},
.match_existing_only = 1,
+ .lpm_U1_disable = 0,
+ .lpm_U2_disable = 0,
};
static struct config_group *gadgets_make(
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 7595056..25fe72b 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -619,7 +619,9 @@ static inline int usb_gadget_activate(struct
usb_gadget *gadget)
* this driver will be bound to any available UDC.
* @pending: UDC core private data used for deferred probe of this
driver.
* @match_existing_only: If udc is not found, return an error and
don't add this
- * gadget driver to list of pending driver
+ * gadget driver to list of pending driver.
+ * @lpm_U1_disable: Instruct the UDC to disable U1 if possible.
+ * @lpm_U2_disable: Instruct the UDC to disable U2 if possible.
WARNING: multiple messages have this Message-ID (diff)
From: claus.stovgaard@gmail.com
To: Anurag Kumar Vulisha <anurag.kumar.vulisha@xilinx.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Rob Herring <robh+dt@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Felipe Balbi <balbi@kernel.org>
Cc: linux-usb@vger.kernel.org, v.anuragkumar@gmail.com,
Rob Weber <rob@gnarbox.com>
Subject: Re: [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries
Date: Thu, 02 May 2019 23:36:26 +0200 [thread overview]
Message-ID: <1556832986.22007.15.camel@gmail.com> (raw)
Message-ID: <20190502213626.e-PUxLSgvHBkFKpiThzOSTvPMW0ee2mE_rTswSmWzM0@z> (raw)
In-Reply-To: <1556792423-4833-1-git-send-email-anurag.kumar.vulisha@xilinx.com>
Hi
On tor, 2019-05-02 at 15:50 +0530, Anurag Kumar Vulisha wrote:
> Gadget applications may have a requirement to disable the U1 and U2
> entry based on the usecase. For example, when performing performance
> benchmarking on mass storage gadget the U1 and U2 entries can be
> disabled.
> Another example is when periodic transfers like ISOC transfers are
> used
> with bInterval of 1 which doesn't require the link to enter into U1
> or U2
> state (since ping is issued from host for every uframe interval). In
> this
> case the U1 and U2 entry can be disabled. This can be done by setting
> U1DevExitLat and U2DevExitLat values to 0 in the BOS descriptor. Host
> on
> seeing 0 value for U1DevExitLat and U2DevExitLat, it doesn't send
> SET_SEL
> commands to the gadget. Thus entry of U1 and U2 states can be
> avioded.
> This patch updates the same.
>
Will just vote for this feature, as I will also be able to use it for
solving Rob Webers and my issue [1]
Just today I was making another solution for this feature, using the
configfs instead of the devicetree. Though thinks your solution is
better, as it uses the U1DevExitLat and U2DevExitLat instead. I just
added my solution to the bottem of the mail for reference.
[1] https://www.spinics.net/lists/linux-usb/msg179393.html
---
From 798ea2f5f365ecdf2dbcf436a2a0302e208c6c73 Mon Sep 17 00:00:00 2001
From: "Claus H. Stovgaard" <cst@phaseone.com>
Date: Thu, 2 May 2019 17:54:45 +0200
Subject: [PATCH] usb: gadget: configfs: Add lpm_Ux_disable
When combining dwc3 with an redriver for a USB Type-C device solution,
it
sometimes have problems with leaving U1/U2 for certain hosts, resulting
in
link training errors and reconnects. This patch create an interface via
configfs for disabling U1/U2, enabling a workaround for devices based
on
dwc3.
Signed-off-by: Claus H. Stovgaard <cst@phaseone.com>
---
drivers/usb/dwc3/ep0.c | 9 ++++++-
drivers/usb/gadget/configfs.c | 56
+++++++++++++++++++++++++++++++++++++++++++
include/linux/usb/gadget.h | 6 ++++-
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 8efde17..5b2d26b 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -379,6 +379,8 @@ static int dwc3_ep0_handle_u1(struct dwc3 *dwc,
enum usb_device_state state,
if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
(dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
return -EINVAL;
+ if (dwc->gadget_driver->lpm_U1_disable)
+ return -EINVAL;
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (set)
@@ -401,6 +403,8 @@ static int dwc3_ep0_handle_u2(struct dwc3 *dwc,
enum usb_device_state state,
if ((dwc->speed != DWC3_DSTS_SUPERSPEED) &&
(dwc->speed != DWC3_DSTS_SUPERSPEED_PLUS))
return -EINVAL;
+ if (dwc->gadget_driver->lpm_U2_disable)
+ return -EINVAL;
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
if (set)
@@ -626,7 +630,10 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc,
struct usb_ctrlrequest *ctrl)
* nothing is pending from application.
*/
reg = dwc3_readl(dwc->regs, DWC3_DCTL);
- reg |= (DWC3_DCTL_ACCEPTU1ENA |
DWC3_DCTL_ACCEPTU2ENA);
+ if (!dwc->gadget_driver->lpm_U1_disable)
+ reg |= DWC3_DCTL_ACCEPTU1ENA;
+ if (!dwc->gadget_driver->lpm_U2_disable)
+ reg |= DWC3_DCTL_ACCEPTU2ENA;
dwc3_writel(dwc->regs, DWC3_DCTL, reg);
}
break;
diff --git a/drivers/usb/gadget/configfs.c
b/drivers/usb/gadget/configfs.c
index 0251299..2ee9d10 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -229,6 +229,56 @@ static ssize_t gadget_dev_desc_bcdUSB_store(struct
config_item *item,
return len;
}
+static ssize_t gadget_dev_desc_lpm_U1_disable_show(struct config_item
*item,
+ char *page)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+
+ return sprintf(page, "%d\n",
+ gi->composite.gadget_driver.lpm_U1_disable);
+}
+
+static ssize_t gadget_dev_desc_lpm_U1_disable_store(struct config_item
*item,
+ const char *page, size_t len)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+ bool disable;
+ int ret;
+
+ ret = strtobool(page, &disable);
+ if (!ret) {
+ gi->composite.gadget_driver.lpm_U1_disable = disable;
+ ret = len;
+ }
+
+ return ret;
+}
+
+static ssize_t gadget_dev_desc_lpm_U2_disable_show(struct config_item
*item,
+ char *page)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+
+ return sprintf(page, "%d\n",
+ gi->composite.gadget_driver.lpm_U2_disable);
+}
+
+static ssize_t gadget_dev_desc_lpm_U2_disable_store(struct config_item
*item,
+ const char *page, size_t len)
+{
+ struct gadget_info *gi = to_gadget_info(item);
+ bool disable;
+ int ret;
+
+ ret = strtobool(page, &disable);
+ if (!ret) {
+ gi->composite.gadget_driver.lpm_U2_disable = disable;
+ ret = len;
+ }
+
+ return ret;
+}
+
static ssize_t gadget_dev_desc_UDC_show(struct config_item *item, char
*page)
{
char *udc_name = to_gadget_info(item)-
>composite.gadget_driver.udc_name;
@@ -299,6 +349,8 @@ CONFIGFS_ATTR(gadget_dev_desc_, idVendor);
CONFIGFS_ATTR(gadget_dev_desc_, idProduct);
CONFIGFS_ATTR(gadget_dev_desc_, bcdDevice);
CONFIGFS_ATTR(gadget_dev_desc_, bcdUSB);
+CONFIGFS_ATTR(gadget_dev_desc_, lpm_U1_disable);
+CONFIGFS_ATTR(gadget_dev_desc_, lpm_U2_disable);
CONFIGFS_ATTR(gadget_dev_desc_, UDC);
static struct configfs_attribute *gadget_root_attrs[] = {
@@ -310,6 +362,8 @@ static struct configfs_attribute
*gadget_root_attrs[] = {
&gadget_dev_desc_attr_idProduct,
&gadget_dev_desc_attr_bcdDevice,
&gadget_dev_desc_attr_bcdUSB,
+ &gadget_dev_desc_attr_lpm_U1_disable,
+ &gadget_dev_desc_attr_lpm_U2_disable,
&gadget_dev_desc_attr_UDC,
NULL,
};
@@ -1408,6 +1462,8 @@ static const struct usb_gadget_driver
configfs_driver_template = {
.name = "configfs-gadget",
},
.match_existing_only = 1,
+ .lpm_U1_disable = 0,
+ .lpm_U2_disable = 0,
};
static struct config_group *gadgets_make(
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 7595056..25fe72b 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -619,7 +619,9 @@ static inline int usb_gadget_activate(struct
usb_gadget *gadget)
* this driver will be bound to any available UDC.
* @pending: UDC core private data used for deferred probe of this
driver.
* @match_existing_only: If udc is not found, return an error and
don't add this
- * gadget driver to list of pending driver
+ * gadget driver to list of pending driver.
+ * @lpm_U1_disable: Instruct the UDC to disable U1 if possible.
+ * @lpm_U2_disable: Instruct the UDC to disable U2 if possible.
*
* Devices are disabled till a gadget driver successfully bind()s,
which
* means the driver will handle setup() requests needed to enumerate
(and
@@ -684,6 +686,8 @@ struct usb_gadget_driver {
char *udc_name;
struct list_head pending;
unsigned match_existing_only:1;
+ unsigned lpm_U1_disable:1;
+ unsigned lpm_U2_disable:1;
};
--
2.7.4
next reply other threads:[~2019-05-02 21:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-05-02 21:36 claus.stovgaard [this message]
2019-05-02 21:36 ` [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries claus.stovgaard
-- strict thread matches above, loose matches on Subject: below --
2019-05-03 13:52 [0/3] " Claus H. Stovgaard
2019-05-03 13:52 ` [PATCH 0/3] " Claus H. Stovgaard
2019-05-03 7:34 [0/3] " Anurag Kumar Vulisha
2019-05-03 7:34 ` [PATCH 0/3] " Anurag Kumar Vulisha
2019-05-02 10:20 [2/3] usb: gadget: send usb_gadget as an argument in get_config_params Anurag Kumar Vulisha
2019-05-02 10:20 ` [PATCH 2/3] " Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 [1/3] doc: dt: bindings: usb: dwc3: Update entries for disabling U1 and U2 Anurag Kumar Vulisha
2019-05-02 10:20 ` [PATCH 1/3] " Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 ` [3/3] usb: dwc3: " Anurag Kumar Vulisha
2019-05-02 10:20 ` [PATCH 3/3] " Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-06 19:21 ` Thinh Nguyen
2019-05-06 20:58 ` Claus H. Stovgaard
2019-05-07 9:50 ` Anurag Kumar Vulisha
2019-05-07 13:17 ` Claus H. Stovgaard
2019-05-07 14:09 ` Anurag Kumar Vulisha
2019-05-07 18:42 ` Thinh Nguyen
2019-05-07 9:46 ` Anurag Kumar Vulisha
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1556832986.22007.15.camel@gmail.com \
--to=claus.stovgaard@gmail.com \
--cc=anurag.kumar.vulisha@xilinx.com \
--cc=balbi@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-usb@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rob@gnarbox.com \
--cc=robh+dt@kernel.org \
--cc=v.anuragkumar@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.