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 <[email protected]>
---
drivers/usb/gadget/function/f_ecm.c | 77 +++++++++++++++++++++++++++++++++++
drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++
drivers/usb/gadget/function/u_ether.h | 4 ++
3 files changed, 144 insertions(+)
diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index a7ab30e..fea07ab 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,77 @@ 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)
+{
+ struct usb_configuration *c = f->config;
+
+ /* D0 and D1 bit set to 0 if device is not wakeup capable */
+ if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
+ return 0;
+
+ 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 usb_configuration *c = f->config;
+
+ DBG(c->cdev, "func susp %u cmd\n", options);
+
+ /* Respond with negative errno if request is not supported */
+ if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
+ return -EINVAL;
+
+ 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 +1025,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
On Wed, Mar 15, 2023, Elson Roy Serrao wrote:
> 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 <[email protected]>
> ---
> drivers/usb/gadget/function/f_ecm.c | 77 +++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++
> drivers/usb/gadget/function/u_ether.h | 4 ++
> 3 files changed, 144 insertions(+)
>
> diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
> index a7ab30e..fea07ab 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,77 @@ 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)
> +{
> + struct usb_configuration *c = f->config;
> +
> + /* D0 and D1 bit set to 0 if device is not wakeup capable */
> + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> + return 0;
> +
> + 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 usb_configuration *c = f->config;
> +
> + DBG(c->cdev, "func susp %u cmd\n", options);
> +
> + /* Respond with negative errno if request is not supported */
> + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> + return -EINVAL;
We only need to return early if the host tries to enable remote wake
while the device isn't capable of it:
wakeup_sel = !!(options & (USB_INTRF_FUNC_SUSPEND_RW >> 8));
if (wakeup_sel && !(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
return -EINVAL;
f->func_wakeup_armed = wakeup_sel;
BR,
Thinh
> +
> + 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;
> +}
> +
On Wed, Mar 15, 2023, Thinh Nguyen wrote:
> On Wed, Mar 15, 2023, Elson Roy Serrao wrote:
> > 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 <[email protected]>
> > ---
> > drivers/usb/gadget/function/f_ecm.c | 77 +++++++++++++++++++++++++++++++++++
> > drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++
> > drivers/usb/gadget/function/u_ether.h | 4 ++
> > 3 files changed, 144 insertions(+)
> >
> > diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
> > index a7ab30e..fea07ab 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,77 @@ 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)
> > +{
> > + struct usb_configuration *c = f->config;
> > +
> > + /* D0 and D1 bit set to 0 if device is not wakeup capable */
> > + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> > + return 0;
> > +
> > + 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 usb_configuration *c = f->config;
> > +
> > + DBG(c->cdev, "func susp %u cmd\n", options);
> > +
> > + /* Respond with negative errno if request is not supported */
> > + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> > + return -EINVAL;
>
> We only need to return early if the host tries to enable remote wake
> while the device isn't capable of it:
>
> wakeup_sel = !!(options & (USB_INTRF_FUNC_SUSPEND_RW >> 8));
> if (wakeup_sel && !(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> return -EINVAL;
>
> f->func_wakeup_armed = wakeup_sel;
>
Also, I notice that we can't differentiate between ClearFeature() and
SetFeature() in f->func_suspend(). Perhaps we should handle arming the
remote wakeup in the composite layer so we can set/clear the remote
wake. It's common across multiple devices. It's probably better to be
there.
Thanks,
Thinh
On 3/15/2023 2:15 PM, Thinh Nguyen wrote:
> On Wed, Mar 15, 2023, Thinh Nguyen wrote:
>> On Wed, Mar 15, 2023, Elson Roy Serrao wrote:
>>> 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 <[email protected]>
>>> ---
>>> drivers/usb/gadget/function/f_ecm.c | 77 +++++++++++++++++++++++++++++++++++
>>> drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++
>>> drivers/usb/gadget/function/u_ether.h | 4 ++
>>> 3 files changed, 144 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
>>> index a7ab30e..fea07ab 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,77 @@ 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)
>>> +{
>>> + struct usb_configuration *c = f->config;
>>> +
>>> + /* D0 and D1 bit set to 0 if device is not wakeup capable */
>>> + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
>>> + return 0;
>>> +
>>> + 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 usb_configuration *c = f->config;
>>> +
>>> + DBG(c->cdev, "func susp %u cmd\n", options);
>>> +
>>> + /* Respond with negative errno if request is not supported */
>>> + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
>>> + return -EINVAL;
>>
>> We only need to return early if the host tries to enable remote wake
>> while the device isn't capable of it:
>>
>> wakeup_sel = !!(options & (USB_INTRF_FUNC_SUSPEND_RW >> 8));
>> if (wakeup_sel && !(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
>> return -EINVAL;
>>
>> f->func_wakeup_armed = wakeup_sel;
>>
>
> Also, I notice that we can't differentiate between ClearFeature() and
> SetFeature() in f->func_suspend(). Perhaps we should handle arming the
> remote wakeup in the composite layer so we can set/clear the remote
> wake. It's common across multiple devices. It's probably better to be
> there.
>
> Thanks,
> Thinh
Yeah agree with that. We check for function_wakeup_armed flag before
sending remote wakeup in composite.c. So it makes more sense
to set/reset this flag in composite.c itself. I will make that change
and upload v12 along with fixing 'value = 0' removal that we discussed
earlier. Please let me know if that is fine
Thanks
Elson
On Wed, Mar 15, 2023, Elson Serrao wrote:
>
> On 3/15/2023 2:15 PM, Thinh Nguyen wrote:
> > On Wed, Mar 15, 2023, Thinh Nguyen wrote:
> > > On Wed, Mar 15, 2023, Elson Roy Serrao wrote:
> > > > 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 <[email protected]>
> > > > ---
> > > > drivers/usb/gadget/function/f_ecm.c | 77 +++++++++++++++++++++++++++++++++++
> > > > drivers/usb/gadget/function/u_ether.c | 63 ++++++++++++++++++++++++++++
> > > > drivers/usb/gadget/function/u_ether.h | 4 ++
> > > > 3 files changed, 144 insertions(+)
> > > >
> > > > diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
> > > > index a7ab30e..fea07ab 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,77 @@ 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)
> > > > +{
> > > > + struct usb_configuration *c = f->config;
> > > > +
> > > > + /* D0 and D1 bit set to 0 if device is not wakeup capable */
> > > > + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> > > > + return 0;
> > > > +
> > > > + 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 usb_configuration *c = f->config;
> > > > +
> > > > + DBG(c->cdev, "func susp %u cmd\n", options);
> > > > +
> > > > + /* Respond with negative errno if request is not supported */
> > > > + if (!(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> > > > + return -EINVAL;
> > >
> > > We only need to return early if the host tries to enable remote wake
> > > while the device isn't capable of it:
> > >
> > > wakeup_sel = !!(options & (USB_INTRF_FUNC_SUSPEND_RW >> 8));
> > > if (wakeup_sel && !(USB_CONFIG_ATT_WAKEUP & c->bmAttributes))
> > > return -EINVAL;
> > >
> > > f->func_wakeup_armed = wakeup_sel;
> > >
> >
> > Also, I notice that we can't differentiate between ClearFeature() and
> > SetFeature() in f->func_suspend(). Perhaps we should handle arming the
> > remote wakeup in the composite layer so we can set/clear the remote
> > wake. It's common across multiple devices. It's probably better to be
> > there.
> >
> > Thanks,
> > Thinh
>
> Yeah agree with that. We check for function_wakeup_armed flag before sending
> remote wakeup in composite.c. So it makes more sense
> to set/reset this flag in composite.c itself. I will make that change
> and upload v12 along with fixing 'value = 0' removal that we discussed
> earlier. Please let me know if that is fine
>
Since we don't go through the func_suspend() anymore, let's keep it as
is for now. If you're in composite.c, then you can skip doing
func_suspend() if remote_wake is not supported when host requested.
Thanks,
Thinh