All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/5] Add function suspend/resume and remote wakeup support
@ 2023-02-16 22:29 Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup Elson Roy Serrao
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

Changes in v5
 - Add wakeup_armed check in patch2 in the link status change event handler
   so that resume gets triggeed only in the remote wakeup context.
 - Costmetic changes in patch3 and patch4 

Changes in v4
 - Moved the wakeup bit check to bind function for warning the user at an early
   stage itself.
 - Added the remote wakeup configured check to gadget_wakeup() and func_wakeup()
   routines so that wakeup can be triggered only if user has configured it.
 - Cosmetic changes with respect to renaming the variables to reflect the operation
   better.

Changes in v3
 - Modified rw_capable flag to reflect the gadgets capability for wakeup
   signalling.
 - Added a check to configure wakeup bit in bmAttributes only if gadget
   is capable of triggering wakeup.
 - Implemented a gadget op for composite layer to inform UDC whether device
   is configured for remote wakeup.
 - Added a check in __usb_gadget_wakeup() API to trigger wakeup only if the
   device is configured for it.
 - Cosmetic changes in dwc3_gadget_func_wakeup() API.

Changes in v2
 - Added a flag to indicate whether the device is remote wakeup capable.
 - Added an async parameter to _dwc3_gadget_wakeup() API and few cosmetic
   changes.
 - Added flags to reflect the state of  function suspend and function remote
   wakeup to usb_function struct rather than function specific struct (f_ecm).
 - Changed the dwc3_gadget_func__wakeup() API to run synchronously by first
   checking the link state and then sending the device notification. Also
   added debug log for DEVICE_NOTIFICATION generic cmd.
 - Added changes to arm the device for remotewakeup/function remotewakeup
   only if device is capable.

An usb device can initate a remote wakeup and bring the link out of
suspend as dictated by the DEVICE_REMOTE_WAKEUP feature selector.
To achieve this an interface can invoke gadget_wakeup op and wait for the
device to come out of LPM. But the current polling based implementation
fails if the host takes a long time to drive the resume signaling specially
in high speed capable devices. Switching to an interrupt based approach is
more robust and efficient. This can be leveraged by enabling link status
change events and triggering a gadget resume when the link comes to active
state.

If the device is enhanced super-speed capable, individual interfaces can
also be put into suspend state. An interface can be in function suspend
state even when the device is not in suspend state. Function suspend state
is retained throughout the device suspend entry and exit process.
A function can be put to function suspend through FUNCTION_SUSPEND feature
selector sent by the host. This setup packet also decides whether that
function is capable of initiating a function remote wakeup. When the
function sends a wakeup notification to the host the link must be first
brought to a non-U0 state and then this notification is sent.

This change adds the infrastructure needed to support the above
functionalities.

Elson Roy Serrao (5):
  usb: gadget: Properly configure the device for remote wakeup
  usb: dwc3: Add remote wakeup handling
  usb: gadget: Add function wakeup support
  usb: dwc3: Add function suspend and function wakeup support
  usb: gadget: f_ecm: Add suspend/resume and remote wakeup support

 drivers/usb/dwc3/core.h               |   5 ++
 drivers/usb/dwc3/debug.h              |   2 +
 drivers/usb/dwc3/ep0.c                |  16 ++---
 drivers/usb/dwc3/gadget.c             | 118 ++++++++++++++++++++++++++++++++--
 drivers/usb/gadget/composite.c        |  42 ++++++++++++
 drivers/usb/gadget/configfs.c         |   3 +
 drivers/usb/gadget/function/f_ecm.c   |  68 ++++++++++++++++++++
 drivers/usb/gadget/function/u_ether.c |  63 ++++++++++++++++++
 drivers/usb/gadget/function/u_ether.h |   4 ++
 drivers/usb/gadget/udc/core.c         |  48 ++++++++++++++
 drivers/usb/gadget/udc/trace.h        |   5 ++
 include/linux/usb/composite.h         |   8 +++
 include/linux/usb/gadget.h            |  12 ++++
 13 files changed, 380 insertions(+), 14 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup
  2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
@ 2023-02-16 22:29 ` Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 2/5] usb: dwc3: Add remote wakeup handling Elson Roy Serrao
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

The wakeup bit in the bmAttributes field indicates whether the device
is configured for remote wakeup. But this field should be allowed to
set only if the UDC supports such wakeup mechanism. So configure this
field based on UDC capability. Also inform the UDC whether the device
is configured for remote wakeup by implementing a gadget op.

Reviewed-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/gadget/composite.c | 18 ++++++++++++++++++
 drivers/usb/gadget/configfs.c  |  3 +++
 drivers/usb/gadget/udc/core.c  | 27 +++++++++++++++++++++++++++
 drivers/usb/gadget/udc/trace.h |  5 +++++
 include/linux/usb/composite.h  |  2 ++
 include/linux/usb/gadget.h     |  8 ++++++++
 6 files changed, 63 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index fa7dd6c..a37a8f4 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -513,6 +513,19 @@ static u8 encode_bMaxPower(enum usb_device_speed speed,
 		return min(val, 900U) / 8;
 }
 
+void check_remote_wakeup_config(struct usb_gadget *g,
+				struct usb_configuration *c)
+{
+	if (USB_CONFIG_ATT_WAKEUP & c->bmAttributes) {
+		/* Reset the rw bit if gadget is not capable of it */
+		if (!g->wakeup_capable && g->ops->set_remote_wakeup) {
+			WARN(c->cdev, "Clearing wakeup bit for config c.%d\n",
+			     c->bConfigurationValue);
+			c->bmAttributes &= ~USB_CONFIG_ATT_WAKEUP;
+		}
+	}
+}
+
 static int config_buf(struct usb_configuration *config,
 		enum usb_device_speed speed, void *buf, u8 type)
 {
@@ -1000,6 +1013,11 @@ static int set_config(struct usb_composite_dev *cdev,
 	else
 		usb_gadget_clear_selfpowered(gadget);
 
+	if (USB_CONFIG_ATT_WAKEUP & c->bmAttributes)
+		usb_gadget_set_remote_wakeup(gadget, 1);
+	else
+		usb_gadget_set_remote_wakeup(gadget, 0);
+
 	usb_gadget_vbus_draw(gadget, power);
 	if (result >= 0 && cdev->delayed_status)
 		result = USB_GADGET_DELAYED_STATUS;
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index b9f1136..4c639e9 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1761,6 +1761,9 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 		if (gadget_is_otg(gadget))
 			c->descriptors = otg_desc;
 
+		/* Properly configure the bmAttributes wakeup bit */
+		check_remote_wakeup_config(gadget, c);
+
 		cfg = container_of(c, struct config_usb_cfg, c);
 		if (!list_empty(&cfg->string_list)) {
 			i = 0;
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 23b0629..3dcbba7 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -514,6 +514,33 @@ int usb_gadget_wakeup(struct usb_gadget *gadget)
 EXPORT_SYMBOL_GPL(usb_gadget_wakeup);
 
 /**
+ * usb_gadget_set_remote_wakeup - configures the device remote wakeup feature.
+ * @gadget:the device being configured for remote wakeup
+ * @set:value to be configured.
+ *
+ * set to one to enable remote wakeup feature and zero to disable it.
+ *
+ * returns zero on success, else negative errno.
+ */
+int usb_gadget_set_remote_wakeup(struct usb_gadget *gadget, int set)
+{
+	int ret = 0;
+
+	if (!gadget->ops->set_remote_wakeup) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
+	ret = gadget->ops->set_remote_wakeup(gadget, set);
+
+out:
+	trace_usb_gadget_set_remote_wakeup(gadget, ret);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_set_remote_wakeup);
+
+/**
  * usb_gadget_set_selfpowered - sets the device selfpowered feature.
  * @gadget:the device being declared as self-powered
  *
diff --git a/drivers/usb/gadget/udc/trace.h b/drivers/usb/gadget/udc/trace.h
index abdbcb1..a5ed26f 100644
--- a/drivers/usb/gadget/udc/trace.h
+++ b/drivers/usb/gadget/udc/trace.h
@@ -91,6 +91,11 @@ DEFINE_EVENT(udc_log_gadget, usb_gadget_wakeup,
 	TP_ARGS(g, ret)
 );
 
+DEFINE_EVENT(udc_log_gadget, usb_gadget_set_remote_wakeup,
+	TP_PROTO(struct usb_gadget *g, int ret),
+	TP_ARGS(g, ret)
+);
+
 DEFINE_EVENT(udc_log_gadget, usb_gadget_set_selfpowered,
 	TP_PROTO(struct usb_gadget *g, int ret),
 	TP_ARGS(g, ret)
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 608dc96..d949e91 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -413,6 +413,8 @@ extern int composite_dev_prepare(struct usb_composite_driver *composite,
 extern int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
 					 struct usb_ep *ep0);
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
+void check_remote_wakeup_config(struct usb_gadget *g,
+				struct usb_configuration *c);
 
 static inline struct usb_composite_driver *to_cdriver(
 		struct usb_gadget_driver *gdrv)
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 00750f7..1d79612 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -310,6 +310,7 @@ struct usb_udc;
 struct usb_gadget_ops {
 	int	(*get_frame)(struct usb_gadget *);
 	int	(*wakeup)(struct usb_gadget *);
+	int	(*set_remote_wakeup)(struct usb_gadget *, int set);
 	int	(*set_selfpowered) (struct usb_gadget *, int is_selfpowered);
 	int	(*vbus_session) (struct usb_gadget *, int is_active);
 	int	(*vbus_draw) (struct usb_gadget *, unsigned mA);
@@ -384,6 +385,8 @@ struct usb_gadget_ops {
  * @connected: True if gadget is connected.
  * @lpm_capable: If the gadget max_speed is FULL or HIGH, this flag
  *	indicates that it supports LPM as per the LPM ECN & errata.
+ * @wakeup_capable: True if gadget is capable of sending remote wakeup.
+ * @wakeup_armed: True if gadget is armed by the host for remote wakeup.
  * @irq: the interrupt number for device controller.
  * @id_number: a unique ID number for ensuring that gadget names are distinct
  *
@@ -445,6 +448,8 @@ struct usb_gadget {
 	unsigned			deactivated:1;
 	unsigned			connected:1;
 	unsigned			lpm_capable:1;
+	unsigned			wakeup_capable:1;
+	unsigned			wakeup_armed:1;
 	int				irq;
 	int				id_number;
 };
@@ -601,6 +606,7 @@ static inline int gadget_is_otg(struct usb_gadget *g)
 #if IS_ENABLED(CONFIG_USB_GADGET)
 int usb_gadget_frame_number(struct usb_gadget *gadget);
 int usb_gadget_wakeup(struct usb_gadget *gadget);
+int usb_gadget_set_remote_wakeup(struct usb_gadget *gadget, int set);
 int usb_gadget_set_selfpowered(struct usb_gadget *gadget);
 int usb_gadget_clear_selfpowered(struct usb_gadget *gadget);
 int usb_gadget_vbus_connect(struct usb_gadget *gadget);
@@ -616,6 +622,8 @@ static inline int usb_gadget_frame_number(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_wakeup(struct usb_gadget *gadget)
 { return 0; }
+static inline int usb_gadget_set_remote_wakeup(struct usb_gadget *gadget, int set)
+{ return 0; }
 static inline int usb_gadget_set_selfpowered(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_clear_selfpowered(struct usb_gadget *gadget)
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/5] usb: dwc3: Add remote wakeup handling
  2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup Elson Roy Serrao
@ 2023-02-16 22:29 ` Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 3/5] usb: gadget: Add function wakeup support Elson Roy Serrao
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

An usb device can initate a remote wakeup and bring the link out of
suspend as dictated by the DEVICE_REMOTE_WAKEUP feature selector.
Add support to handle this packet and set the remote wakeup capability.

Some hosts may take longer time to initiate the resume signaling after
device triggers a remote wakeup. So add async support to the wakeup API
by enabling link status change events.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/dwc3/core.h   |  2 ++
 drivers/usb/dwc3/ep0.c    |  4 +++
 drivers/usb/dwc3/gadget.c | 76 +++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 582ebd9..cc78236 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1110,6 +1110,7 @@ struct dwc3_scratchpad_array {
  *	3	- Reserved
  * @dis_metastability_quirk: set to disable metastability quirk.
  * @dis_split_quirk: set to disable split boundary.
+ * @wakeup_configured: set if the device is configured for remote wakeup.
  * @imod_interval: set the interrupt moderation interval in 250ns
  *			increments or 0 to disable.
  * @max_cfg_eps: current max number of IN eps used across all USB configs.
@@ -1327,6 +1328,7 @@ struct dwc3 {
 
 	unsigned		dis_split_quirk:1;
 	unsigned		async_callbacks:1;
+	unsigned		wakeup_configured:1;
 
 	u16			imod_interval;
 
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 61de693..f90fa55 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -356,6 +356,9 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
 				usb_status |= 1 << USB_DEV_STAT_U1_ENABLED;
 			if (reg & DWC3_DCTL_INITU2ENA)
 				usb_status |= 1 << USB_DEV_STAT_U2_ENABLED;
+		} else {
+			usb_status |= dwc->gadget->wakeup_armed <<
+					USB_DEVICE_REMOTE_WAKEUP;
 		}
 
 		break;
@@ -476,6 +479,7 @@ static int dwc3_ep0_handle_device(struct dwc3 *dwc,
 
 	switch (wValue) {
 	case USB_DEVICE_REMOTE_WAKEUP:
+		dwc->gadget->wakeup_armed = set;
 		break;
 	/*
 	 * 9.4.1 says only for SS, in AddressState only for
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3c63fa9..10003b7 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -258,7 +258,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, unsigned int cmd,
 	return ret;
 }
 
-static int __dwc3_gadget_wakeup(struct dwc3 *dwc);
+static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async);
 
 /**
  * dwc3_send_gadget_ep_cmd - issue an endpoint command
@@ -325,7 +325,7 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
 
 			fallthrough;
 		case DWC3_LINK_STATE_U3:
-			ret = __dwc3_gadget_wakeup(dwc);
+			ret = __dwc3_gadget_wakeup(dwc, false);
 			dev_WARN_ONCE(dwc->dev, ret, "wakeup failed --> %d\n",
 					ret);
 			break;
@@ -2269,6 +2269,22 @@ static const struct usb_ep_ops dwc3_gadget_ep_ops = {
 
 /* -------------------------------------------------------------------------- */
 
+static void dwc3_gadget_enable_linksts_evts(struct dwc3 *dwc, bool set)
+{
+	u32 reg;
+
+	if (DWC3_VER_IS_PRIOR(DWC3, 250A))
+		return;
+
+	reg = dwc3_readl(dwc->regs, DWC3_DEVTEN);
+	if (set)
+		reg |= DWC3_DEVTEN_ULSTCNGEN;
+	else
+		reg &= ~DWC3_DEVTEN_ULSTCNGEN;
+
+	dwc3_writel(dwc->regs, DWC3_DEVTEN, reg);
+}
+
 static int dwc3_gadget_get_frame(struct usb_gadget *g)
 {
 	struct dwc3		*dwc = gadget_to_dwc(g);
@@ -2276,7 +2292,7 @@ static int dwc3_gadget_get_frame(struct usb_gadget *g)
 	return __dwc3_gadget_get_frame(dwc);
 }
 
-static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
+static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async)
 {
 	int			retries;
 
@@ -2307,9 +2323,13 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
 		return -EINVAL;
 	}
 
+	if (async)
+		dwc3_gadget_enable_linksts_evts(dwc, true);
+
 	ret = dwc3_gadget_set_link_state(dwc, DWC3_LINK_STATE_RECOV);
 	if (ret < 0) {
 		dev_err(dwc->dev, "failed to put link in Recovery\n");
+		dwc3_gadget_enable_linksts_evts(dwc, false);
 		return ret;
 	}
 
@@ -2321,6 +2341,13 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc)
 		dwc3_writel(dwc->regs, DWC3_DCTL, reg);
 	}
 
+	/*
+	 * Since link status change events are enabled we will receive
+	 * an U0 event when wakeup is successful. So bail out.
+	 */
+	if (async)
+		return 0;
+
 	/* poll until Link State changes to ON */
 	retries = 20000;
 
@@ -2346,13 +2373,36 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g)
 	unsigned long		flags;
 	int			ret;
 
+	if (!dwc->wakeup_configured) {
+		dev_err(dwc->dev, "remote wakeup not configured\n");
+		return -EINVAL;
+	}
+
 	spin_lock_irqsave(&dwc->lock, flags);
-	ret = __dwc3_gadget_wakeup(dwc);
+	if (!dwc->gadget->wakeup_armed) {
+		dev_err(dwc->dev, "not armed for remote wakeup\n");
+		spin_unlock_irqrestore(&dwc->lock, flags);
+		return -EINVAL;
+	}
+	ret = __dwc3_gadget_wakeup(dwc, true);
+
 	spin_unlock_irqrestore(&dwc->lock, flags);
 
 	return ret;
 }
 
+static int dwc3_gadget_set_remote_wakeup(struct usb_gadget *g, int set)
+{
+	struct dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	dwc->wakeup_configured = !!set;
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	return 0;
+}
+
 static int dwc3_gadget_set_selfpowered(struct usb_gadget *g,
 		int is_selfpowered)
 {
@@ -2978,6 +3028,7 @@ static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
 static const struct usb_gadget_ops dwc3_gadget_ops = {
 	.get_frame		= dwc3_gadget_get_frame,
 	.wakeup			= dwc3_gadget_wakeup,
+	.set_remote_wakeup	= dwc3_gadget_set_remote_wakeup,
 	.set_selfpowered	= dwc3_gadget_set_selfpowered,
 	.pullup			= dwc3_gadget_pullup,
 	.udc_start		= dwc3_gadget_start,
@@ -3819,6 +3870,8 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc)
 
 	dwc->gadget->speed = USB_SPEED_UNKNOWN;
 	dwc->setup_packet_pending = false;
+	dwc->gadget->wakeup_armed = false;
+	dwc3_gadget_enable_linksts_evts(dwc, false);
 	usb_gadget_set_state(dwc->gadget, USB_STATE_NOTATTACHED);
 
 	if (dwc->ep0state != EP0_SETUP_PHASE) {
@@ -3912,6 +3965,8 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
 	reg &= ~DWC3_DCTL_TSTCTRL_MASK;
 	dwc3_gadget_dctl_write_safe(dwc, reg);
 	dwc->test_mode = false;
+	dwc->gadget->wakeup_armed = false;
+	dwc3_gadget_enable_linksts_evts(dwc, false);
 	dwc3_clear_stall_all_ep(dwc);
 
 	/* Reset device address to zero */
@@ -4064,7 +4119,7 @@ static void dwc3_gadget_conndone_interrupt(struct dwc3 *dwc)
 	 */
 }
 
-static void dwc3_gadget_wakeup_interrupt(struct dwc3 *dwc)
+static void dwc3_gadget_wakeup_interrupt(struct dwc3 *dwc, unsigned int evtinfo)
 {
 	/*
 	 * TODO take core out of low power mode when that's
@@ -4076,6 +4131,8 @@ static void dwc3_gadget_wakeup_interrupt(struct dwc3 *dwc)
 		dwc->gadget_driver->resume(dwc->gadget);
 		spin_lock(&dwc->lock);
 	}
+
+	dwc->link_state = evtinfo & DWC3_LINK_STATE_MASK;
 }
 
 static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
@@ -4157,6 +4214,12 @@ static void dwc3_gadget_linksts_change_interrupt(struct dwc3 *dwc,
 	}
 
 	switch (next) {
+	case DWC3_LINK_STATE_U0:
+		if (dwc->gadget->wakeup_armed) {
+			dwc3_gadget_enable_linksts_evts(dwc, false);
+			dwc3_resume_gadget(dwc);
+		}
+		break;
 	case DWC3_LINK_STATE_U1:
 		if (dwc->speed == USB_SPEED_SUPER)
 			dwc3_suspend_gadget(dwc);
@@ -4225,7 +4288,7 @@ static void dwc3_gadget_interrupt(struct dwc3 *dwc,
 		dwc3_gadget_conndone_interrupt(dwc);
 		break;
 	case DWC3_DEVICE_EVENT_WAKEUP:
-		dwc3_gadget_wakeup_interrupt(dwc);
+		dwc3_gadget_wakeup_interrupt(dwc, event->event_info);
 		break;
 	case DWC3_DEVICE_EVENT_HIBER_REQ:
 		if (dev_WARN_ONCE(dwc->dev, !dwc->has_hibernation,
@@ -4485,6 +4548,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	dwc->gadget->sg_supported	= true;
 	dwc->gadget->name		= "dwc3-gadget";
 	dwc->gadget->lpm_capable	= !dwc->usb2_gadget_lpm_disable;
+	dwc->gadget->wakeup_capable	= true;
 
 	/*
 	 * FIXME We might be setting max_speed to <SUPER, however versions
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/5] usb: gadget: Add function wakeup support
  2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 2/5] usb: dwc3: Add remote wakeup handling Elson Roy Serrao
@ 2023-02-16 22:29 ` Elson Roy Serrao
  2023-02-16 23:58   ` Thinh Nguyen
  2023-02-16 22:29 ` [PATCH v5 4/5] usb: dwc3: Add function suspend and " Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 5/5] usb: gadget: f_ecm: Add suspend/resume and remote " Elson Roy Serrao
  4 siblings, 1 reply; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

A function which is in function suspend state has to send a
function wake notification to the host in case it needs to
exit from this state and resume data transfer. Add support to
handle such requests by exposing a new gadget op.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++
 drivers/usb/gadget/udc/core.c  | 21 +++++++++++++++++++++
 include/linux/usb/composite.h  |  6 ++++++
 include/linux/usb/gadget.h     |  4 ++++
 4 files changed, 55 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a37a8f4..f649f997 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config,
 }
 EXPORT_SYMBOL_GPL(usb_interface_id);
 
+int usb_func_wakeup(struct usb_function *func)
+{
+	int ret, id;
+
+	if (!func->func_wakeup_armed) {
+		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
+		return -EINVAL;
+	}
+
+	for (id = 0; id < MAX_CONFIG_INTERFACES; id++)
+		if (func->config->interface[id] == func)
+			break;
+
+	if (id == MAX_CONFIG_INTERFACES) {
+		ERROR(func->config->cdev, "Invalid function\n");
+		return -EINVAL;
+	}
+
+	ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(usb_func_wakeup);
+
 static u8 encode_bMaxPower(enum usb_device_speed speed,
 		struct usb_configuration *c)
 {
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 3dcbba7..59e7a7e 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget)
 }
 EXPORT_SYMBOL_GPL(usb_gadget_activate);
 
+/**
+ * usb_gadget_func_wakeup - sends function wake notification to the host.
+ * @gadget: controller used to wake up the host
+ * @interface_id: interface on which function wake notification is sent.
+ *
+ * On completion, function wake notification is sent. If the device is in
+ * low power state it tries to bring the device to active state before sending
+ * the wake notification. Since it is a synchronous call, caller must take care
+ * of not calling it in interrupt context.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id)
+{
+	if (!gadget->ops->func_wakeup)
+		return -EOPNOTSUPP;
+
+	return gadget->ops->func_wakeup(gadget, intf_id);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup);
+
 /* ------------------------------------------------------------------------- */
 
 #ifdef	CONFIG_HAS_DMA
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index d949e91..a2448e9 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -150,6 +150,9 @@ struct usb_os_desc_table {
  *	GetStatus() request when the recipient is Interface.
  * @func_suspend: callback to be called when
  *	SetFeature(FUNCTION_SUSPEND) is reseived
+ * @func_suspended: Indicates whether the function is in function suspend state.
+ * @func_wakeup_armed: Indicates whether the function is armed by the host for
+ *	wakeup signaling.
  *
  * A single USB function uses one or more interfaces, and should in most
  * cases support operation at both full and high speeds.  Each function is
@@ -220,6 +223,8 @@ struct usb_function {
 	int			(*get_status)(struct usb_function *);
 	int			(*func_suspend)(struct usb_function *,
 						u8 suspend_opt);
+	bool			func_suspended;
+	bool			func_wakeup_armed;
 	/* private: */
 	/* internals */
 	struct list_head		list;
@@ -241,6 +246,7 @@ int config_ep_by_speed_and_alt(struct usb_gadget *g, struct usb_function *f,
 
 int config_ep_by_speed(struct usb_gadget *g, struct usb_function *f,
 			struct usb_ep *_ep);
+int usb_func_wakeup(struct usb_function *func);
 
 #define	MAX_CONFIG_INTERFACES		16	/* arbitrary; max 255 */
 
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 1d79612..2a512c5 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -310,6 +310,7 @@ struct usb_udc;
 struct usb_gadget_ops {
 	int	(*get_frame)(struct usb_gadget *);
 	int	(*wakeup)(struct usb_gadget *);
+	int     (*func_wakeup)(struct usb_gadget *gadget, int intf_id);
 	int	(*set_remote_wakeup)(struct usb_gadget *, int set);
 	int	(*set_selfpowered) (struct usb_gadget *, int is_selfpowered);
 	int	(*vbus_session) (struct usb_gadget *, int is_active);
@@ -617,6 +618,7 @@ int usb_gadget_disconnect(struct usb_gadget *gadget);
 int usb_gadget_deactivate(struct usb_gadget *gadget);
 int usb_gadget_activate(struct usb_gadget *gadget);
 int usb_gadget_check_config(struct usb_gadget *gadget);
+int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id);
 #else
 static inline int usb_gadget_frame_number(struct usb_gadget *gadget)
 { return 0; }
@@ -644,6 +646,8 @@ static inline int usb_gadget_activate(struct usb_gadget *gadget)
 { return 0; }
 static inline int usb_gadget_check_config(struct usb_gadget *gadget)
 { return 0; }
+static inline int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id)
+{ return 0; }
 #endif /* CONFIG_USB_GADGET */
 
 /*-------------------------------------------------------------------------*/
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 4/5] usb: dwc3: Add function suspend and function wakeup support
  2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
                   ` (2 preceding siblings ...)
  2023-02-16 22:29 ` [PATCH v5 3/5] usb: gadget: Add function wakeup support Elson Roy Serrao
@ 2023-02-16 22:29 ` Elson Roy Serrao
  2023-02-16 22:29 ` [PATCH v5 5/5] usb: gadget: f_ecm: Add suspend/resume and remote " Elson Roy Serrao
  4 siblings, 0 replies; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

USB host sends function suspend and function resume notifications to
the interface through SET_FEATURE/CLEAR_FEATURE setup packets.
Add support to handle these packets by delegating the requests to
composite layer. Also add support to handle function wake notification
requests to exit from function suspend state.

Acked-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/dwc3/core.h   |  3 +++
 drivers/usb/dwc3/debug.h  |  2 ++
 drivers/usb/dwc3/ep0.c    | 12 ++++--------
 drivers/usb/dwc3/gadget.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index cc78236..1565b21 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -526,6 +526,7 @@
 #define DWC3_DGCMD_SET_ENDPOINT_NRDY	0x0c
 #define DWC3_DGCMD_SET_ENDPOINT_PRIME	0x0d
 #define DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK	0x10
+#define DWC3_DGCMD_DEV_NOTIFICATION	0x07
 
 #define DWC3_DGCMD_STATUS(n)		(((n) >> 12) & 0x0F)
 #define DWC3_DGCMD_CMDACT		BIT(10)
@@ -538,6 +539,8 @@
 #define DWC3_DGCMDPAR_TX_FIFO			BIT(5)
 #define DWC3_DGCMDPAR_LOOPBACK_DIS		(0 << 0)
 #define DWC3_DGCMDPAR_LOOPBACK_ENA		BIT(0)
+#define DWC3_DGCMDPAR_DN_FUNC_WAKE		BIT(0)
+#define DWC3_DGCMDPAR_INTF_SEL(n)		((n) << 4)
 
 /* Device Endpoint Command Register */
 #define DWC3_DEPCMD_PARAM_SHIFT		16
diff --git a/drivers/usb/dwc3/debug.h b/drivers/usb/dwc3/debug.h
index 8bb2c9e..09d7038 100644
--- a/drivers/usb/dwc3/debug.h
+++ b/drivers/usb/dwc3/debug.h
@@ -72,6 +72,8 @@ dwc3_gadget_generic_cmd_string(u8 cmd)
 		return "Set Endpoint Prime";
 	case DWC3_DGCMD_RUN_SOC_BUS_LOOPBACK:
 		return "Run SoC Bus Loopback Test";
+	case DWC3_DGCMD_DEV_NOTIFICATION:
+		return "Device Notification";
 	default:
 		return "UNKNOWN";
 	}
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index f90fa55..1e7cc15 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -30,6 +30,8 @@
 static void __dwc3_ep0_do_control_status(struct dwc3 *dwc, struct dwc3_ep *dep);
 static void __dwc3_ep0_do_control_data(struct dwc3 *dwc,
 		struct dwc3_ep *dep, struct dwc3_request *req);
+static int dwc3_ep0_delegate_req(struct dwc3 *dwc,
+				 struct usb_ctrlrequest *ctrl);
 
 static void dwc3_ep0_prepare_one_trb(struct dwc3_ep *dep,
 		dma_addr_t buf_dma, u32 len, u32 type, bool chain)
@@ -368,7 +370,7 @@ static int dwc3_ep0_handle_status(struct dwc3 *dwc,
 		 * Function Remote Wake Capable	D0
 		 * Function Remote Wakeup	D1
 		 */
-		break;
+		return dwc3_ep0_delegate_req(dwc, ctrl);
 
 	case USB_RECIP_ENDPOINT:
 		dep = dwc3_wIndex_to_dep(dwc, ctrl->wIndex);
@@ -514,13 +516,7 @@ static int dwc3_ep0_handle_intf(struct dwc3 *dwc,
 
 	switch (wValue) {
 	case USB_INTRF_FUNC_SUSPEND:
-		/*
-		 * REVISIT: Ideally we would enable some low power mode here,
-		 * however it's unclear what we should be doing here.
-		 *
-		 * For now, we're not doing anything, just making sure we return
-		 * 0 so USB Command Verifier tests pass without any errors.
-		 */
+		ret = dwc3_ep0_delegate_req(dwc, ctrl);
 		break;
 	default:
 		ret = -EINVAL;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 10003b7..9d6b753 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2391,6 +2391,47 @@ static int dwc3_gadget_wakeup(struct usb_gadget *g)
 	return ret;
 }
 
+static void dwc3_resume_gadget(struct dwc3 *dwc);
+
+static int dwc3_gadget_func_wakeup(struct usb_gadget *g, int intf_id)
+{
+	struct  dwc3		*dwc = gadget_to_dwc(g);
+	unsigned long		flags;
+	int			ret;
+	int			link_state;
+
+	if (!dwc->wakeup_configured) {
+		dev_err(dwc->dev, "remote wakeup not configured\n");
+		return -EINVAL;
+	}
+
+	spin_lock_irqsave(&dwc->lock, flags);
+	/*
+	 * If the link is in U3, signal for remote wakeup and wait for the
+	 * link to transition to U0 before sending device notification.
+	 */
+	link_state = dwc3_gadget_get_link_state(dwc);
+	if (link_state == DWC3_LINK_STATE_U3) {
+		ret = __dwc3_gadget_wakeup(dwc, false);
+		if (ret) {
+			spin_unlock_irqrestore(&dwc->lock, flags);
+			return -EINVAL;
+		}
+		dwc3_resume_gadget(dwc);
+		dwc->link_state = DWC3_LINK_STATE_U0;
+	}
+
+	ret = dwc3_send_gadget_generic_command(dwc, DWC3_DGCMD_DEV_NOTIFICATION,
+					       DWC3_DGCMDPAR_DN_FUNC_WAKE |
+					       DWC3_DGCMDPAR_INTF_SEL(intf_id));
+	if (ret)
+		dev_err(dwc->dev, "function remote wakeup failed, ret:%d\n", ret);
+
+	spin_unlock_irqrestore(&dwc->lock, flags);
+
+	return ret;
+}
+
 static int dwc3_gadget_set_remote_wakeup(struct usb_gadget *g, int set)
 {
 	struct dwc3		*dwc = gadget_to_dwc(g);
@@ -3028,6 +3069,7 @@ static void dwc3_gadget_async_callbacks(struct usb_gadget *g, bool enable)
 static const struct usb_gadget_ops dwc3_gadget_ops = {
 	.get_frame		= dwc3_gadget_get_frame,
 	.wakeup			= dwc3_gadget_wakeup,
+	.func_wakeup		= dwc3_gadget_func_wakeup,
 	.set_remote_wakeup	= dwc3_gadget_set_remote_wakeup,
 	.set_selfpowered	= dwc3_gadget_set_selfpowered,
 	.pullup			= dwc3_gadget_pullup,
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 5/5] usb: gadget: f_ecm: Add suspend/resume and remote wakeup support
  2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
                   ` (3 preceding siblings ...)
  2023-02-16 22:29 ` [PATCH v5 4/5] usb: dwc3: Add function suspend and " Elson Roy Serrao
@ 2023-02-16 22:29 ` Elson Roy Serrao
  4 siblings, 0 replies; 11+ messages in thread
From: Elson Roy Serrao @ 2023-02-16 22:29 UTC (permalink / raw)
  To: gregkh, Thinh.Nguyen, balbi
  Cc: linux-kernel, linux-usb, quic_wcheng, quic_jackp, Elson Roy Serrao

When host sends a suspend notification to the device, handle
the suspend callbacks in the function driver. Enhanced super
speed devices can support function suspend feature to put the
function in suspend state. Handle function suspend callback.

Depending on the remote wakeup capability the device can either
trigger a remote wakeup or wait for the host initiated resume to
start data transfer again.

Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
---
 drivers/usb/gadget/function/f_ecm.c   | 68 +++++++++++++++++++++++++++++++++++
 drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++++++
 drivers/usb/gadget/function/u_ether.h |  4 +++
 3 files changed, 135 insertions(+)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index a7ab30e..d50c1a4 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -633,6 +633,8 @@ static void ecm_disable(struct usb_function *f)
 
 	usb_ep_disable(ecm->notify);
 	ecm->notify->desc = NULL;
+	f->func_suspended = false;
+	f->func_wakeup_armed = false;
 }
 
 /*-------------------------------------------------------------------------*/
@@ -885,6 +887,68 @@ static struct usb_function_instance *ecm_alloc_inst(void)
 	return &opts->func_inst;
 }
 
+static void ecm_suspend(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	if (f->func_suspended) {
+		DBG(cdev, "Function already suspended\n");
+		return;
+	}
+
+	DBG(cdev, "ECM Suspend\n");
+
+	gether_suspend(&ecm->port);
+}
+
+static void ecm_resume(struct usb_function *f)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	/*
+	 * If the function is in USB3 Function Suspend state, resume is
+	 * canceled. In this case resume is done by a Function Resume request.
+	 */
+	if (f->func_suspended)
+		return;
+
+	DBG(cdev, "ECM Resume\n");
+
+	gether_resume(&ecm->port);
+}
+
+static int ecm_get_status(struct usb_function *f)
+{
+	return (f->func_wakeup_armed ? USB_INTRF_STAT_FUNC_RW : 0) |
+		USB_INTRF_STAT_FUNC_RW_CAP;
+}
+
+static int ecm_func_suspend(struct usb_function *f, u8 options)
+{
+	struct f_ecm *ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = ecm->port.func.config->cdev;
+
+	DBG(cdev, "func susp %u cmd\n", options);
+
+	f->func_wakeup_armed = !!(options & (USB_INTRF_FUNC_SUSPEND_RW >> 8));
+
+	if (options & (USB_INTRF_FUNC_SUSPEND_LP >> 8)) {
+		if (!f->func_suspended) {
+			ecm_suspend(f);
+			f->func_suspended = true;
+		}
+	} else {
+		if (f->func_suspended) {
+			f->func_suspended = false;
+			ecm_resume(f);
+		}
+	}
+
+	return 0;
+}
+
 static void ecm_free(struct usb_function *f)
 {
 	struct f_ecm *ecm;
@@ -952,6 +1016,10 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 	ecm->port.func.setup = ecm_setup;
 	ecm->port.func.disable = ecm_disable;
 	ecm->port.func.free_func = ecm_free;
+	ecm->port.func.suspend = ecm_suspend;
+	ecm->port.func.get_status = ecm_get_status;
+	ecm->port.func.func_suspend = ecm_func_suspend;
+	ecm->port.func.resume = ecm_resume;
 
 	return &ecm->port.func;
 }
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index f259975..8eba018 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -437,6 +437,20 @@ static inline int is_promisc(u16 cdc_filter)
 	return cdc_filter & USB_CDC_PACKET_TYPE_PROMISCUOUS;
 }
 
+static int ether_wakeup_host(struct gether *port)
+{
+	int			ret;
+	struct usb_function	*func = &port->func;
+	struct usb_gadget	*gadget = func->config->cdev->gadget;
+
+	if (func->func_suspended)
+		ret = usb_func_wakeup(func);
+	else
+		ret = usb_gadget_wakeup(gadget);
+
+	return ret;
+}
+
 static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 					struct net_device *net)
 {
@@ -456,6 +470,15 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
 		in = NULL;
 		cdc_filter = 0;
 	}
+
+	if (dev->port_usb->is_suspend) {
+		DBG(dev, "Port suspended. Triggering wakeup\n");
+		netif_stop_queue(net);
+		spin_unlock_irqrestore(&dev->lock, flags);
+		ether_wakeup_host(dev->port_usb);
+		return NETDEV_TX_BUSY;
+	}
+
 	spin_unlock_irqrestore(&dev->lock, flags);
 
 	if (!in) {
@@ -1014,6 +1037,45 @@ int gether_set_ifname(struct net_device *net, const char *name, int len)
 }
 EXPORT_SYMBOL_GPL(gether_set_ifname);
 
+void gether_suspend(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	unsigned long flags;
+
+	if (!dev)
+		return;
+
+	if (atomic_read(&dev->tx_qlen)) {
+		/*
+		 * There is a transfer in progress. So we trigger a remote
+		 * wakeup to inform the host.
+		 */
+		ether_wakeup_host(dev->port_usb);
+		return;
+	}
+	spin_lock_irqsave(&dev->lock, flags);
+	link->is_suspend = true;
+	spin_unlock_irqrestore(&dev->lock, flags);
+}
+EXPORT_SYMBOL_GPL(gether_suspend);
+
+void gether_resume(struct gether *link)
+{
+	struct eth_dev *dev = link->ioport;
+	unsigned long flags;
+
+	if (!dev)
+		return;
+
+	if (netif_queue_stopped(dev->net))
+		netif_start_queue(dev->net);
+
+	spin_lock_irqsave(&dev->lock, flags);
+	link->is_suspend = false;
+	spin_unlock_irqrestore(&dev->lock, flags);
+}
+EXPORT_SYMBOL_GPL(gether_resume);
+
 /*
  * gether_cleanup - remove Ethernet-over-USB device
  * Context: may sleep
@@ -1176,6 +1238,7 @@ void gether_disconnect(struct gether *link)
 
 	spin_lock(&dev->lock);
 	dev->port_usb = NULL;
+	link->is_suspend = false;
 	spin_unlock(&dev->lock);
 }
 EXPORT_SYMBOL_GPL(gether_disconnect);
diff --git a/drivers/usb/gadget/function/u_ether.h b/drivers/usb/gadget/function/u_ether.h
index 4014454..851ee10 100644
--- a/drivers/usb/gadget/function/u_ether.h
+++ b/drivers/usb/gadget/function/u_ether.h
@@ -79,6 +79,7 @@ struct gether {
 	/* called on network open/close */
 	void				(*open)(struct gether *);
 	void				(*close)(struct gether *);
+	bool				is_suspend;
 };
 
 #define	DEFAULT_FILTER	(USB_CDC_PACKET_TYPE_BROADCAST \
@@ -258,6 +259,9 @@ int gether_set_ifname(struct net_device *net, const char *name, int len);
 
 void gether_cleanup(struct eth_dev *dev);
 
+void gether_suspend(struct gether *link);
+void gether_resume(struct gether *link);
+
 /* connect/disconnect is handled by individual functions */
 struct net_device *gether_connect(struct gether *);
 void gether_disconnect(struct gether *);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] usb: gadget: Add function wakeup support
  2023-02-16 22:29 ` [PATCH v5 3/5] usb: gadget: Add function wakeup support Elson Roy Serrao
@ 2023-02-16 23:58   ` Thinh Nguyen
  2023-02-17  0:38     ` Elson Serrao
  0 siblings, 1 reply; 11+ messages in thread
From: Thinh Nguyen @ 2023-02-16 23:58 UTC (permalink / raw)
  To: Elson Roy Serrao
  Cc: gregkh, Thinh Nguyen, balbi, linux-kernel, linux-usb,
	quic_wcheng, quic_jackp

On Thu, Feb 16, 2023, Elson Roy Serrao wrote:
> A function which is in function suspend state has to send a
> function wake notification to the host in case it needs to
> exit from this state and resume data transfer. Add support to
> handle such requests by exposing a new gadget op.
> 
> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
> ---
>  drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++
>  drivers/usb/gadget/udc/core.c  | 21 +++++++++++++++++++++
>  include/linux/usb/composite.h  |  6 ++++++
>  include/linux/usb/gadget.h     |  4 ++++
>  4 files changed, 55 insertions(+)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index a37a8f4..f649f997 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config,
>  }
>  EXPORT_SYMBOL_GPL(usb_interface_id);
>  
> +int usb_func_wakeup(struct usb_function *func)
> +{
> +	int ret, id;
> +
> +	if (!func->func_wakeup_armed) {
> +		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
> +		return -EINVAL;
> +	}
> +
> +	for (id = 0; id < MAX_CONFIG_INTERFACES; id++)
> +		if (func->config->interface[id] == func)
> +			break;
> +
> +	if (id == MAX_CONFIG_INTERFACES) {
> +		ERROR(func->config->cdev, "Invalid function\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(usb_func_wakeup);
> +
>  static u8 encode_bMaxPower(enum usb_device_speed speed,
>  		struct usb_configuration *c)
>  {
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 3dcbba7..59e7a7e 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget)
>  }
>  EXPORT_SYMBOL_GPL(usb_gadget_activate);
>  
> +/**
> + * usb_gadget_func_wakeup - sends function wake notification to the host.
> + * @gadget: controller used to wake up the host
> + * @interface_id: interface on which function wake notification is sent.

Device notification is only applicable for eSS devices. What will happen
if the device is operating in lower speed and the driver calls this
function?

Thanks,
Thinh

> + *
> + * On completion, function wake notification is sent. If the device is in
> + * low power state it tries to bring the device to active state before sending
> + * the wake notification. Since it is a synchronous call, caller must take care
> + * of not calling it in interrupt context.
> + *
> + * Returns zero on success, else negative errno.
> + */
> +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id)
> +{
> +	if (!gadget->ops->func_wakeup)
> +		return -EOPNOTSUPP;
> +
> +	return gadget->ops->func_wakeup(gadget, intf_id);
> +}
> +EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] usb: gadget: Add function wakeup support
  2023-02-16 23:58   ` Thinh Nguyen
@ 2023-02-17  0:38     ` Elson Serrao
  2023-02-17  2:02       ` Thinh Nguyen
  0 siblings, 1 reply; 11+ messages in thread
From: Elson Serrao @ 2023-02-17  0:38 UTC (permalink / raw)
  To: Thinh Nguyen
  Cc: gregkh, balbi, linux-kernel, linux-usb, quic_wcheng, quic_jackp



On 2/16/2023 3:58 PM, Thinh Nguyen wrote:
> On Thu, Feb 16, 2023, Elson Roy Serrao wrote:
>> A function which is in function suspend state has to send a
>> function wake notification to the host in case it needs to
>> exit from this state and resume data transfer. Add support to
>> handle such requests by exposing a new gadget op.
>>
>> Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
>> ---
>>   drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++
>>   drivers/usb/gadget/udc/core.c  | 21 +++++++++++++++++++++
>>   include/linux/usb/composite.h  |  6 ++++++
>>   include/linux/usb/gadget.h     |  4 ++++
>>   4 files changed, 55 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> index a37a8f4..f649f997 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config,
>>   }
>>   EXPORT_SYMBOL_GPL(usb_interface_id);
>>   
>> +int usb_func_wakeup(struct usb_function *func)
>> +{
>> +	int ret, id;
>> +
>> +	if (!func->func_wakeup_armed) {
>> +		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	for (id = 0; id < MAX_CONFIG_INTERFACES; id++)
>> +		if (func->config->interface[id] == func)
>> +			break;
>> +
>> +	if (id == MAX_CONFIG_INTERFACES) {
>> +		ERROR(func->config->cdev, "Invalid function\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id);
>> +
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(usb_func_wakeup);
>> +
>>   static u8 encode_bMaxPower(enum usb_device_speed speed,
>>   		struct usb_configuration *c)
>>   {
>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>> index 3dcbba7..59e7a7e 100644
>> --- a/drivers/usb/gadget/udc/core.c
>> +++ b/drivers/usb/gadget/udc/core.c
>> @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget)
>>   }
>>   EXPORT_SYMBOL_GPL(usb_gadget_activate);
>>   
>> +/**
>> + * usb_gadget_func_wakeup - sends function wake notification to the host.
>> + * @gadget: controller used to wake up the host
>> + * @interface_id: interface on which function wake notification is sent.
> 
> Device notification is only applicable for eSS devices. What will happen
> if the device is operating in lower speed and the driver calls this
> function?
> 
> Thanks,
> Thinh
> 

Since the non-eSS devices dont support function suspend, the function 
suspend feature selector is not sent by the host and the function is not 
armed for sending function remote wakeup. So the usb_func_wakeup() API 
that is called from the function drivers fails the attempt

int usb_func_wakeup(struct usb_function *func)
{
	int ret, id;

	if (!func->func_wakeup_armed) {
		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
		return -EINVAL;
	}

Let me know if its better to add an explicit speed check as well here.

Thanks
Elson

>> + *
>> + * On completion, function wake notification is sent. If the device is in
>> + * low power state it tries to bring the device to active state before sending
>> + * the wake notification. Since it is a synchronous call, caller must take care
>> + * of not calling it in interrupt context.
>> + *
>> + * Returns zero on success, else negative errno.
>> + */
>> +int usb_gadget_func_wakeup(struct usb_gadget *gadget, int intf_id)
>> +{
>> +	if (!gadget->ops->func_wakeup)
>> +		return -EOPNOTSUPP;
>> +
>> +	return gadget->ops->func_wakeup(gadget, intf_id);
>> +}
>> +EXPORT_SYMBOL_GPL(usb_gadget_func_wakeup);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 3/5] usb: gadget: Add function wakeup support
  2023-02-17  0:38     ` Elson Serrao
@ 2023-02-17  2:02       ` Thinh Nguyen
  0 siblings, 0 replies; 11+ messages in thread
From: Thinh Nguyen @ 2023-02-17  2:02 UTC (permalink / raw)
  To: Elson Serrao
  Cc: Thinh Nguyen, gregkh, balbi, linux-kernel, linux-usb,
	quic_wcheng, quic_jackp

On Thu, Feb 16, 2023, Elson Serrao wrote:
> 
> 
> On 2/16/2023 3:58 PM, Thinh Nguyen wrote:
> > On Thu, Feb 16, 2023, Elson Roy Serrao wrote:
> > > A function which is in function suspend state has to send a
> > > function wake notification to the host in case it needs to
> > > exit from this state and resume data transfer. Add support to
> > > handle such requests by exposing a new gadget op.
> > > 
> > > Signed-off-by: Elson Roy Serrao <quic_eserrao@quicinc.com>
> > > ---
> > >   drivers/usb/gadget/composite.c | 24 ++++++++++++++++++++++++
> > >   drivers/usb/gadget/udc/core.c  | 21 +++++++++++++++++++++
> > >   include/linux/usb/composite.h  |  6 ++++++
> > >   include/linux/usb/gadget.h     |  4 ++++
> > >   4 files changed, 55 insertions(+)
> > > 
> > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> > > index a37a8f4..f649f997 100644
> > > --- a/drivers/usb/gadget/composite.c
> > > +++ b/drivers/usb/gadget/composite.c
> > > @@ -492,6 +492,30 @@ int usb_interface_id(struct usb_configuration *config,
> > >   }
> > >   EXPORT_SYMBOL_GPL(usb_interface_id);
> > > +int usb_func_wakeup(struct usb_function *func)
> > > +{
> > > +	int ret, id;
> > > +
> > > +	if (!func->func_wakeup_armed) {
> > > +		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	for (id = 0; id < MAX_CONFIG_INTERFACES; id++)
> > > +		if (func->config->interface[id] == func)
> > > +			break;
> > > +
> > > +	if (id == MAX_CONFIG_INTERFACES) {
> > > +		ERROR(func->config->cdev, "Invalid function\n");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	ret = usb_gadget_func_wakeup(func->config->cdev->gadget, id);
> > > +
> > > +	return ret;
> > > +}
> > > +EXPORT_SYMBOL_GPL(usb_func_wakeup);
> > > +
> > >   static u8 encode_bMaxPower(enum usb_device_speed speed,
> > >   		struct usb_configuration *c)
> > >   {
> > > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> > > index 3dcbba7..59e7a7e 100644
> > > --- a/drivers/usb/gadget/udc/core.c
> > > +++ b/drivers/usb/gadget/udc/core.c
> > > @@ -846,6 +846,27 @@ int usb_gadget_activate(struct usb_gadget *gadget)
> > >   }
> > >   EXPORT_SYMBOL_GPL(usb_gadget_activate);
> > > +/**
> > > + * usb_gadget_func_wakeup - sends function wake notification to the host.
> > > + * @gadget: controller used to wake up the host
> > > + * @interface_id: interface on which function wake notification is sent.
> > 
> > Device notification is only applicable for eSS devices. What will happen
> > if the device is operating in lower speed and the driver calls this
> > function?
> > 
> > Thanks,
> > Thinh
> > 
> 
> Since the non-eSS devices dont support function suspend, the function
> suspend feature selector is not sent by the host and the function is not
> armed for sending function remote wakeup. So the usb_func_wakeup() API that
> is called from the function drivers fails the attempt

We may be able to say that for usb_func_wakeup() and document more on
the func_wakeup_armed flag. However, the driver can still call
usb_gadget_func_wakeup() directly right? Can you document the expected
behavior of usb_gadget_func_wakeup(). Would it fall back to behave like
usb_gadget_wakeup()? Or would it become no-op? Whichever you choose,
please document it.

Thanks,
Thinh

> 
> int usb_func_wakeup(struct usb_function *func)
> {
> 	int ret, id;
> 
> 	if (!func->func_wakeup_armed) {
> 		ERROR(func->config->cdev, "not armed for func remote wakeup\n");
> 		return -EINVAL;
> 	}
> 
> Let me know if its better to add an explicit speed check as well here.
> 
> Thanks
> Elson
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup
@ 2023-02-17 15:55 ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-02-17 14:47 UTC (permalink / raw)
  To: oe-kbuild; +Cc: lkp, Dan Carpenter

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <1676586588-25989-2-git-send-email-quic_eserrao@quicinc.com>
References: <1676586588-25989-2-git-send-email-quic_eserrao@quicinc.com>
TO: Elson Roy Serrao <quic_eserrao@quicinc.com>
TO: gregkh@linuxfoundation.org
TO: Thinh.Nguyen@synopsys.com
TO: balbi@kernel.org
CC: linux-kernel@vger.kernel.org
CC: linux-usb@vger.kernel.org
CC: quic_wcheng@quicinc.com
CC: quic_jackp@quicinc.com
CC: Elson Roy Serrao <quic_eserrao@quicinc.com>

Hi Elson,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on usb/usb-next usb/usb-linus linus/master v6.2-rc8 next-20230217]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Elson-Roy-Serrao/usb-gadget-Properly-configure-the-device-for-remote-wakeup/20230217-063114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1676586588-25989-2-git-send-email-quic_eserrao%40quicinc.com
patch subject: [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup
:::::: branch date: 16 hours ago
:::::: commit date: 16 hours ago
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230217/202302172243.hKUsQl2q-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302172243.hKUsQl2q-lkp@intel.com/

smatch warnings:
drivers/usb/gadget/composite.c:1016 set_config() error: we previously assumed 'c' could be null (see line 946)

vim +/c +1016 drivers/usb/gadget/composite.c

40982be52d8f64 David Brownell    2008-06-19   909  
40982be52d8f64 David Brownell    2008-06-19   910  static int set_config(struct usb_composite_dev *cdev,
40982be52d8f64 David Brownell    2008-06-19   911  		const struct usb_ctrlrequest *ctrl, unsigned number)
40982be52d8f64 David Brownell    2008-06-19   912  {
40982be52d8f64 David Brownell    2008-06-19   913  	struct usb_gadget	*gadget = cdev->gadget;
d6f4663664cbd5 Jakob Koschel     2022-03-08   914  	struct usb_configuration *c = NULL, *iter;
40982be52d8f64 David Brownell    2008-06-19   915  	int			result = -EINVAL;
40982be52d8f64 David Brownell    2008-06-19   916  	unsigned		power = gadget_is_otg(gadget) ? 8 : 100;
40982be52d8f64 David Brownell    2008-06-19   917  	int			tmp;
40982be52d8f64 David Brownell    2008-06-19   918  
40982be52d8f64 David Brownell    2008-06-19   919  	if (number) {
d6f4663664cbd5 Jakob Koschel     2022-03-08   920  		list_for_each_entry(iter, &cdev->configs, list) {
d6f4663664cbd5 Jakob Koschel     2022-03-08   921  			if (iter->bConfigurationValue != number)
d6f4663664cbd5 Jakob Koschel     2022-03-08   922  				continue;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   923  			/*
bdb64d727216b4 Tatyana Brokhman  2011-06-29   924  			 * We disable the FDs of the previous
bdb64d727216b4 Tatyana Brokhman  2011-06-29   925  			 * configuration only if the new configuration
bdb64d727216b4 Tatyana Brokhman  2011-06-29   926  			 * is a valid one
bdb64d727216b4 Tatyana Brokhman  2011-06-29   927  			 */
bdb64d727216b4 Tatyana Brokhman  2011-06-29   928  			if (cdev->config)
bdb64d727216b4 Tatyana Brokhman  2011-06-29   929  				reset_config(cdev);
d6f4663664cbd5 Jakob Koschel     2022-03-08   930  			c = iter;
40982be52d8f64 David Brownell    2008-06-19   931  			result = 0;
40982be52d8f64 David Brownell    2008-06-19   932  			break;
40982be52d8f64 David Brownell    2008-06-19   933  		}
40982be52d8f64 David Brownell    2008-06-19   934  		if (result < 0)
40982be52d8f64 David Brownell    2008-06-19   935  			goto done;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   936  	} else { /* Zero configuration value - need to reset the config */
bdb64d727216b4 Tatyana Brokhman  2011-06-29   937  		if (cdev->config)
bdb64d727216b4 Tatyana Brokhman  2011-06-29   938  			reset_config(cdev);
40982be52d8f64 David Brownell    2008-06-19   939  		result = 0;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   940  	}
40982be52d8f64 David Brownell    2008-06-19   941  
1cbfb8c4f62d66 Joel Stanley      2019-09-30   942  	DBG(cdev, "%s config #%d: %s\n",
e538dfdae85244 Michal Nazarewicz 2011-08-30   943  	    usb_speed_string(gadget->speed),
e538dfdae85244 Michal Nazarewicz 2011-08-30   944  	    number, c ? c->label : "unconfigured");
40982be52d8f64 David Brownell    2008-06-19   945  
40982be52d8f64 David Brownell    2008-06-19  @946  	if (!c)
40982be52d8f64 David Brownell    2008-06-19   947  		goto done;
40982be52d8f64 David Brownell    2008-06-19   948  
6027f3173e797b Peter Chen        2014-04-29   949  	usb_gadget_set_state(gadget, USB_STATE_CONFIGURED);
40982be52d8f64 David Brownell    2008-06-19   950  	cdev->config = c;
40982be52d8f64 David Brownell    2008-06-19   951  
40982be52d8f64 David Brownell    2008-06-19   952  	/* Initialize all interfaces by setting them to altsetting zero. */
40982be52d8f64 David Brownell    2008-06-19   953  	for (tmp = 0; tmp < MAX_CONFIG_INTERFACES; tmp++) {
40982be52d8f64 David Brownell    2008-06-19   954  		struct usb_function	*f = c->interface[tmp];
5242658d1b9777 Laurent Pinchart  2009-10-21   955  		struct usb_descriptor_header **descriptors;
40982be52d8f64 David Brownell    2008-06-19   956  
40982be52d8f64 David Brownell    2008-06-19   957  		if (!f)
40982be52d8f64 David Brownell    2008-06-19   958  			break;
40982be52d8f64 David Brownell    2008-06-19   959  
5242658d1b9777 Laurent Pinchart  2009-10-21   960  		/*
5242658d1b9777 Laurent Pinchart  2009-10-21   961  		 * Record which endpoints are used by the function. This is used
5242658d1b9777 Laurent Pinchart  2009-10-21   962  		 * to dispatch control requests targeted at that endpoint to the
5242658d1b9777 Laurent Pinchart  2009-10-21   963  		 * function's setup callback instead of the current
5242658d1b9777 Laurent Pinchart  2009-10-21   964  		 * configuration's setup callback.
5242658d1b9777 Laurent Pinchart  2009-10-21   965  		 */
f3bdbe36682631 John Youn         2016-02-05   966  		descriptors = function_descriptors(f, gadget->speed);
5242658d1b9777 Laurent Pinchart  2009-10-21   967  
5242658d1b9777 Laurent Pinchart  2009-10-21   968  		for (; *descriptors; ++descriptors) {
5242658d1b9777 Laurent Pinchart  2009-10-21   969  			struct usb_endpoint_descriptor *ep;
5242658d1b9777 Laurent Pinchart  2009-10-21   970  			int addr;
5242658d1b9777 Laurent Pinchart  2009-10-21   971  
5242658d1b9777 Laurent Pinchart  2009-10-21   972  			if ((*descriptors)->bDescriptorType != USB_DT_ENDPOINT)
5242658d1b9777 Laurent Pinchart  2009-10-21   973  				continue;
5242658d1b9777 Laurent Pinchart  2009-10-21   974  
5242658d1b9777 Laurent Pinchart  2009-10-21   975  			ep = (struct usb_endpoint_descriptor *)*descriptors;
5242658d1b9777 Laurent Pinchart  2009-10-21   976  			addr = ((ep->bEndpointAddress & 0x80) >> 3)
5242658d1b9777 Laurent Pinchart  2009-10-21   977  			     |  (ep->bEndpointAddress & 0x0f);
5242658d1b9777 Laurent Pinchart  2009-10-21   978  			set_bit(addr, f->endpoints);
5242658d1b9777 Laurent Pinchart  2009-10-21   979  		}
5242658d1b9777 Laurent Pinchart  2009-10-21   980  
40982be52d8f64 David Brownell    2008-06-19   981  		result = f->set_alt(f, tmp, 0);
40982be52d8f64 David Brownell    2008-06-19   982  		if (result < 0) {
40982be52d8f64 David Brownell    2008-06-19   983  			DBG(cdev, "interface %d (%s/%p) alt 0 --> %d\n",
40982be52d8f64 David Brownell    2008-06-19   984  					tmp, f->name, f, result);
40982be52d8f64 David Brownell    2008-06-19   985  
40982be52d8f64 David Brownell    2008-06-19   986  			reset_config(cdev);
40982be52d8f64 David Brownell    2008-06-19   987  			goto done;
40982be52d8f64 David Brownell    2008-06-19   988  		}
1b9ba000177ee4 Roger Quadros     2011-05-09   989  
1b9ba000177ee4 Roger Quadros     2011-05-09   990  		if (result == USB_GADGET_DELAYED_STATUS) {
1b9ba000177ee4 Roger Quadros     2011-05-09   991  			DBG(cdev,
1b9ba000177ee4 Roger Quadros     2011-05-09   992  			 "%s: interface %d (%s) requested delayed status\n",
1b9ba000177ee4 Roger Quadros     2011-05-09   993  					__func__, tmp, f->name);
1b9ba000177ee4 Roger Quadros     2011-05-09   994  			cdev->delayed_status++;
1b9ba000177ee4 Roger Quadros     2011-05-09   995  			DBG(cdev, "delayed_status count %d\n",
1b9ba000177ee4 Roger Quadros     2011-05-09   996  					cdev->delayed_status);
1b9ba000177ee4 Roger Quadros     2011-05-09   997  		}
40982be52d8f64 David Brownell    2008-06-19   998  	}
40982be52d8f64 David Brownell    2008-06-19   999  
40982be52d8f64 David Brownell    2008-06-19  1000  	/* when we return, be sure our power usage is valid */
bcacbf06c89137 Jack Pham         2021-07-20  1001  	if (c->MaxPower || (c->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
bcacbf06c89137 Jack Pham         2021-07-20  1002  		power = c->MaxPower;
bcacbf06c89137 Jack Pham         2021-07-20  1003  	else
bcacbf06c89137 Jack Pham         2021-07-20  1004  		power = CONFIG_USB_GADGET_VBUS_DRAW;
bcacbf06c89137 Jack Pham         2021-07-20  1005  
a2035411fa1d12 Jack Pham         2020-01-30  1006  	if (gadget->speed < USB_SPEED_SUPER)
a2035411fa1d12 Jack Pham         2020-01-30  1007  		power = min(power, 500U);
a2035411fa1d12 Jack Pham         2020-01-30  1008  	else
a2035411fa1d12 Jack Pham         2020-01-30  1009  		power = min(power, 900U);
40982be52d8f64 David Brownell    2008-06-19  1010  done:
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1011  	if (power <= USB_SELF_POWER_VBUS_MAX_DRAW)
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1012  		usb_gadget_set_selfpowered(gadget);
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1013  	else
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1014  		usb_gadget_clear_selfpowered(gadget);
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1015  
24ee47b2693b2d Elson Roy Serrao  2023-02-16 @1016  	if (USB_CONFIG_ATT_WAKEUP & c->bmAttributes)
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1017  		usb_gadget_set_remote_wakeup(gadget, 1);
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1018  	else
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1019  		usb_gadget_set_remote_wakeup(gadget, 0);
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1020  
40982be52d8f64 David Brownell    2008-06-19  1021  	usb_gadget_vbus_draw(gadget, power);
1b9ba000177ee4 Roger Quadros     2011-05-09  1022  	if (result >= 0 && cdev->delayed_status)
1b9ba000177ee4 Roger Quadros     2011-05-09  1023  		result = USB_GADGET_DELAYED_STATUS;
40982be52d8f64 David Brownell    2008-06-19  1024  	return result;
40982be52d8f64 David Brownell    2008-06-19  1025  }
40982be52d8f64 David Brownell    2008-06-19  1026  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup
@ 2023-02-17 15:55 ` Dan Carpenter
  0 siblings, 0 replies; 11+ messages in thread
From: Dan Carpenter @ 2023-02-17 15:55 UTC (permalink / raw)
  To: oe-kbuild, Elson Roy Serrao, gregkh, Thinh.Nguyen, balbi
  Cc: lkp, oe-kbuild-all, linux-kernel, linux-usb, quic_wcheng,
	quic_jackp, Elson Roy Serrao

Hi Elson,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Elson-Roy-Serrao/usb-gadget-Properly-configure-the-device-for-remote-wakeup/20230217-063114
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/1676586588-25989-2-git-send-email-quic_eserrao%40quicinc.com
patch subject: [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup
config: x86_64-randconfig-m001 (https://download.01.org/0day-ci/archive/20230217/202302172243.hKUsQl2q-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
| Link: https://lore.kernel.org/r/202302172243.hKUsQl2q-lkp@intel.com/

smatch warnings:
drivers/usb/gadget/composite.c:1016 set_config() error: we previously assumed 'c' could be null (see line 946)

vim +/c +1016 drivers/usb/gadget/composite.c

40982be52d8f64 David Brownell    2008-06-19   910  static int set_config(struct usb_composite_dev *cdev,
40982be52d8f64 David Brownell    2008-06-19   911  		const struct usb_ctrlrequest *ctrl, unsigned number)
40982be52d8f64 David Brownell    2008-06-19   912  {
40982be52d8f64 David Brownell    2008-06-19   913  	struct usb_gadget	*gadget = cdev->gadget;
d6f4663664cbd5 Jakob Koschel     2022-03-08   914  	struct usb_configuration *c = NULL, *iter;
40982be52d8f64 David Brownell    2008-06-19   915  	int			result = -EINVAL;
40982be52d8f64 David Brownell    2008-06-19   916  	unsigned		power = gadget_is_otg(gadget) ? 8 : 100;
40982be52d8f64 David Brownell    2008-06-19   917  	int			tmp;
40982be52d8f64 David Brownell    2008-06-19   918  
40982be52d8f64 David Brownell    2008-06-19   919  	if (number) {
d6f4663664cbd5 Jakob Koschel     2022-03-08   920  		list_for_each_entry(iter, &cdev->configs, list) {
d6f4663664cbd5 Jakob Koschel     2022-03-08   921  			if (iter->bConfigurationValue != number)
d6f4663664cbd5 Jakob Koschel     2022-03-08   922  				continue;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   923  			/*
bdb64d727216b4 Tatyana Brokhman  2011-06-29   924  			 * We disable the FDs of the previous
bdb64d727216b4 Tatyana Brokhman  2011-06-29   925  			 * configuration only if the new configuration
bdb64d727216b4 Tatyana Brokhman  2011-06-29   926  			 * is a valid one
bdb64d727216b4 Tatyana Brokhman  2011-06-29   927  			 */
bdb64d727216b4 Tatyana Brokhman  2011-06-29   928  			if (cdev->config)
bdb64d727216b4 Tatyana Brokhman  2011-06-29   929  				reset_config(cdev);
d6f4663664cbd5 Jakob Koschel     2022-03-08   930  			c = iter;
40982be52d8f64 David Brownell    2008-06-19   931  			result = 0;
40982be52d8f64 David Brownell    2008-06-19   932  			break;
40982be52d8f64 David Brownell    2008-06-19   933  		}
40982be52d8f64 David Brownell    2008-06-19   934  		if (result < 0)
40982be52d8f64 David Brownell    2008-06-19   935  			goto done;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   936  	} else { /* Zero configuration value - need to reset the config */
bdb64d727216b4 Tatyana Brokhman  2011-06-29   937  		if (cdev->config)
bdb64d727216b4 Tatyana Brokhman  2011-06-29   938  			reset_config(cdev);
40982be52d8f64 David Brownell    2008-06-19   939  		result = 0;
bdb64d727216b4 Tatyana Brokhman  2011-06-29   940  	}
40982be52d8f64 David Brownell    2008-06-19   941  
1cbfb8c4f62d66 Joel Stanley      2019-09-30   942  	DBG(cdev, "%s config #%d: %s\n",
e538dfdae85244 Michal Nazarewicz 2011-08-30   943  	    usb_speed_string(gadget->speed),
e538dfdae85244 Michal Nazarewicz 2011-08-30   944  	    number, c ? c->label : "unconfigured");
                                                                    ^

40982be52d8f64 David Brownell    2008-06-19   945  
40982be52d8f64 David Brownell    2008-06-19  @946  	if (!c)
                                                             ^
Check for NULL

40982be52d8f64 David Brownell    2008-06-19   947  		goto done;
40982be52d8f64 David Brownell    2008-06-19   948  
6027f3173e797b Peter Chen        2014-04-29   949  	usb_gadget_set_state(gadget, USB_STATE_CONFIGURED);
40982be52d8f64 David Brownell    2008-06-19   950  	cdev->config = c;
40982be52d8f64 David Brownell    2008-06-19   951  
40982be52d8f64 David Brownell    2008-06-19   952  	/* Initialize all interfaces by setting them to altsetting zero. */
40982be52d8f64 David Brownell    2008-06-19   953  	for (tmp = 0; tmp < MAX_CONFIG_INTERFACES; tmp++) {
40982be52d8f64 David Brownell    2008-06-19   954  		struct usb_function	*f = c->interface[tmp];
5242658d1b9777 Laurent Pinchart  2009-10-21   955  		struct usb_descriptor_header **descriptors;
40982be52d8f64 David Brownell    2008-06-19   956  
40982be52d8f64 David Brownell    2008-06-19   957  		if (!f)
40982be52d8f64 David Brownell    2008-06-19   958  			break;
40982be52d8f64 David Brownell    2008-06-19   959  
5242658d1b9777 Laurent Pinchart  2009-10-21   960  		/*
5242658d1b9777 Laurent Pinchart  2009-10-21   961  		 * Record which endpoints are used by the function. This is used
5242658d1b9777 Laurent Pinchart  2009-10-21   962  		 * to dispatch control requests targeted at that endpoint to the
5242658d1b9777 Laurent Pinchart  2009-10-21   963  		 * function's setup callback instead of the current
5242658d1b9777 Laurent Pinchart  2009-10-21   964  		 * configuration's setup callback.
5242658d1b9777 Laurent Pinchart  2009-10-21   965  		 */
f3bdbe36682631 John Youn         2016-02-05   966  		descriptors = function_descriptors(f, gadget->speed);
5242658d1b9777 Laurent Pinchart  2009-10-21   967  
5242658d1b9777 Laurent Pinchart  2009-10-21   968  		for (; *descriptors; ++descriptors) {
5242658d1b9777 Laurent Pinchart  2009-10-21   969  			struct usb_endpoint_descriptor *ep;
5242658d1b9777 Laurent Pinchart  2009-10-21   970  			int addr;
5242658d1b9777 Laurent Pinchart  2009-10-21   971  
5242658d1b9777 Laurent Pinchart  2009-10-21   972  			if ((*descriptors)->bDescriptorType != USB_DT_ENDPOINT)
5242658d1b9777 Laurent Pinchart  2009-10-21   973  				continue;
5242658d1b9777 Laurent Pinchart  2009-10-21   974  
5242658d1b9777 Laurent Pinchart  2009-10-21   975  			ep = (struct usb_endpoint_descriptor *)*descriptors;
5242658d1b9777 Laurent Pinchart  2009-10-21   976  			addr = ((ep->bEndpointAddress & 0x80) >> 3)
5242658d1b9777 Laurent Pinchart  2009-10-21   977  			     |  (ep->bEndpointAddress & 0x0f);
5242658d1b9777 Laurent Pinchart  2009-10-21   978  			set_bit(addr, f->endpoints);
5242658d1b9777 Laurent Pinchart  2009-10-21   979  		}
5242658d1b9777 Laurent Pinchart  2009-10-21   980  
40982be52d8f64 David Brownell    2008-06-19   981  		result = f->set_alt(f, tmp, 0);
40982be52d8f64 David Brownell    2008-06-19   982  		if (result < 0) {
40982be52d8f64 David Brownell    2008-06-19   983  			DBG(cdev, "interface %d (%s/%p) alt 0 --> %d\n",
40982be52d8f64 David Brownell    2008-06-19   984  					tmp, f->name, f, result);
40982be52d8f64 David Brownell    2008-06-19   985  
40982be52d8f64 David Brownell    2008-06-19   986  			reset_config(cdev);
40982be52d8f64 David Brownell    2008-06-19   987  			goto done;
40982be52d8f64 David Brownell    2008-06-19   988  		}
1b9ba000177ee4 Roger Quadros     2011-05-09   989  
1b9ba000177ee4 Roger Quadros     2011-05-09   990  		if (result == USB_GADGET_DELAYED_STATUS) {
1b9ba000177ee4 Roger Quadros     2011-05-09   991  			DBG(cdev,
1b9ba000177ee4 Roger Quadros     2011-05-09   992  			 "%s: interface %d (%s) requested delayed status\n",
1b9ba000177ee4 Roger Quadros     2011-05-09   993  					__func__, tmp, f->name);
1b9ba000177ee4 Roger Quadros     2011-05-09   994  			cdev->delayed_status++;
1b9ba000177ee4 Roger Quadros     2011-05-09   995  			DBG(cdev, "delayed_status count %d\n",
1b9ba000177ee4 Roger Quadros     2011-05-09   996  					cdev->delayed_status);
1b9ba000177ee4 Roger Quadros     2011-05-09   997  		}
40982be52d8f64 David Brownell    2008-06-19   998  	}
40982be52d8f64 David Brownell    2008-06-19   999  
40982be52d8f64 David Brownell    2008-06-19  1000  	/* when we return, be sure our power usage is valid */
bcacbf06c89137 Jack Pham         2021-07-20  1001  	if (c->MaxPower || (c->bmAttributes & USB_CONFIG_ATT_SELFPOWER))
bcacbf06c89137 Jack Pham         2021-07-20  1002  		power = c->MaxPower;
bcacbf06c89137 Jack Pham         2021-07-20  1003  	else
bcacbf06c89137 Jack Pham         2021-07-20  1004  		power = CONFIG_USB_GADGET_VBUS_DRAW;
bcacbf06c89137 Jack Pham         2021-07-20  1005  
a2035411fa1d12 Jack Pham         2020-01-30  1006  	if (gadget->speed < USB_SPEED_SUPER)
a2035411fa1d12 Jack Pham         2020-01-30  1007  		power = min(power, 500U);
a2035411fa1d12 Jack Pham         2020-01-30  1008  	else
a2035411fa1d12 Jack Pham         2020-01-30  1009  		power = min(power, 900U);
40982be52d8f64 David Brownell    2008-06-19  1010  done:
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1011  	if (power <= USB_SELF_POWER_VBUS_MAX_DRAW)
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1012  		usb_gadget_set_selfpowered(gadget);
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1013  	else
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1014  		usb_gadget_clear_selfpowered(gadget);
5e5caf4fa8d303 Thinh Nguyen      2020-02-03  1015  
24ee47b2693b2d Elson Roy Serrao  2023-02-16 @1016  	if (USB_CONFIG_ATT_WAKEUP & c->bmAttributes)
                                                                                    ^^^^^^^^^^^^^^^
Unchecked dereference

24ee47b2693b2d Elson Roy Serrao  2023-02-16  1017  		usb_gadget_set_remote_wakeup(gadget, 1);
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1018  	else
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1019  		usb_gadget_set_remote_wakeup(gadget, 0);
24ee47b2693b2d Elson Roy Serrao  2023-02-16  1020  
40982be52d8f64 David Brownell    2008-06-19  1021  	usb_gadget_vbus_draw(gadget, power);
1b9ba000177ee4 Roger Quadros     2011-05-09  1022  	if (result >= 0 && cdev->delayed_status)
1b9ba000177ee4 Roger Quadros     2011-05-09  1023  		result = USB_GADGET_DELAYED_STATUS;
40982be52d8f64 David Brownell    2008-06-19  1024  	return result;
40982be52d8f64 David Brownell    2008-06-19  1025  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-02-17 15:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-16 22:29 [PATCH v5 0/5] Add function suspend/resume and remote wakeup support Elson Roy Serrao
2023-02-16 22:29 ` [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup Elson Roy Serrao
2023-02-16 22:29 ` [PATCH v5 2/5] usb: dwc3: Add remote wakeup handling Elson Roy Serrao
2023-02-16 22:29 ` [PATCH v5 3/5] usb: gadget: Add function wakeup support Elson Roy Serrao
2023-02-16 23:58   ` Thinh Nguyen
2023-02-17  0:38     ` Elson Serrao
2023-02-17  2:02       ` Thinh Nguyen
2023-02-16 22:29 ` [PATCH v5 4/5] usb: dwc3: Add function suspend and " Elson Roy Serrao
2023-02-16 22:29 ` [PATCH v5 5/5] usb: gadget: f_ecm: Add suspend/resume and remote " Elson Roy Serrao
2023-02-17 14:47 [PATCH v5 1/5] usb: gadget: Properly configure the device for remote wakeup kernel test robot
2023-02-17 15:55 ` Dan Carpenter

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.