Re: [RFC/PATCH 3/3] usb:gadget: SuperSpeed function power management testing support

2011-07-27 Thread Felipe Balbi
On Sun, Jul 03, 2011 at 05:29:33PM +0300, Amit Blay wrote:
 1. Fix dummy_hcd to let the function handle GET_STATUS(Interface) request.
 2. Fix a bug in f_loopback Gadget which updated the bmAttribute of
 f_sourcesink instead of f_loopback.
 3. Update f_sourcesink Gadget to support function suspend  wakeup.
 This is done by adding get_status()  func_suspend() handlers.
 The current status of the function is controlable via debug FS:
 (wakeup capable, wakeup enabled, suspended).
 
 Signed-off-by: Amit Blay ab...@codeaurora.org
 
 ---
  drivers/usb/gadget/dummy_hcd.c|3 +-
  drivers/usb/gadget/f_loopback.c   |2 +-
  drivers/usb/gadget/f_sourcesink.c |  162 
 +
  3 files changed, 165 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
 index e755a9d..7f6a2d8 100644
 --- a/drivers/usb/gadget/dummy_hcd.c
 +++ b/drivers/usb/gadget/dummy_hcd.c
 @@ -1496,7 +1496,8 @@ static int handle_control_request(struct dummy_hcd 
 *dum_hcd, struct urb *urb,
  Dev_InRequest) {
   buf[0] = (u8)dum-devstatus;
   } else
 - buf[0] = 0;
 + /* Let the function handle this */
 + break;
   }
   if (urb-transfer_buffer_length  1)
   buf[1] = 0;
 diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
 index ca660d4..75bac6d 100644
 --- a/drivers/usb/gadget/f_loopback.c
 +++ b/drivers/usb/gadget/f_loopback.c
 @@ -427,7 +427,7 @@ int __init loopback_add(struct usb_composite_dev *cdev, 
 bool autoresume)
  
   /* support autoresume for remote wakeup testing */
   if (autoresume)
 - sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 + loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;

this change doesn't belong here.

 diff --git a/drivers/usb/gadget/f_sourcesink.c 
 b/drivers/usb/gadget/f_sourcesink.c
 index 5247f07..5c5da19 100644
 --- a/drivers/usb/gadget/f_sourcesink.c
 +++ b/drivers/usb/gadget/f_sourcesink.c
 @@ -59,6 +59,12 @@ struct f_sourcesink {
  
   struct usb_ep   *in_ep;
   struct usb_ep   *out_ep;
 +
 + /* Function Power Management */
 + boolfunc_suspended;
 + boolfunc_wakeup_capable;
 + boolfunc_wakeup_enabled;
 + struct  device dev;

this should *NOT* have a device of its own. And the way you tabified is
wrong.

  };
  
  static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
 @@ -191,6 +197,79 @@ static struct usb_gadget_strings *sourcesink_strings[] = 
 {
   NULL,
  };
  
 +/*** DEVICE ATTRIBUTES ***/
 +
 +static ssize_t f_sourcesink_show_func_suspend(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_suspended);
 +}
 +
 +static ssize_t f_sourcesink_show_func_wakeup_enabled(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_wakeup_enabled);
 +}
 +
 +static ssize_t f_sourcesink_show_func_wakeup_capable(struct device *dev,
 + struct device_attribute *attr,
 + char *buf)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
 + dev);
 + return sprintf(buf, %d\n, ss-func_wakeup_capable);
 +}
 +
 +static ssize_t f_sourcesink_store_func_wakeup_capable(struct device *dev,
 + struct device_attribute *attr, const char *buf, size_t count)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
 + unsigned long func_wakeup_capable;
 +
 + /* Allows changing function wakeup capable field from the file system */
 + if (strict_strtoul(buf, 2, func_wakeup_capable))
 + return -EINVAL;
 + ss-func_wakeup_capable = (bool)func_wakeup_capable;
 + return count;
 +}
 +
 +static ssize_t f_sourcesink_store_func_wakeup_trigger(struct device *dev,
 + struct device_attribute *attr, const char *buf, size_t count)
 +{
 + struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
 +
 + /* Allows trigerring function wakeup from the file system */
 + if (!ss-func_wakeup_capable || !ss-func_wakeup_enabled)
 + return -EINVAL;
 +
 + if (usb_gadget_wakeup(ss-function.config-cdev-gadget,
 +source_sink_intf.bInterfaceNumber)  0)
 + return -EINVAL;
 + return count;
 +}

why didn't you 

[RFC/PATCH 3/3] usb:gadget: SuperSpeed function power management testing support

2011-07-03 Thread Amit Blay
1. Fix dummy_hcd to let the function handle GET_STATUS(Interface) request.
2. Fix a bug in f_loopback Gadget which updated the bmAttribute of
f_sourcesink instead of f_loopback.
3. Update f_sourcesink Gadget to support function suspend  wakeup.
This is done by adding get_status()  func_suspend() handlers.
The current status of the function is controlable via debug FS:
(wakeup capable, wakeup enabled, suspended).

Signed-off-by: Amit Blay ab...@codeaurora.org

---
 drivers/usb/gadget/dummy_hcd.c|3 +-
 drivers/usb/gadget/f_loopback.c   |2 +-
 drivers/usb/gadget/f_sourcesink.c |  162 +
 3 files changed, 165 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c
index e755a9d..7f6a2d8 100644
--- a/drivers/usb/gadget/dummy_hcd.c
+++ b/drivers/usb/gadget/dummy_hcd.c
@@ -1496,7 +1496,8 @@ static int handle_control_request(struct dummy_hcd 
*dum_hcd, struct urb *urb,
   Dev_InRequest) {
buf[0] = (u8)dum-devstatus;
} else
-   buf[0] = 0;
+   /* Let the function handle this */
+   break;
}
if (urb-transfer_buffer_length  1)
buf[1] = 0;
diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index ca660d4..75bac6d 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -427,7 +427,7 @@ int __init loopback_add(struct usb_composite_dev *cdev, 
bool autoresume)
 
/* support autoresume for remote wakeup testing */
if (autoresume)
-   sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
+   loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
 
/* support OTG systems */
if (gadget_is_otg(cdev-gadget)) {
diff --git a/drivers/usb/gadget/f_sourcesink.c 
b/drivers/usb/gadget/f_sourcesink.c
index 5247f07..5c5da19 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -59,6 +59,12 @@ struct f_sourcesink {
 
struct usb_ep   *in_ep;
struct usb_ep   *out_ep;
+
+   /* Function Power Management */
+   boolfunc_suspended;
+   boolfunc_wakeup_capable;
+   boolfunc_wakeup_enabled;
+   struct  device dev;
 };
 
 static inline struct f_sourcesink *func_to_ss(struct usb_function *f)
@@ -191,6 +197,79 @@ static struct usb_gadget_strings *sourcesink_strings[] = {
NULL,
 };
 
+/*** DEVICE ATTRIBUTES ***/
+
+static ssize_t f_sourcesink_show_func_suspend(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
+   dev);
+   return sprintf(buf, %d\n, ss-func_suspended);
+}
+
+static ssize_t f_sourcesink_show_func_wakeup_enabled(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
+   dev);
+   return sprintf(buf, %d\n, ss-func_wakeup_enabled);
+}
+
+static ssize_t f_sourcesink_show_func_wakeup_capable(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   struct f_sourcesink *ss = container_of(dev, struct f_sourcesink,
+   dev);
+   return sprintf(buf, %d\n, ss-func_wakeup_capable);
+}
+
+static ssize_t f_sourcesink_store_func_wakeup_capable(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
+   unsigned long func_wakeup_capable;
+
+   /* Allows changing function wakeup capable field from the file system */
+   if (strict_strtoul(buf, 2, func_wakeup_capable))
+   return -EINVAL;
+   ss-func_wakeup_capable = (bool)func_wakeup_capable;
+   return count;
+}
+
+static ssize_t f_sourcesink_store_func_wakeup_trigger(struct device *dev,
+   struct device_attribute *attr, const char *buf, size_t count)
+{
+   struct f_sourcesink *ss = container_of(dev, struct f_sourcesink, dev);
+
+   /* Allows trigerring function wakeup from the file system */
+   if (!ss-func_wakeup_capable || !ss-func_wakeup_enabled)
+   return -EINVAL;
+
+   if (usb_gadget_wakeup(ss-function.config-cdev-gadget,
+  source_sink_intf.bInterfaceNumber)  0)
+   return -EINVAL;
+   return count;
+}
+
+static DEVICE_ATTR(func_suspend, 0444, f_sourcesink_show_func_suspend, NULL);
+static DEVICE_ATTR(func_wakeup_enabled, 0444,
+