* [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-03-23 8:04 ` Tatyana Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tatyana Brokhman @ 2011-03-23 8:04 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-msm, balbi, ablay, Tatyana Brokhman,
open list:USB GADGET/PERIPH...,
open list
This patch adds the SuperSpeed functionality to the gadget framework.
In order not to force all the gadget drivers to supply SuperSpeed
descriptors when operating in SuperSpeed mode the following approach was
taken:
If we're operating in SuperSpeed mode and the gadget driver didn't supply
SuperSpeed descriptors, the composite layer will automatically create
SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added.
Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
added.
Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bc5123c..b4130bc 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -605,6 +605,18 @@ config USB_GADGET_DUALSPEED
Means that gadget drivers should include extra descriptors
and code to handle dual-speed controllers.
+config USB_GADGET_SUPERSPEED
+ boolean "Gadget operating in Super Speed"
+ depends on USB_GADGET
+ depends on USB_GADGET_DUALSPEED
+ default n
+ help
+ Enabling this feature enables Super Speed support in the Gadget
+ driver. It means that gadget drivers should provide extra (SuperSpeed)
+ descriptors to the host.
+ For composite devices: if SuperSpeed descriptors weren't supplied by
+ the FD, they will be automatically generated with default values.
+
#
# USB Gadget Drivers
#
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index fb7e488..d5fe1c2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
static char composite_manufacturer[50];
+/* Default endpoint companion descriptor */
+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+ .bLength = 0x06,
+ .bMaxBurst = 0, /* the default is we don't support bursting */
+ .bmAttributes = 0, /* 2^0 streams supported */
+ .wBytesPerInterval = 0,
+};
+
+/**
+ * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * with default values
+ * @f: pointer to usb_function to generate the descriptors for
+ *
+ * This function receives a pointer to usb_function and adds
+ * missing super speed descriptors in the ss_descriptor field
+ * according to its hs_descriptors field.
+ *
+ * This function copies f->hs_descriptors while updating the
+ * endpoint descriptor and adding endpoint companion descriptor.
+ */
+static void create_ss_descriptors(struct usb_function *f)
+{
+ unsigned bytes; /* number of bytes to allocate */
+ unsigned n_desc; /* number of descriptors */
+ void *mem; /* allocated memory to copy to */
+ struct usb_descriptor_header **tmp;
+ struct usb_endpoint_descriptor *ep_desc ;
+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+ struct usb_descriptor_header **src = f->hs_descriptors;
+
+ if (!f->hs_descriptors)
+ return;
+
+ /*
+ * Count number of EPs (in order to know how many SS_EP_COMPANION
+ * descriptors to add), the total number of descriptors and the sum of
+ * each descriptor bLength field in order to know how much memory to
+ * allocate.
+ */
+ for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
+ if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
+ bytes += default_ep_comp_desc.bLength;
+ n_desc++;
+ }
+ bytes += (*tmp)->bLength;
+ }
+
+ bytes += (n_desc + 1) * sizeof(*tmp);
+ mem = kmalloc(bytes, GFP_KERNEL);
+ if (!mem)
+ return;
+
+ /*
+ * Fill in pointers starting at "tmp", to descriptors copied starting
+ * at "mem" and return "ret"
+ */
+ tmp = mem;
+ f->ss_descriptors = mem;
+ mem += (n_desc + 1) * sizeof(*tmp);
+ while (*src) {
+ /* Copy the original descriptor */
+ memcpy(mem, *src, (*src)->bLength);
+ switch ((*src)->bDescriptorType) {
+ case USB_DT_ENDPOINT:
+ /* update ep descriptor */
+ ep_desc = (struct usb_endpoint_descriptor *)mem;
+ switch (ep_desc->bmAttributes &
+ USB_ENDPOINT_XFERTYPE_MASK) {
+ case USB_ENDPOINT_XFER_CONTROL:
+ ep_desc->wMaxPacketSize = cpu_to_le16(512);
+ ep_desc->bInterval = 0;
+ break;
+ case USB_ENDPOINT_XFER_BULK:
+ ep_desc->wMaxPacketSize = cpu_to_le16(1024);
+ ep_desc->bInterval = 0;
+ break;
+ case USB_ENDPOINT_XFER_INT:
+ case USB_ENDPOINT_XFER_ISOC:
+ break;
+ }
+ *tmp = mem;
+ tmp++;
+ mem += (*src)->bLength;
+ /* add ep companion descriptor */
+ memcpy(mem, &default_ep_comp_desc,
+ default_ep_comp_desc.bLength);
+ *tmp = mem;
+ tmp++;
+ /* Update wBytesPerInterval for periodic endpoints */
+ ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
+ switch (ep_desc->bmAttributes &
+ USB_ENDPOINT_XFERTYPE_MASK) {
+ case USB_ENDPOINT_XFER_INT:
+ case USB_ENDPOINT_XFER_ISOC:
+ ep_comp_desc->wBytesPerInterval =
+ ep_desc->wMaxPacketSize;
+ break;
+ }
+ mem += default_ep_comp_desc.bLength;
+ break;
+ default:
+ *tmp = mem;
+ tmp++;
+ mem += (*src)->bLength;
+ break;
+ }
+ src++;
+ }
+ /*
+ * The last (struct usb_descriptor_header *) in the descriptors
+ * vector is NULL
+ */
+ *tmp = NULL;
+ f->ss_desc_allocated = true;
+}
+
/*-------------------------------------------------------------------------*/
/**
* next_ep_desc() - advance to the next EP descriptor
@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
struct usb_endpoint_descriptor *chosen_desc = NULL;
struct usb_descriptor_header **speed_desc = NULL;
+ struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
+ int want_comp_desc = 0;
+
struct usb_descriptor_header **d_spd; /* cursor for speed desc */
if (!g || !f || !_ep)
@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
/* select desired speed */
switch (g->speed) {
+ case USB_SPEED_SUPER:
+ if (gadget_is_superspeed(g)) {
+ speed_desc = f->ss_descriptors;
+ want_comp_desc = 1;
+ break;
+ }
+ /* else: Fall trough */
case USB_SPEED_HIGH:
if (gadget_is_dualspeed(g)) {
speed_desc = f->hs_descriptors;
@@ -147,7 +274,31 @@ ep_found:
/* commit results */
_ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
_ep->desc = chosen_desc;
-
+ _ep->comp_desc = NULL;
+ _ep->maxburst = 0;
+ _ep->mult = 0;
+ if (want_comp_desc) {
+ /*
+ * Companion descriptor should follow EP descriptor
+ * USB 3.0 spec, #9.6.7
+ */
+ comp_desc = (struct usb_ss_ep_comp_descriptor *)*(++d_spd);
+ if (!comp_desc ||
+ (comp_desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP))
+ return -EIO;
+ _ep->comp_desc = comp_desc;
+ if (g->speed == USB_SPEED_SUPER) {
+ int xfer_type = _ep->bEndpointAddress &
+ USB_ENDPOINT_XFERTYPE_MASK ;
+ if (xfer_type == USB_ENDPOINT_XFER_BULK)
+ _ep->maxburst = comp_desc->bMaxBurst;
+ else if (xfer_type == USB_ENDPOINT_XFER_INT)
+ _ep->maxburst = comp_desc->bMaxBurst;
+ else if (xfer_type == USB_ENDPOINT_XFER_ISOC)
+ /* mult: bits 1:0 of bmAttributes */
+ _ep->mult = comp_desc->bmAttributes & 0x3;
+ }
+ }
return 0;
}
@@ -187,6 +338,14 @@ int usb_add_function(struct usb_configuration *config,
list_del(&function->list);
function->config = NULL;
}
+ /*
+ * Add SS descriptors if there are any. This has to be done
+ * after the bind since we need the hs_descriptors to be set in
+ * usb_function and some of the FDs does it in the bind.
+ */
+ if ((gadget_is_superspeed(config->cdev->gadget)) &&
+ (!function->ss_not_capable) && (!function->ss_descriptors))
+ create_ss_descriptors(function);
} else
value = 0;
@@ -199,6 +358,8 @@ int usb_add_function(struct usb_configuration *config,
config->fullspeed = true;
if (!config->highspeed && function->hs_descriptors)
config->highspeed = true;
+ if (!config->superspeed && function->ss_descriptors)
+ config->superspeed = true;
done:
if (value)
@@ -342,7 +503,9 @@ static int config_buf(struct usb_configuration *config,
list_for_each_entry(f, &config->functions, list) {
struct usb_descriptor_header **descriptors;
- if (speed == USB_SPEED_HIGH)
+ if (speed == USB_SPEED_SUPER)
+ descriptors = f->ss_descriptors;
+ else if (speed == USB_SPEED_HIGH)
descriptors = f->hs_descriptors;
else
descriptors = f->descriptors;
@@ -368,9 +531,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
u8 type = w_value >> 8;
enum usb_device_speed speed = USB_SPEED_UNKNOWN;
- if (gadget_is_dualspeed(gadget)) {
- int hs = 0;
-
+ if (gadget->speed == USB_SPEED_SUPER)
+ speed = gadget->speed;
+ else if (gadget_is_dualspeed(gadget)) {
+ int hs = 0;
if (gadget->speed == USB_SPEED_HIGH)
hs = 1;
if (type == USB_DT_OTHER_SPEED_CONFIG)
@@ -384,7 +548,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
w_value &= 0xff;
list_for_each_entry(c, &cdev->configs, list) {
/* ignore configs that won't work at this speed */
- if (speed == USB_SPEED_HIGH) {
+ if (speed == USB_SPEED_SUPER) {
+ if (!c->superspeed)
+ continue;
+ } else if (speed == USB_SPEED_HIGH) {
if (!c->highspeed)
continue;
} else {
@@ -404,16 +571,22 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
struct usb_configuration *c;
unsigned count = 0;
int hs = 0;
+ int ss = 0;
if (gadget_is_dualspeed(gadget)) {
if (gadget->speed == USB_SPEED_HIGH)
hs = 1;
+ if (gadget->speed == USB_SPEED_SUPER)
+ ss = 1;
if (type == USB_DT_DEVICE_QUALIFIER)
hs = !hs;
}
list_for_each_entry(c, &cdev->configs, list) {
/* ignore configs that won't work at this speed */
- if (hs) {
+ if (ss) {
+ if (!c->superspeed)
+ continue;
+ } else if (hs) {
if (!c->highspeed)
continue;
} else {
@@ -425,6 +598,73 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
return count;
}
+/**
+ * bos() - prepares the BOS descriptor.
+ * @cdev: pointer to usb_composite device to generate the bos
+ * descriptor for
+ *
+ * This function generates the BOS (Binary Device Object)
+ * descriptor and its device capabilities descriptors. The BOS
+ * descriptor should be supported by a SuperSpeed device.
+ */
+static int bos(struct usb_composite_dev *cdev)
+{
+ struct usb_bos_descriptor *bos = cdev->req->buf;
+ struct usb_ext_cap_descriptor *usb_ext = NULL;
+ struct usb_ss_cap_descriptor *ss_cap = NULL;
+
+ struct usb_dcd_config_params dcd_config_params;
+
+ bos->bLength = USB_DT_BOS_SIZE;
+ bos->bDescriptorType = USB_DT_BOS;
+
+ bos->wTotalLength = USB_DT_BOS_SIZE;
+ bos->bNumDeviceCaps = 0;
+
+ /*
+ * A SuperSpeed device shall include the USB2.0 extension descriptor
+ * and shall support LPM when operating in USB2.0 HS mode.
+ */
+ usb_ext = (struct usb_ext_cap_descriptor *)
+ (cdev->req->buf+bos->wTotalLength);
+ bos->bNumDeviceCaps++;
+ bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
+ usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
+ usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+ usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
+ usb_ext->bmAttributes = USB_LPM_SUPPORT;
+
+ /*
+ * The Superspeed USB Capability descriptor shall be implemented by all
+ * SuperSpeed devices.
+ */
+ ss_cap = (struct usb_ss_cap_descriptor *)
+ (cdev->req->buf+bos->wTotalLength);
+ bos->bNumDeviceCaps++;
+ bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
+ ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
+ USB_FULL_SPEED_OPERATION |
+ USB_HIGH_SPEED_OPERATION |
+ USB_5GBPS_OPERATION;
+ ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
+
+ /* Get Controller configuration */
+ if (cdev->gadget->ops->get_config_params)
+ cdev->gadget->ops->get_config_params(&dcd_config_params);
+ else {
+ dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
+ dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
+ }
+ ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
+ ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
+
+ return bos->wTotalLength;
+}
+
static void device_qual(struct usb_composite_dev *cdev)
{
struct usb_qualifier_descriptor *qual = cdev->req->buf;
@@ -468,27 +708,42 @@ static int set_config(struct usb_composite_dev *cdev,
unsigned power = gadget_is_otg(gadget) ? 8 : 100;
int tmp;
- if (cdev->config)
- reset_config(cdev);
-
if (number) {
list_for_each_entry(c, &cdev->configs, list) {
if (c->bConfigurationValue == number) {
+ /*
+ * Need to disable the FDs of the previous
+ * configuration
+ */
+ if (cdev->config)
+ reset_config(cdev);
result = 0;
break;
}
}
if (result < 0)
goto done;
- } else
+ } else { /* Zero configuration value - need to reset the config */
+ if (cdev->config)
+ reset_config(cdev);
result = 0;
+ }
INFO(cdev, "%s speed config #%d: %s\n",
({ char *speed;
switch (gadget->speed) {
- case USB_SPEED_LOW: speed = "low"; break;
- case USB_SPEED_FULL: speed = "full"; break;
- case USB_SPEED_HIGH: speed = "high"; break;
+ case USB_SPEED_LOW:
+ speed = "low";
+ break;
+ case USB_SPEED_FULL:
+ speed = "full";
+ break;
+ case USB_SPEED_HIGH:
+ speed = "high";
+ break;
+ case USB_SPEED_SUPER:
+ speed = "super";
+ break;
default: speed = "?"; break;
} ; speed; }), number, c ? c->label : "unconfigured");
@@ -511,7 +766,9 @@ static int set_config(struct usb_composite_dev *cdev,
* function's setup callback instead of the current
* configuration's setup callback.
*/
- if (gadget->speed == USB_SPEED_HIGH)
+ if (gadget->speed == USB_SPEED_SUPER)
+ descriptors = f->ss_descriptors;
+ else if (gadget->speed == USB_SPEED_HIGH)
descriptors = f->hs_descriptors;
else
descriptors = f->descriptors;
@@ -596,14 +853,14 @@ int usb_add_config(struct usb_composite_dev *cdev,
} else {
unsigned i;
- DBG(cdev, "cfg %d/%p speeds:%s%s\n",
+ DBG(cdev, "cfg %d/%p speeds:%s%s%s\n",
config->bConfigurationValue, config,
+ config->superspeed ? " super" : "",
config->highspeed ? " high" : "",
config->fullspeed
? (gadget_is_dualspeed(cdev->gadget)
? " full"
- : " full/low")
- : "");
+ : " full/low") : "");
for (i = 0; i < MAX_CONFIG_INTERFACES; i++) {
struct usb_function *f = config->interface[i];
@@ -721,7 +978,8 @@ static int get_string(struct usb_composite_dev *cdev,
return s->bLength;
}
- /* Otherwise, look up and return a specified string. First
+ /*
+ * Otherwise, look up and return a specified string. First
* check if the string has not been overridden.
*/
if (cdev->manufacturer_override == id)
@@ -876,6 +1134,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
struct usb_composite_dev *cdev = get_gadget_data(gadget);
struct usb_request *req = cdev->req;
int value = -EOPNOTSUPP;
+ int status = 0;
u16 w_index = le16_to_cpu(ctrl->wIndex);
u8 intf = w_index & 0xFF;
u16 w_value = le16_to_cpu(ctrl->wValue);
@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
case USB_DT_DEVICE:
cdev->desc.bNumConfigurations =
count_configs(cdev, USB_DT_DEVICE);
+ cdev->desc.bMaxPacketSize0 =
+ cdev->gadget->ep0->maxpacket;
+ if (gadget->speed >= USB_SPEED_SUPER)
+ cdev->desc.bcdUSB = cpu_to_le16(0x0300);
+ else if ((gadget_is_superspeed(gadget)) &&
+ (gadget->speed <= USB_SPEED_HIGH))
+ cdev->desc.bcdUSB = cpu_to_le16(0x0210);
+
value = min(w_length, (u16) sizeof cdev->desc);
memcpy(req->buf, &cdev->desc, value);
break;
case USB_DT_DEVICE_QUALIFIER:
- if (!gadget_is_dualspeed(gadget))
+ if (!gadget_is_dualspeed(gadget) ||
+ gadget->speed >= USB_SPEED_SUPER)
break;
device_qual(cdev);
value = min_t(int, w_length,
sizeof(struct usb_qualifier_descriptor));
break;
case USB_DT_OTHER_SPEED_CONFIG:
- if (!gadget_is_dualspeed(gadget))
+ if (!gadget_is_dualspeed(gadget) ||
+ gadget->speed >= USB_SPEED_SUPER)
break;
/* FALLTHROUGH */
case USB_DT_CONFIG:
@@ -928,6 +1197,12 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
if (value >= 0)
value = min(w_length, (u16) value);
break;
+ case USB_DT_BOS:
+ if (gadget_is_superspeed(gadget)) {
+ value = bos(cdev);
+ value = min(w_length, (u16) value);
+ }
+ break;
}
break;
@@ -987,6 +1262,65 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
*((u8 *)req->buf) = value;
value = min(w_length, (u16) 1);
break;
+
+ /*
+ * USB 3.0 additions:
+ * Function driver should handle get_status request. If such cb
+ * wasn't supplied we respond with default value = 0
+ * Note: function driver should supply such cb only for the first
+ * interface of the function
+ */
+ case USB_REQ_GET_STATUS:
+ if (!gadget_is_superspeed(gadget))
+ goto unknown;
+ if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
+ goto unknown;
+ value = 2; /* This is the length of the get_status reply */
+ *((u16 *)req->buf) = 0;
+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ break;
+ f = cdev->config->interface[intf];
+ if (!f)
+ break;
+ status = f->get_status ? f->get_status(f) : 0;
+ if (status < 0)
+ break;
+ *((u16 *)req->buf) = status & 0x0000ffff;
+ break;
+ /*
+ * Function drivers should handle SetFeature/ClearFeature
+ * (FUNCTION_SUSPEND) request. function_suspend cb should be supplied
+ * only for the first interface of the function
+ */
+ case USB_REQ_CLEAR_FEATURE:
+ case USB_REQ_SET_FEATURE:
+ if (!gadget_is_superspeed(gadget))
+ goto unknown;
+ if (ctrl->bRequestType != (USB_DIR_OUT | USB_RECIP_INTERFACE))
+ goto unknown;
+ switch (w_value) {
+ case USB_INTRF_FUNC_SUSPEND:
+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ break;
+ f = cdev->config->interface[intf];
+ if (!f)
+ break;
+ value = f->func_suspend ?
+ f->func_suspend(f,
+ (u8)((w_index &
+ USB_INTR_FUNC_SUSPEND_OPT_MASK)
+ >> 8)) :
+ 0;
+ if (value < 0) {
+ ERROR(cdev, "func_suspend() returned "
+ "error %d\n", value);
+ value = 0;
+ }
+ break;
+ default:
+ break;
+ }
+ break;
default:
unknown:
VDBG(cdev,
@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
DBG(cdev, "unbind function '%s'/%p\n",
f->name, f);
f->unbind(c, f);
- /* may free memory for "f" */
}
+ /* Free memory allocated for ss descriptors */
+ if (f->ss_desc_allocated && f->ss_descriptors)
+ usb_free_descriptors(f->ss_descriptors);
+ /* may free memory for "f" */
}
list_del(&c->list);
if (c->unbind) {
@@ -1346,6 +1683,9 @@ int usb_composite_probe(struct usb_composite_driver *driver,
driver->iProduct = driver->name;
composite_driver.function = (char *) driver->name;
composite_driver.driver.name = driver->name;
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+ composite_driver.speed = USB_SPEED_SUPER;
+#endif /* CONFIG_USB_GADGET_SUPERSPEED */
composite = driver;
composite_gadget_bind = bind;
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index de3461a..35c5818 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -142,13 +142,13 @@ ep_matches (
max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
switch (type) {
case USB_ENDPOINT_XFER_INT:
- /* INT: limit 64 bytes full speed, 1024 high speed */
+ /* INT: limit 64 bytes full speed, 1024 high/super speed */
if (!gadget->is_dualspeed && max > 64)
return 0;
/* FALLTHROUGH */
case USB_ENDPOINT_XFER_ISOC:
- /* ISO: limit 1023 bytes full speed, 1024 high speed */
+ /* ISO: limit 1023 bytes full speed, 1024 high/super speed */
if (ep->maxpacket < max)
return 0;
if (!gadget->is_dualspeed && max > 1023)
@@ -183,7 +183,7 @@ ep_matches (
}
/* report (variable) full speed bulk maxpacket */
- if (USB_ENDPOINT_XFER_BULK == type) {
+ if ((USB_ENDPOINT_XFER_BULK == type) & !gadget->is_dualspeed) {
int size = ep->maxpacket;
/* min() doesn't work on bitfields with gcc-3.5 */
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 4f56e44..e9fa7d8 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -51,6 +51,18 @@ struct usb_configuration;
* @hs_descriptors: Table of high speed descriptors, using interface and
* string identifiers assigned during @bind(). If this pointer is null,
* the function will not be available at high speed.
+ * @ss_descriptors: Table of super speed descriptors. If
+ * wasnt supplied by the FD during @bind() and
+ * !ss_not_capble, will be generated automaticly with
+ * default values while working in superspeed mode. If this
+ * pointer is null after initiation, the function will not
+ * be available at super speed.
+ * @ss_not_capable: This flag is used by the FD to indicate if
+ * this function is SS capble. Meaning: if SS descriptors
+ * weren't supplied by the FD, and the flag is set ss
+ * descriptors will NOT be automatically generated
+ * @ss_desc_allocated: This flag indicates whether the ss descriptors were
+ * dynamically allocated (and needs to be released).
* @config: assigned when @usb_add_function() is called; this is the
* configuration with which this function is associated.
* @bind: Before the gadget can register, all of its functions bind() to the
@@ -69,6 +81,10 @@ struct usb_configuration;
* @setup: Used for interface-specific control requests.
* @suspend: Notifies functions when the host stops sending USB traffic.
* @resume: Notifies functions when the host restarts USB traffic.
+ * @get_status: Returns function status as a reply to
+ * GetStatus() request when the recepient is Interface.
+ * @func_suspend: callback to be called when
+ * SetFeature(FUNCTION_SUSPEND) is reseived
*
* 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
@@ -98,6 +114,10 @@ struct usb_function {
struct usb_gadget_strings **strings;
struct usb_descriptor_header **descriptors;
struct usb_descriptor_header **hs_descriptors;
+ struct usb_descriptor_header **ss_descriptors;
+
+ unsigned ss_desc_allocated:1;
+ unsigned ss_not_capable:1;
struct usb_configuration *config;
@@ -124,6 +144,10 @@ struct usb_function {
void (*suspend)(struct usb_function *);
void (*resume)(struct usb_function *);
+ /* USB 3.0 additions */
+ int (*get_status)(struct usb_function *);
+ int (*func_suspend)(struct usb_function *,
+ u8 suspend_opt);
/* private: */
/* internals */
struct list_head list;
@@ -229,6 +253,7 @@ struct usb_configuration {
struct list_head list;
struct list_head functions;
u8 next_interface_id;
+ unsigned superspeed:1;
unsigned highspeed:1;
unsigned fullspeed:1;
struct usb_function *interface[MAX_CONFIG_INTERFACES];
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 1bb7b3e..92c2d62 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -131,11 +131,15 @@ struct usb_ep_ops {
* @maxpacket:The maximum packet size used on this endpoint. The initial
* value can sometimes be reduced (hardware allowing), according to
* the endpoint descriptor used to configure the endpoint.
+ * @mult: multiplier, 'mult' value for SS Isoc EPs
+ * @maxburst: the maximum number of bursts supported by this EP (for usb3)
* @driver_data:for use by the gadget driver.
* @bEndpointAddress: used to identify the endpoint when finding
* descriptor that matches connection speed
* @desc: endpoint descriptor. This pointer is set before the endpoint is
* enabled and remains valid until the endpoint is disabled.
+ * @comp_desc: In case of SuperSpeed support, this is the endpoint companion
+ * descriptor that is used to configure the endpoint
*
* the bus controller driver lists all the general purpose endpoints in
* gadget->ep_list. the control endpoint (gadget->ep0) is not in that list,
@@ -148,8 +152,12 @@ struct usb_ep {
const struct usb_ep_ops *ops;
struct list_head ep_list;
unsigned maxpacket:16;
+ unsigned mult:2;
+ unsigned pad:1;
+ unsigned maxburst:4;
u8 bEndpointAddress;
const struct usb_endpoint_descriptor *desc;
+ const struct usb_ss_ep_comp_descriptor *comp_desc;
};
/*-------------------------------------------------------------------------*/
@@ -417,6 +425,14 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
/*-------------------------------------------------------------------------*/
+struct usb_dcd_config_params {
+ __u8 bU1devExitLat; /* U1 Device exit Latency */
+#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
+ __u16 bU2DevExitLat; /* U2 Device exit Latency */
+#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
+};
+
+
struct usb_gadget;
/* the rest of the api to the controller hardware: device operations,
@@ -431,6 +447,7 @@ struct usb_gadget_ops {
int (*pullup) (struct usb_gadget *, int is_on);
int (*ioctl)(struct usb_gadget *,
unsigned code, unsigned long param);
+ void (*get_config_params)(struct usb_dcd_config_params *);
};
/**
@@ -522,6 +539,23 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
}
/**
+ * gadget_is_superspeed() - return true if the hardware handles
+ * supperspeed
+ * @g: controller that might support supper speed
+ */
+static inline int gadget_is_superspeed(struct usb_gadget *g)
+{
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+ /* runtime test would check "g->is_superspeed" ... that might be
+ * useful to work around hardware bugs, but is mostly pointless
+ */
+ return 1;
+#else
+ return 0;
+#endif
+}
+
+/**
* gadget_is_otg - return true iff the hardware is OTG-ready
* @g: controller that might have a Mini-AB connector
*
--
1.7.3.3
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-03-23 8:04 ` Tatyana Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tatyana Brokhman @ 2011-03-23 8:04 UTC (permalink / raw)
To: gregkh
Cc: linux-arm-msm, balbi, ablay, Tatyana Brokhman,
open list:USB GADGET/PERIPH...,
open list
This patch adds the SuperSpeed functionality to the gadget framework.
In order not to force all the gadget drivers to supply SuperSpeed
descriptors when operating in SuperSpeed mode the following approach was
taken:
If we're operating in SuperSpeed mode and the gadget driver didn't supply
SuperSpeed descriptors, the composite layer will automatically create
SuperSpeed descriptors with default values.
Support for new SuperSpeed BOS descriptor was added.
Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
added.
Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index bc5123c..b4130bc 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -605,6 +605,18 @@ config USB_GADGET_DUALSPEED
Means that gadget drivers should include extra descriptors
and code to handle dual-speed controllers.
+config USB_GADGET_SUPERSPEED
+ boolean "Gadget operating in Super Speed"
+ depends on USB_GADGET
+ depends on USB_GADGET_DUALSPEED
+ default n
+ help
+ Enabling this feature enables Super Speed support in the Gadget
+ driver. It means that gadget drivers should provide extra (SuperSpeed)
+ descriptors to the host.
+ For composite devices: if SuperSpeed descriptors weren't supplied by
+ the FD, they will be automatically generated with default values.
+
#
# USB Gadget Drivers
#
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index fb7e488..d5fe1c2 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
static char composite_manufacturer[50];
+/* Default endpoint companion descriptor */
+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
+ .bLength = 0x06,
+ .bMaxBurst = 0, /* the default is we don't support bursting */
+ .bmAttributes = 0, /* 2^0 streams supported */
+ .wBytesPerInterval = 0,
+};
+
+/**
+ * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * with default values
+ * @f: pointer to usb_function to generate the descriptors for
+ *
+ * This function receives a pointer to usb_function and adds
+ * missing super speed descriptors in the ss_descriptor field
+ * according to its hs_descriptors field.
+ *
+ * This function copies f->hs_descriptors while updating the
+ * endpoint descriptor and adding endpoint companion descriptor.
+ */
+static void create_ss_descriptors(struct usb_function *f)
+{
+ unsigned bytes; /* number of bytes to allocate */
+ unsigned n_desc; /* number of descriptors */
+ void *mem; /* allocated memory to copy to */
+ struct usb_descriptor_header **tmp;
+ struct usb_endpoint_descriptor *ep_desc ;
+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+ struct usb_descriptor_header **src = f->hs_descriptors;
+
+ if (!f->hs_descriptors)
+ return;
+
+ /*
+ * Count number of EPs (in order to know how many SS_EP_COMPANION
+ * descriptors to add), the total number of descriptors and the sum of
+ * each descriptor bLength field in order to know how much memory to
+ * allocate.
+ */
+ for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
+ if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
+ bytes += default_ep_comp_desc.bLength;
+ n_desc++;
+ }
+ bytes += (*tmp)->bLength;
+ }
+
+ bytes += (n_desc + 1) * sizeof(*tmp);
+ mem = kmalloc(bytes, GFP_KERNEL);
+ if (!mem)
+ return;
+
+ /*
+ * Fill in pointers starting at "tmp", to descriptors copied starting
+ * at "mem" and return "ret"
+ */
+ tmp = mem;
+ f->ss_descriptors = mem;
+ mem += (n_desc + 1) * sizeof(*tmp);
+ while (*src) {
+ /* Copy the original descriptor */
+ memcpy(mem, *src, (*src)->bLength);
+ switch ((*src)->bDescriptorType) {
+ case USB_DT_ENDPOINT:
+ /* update ep descriptor */
+ ep_desc = (struct usb_endpoint_descriptor *)mem;
+ switch (ep_desc->bmAttributes &
+ USB_ENDPOINT_XFERTYPE_MASK) {
+ case USB_ENDPOINT_XFER_CONTROL:
+ ep_desc->wMaxPacketSize = cpu_to_le16(512);
+ ep_desc->bInterval = 0;
+ break;
+ case USB_ENDPOINT_XFER_BULK:
+ ep_desc->wMaxPacketSize = cpu_to_le16(1024);
+ ep_desc->bInterval = 0;
+ break;
+ case USB_ENDPOINT_XFER_INT:
+ case USB_ENDPOINT_XFER_ISOC:
+ break;
+ }
+ *tmp = mem;
+ tmp++;
+ mem += (*src)->bLength;
+ /* add ep companion descriptor */
+ memcpy(mem, &default_ep_comp_desc,
+ default_ep_comp_desc.bLength);
+ *tmp = mem;
+ tmp++;
+ /* Update wBytesPerInterval for periodic endpoints */
+ ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
+ switch (ep_desc->bmAttributes &
+ USB_ENDPOINT_XFERTYPE_MASK) {
+ case USB_ENDPOINT_XFER_INT:
+ case USB_ENDPOINT_XFER_ISOC:
+ ep_comp_desc->wBytesPerInterval =
+ ep_desc->wMaxPacketSize;
+ break;
+ }
+ mem += default_ep_comp_desc.bLength;
+ break;
+ default:
+ *tmp = mem;
+ tmp++;
+ mem += (*src)->bLength;
+ break;
+ }
+ src++;
+ }
+ /*
+ * The last (struct usb_descriptor_header *) in the descriptors
+ * vector is NULL
+ */
+ *tmp = NULL;
+ f->ss_desc_allocated = true;
+}
+
/*-------------------------------------------------------------------------*/
/**
* next_ep_desc() - advance to the next EP descriptor
@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
struct usb_endpoint_descriptor *chosen_desc = NULL;
struct usb_descriptor_header **speed_desc = NULL;
+ struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
+ int want_comp_desc = 0;
+
struct usb_descriptor_header **d_spd; /* cursor for speed desc */
if (!g || !f || !_ep)
@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
/* select desired speed */
switch (g->speed) {
+ case USB_SPEED_SUPER:
+ if (gadget_is_superspeed(g)) {
+ speed_desc = f->ss_descriptors;
+ want_comp_desc = 1;
+ break;
+ }
+ /* else: Fall trough */
case USB_SPEED_HIGH:
if (gadget_is_dualspeed(g)) {
speed_desc = f->hs_descriptors;
@@ -147,7 +274,31 @@ ep_found:
/* commit results */
_ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
_ep->desc = chosen_desc;
-
+ _ep->comp_desc = NULL;
+ _ep->maxburst = 0;
+ _ep->mult = 0;
+ if (want_comp_desc) {
+ /*
+ * Companion descriptor should follow EP descriptor
+ * USB 3.0 spec, #9.6.7
+ */
+ comp_desc = (struct usb_ss_ep_comp_descriptor *)*(++d_spd);
+ if (!comp_desc ||
+ (comp_desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP))
+ return -EIO;
+ _ep->comp_desc = comp_desc;
+ if (g->speed == USB_SPEED_SUPER) {
+ int xfer_type = _ep->bEndpointAddress &
+ USB_ENDPOINT_XFERTYPE_MASK ;
+ if (xfer_type == USB_ENDPOINT_XFER_BULK)
+ _ep->maxburst = comp_desc->bMaxBurst;
+ else if (xfer_type == USB_ENDPOINT_XFER_INT)
+ _ep->maxburst = comp_desc->bMaxBurst;
+ else if (xfer_type == USB_ENDPOINT_XFER_ISOC)
+ /* mult: bits 1:0 of bmAttributes */
+ _ep->mult = comp_desc->bmAttributes & 0x3;
+ }
+ }
return 0;
}
@@ -187,6 +338,14 @@ int usb_add_function(struct usb_configuration *config,
list_del(&function->list);
function->config = NULL;
}
+ /*
+ * Add SS descriptors if there are any. This has to be done
+ * after the bind since we need the hs_descriptors to be set in
+ * usb_function and some of the FDs does it in the bind.
+ */
+ if ((gadget_is_superspeed(config->cdev->gadget)) &&
+ (!function->ss_not_capable) && (!function->ss_descriptors))
+ create_ss_descriptors(function);
} else
value = 0;
@@ -199,6 +358,8 @@ int usb_add_function(struct usb_configuration *config,
config->fullspeed = true;
if (!config->highspeed && function->hs_descriptors)
config->highspeed = true;
+ if (!config->superspeed && function->ss_descriptors)
+ config->superspeed = true;
done:
if (value)
@@ -342,7 +503,9 @@ static int config_buf(struct usb_configuration *config,
list_for_each_entry(f, &config->functions, list) {
struct usb_descriptor_header **descriptors;
- if (speed == USB_SPEED_HIGH)
+ if (speed == USB_SPEED_SUPER)
+ descriptors = f->ss_descriptors;
+ else if (speed == USB_SPEED_HIGH)
descriptors = f->hs_descriptors;
else
descriptors = f->descriptors;
@@ -368,9 +531,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
u8 type = w_value >> 8;
enum usb_device_speed speed = USB_SPEED_UNKNOWN;
- if (gadget_is_dualspeed(gadget)) {
- int hs = 0;
-
+ if (gadget->speed == USB_SPEED_SUPER)
+ speed = gadget->speed;
+ else if (gadget_is_dualspeed(gadget)) {
+ int hs = 0;
if (gadget->speed == USB_SPEED_HIGH)
hs = 1;
if (type == USB_DT_OTHER_SPEED_CONFIG)
@@ -384,7 +548,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
w_value &= 0xff;
list_for_each_entry(c, &cdev->configs, list) {
/* ignore configs that won't work at this speed */
- if (speed == USB_SPEED_HIGH) {
+ if (speed == USB_SPEED_SUPER) {
+ if (!c->superspeed)
+ continue;
+ } else if (speed == USB_SPEED_HIGH) {
if (!c->highspeed)
continue;
} else {
@@ -404,16 +571,22 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
struct usb_configuration *c;
unsigned count = 0;
int hs = 0;
+ int ss = 0;
if (gadget_is_dualspeed(gadget)) {
if (gadget->speed == USB_SPEED_HIGH)
hs = 1;
+ if (gadget->speed == USB_SPEED_SUPER)
+ ss = 1;
if (type == USB_DT_DEVICE_QUALIFIER)
hs = !hs;
}
list_for_each_entry(c, &cdev->configs, list) {
/* ignore configs that won't work at this speed */
- if (hs) {
+ if (ss) {
+ if (!c->superspeed)
+ continue;
+ } else if (hs) {
if (!c->highspeed)
continue;
} else {
@@ -425,6 +598,73 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
return count;
}
+/**
+ * bos() - prepares the BOS descriptor.
+ * @cdev: pointer to usb_composite device to generate the bos
+ * descriptor for
+ *
+ * This function generates the BOS (Binary Device Object)
+ * descriptor and its device capabilities descriptors. The BOS
+ * descriptor should be supported by a SuperSpeed device.
+ */
+static int bos(struct usb_composite_dev *cdev)
+{
+ struct usb_bos_descriptor *bos = cdev->req->buf;
+ struct usb_ext_cap_descriptor *usb_ext = NULL;
+ struct usb_ss_cap_descriptor *ss_cap = NULL;
+
+ struct usb_dcd_config_params dcd_config_params;
+
+ bos->bLength = USB_DT_BOS_SIZE;
+ bos->bDescriptorType = USB_DT_BOS;
+
+ bos->wTotalLength = USB_DT_BOS_SIZE;
+ bos->bNumDeviceCaps = 0;
+
+ /*
+ * A SuperSpeed device shall include the USB2.0 extension descriptor
+ * and shall support LPM when operating in USB2.0 HS mode.
+ */
+ usb_ext = (struct usb_ext_cap_descriptor *)
+ (cdev->req->buf+bos->wTotalLength);
+ bos->bNumDeviceCaps++;
+ bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
+ usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
+ usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+ usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
+ usb_ext->bmAttributes = USB_LPM_SUPPORT;
+
+ /*
+ * The Superspeed USB Capability descriptor shall be implemented by all
+ * SuperSpeed devices.
+ */
+ ss_cap = (struct usb_ss_cap_descriptor *)
+ (cdev->req->buf+bos->wTotalLength);
+ bos->bNumDeviceCaps++;
+ bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
+ ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
+ USB_FULL_SPEED_OPERATION |
+ USB_HIGH_SPEED_OPERATION |
+ USB_5GBPS_OPERATION;
+ ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
+
+ /* Get Controller configuration */
+ if (cdev->gadget->ops->get_config_params)
+ cdev->gadget->ops->get_config_params(&dcd_config_params);
+ else {
+ dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
+ dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
+ }
+ ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
+ ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
+
+ return bos->wTotalLength;
+}
+
static void device_qual(struct usb_composite_dev *cdev)
{
struct usb_qualifier_descriptor *qual = cdev->req->buf;
@@ -468,27 +708,42 @@ static int set_config(struct usb_composite_dev *cdev,
unsigned power = gadget_is_otg(gadget) ? 8 : 100;
int tmp;
- if (cdev->config)
- reset_config(cdev);
-
if (number) {
list_for_each_entry(c, &cdev->configs, list) {
if (c->bConfigurationValue == number) {
+ /*
+ * Need to disable the FDs of the previous
+ * configuration
+ */
+ if (cdev->config)
+ reset_config(cdev);
result = 0;
break;
}
}
if (result < 0)
goto done;
- } else
+ } else { /* Zero configuration value - need to reset the config */
+ if (cdev->config)
+ reset_config(cdev);
result = 0;
+ }
INFO(cdev, "%s speed config #%d: %s\n",
({ char *speed;
switch (gadget->speed) {
- case USB_SPEED_LOW: speed = "low"; break;
- case USB_SPEED_FULL: speed = "full"; break;
- case USB_SPEED_HIGH: speed = "high"; break;
+ case USB_SPEED_LOW:
+ speed = "low";
+ break;
+ case USB_SPEED_FULL:
+ speed = "full";
+ break;
+ case USB_SPEED_HIGH:
+ speed = "high";
+ break;
+ case USB_SPEED_SUPER:
+ speed = "super";
+ break;
default: speed = "?"; break;
} ; speed; }), number, c ? c->label : "unconfigured");
@@ -511,7 +766,9 @@ static int set_config(struct usb_composite_dev *cdev,
* function's setup callback instead of the current
* configuration's setup callback.
*/
- if (gadget->speed == USB_SPEED_HIGH)
+ if (gadget->speed == USB_SPEED_SUPER)
+ descriptors = f->ss_descriptors;
+ else if (gadget->speed == USB_SPEED_HIGH)
descriptors = f->hs_descriptors;
else
descriptors = f->descriptors;
@@ -596,14 +853,14 @@ int usb_add_config(struct usb_composite_dev *cdev,
} else {
unsigned i;
- DBG(cdev, "cfg %d/%p speeds:%s%s\n",
+ DBG(cdev, "cfg %d/%p speeds:%s%s%s\n",
config->bConfigurationValue, config,
+ config->superspeed ? " super" : "",
config->highspeed ? " high" : "",
config->fullspeed
? (gadget_is_dualspeed(cdev->gadget)
? " full"
- : " full/low")
- : "");
+ : " full/low") : "");
for (i = 0; i < MAX_CONFIG_INTERFACES; i++) {
struct usb_function *f = config->interface[i];
@@ -721,7 +978,8 @@ static int get_string(struct usb_composite_dev *cdev,
return s->bLength;
}
- /* Otherwise, look up and return a specified string. First
+ /*
+ * Otherwise, look up and return a specified string. First
* check if the string has not been overridden.
*/
if (cdev->manufacturer_override == id)
@@ -876,6 +1134,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
struct usb_composite_dev *cdev = get_gadget_data(gadget);
struct usb_request *req = cdev->req;
int value = -EOPNOTSUPP;
+ int status = 0;
u16 w_index = le16_to_cpu(ctrl->wIndex);
u8 intf = w_index & 0xFF;
u16 w_value = le16_to_cpu(ctrl->wValue);
@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
case USB_DT_DEVICE:
cdev->desc.bNumConfigurations =
count_configs(cdev, USB_DT_DEVICE);
+ cdev->desc.bMaxPacketSize0 =
+ cdev->gadget->ep0->maxpacket;
+ if (gadget->speed >= USB_SPEED_SUPER)
+ cdev->desc.bcdUSB = cpu_to_le16(0x0300);
+ else if ((gadget_is_superspeed(gadget)) &&
+ (gadget->speed <= USB_SPEED_HIGH))
+ cdev->desc.bcdUSB = cpu_to_le16(0x0210);
+
value = min(w_length, (u16) sizeof cdev->desc);
memcpy(req->buf, &cdev->desc, value);
break;
case USB_DT_DEVICE_QUALIFIER:
- if (!gadget_is_dualspeed(gadget))
+ if (!gadget_is_dualspeed(gadget) ||
+ gadget->speed >= USB_SPEED_SUPER)
break;
device_qual(cdev);
value = min_t(int, w_length,
sizeof(struct usb_qualifier_descriptor));
break;
case USB_DT_OTHER_SPEED_CONFIG:
- if (!gadget_is_dualspeed(gadget))
+ if (!gadget_is_dualspeed(gadget) ||
+ gadget->speed >= USB_SPEED_SUPER)
break;
/* FALLTHROUGH */
case USB_DT_CONFIG:
@@ -928,6 +1197,12 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
if (value >= 0)
value = min(w_length, (u16) value);
break;
+ case USB_DT_BOS:
+ if (gadget_is_superspeed(gadget)) {
+ value = bos(cdev);
+ value = min(w_length, (u16) value);
+ }
+ break;
}
break;
@@ -987,6 +1262,65 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
*((u8 *)req->buf) = value;
value = min(w_length, (u16) 1);
break;
+
+ /*
+ * USB 3.0 additions:
+ * Function driver should handle get_status request. If such cb
+ * wasn't supplied we respond with default value = 0
+ * Note: function driver should supply such cb only for the first
+ * interface of the function
+ */
+ case USB_REQ_GET_STATUS:
+ if (!gadget_is_superspeed(gadget))
+ goto unknown;
+ if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
+ goto unknown;
+ value = 2; /* This is the length of the get_status reply */
+ *((u16 *)req->buf) = 0;
+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ break;
+ f = cdev->config->interface[intf];
+ if (!f)
+ break;
+ status = f->get_status ? f->get_status(f) : 0;
+ if (status < 0)
+ break;
+ *((u16 *)req->buf) = status & 0x0000ffff;
+ break;
+ /*
+ * Function drivers should handle SetFeature/ClearFeature
+ * (FUNCTION_SUSPEND) request. function_suspend cb should be supplied
+ * only for the first interface of the function
+ */
+ case USB_REQ_CLEAR_FEATURE:
+ case USB_REQ_SET_FEATURE:
+ if (!gadget_is_superspeed(gadget))
+ goto unknown;
+ if (ctrl->bRequestType != (USB_DIR_OUT | USB_RECIP_INTERFACE))
+ goto unknown;
+ switch (w_value) {
+ case USB_INTRF_FUNC_SUSPEND:
+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ break;
+ f = cdev->config->interface[intf];
+ if (!f)
+ break;
+ value = f->func_suspend ?
+ f->func_suspend(f,
+ (u8)((w_index &
+ USB_INTR_FUNC_SUSPEND_OPT_MASK)
+ >> 8)) :
+ 0;
+ if (value < 0) {
+ ERROR(cdev, "func_suspend() returned "
+ "error %d\n", value);
+ value = 0;
+ }
+ break;
+ default:
+ break;
+ }
+ break;
default:
unknown:
VDBG(cdev,
@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
DBG(cdev, "unbind function '%s'/%p\n",
f->name, f);
f->unbind(c, f);
- /* may free memory for "f" */
}
+ /* Free memory allocated for ss descriptors */
+ if (f->ss_desc_allocated && f->ss_descriptors)
+ usb_free_descriptors(f->ss_descriptors);
+ /* may free memory for "f" */
}
list_del(&c->list);
if (c->unbind) {
@@ -1346,6 +1683,9 @@ int usb_composite_probe(struct usb_composite_driver *driver,
driver->iProduct = driver->name;
composite_driver.function = (char *) driver->name;
composite_driver.driver.name = driver->name;
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+ composite_driver.speed = USB_SPEED_SUPER;
+#endif /* CONFIG_USB_GADGET_SUPERSPEED */
composite = driver;
composite_gadget_bind = bind;
diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
index de3461a..35c5818 100644
--- a/drivers/usb/gadget/epautoconf.c
+++ b/drivers/usb/gadget/epautoconf.c
@@ -142,13 +142,13 @@ ep_matches (
max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
switch (type) {
case USB_ENDPOINT_XFER_INT:
- /* INT: limit 64 bytes full speed, 1024 high speed */
+ /* INT: limit 64 bytes full speed, 1024 high/super speed */
if (!gadget->is_dualspeed && max > 64)
return 0;
/* FALLTHROUGH */
case USB_ENDPOINT_XFER_ISOC:
- /* ISO: limit 1023 bytes full speed, 1024 high speed */
+ /* ISO: limit 1023 bytes full speed, 1024 high/super speed */
if (ep->maxpacket < max)
return 0;
if (!gadget->is_dualspeed && max > 1023)
@@ -183,7 +183,7 @@ ep_matches (
}
/* report (variable) full speed bulk maxpacket */
- if (USB_ENDPOINT_XFER_BULK == type) {
+ if ((USB_ENDPOINT_XFER_BULK == type) & !gadget->is_dualspeed) {
int size = ep->maxpacket;
/* min() doesn't work on bitfields with gcc-3.5 */
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 4f56e44..e9fa7d8 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -51,6 +51,18 @@ struct usb_configuration;
* @hs_descriptors: Table of high speed descriptors, using interface and
* string identifiers assigned during @bind(). If this pointer is null,
* the function will not be available at high speed.
+ * @ss_descriptors: Table of super speed descriptors. If
+ * wasnt supplied by the FD during @bind() and
+ * !ss_not_capble, will be generated automaticly with
+ * default values while working in superspeed mode. If this
+ * pointer is null after initiation, the function will not
+ * be available at super speed.
+ * @ss_not_capable: This flag is used by the FD to indicate if
+ * this function is SS capble. Meaning: if SS descriptors
+ * weren't supplied by the FD, and the flag is set ss
+ * descriptors will NOT be automatically generated
+ * @ss_desc_allocated: This flag indicates whether the ss descriptors were
+ * dynamically allocated (and needs to be released).
* @config: assigned when @usb_add_function() is called; this is the
* configuration with which this function is associated.
* @bind: Before the gadget can register, all of its functions bind() to the
@@ -69,6 +81,10 @@ struct usb_configuration;
* @setup: Used for interface-specific control requests.
* @suspend: Notifies functions when the host stops sending USB traffic.
* @resume: Notifies functions when the host restarts USB traffic.
+ * @get_status: Returns function status as a reply to
+ * GetStatus() request when the recepient is Interface.
+ * @func_suspend: callback to be called when
+ * SetFeature(FUNCTION_SUSPEND) is reseived
*
* 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
@@ -98,6 +114,10 @@ struct usb_function {
struct usb_gadget_strings **strings;
struct usb_descriptor_header **descriptors;
struct usb_descriptor_header **hs_descriptors;
+ struct usb_descriptor_header **ss_descriptors;
+
+ unsigned ss_desc_allocated:1;
+ unsigned ss_not_capable:1;
struct usb_configuration *config;
@@ -124,6 +144,10 @@ struct usb_function {
void (*suspend)(struct usb_function *);
void (*resume)(struct usb_function *);
+ /* USB 3.0 additions */
+ int (*get_status)(struct usb_function *);
+ int (*func_suspend)(struct usb_function *,
+ u8 suspend_opt);
/* private: */
/* internals */
struct list_head list;
@@ -229,6 +253,7 @@ struct usb_configuration {
struct list_head list;
struct list_head functions;
u8 next_interface_id;
+ unsigned superspeed:1;
unsigned highspeed:1;
unsigned fullspeed:1;
struct usb_function *interface[MAX_CONFIG_INTERFACES];
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 1bb7b3e..92c2d62 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -131,11 +131,15 @@ struct usb_ep_ops {
* @maxpacket:The maximum packet size used on this endpoint. The initial
* value can sometimes be reduced (hardware allowing), according to
* the endpoint descriptor used to configure the endpoint.
+ * @mult: multiplier, 'mult' value for SS Isoc EPs
+ * @maxburst: the maximum number of bursts supported by this EP (for usb3)
* @driver_data:for use by the gadget driver.
* @bEndpointAddress: used to identify the endpoint when finding
* descriptor that matches connection speed
* @desc: endpoint descriptor. This pointer is set before the endpoint is
* enabled and remains valid until the endpoint is disabled.
+ * @comp_desc: In case of SuperSpeed support, this is the endpoint companion
+ * descriptor that is used to configure the endpoint
*
* the bus controller driver lists all the general purpose endpoints in
* gadget->ep_list. the control endpoint (gadget->ep0) is not in that list,
@@ -148,8 +152,12 @@ struct usb_ep {
const struct usb_ep_ops *ops;
struct list_head ep_list;
unsigned maxpacket:16;
+ unsigned mult:2;
+ unsigned pad:1;
+ unsigned maxburst:4;
u8 bEndpointAddress;
const struct usb_endpoint_descriptor *desc;
+ const struct usb_ss_ep_comp_descriptor *comp_desc;
};
/*-------------------------------------------------------------------------*/
@@ -417,6 +425,14 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
/*-------------------------------------------------------------------------*/
+struct usb_dcd_config_params {
+ __u8 bU1devExitLat; /* U1 Device exit Latency */
+#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
+ __u16 bU2DevExitLat; /* U2 Device exit Latency */
+#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
+};
+
+
struct usb_gadget;
/* the rest of the api to the controller hardware: device operations,
@@ -431,6 +447,7 @@ struct usb_gadget_ops {
int (*pullup) (struct usb_gadget *, int is_on);
int (*ioctl)(struct usb_gadget *,
unsigned code, unsigned long param);
+ void (*get_config_params)(struct usb_dcd_config_params *);
};
/**
@@ -522,6 +539,23 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
}
/**
+ * gadget_is_superspeed() - return true if the hardware handles
+ * supperspeed
+ * @g: controller that might support supper speed
+ */
+static inline int gadget_is_superspeed(struct usb_gadget *g)
+{
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
+ /* runtime test would check "g->is_superspeed" ... that might be
+ * useful to work around hardware bugs, but is mostly pointless
+ */
+ return 1;
+#else
+ return 0;
+#endif
+}
+
+/**
* gadget_is_otg - return true iff the hardware is OTG-ready
* @g: controller that might have a Mini-AB connector
*
--
1.7.3.3
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-03-23 8:04 ` Tatyana Brokhman
@ 2011-03-25 13:41 ` Felipe Balbi
-1 siblings, 0 replies; 28+ messages in thread
From: Felipe Balbi @ 2011-03-25 13:41 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh-l3A5Bk7waGM, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
balbi-l0cyMroinI0, ablay-sgV2jX0FEOL9JmXXK+q4OQ,
open list:USB GADGET/PERIPH...,
open list
Hi,
On Wed, Mar 23, 2011 at 10:04:57AM +0200, Tatyana Brokhman wrote:
> This patch adds the SuperSpeed functionality to the gadget framework.
> In order not to force all the gadget drivers to supply SuperSpeed
> descriptors when operating in SuperSpeed mode the following approach was
> taken:
> If we're operating in SuperSpeed mode and the gadget driver didn't supply
> SuperSpeed descriptors, the composite layer will automatically create
> SuperSpeed descriptors with default values.
> Support for new SuperSpeed BOS descriptor was added.
> Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
> added.
IMHO, this patch tries to do too much in one huge patch. It needs major
splitting.
Also, I don't get why you decided to go the path of letting composite.c
generate the super speed descriptors instead of expecting gadget drivers
to pass those should they support super speed.
I would like the other way much better: first, it's what we already do
for full/low speed and high speed. Second, we can easily see the
descriptors by opening the gadget driver's code.
There are a few more comments below, but this patch needs major, major
splitting.
> Signed-off-by: Tatyana Brokhman <tlinder-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
missing the tear line --- here
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index fb7e488..d5fe1c2 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
>
> static char composite_manufacturer[50];
>
> +/* Default endpoint companion descriptor */
> +static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> + .bLength = 0x06,
there's a define for this:
#define USB_DT_SS_EP_COMP_SIZE 6
> +/**
> + * create_ss_descriptors() - Generate SuperSpeed descriptors
> + * with default values
> + * @f: pointer to usb_function to generate the descriptors for
> + *
> + * This function receives a pointer to usb_function and adds
> + * missing super speed descriptors in the ss_descriptor field
> + * according to its hs_descriptors field.
> + *
> + * This function copies f->hs_descriptors while updating the
> + * endpoint descriptor and adding endpoint companion descriptor.
> + */
> +static void create_ss_descriptors(struct usb_function *f)
> +{
> + unsigned bytes; /* number of bytes to allocate */
> + unsigned n_desc; /* number of descriptors */
> + void *mem; /* allocated memory to copy to */
> + struct usb_descriptor_header **tmp;
> + struct usb_endpoint_descriptor *ep_desc ;
> + struct usb_ss_ep_comp_descriptor *ep_comp_desc;
> + struct usb_descriptor_header **src = f->hs_descriptors;
> +
> + if (!f->hs_descriptors)
> + return;
> +
> + /*
> + * Count number of EPs (in order to know how many SS_EP_COMPANION
> + * descriptors to add), the total number of descriptors and the sum of
> + * each descriptor bLength field in order to know how much memory to
> + * allocate.
> + */
> + for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
> + if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
> + bytes += default_ep_comp_desc.bLength;
> + n_desc++;
> + }
> + bytes += (*tmp)->bLength;
> + }
> +
> + bytes += (n_desc + 1) * sizeof(*tmp);
> + mem = kmalloc(bytes, GFP_KERNEL);
> + if (!mem)
> + return;
> +
> + /*
> + * Fill in pointers starting at "tmp", to descriptors copied starting
> + * at "mem" and return "ret"
> + */
> + tmp = mem;
> + f->ss_descriptors = mem;
> + mem += (n_desc + 1) * sizeof(*tmp);
> + while (*src) {
> + /* Copy the original descriptor */
> + memcpy(mem, *src, (*src)->bLength);
> + switch ((*src)->bDescriptorType) {
> + case USB_DT_ENDPOINT:
> + /* update ep descriptor */
> + ep_desc = (struct usb_endpoint_descriptor *)mem;
> + switch (ep_desc->bmAttributes &
> + USB_ENDPOINT_XFERTYPE_MASK) {
> + case USB_ENDPOINT_XFER_CONTROL:
> + ep_desc->wMaxPacketSize = cpu_to_le16(512);
> + ep_desc->bInterval = 0;
> + break;
> + case USB_ENDPOINT_XFER_BULK:
> + ep_desc->wMaxPacketSize = cpu_to_le16(1024);
> + ep_desc->bInterval = 0;
> + break;
> + case USB_ENDPOINT_XFER_INT:
> + case USB_ENDPOINT_XFER_ISOC:
> + break;
> + }
> + *tmp = mem;
> + tmp++;
> + mem += (*src)->bLength;
> + /* add ep companion descriptor */
> + memcpy(mem, &default_ep_comp_desc,
> + default_ep_comp_desc.bLength);
> + *tmp = mem;
> + tmp++;
> + /* Update wBytesPerInterval for periodic endpoints */
> + ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
> + switch (ep_desc->bmAttributes &
> + USB_ENDPOINT_XFERTYPE_MASK) {
> + case USB_ENDPOINT_XFER_INT:
> + case USB_ENDPOINT_XFER_ISOC:
> + ep_comp_desc->wBytesPerInterval =
> + ep_desc->wMaxPacketSize;
> + break;
> + }
> + mem += default_ep_comp_desc.bLength;
> + break;
> + default:
> + *tmp = mem;
> + tmp++;
> + mem += (*src)->bLength;
> + break;
> + }
> + src++;
> + }
> + /*
> + * The last (struct usb_descriptor_header *) in the descriptors
> + * vector is NULL
> + */
> + *tmp = NULL;
> + f->ss_desc_allocated = true;
> +}
I would rather expect gadget drivers to provide their own descriptors
just like we do for full/low speed and high speed descriptors. Why you
decided to take this approach ?
> +
> /*-------------------------------------------------------------------------*/
> /**
> * next_ep_desc() - advance to the next EP descriptor
> @@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> struct usb_endpoint_descriptor *chosen_desc = NULL;
> struct usb_descriptor_header **speed_desc = NULL;
>
> + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
> + int want_comp_desc = 0;
> +
> struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>
> if (!g || !f || !_ep)
> @@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
>
> /* select desired speed */
> switch (g->speed) {
> + case USB_SPEED_SUPER:
> + if (gadget_is_superspeed(g)) {
> + speed_desc = f->ss_descriptors;
> + want_comp_desc = 1;
> + break;
> + }
same here, I would expect gadget drivers to be changed providing the
correct descriptors. Just like I did when I sent my version.
In my opinion, it would be easier that way. We could see the descriptors
just by opening the gadget driver code. Besides, different gadget
drivers might want different "sane defaults".
> INFO(cdev, "%s speed config #%d: %s\n",
> ({ char *speed;
> switch (gadget->speed) {
> - case USB_SPEED_LOW: speed = "low"; break;
> - case USB_SPEED_FULL: speed = "full"; break;
> - case USB_SPEED_HIGH: speed = "high"; break;
> + case USB_SPEED_LOW:
> + speed = "low";
> + break;
> + case USB_SPEED_FULL:
> + speed = "full";
> + break;
> + case USB_SPEED_HIGH:
> + speed = "high";
> + break;
> + case USB_SPEED_SUPER:
> + speed = "super";
> + break;
> default: speed = "?"; break;
> } ; speed; }), number, c ? c->label : "unconfigured");
this part isn't part of this patch.
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-03-25 13:41 ` Felipe Balbi
0 siblings, 0 replies; 28+ messages in thread
From: Felipe Balbi @ 2011-03-25 13:41 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay,
open list:USB GADGET/PERIPH...,
open list
Hi,
On Wed, Mar 23, 2011 at 10:04:57AM +0200, Tatyana Brokhman wrote:
> This patch adds the SuperSpeed functionality to the gadget framework.
> In order not to force all the gadget drivers to supply SuperSpeed
> descriptors when operating in SuperSpeed mode the following approach was
> taken:
> If we're operating in SuperSpeed mode and the gadget driver didn't supply
> SuperSpeed descriptors, the composite layer will automatically create
> SuperSpeed descriptors with default values.
> Support for new SuperSpeed BOS descriptor was added.
> Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
> added.
IMHO, this patch tries to do too much in one huge patch. It needs major
splitting.
Also, I don't get why you decided to go the path of letting composite.c
generate the super speed descriptors instead of expecting gadget drivers
to pass those should they support super speed.
I would like the other way much better: first, it's what we already do
for full/low speed and high speed. Second, we can easily see the
descriptors by opening the gadget driver's code.
There are a few more comments below, but this patch needs major, major
splitting.
> Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
missing the tear line --- here
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index fb7e488..d5fe1c2 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
>
> static char composite_manufacturer[50];
>
> +/* Default endpoint companion descriptor */
> +static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> + .bLength = 0x06,
there's a define for this:
#define USB_DT_SS_EP_COMP_SIZE 6
> +/**
> + * create_ss_descriptors() - Generate SuperSpeed descriptors
> + * with default values
> + * @f: pointer to usb_function to generate the descriptors for
> + *
> + * This function receives a pointer to usb_function and adds
> + * missing super speed descriptors in the ss_descriptor field
> + * according to its hs_descriptors field.
> + *
> + * This function copies f->hs_descriptors while updating the
> + * endpoint descriptor and adding endpoint companion descriptor.
> + */
> +static void create_ss_descriptors(struct usb_function *f)
> +{
> + unsigned bytes; /* number of bytes to allocate */
> + unsigned n_desc; /* number of descriptors */
> + void *mem; /* allocated memory to copy to */
> + struct usb_descriptor_header **tmp;
> + struct usb_endpoint_descriptor *ep_desc ;
> + struct usb_ss_ep_comp_descriptor *ep_comp_desc;
> + struct usb_descriptor_header **src = f->hs_descriptors;
> +
> + if (!f->hs_descriptors)
> + return;
> +
> + /*
> + * Count number of EPs (in order to know how many SS_EP_COMPANION
> + * descriptors to add), the total number of descriptors and the sum of
> + * each descriptor bLength field in order to know how much memory to
> + * allocate.
> + */
> + for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
> + if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
> + bytes += default_ep_comp_desc.bLength;
> + n_desc++;
> + }
> + bytes += (*tmp)->bLength;
> + }
> +
> + bytes += (n_desc + 1) * sizeof(*tmp);
> + mem = kmalloc(bytes, GFP_KERNEL);
> + if (!mem)
> + return;
> +
> + /*
> + * Fill in pointers starting at "tmp", to descriptors copied starting
> + * at "mem" and return "ret"
> + */
> + tmp = mem;
> + f->ss_descriptors = mem;
> + mem += (n_desc + 1) * sizeof(*tmp);
> + while (*src) {
> + /* Copy the original descriptor */
> + memcpy(mem, *src, (*src)->bLength);
> + switch ((*src)->bDescriptorType) {
> + case USB_DT_ENDPOINT:
> + /* update ep descriptor */
> + ep_desc = (struct usb_endpoint_descriptor *)mem;
> + switch (ep_desc->bmAttributes &
> + USB_ENDPOINT_XFERTYPE_MASK) {
> + case USB_ENDPOINT_XFER_CONTROL:
> + ep_desc->wMaxPacketSize = cpu_to_le16(512);
> + ep_desc->bInterval = 0;
> + break;
> + case USB_ENDPOINT_XFER_BULK:
> + ep_desc->wMaxPacketSize = cpu_to_le16(1024);
> + ep_desc->bInterval = 0;
> + break;
> + case USB_ENDPOINT_XFER_INT:
> + case USB_ENDPOINT_XFER_ISOC:
> + break;
> + }
> + *tmp = mem;
> + tmp++;
> + mem += (*src)->bLength;
> + /* add ep companion descriptor */
> + memcpy(mem, &default_ep_comp_desc,
> + default_ep_comp_desc.bLength);
> + *tmp = mem;
> + tmp++;
> + /* Update wBytesPerInterval for periodic endpoints */
> + ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
> + switch (ep_desc->bmAttributes &
> + USB_ENDPOINT_XFERTYPE_MASK) {
> + case USB_ENDPOINT_XFER_INT:
> + case USB_ENDPOINT_XFER_ISOC:
> + ep_comp_desc->wBytesPerInterval =
> + ep_desc->wMaxPacketSize;
> + break;
> + }
> + mem += default_ep_comp_desc.bLength;
> + break;
> + default:
> + *tmp = mem;
> + tmp++;
> + mem += (*src)->bLength;
> + break;
> + }
> + src++;
> + }
> + /*
> + * The last (struct usb_descriptor_header *) in the descriptors
> + * vector is NULL
> + */
> + *tmp = NULL;
> + f->ss_desc_allocated = true;
> +}
I would rather expect gadget drivers to provide their own descriptors
just like we do for full/low speed and high speed descriptors. Why you
decided to take this approach ?
> +
> /*-------------------------------------------------------------------------*/
> /**
> * next_ep_desc() - advance to the next EP descriptor
> @@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> struct usb_endpoint_descriptor *chosen_desc = NULL;
> struct usb_descriptor_header **speed_desc = NULL;
>
> + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
> + int want_comp_desc = 0;
> +
> struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>
> if (!g || !f || !_ep)
> @@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
>
> /* select desired speed */
> switch (g->speed) {
> + case USB_SPEED_SUPER:
> + if (gadget_is_superspeed(g)) {
> + speed_desc = f->ss_descriptors;
> + want_comp_desc = 1;
> + break;
> + }
same here, I would expect gadget drivers to be changed providing the
correct descriptors. Just like I did when I sent my version.
In my opinion, it would be easier that way. We could see the descriptors
just by opening the gadget driver code. Besides, different gadget
drivers might want different "sane defaults".
> INFO(cdev, "%s speed config #%d: %s\n",
> ({ char *speed;
> switch (gadget->speed) {
> - case USB_SPEED_LOW: speed = "low"; break;
> - case USB_SPEED_FULL: speed = "full"; break;
> - case USB_SPEED_HIGH: speed = "high"; break;
> + case USB_SPEED_LOW:
> + speed = "low";
> + break;
> + case USB_SPEED_FULL:
> + speed = "full";
> + break;
> + case USB_SPEED_HIGH:
> + speed = "high";
> + break;
> + case USB_SPEED_SUPER:
> + speed = "super";
> + break;
> default: speed = "?"; break;
> } ; speed; }), number, c ? c->label : "unconfigured");
this part isn't part of this patch.
--
balbi
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-03-25 13:41 ` Felipe Balbi
@ 2011-03-28 8:45 ` Tanya Brokhman
-1 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-03-28 8:45 UTC (permalink / raw)
To: balbi
Cc: gregkh, linux-arm-msm, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi
>
> On Wed, Mar 23, 2011 at 10:04:57AM +0200, Tatyana Brokhman wrote:
> > This patch adds the SuperSpeed functionality to the gadget framework.
> > In order not to force all the gadget drivers to supply SuperSpeed
> > descriptors when operating in SuperSpeed mode the following approach
> was
> > taken:
> > If we're operating in SuperSpeed mode and the gadget driver didn't
> supply
> > SuperSpeed descriptors, the composite layer will automatically create
> > SuperSpeed descriptors with default values.
> > Support for new SuperSpeed BOS descriptor was added.
> > Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode
> was
> > added.
>
> IMHO, this patch tries to do too much in one huge patch. It needs major
> splitting.
>
> Also, I don't get why you decided to go the path of letting composite.c
> generate the super speed descriptors instead of expecting gadget
> drivers
> to pass those should they support super speed.
>
> I would like the other way much better: first, it's what we already do
> for full/low speed and high speed. Second, we can easily see the
> descriptors by opening the gadget driver's code.
>
Our goal was to provide a generic solution that will enable existing gadget
drivers to operate in SS mode with minimum changes to their code. If a
certain gadget driver wishes to operate in SS mode and the values in the
descriptors are important to it, this FD should define its own SS
descriptors as already done for HS/LS and those (SS) descriptors will be
chosen.
Please note that a gadget driver can also choose not to have SS descriptors
generated for it all, even if it didn't provide any of its own and thus
choosing not to operate in SS mode.
If you look at our implementation of the UAS gadget driver you'll see that
we define the SS descriptors just as is done for HS/LS and those descriptors
are chosen in enumeration.
I believe that in the future all gadget drivers will define their own SS
descriptors with values that are suitable for that gadget driver and the
create_ss_descriptors() will become obsolete. As I've already mentioned this
is just a way to allow all the existing drivers to operate in SS mode with
minimum changes to their code. So this patch doesn't contradict your
approach, just extends it.
I hope the above addressed your other questions on the design bellow. Please
let me know if this is not the case.
> There are a few more comments below, but this patch needs major, major
> splitting.
I'll address your comments and split it up in the next version. I would like
to give Greg and others a chance to comment as well before uploading another
version.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>
> missing the tear line --- here
>
> > diff --git a/drivers/usb/gadget/composite.c
> b/drivers/usb/gadget/composite.c
> > index fb7e488..d5fe1c2 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber
> string");
> >
> > static char composite_manufacturer[50];
> >
> > +/* Default endpoint companion descriptor */
> > +static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> > + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> > + .bLength = 0x06,
>
> there's a define for this:
>
> #define USB_DT_SS_EP_COMP_SIZE 6
>
> > +/**
> > + * create_ss_descriptors() - Generate SuperSpeed descriptors
> > + * with default values
> > + * @f: pointer to usb_function to generate the descriptors for
> > + *
> > + * This function receives a pointer to usb_function and adds
> > + * missing super speed descriptors in the ss_descriptor field
> > + * according to its hs_descriptors field.
> > + *
> > + * This function copies f->hs_descriptors while updating the
> > + * endpoint descriptor and adding endpoint companion descriptor.
> > + */
> > +static void create_ss_descriptors(struct usb_function *f)
> > +{
> > + unsigned bytes; /* number of bytes to allocate */
> > + unsigned n_desc; /* number of descriptors */
> > + void *mem; /* allocated memory to copy to */
> > + struct usb_descriptor_header **tmp;
> > + struct usb_endpoint_descriptor *ep_desc ;
> > + struct usb_ss_ep_comp_descriptor *ep_comp_desc;
> > + struct usb_descriptor_header **src = f->hs_descriptors;
> > +
> > + if (!f->hs_descriptors)
> > + return;
> > +
> > + /*
> > + * Count number of EPs (in order to know how many SS_EP_COMPANION
> > + * descriptors to add), the total number of descriptors and the
> sum of
> > + * each descriptor bLength field in order to know how much memory
> to
> > + * allocate.
> > + */
> > + for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
> > + if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
> > + bytes += default_ep_comp_desc.bLength;
> > + n_desc++;
> > + }
> > + bytes += (*tmp)->bLength;
> > + }
> > +
> > + bytes += (n_desc + 1) * sizeof(*tmp);
> > + mem = kmalloc(bytes, GFP_KERNEL);
> > + if (!mem)
> > + return;
> > +
> > + /*
> > + * Fill in pointers starting at "tmp", to descriptors copied
> starting
> > + * at "mem" and return "ret"
> > + */
> > + tmp = mem;
> > + f->ss_descriptors = mem;
> > + mem += (n_desc + 1) * sizeof(*tmp);
> > + while (*src) {
> > + /* Copy the original descriptor */
> > + memcpy(mem, *src, (*src)->bLength);
> > + switch ((*src)->bDescriptorType) {
> > + case USB_DT_ENDPOINT:
> > + /* update ep descriptor */
> > + ep_desc = (struct usb_endpoint_descriptor *)mem;
> > + switch (ep_desc->bmAttributes &
> > + USB_ENDPOINT_XFERTYPE_MASK) {
> > + case USB_ENDPOINT_XFER_CONTROL:
> > + ep_desc->wMaxPacketSize = cpu_to_le16(512);
> > + ep_desc->bInterval = 0;
> > + break;
> > + case USB_ENDPOINT_XFER_BULK:
> > + ep_desc->wMaxPacketSize = cpu_to_le16(1024);
> > + ep_desc->bInterval = 0;
> > + break;
> > + case USB_ENDPOINT_XFER_INT:
> > + case USB_ENDPOINT_XFER_ISOC:
> > + break;
> > + }
> > + *tmp = mem;
> > + tmp++;
> > + mem += (*src)->bLength;
> > + /* add ep companion descriptor */
> > + memcpy(mem, &default_ep_comp_desc,
> > + default_ep_comp_desc.bLength);
> > + *tmp = mem;
> > + tmp++;
> > + /* Update wBytesPerInterval for periodic endpoints
*/
> > + ep_comp_desc = (struct usb_ss_ep_comp_descriptor
> *)mem;
> > + switch (ep_desc->bmAttributes &
> > + USB_ENDPOINT_XFERTYPE_MASK) {
> > + case USB_ENDPOINT_XFER_INT:
> > + case USB_ENDPOINT_XFER_ISOC:
> > + ep_comp_desc->wBytesPerInterval =
> > + ep_desc->wMaxPacketSize;
> > + break;
> > + }
> > + mem += default_ep_comp_desc.bLength;
> > + break;
> > + default:
> > + *tmp = mem;
> > + tmp++;
> > + mem += (*src)->bLength;
> > + break;
> > + }
> > + src++;
> > + }
> > + /*
> > + * The last (struct usb_descriptor_header *) in the descriptors
> > + * vector is NULL
> > + */
> > + *tmp = NULL;
> > + f->ss_desc_allocated = true;
> > +}
>
> I would rather expect gadget drivers to provide their own descriptors
> just like we do for full/low speed and high speed descriptors. Why you
> decided to take this approach ?
>
> > +
> > /*------------------------------------------------------------------
> -------*/
> > /**
> > * next_ep_desc() - advance to the next EP descriptor
> > @@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> > struct usb_endpoint_descriptor *chosen_desc = NULL;
> > struct usb_descriptor_header **speed_desc = NULL;
> >
> > + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
> > + int want_comp_desc = 0;
> > +
> > struct usb_descriptor_header **d_spd; /* cursor for speed desc */
> >
> > if (!g || !f || !_ep)
> > @@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
> >
> > /* select desired speed */
> > switch (g->speed) {
> > + case USB_SPEED_SUPER:
> > + if (gadget_is_superspeed(g)) {
> > + speed_desc = f->ss_descriptors;
> > + want_comp_desc = 1;
> > + break;
> > + }
>
> same here, I would expect gadget drivers to be changed providing the
> correct descriptors. Just like I did when I sent my version.
>
> In my opinion, it would be easier that way. We could see the
> descriptors
> just by opening the gadget driver code. Besides, different gadget
> drivers might want different "sane defaults".
>
> > INFO(cdev, "%s speed config #%d: %s\n",
> > ({ char *speed;
> > switch (gadget->speed) {
> > - case USB_SPEED_LOW: speed = "low"; break;
> > - case USB_SPEED_FULL: speed = "full"; break;
> > - case USB_SPEED_HIGH: speed = "high"; break;
> > + case USB_SPEED_LOW:
> > + speed = "low";
> > + break;
> > + case USB_SPEED_FULL:
> > + speed = "full";
> > + break;
> > + case USB_SPEED_HIGH:
> > + speed = "high";
> > + break;
> > + case USB_SPEED_SUPER:
> > + speed = "super";
> > + break;
> > default: speed = "?"; break;
> > } ; speed; }), number, c ? c->label : "unconfigured");
>
> this part isn't part of this patch.
>
> --
> balbi
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-03-28 8:45 ` Tanya Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-03-28 8:45 UTC (permalink / raw)
To: balbi
Cc: gregkh, linux-arm-msm, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi
>
> On Wed, Mar 23, 2011 at 10:04:57AM +0200, Tatyana Brokhman wrote:
> > This patch adds the SuperSpeed functionality to the gadget framework.
> > In order not to force all the gadget drivers to supply SuperSpeed
> > descriptors when operating in SuperSpeed mode the following approach
> was
> > taken:
> > If we're operating in SuperSpeed mode and the gadget driver didn't
> supply
> > SuperSpeed descriptors, the composite layer will automatically create
> > SuperSpeed descriptors with default values.
> > Support for new SuperSpeed BOS descriptor was added.
> > Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode
> was
> > added.
>
> IMHO, this patch tries to do too much in one huge patch. It needs major
> splitting.
>
> Also, I don't get why you decided to go the path of letting composite.c
> generate the super speed descriptors instead of expecting gadget
> drivers
> to pass those should they support super speed.
>
> I would like the other way much better: first, it's what we already do
> for full/low speed and high speed. Second, we can easily see the
> descriptors by opening the gadget driver's code.
>
Our goal was to provide a generic solution that will enable existing gadget
drivers to operate in SS mode with minimum changes to their code. If a
certain gadget driver wishes to operate in SS mode and the values in the
descriptors are important to it, this FD should define its own SS
descriptors as already done for HS/LS and those (SS) descriptors will be
chosen.
Please note that a gadget driver can also choose not to have SS descriptors
generated for it all, even if it didn't provide any of its own and thus
choosing not to operate in SS mode.
If you look at our implementation of the UAS gadget driver you'll see that
we define the SS descriptors just as is done for HS/LS and those descriptors
are chosen in enumeration.
I believe that in the future all gadget drivers will define their own SS
descriptors with values that are suitable for that gadget driver and the
create_ss_descriptors() will become obsolete. As I've already mentioned this
is just a way to allow all the existing drivers to operate in SS mode with
minimum changes to their code. So this patch doesn't contradict your
approach, just extends it.
I hope the above addressed your other questions on the design bellow. Please
let me know if this is not the case.
> There are a few more comments below, but this patch needs major, major
> splitting.
I'll address your comments and split it up in the next version. I would like
to give Greg and others a chance to comment as well before uploading another
version.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> > Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>
> missing the tear line --- here
>
> > diff --git a/drivers/usb/gadget/composite.c
> b/drivers/usb/gadget/composite.c
> > index fb7e488..d5fe1c2 100644
> > --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber
> string");
> >
> > static char composite_manufacturer[50];
> >
> > +/* Default endpoint companion descriptor */
> > +static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> > + .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> > + .bLength = 0x06,
>
> there's a define for this:
>
> #define USB_DT_SS_EP_COMP_SIZE 6
>
> > +/**
> > + * create_ss_descriptors() - Generate SuperSpeed descriptors
> > + * with default values
> > + * @f: pointer to usb_function to generate the descriptors for
> > + *
> > + * This function receives a pointer to usb_function and adds
> > + * missing super speed descriptors in the ss_descriptor field
> > + * according to its hs_descriptors field.
> > + *
> > + * This function copies f->hs_descriptors while updating the
> > + * endpoint descriptor and adding endpoint companion descriptor.
> > + */
> > +static void create_ss_descriptors(struct usb_function *f)
> > +{
> > + unsigned bytes; /* number of bytes to allocate */
> > + unsigned n_desc; /* number of descriptors */
> > + void *mem; /* allocated memory to copy to */
> > + struct usb_descriptor_header **tmp;
> > + struct usb_endpoint_descriptor *ep_desc ;
> > + struct usb_ss_ep_comp_descriptor *ep_comp_desc;
> > + struct usb_descriptor_header **src = f->hs_descriptors;
> > +
> > + if (!f->hs_descriptors)
> > + return;
> > +
> > + /*
> > + * Count number of EPs (in order to know how many SS_EP_COMPANION
> > + * descriptors to add), the total number of descriptors and the
> sum of
> > + * each descriptor bLength field in order to know how much memory
> to
> > + * allocate.
> > + */
> > + for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
> > + if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
> > + bytes += default_ep_comp_desc.bLength;
> > + n_desc++;
> > + }
> > + bytes += (*tmp)->bLength;
> > + }
> > +
> > + bytes += (n_desc + 1) * sizeof(*tmp);
> > + mem = kmalloc(bytes, GFP_KERNEL);
> > + if (!mem)
> > + return;
> > +
> > + /*
> > + * Fill in pointers starting at "tmp", to descriptors copied
> starting
> > + * at "mem" and return "ret"
> > + */
> > + tmp = mem;
> > + f->ss_descriptors = mem;
> > + mem += (n_desc + 1) * sizeof(*tmp);
> > + while (*src) {
> > + /* Copy the original descriptor */
> > + memcpy(mem, *src, (*src)->bLength);
> > + switch ((*src)->bDescriptorType) {
> > + case USB_DT_ENDPOINT:
> > + /* update ep descriptor */
> > + ep_desc = (struct usb_endpoint_descriptor *)mem;
> > + switch (ep_desc->bmAttributes &
> > + USB_ENDPOINT_XFERTYPE_MASK) {
> > + case USB_ENDPOINT_XFER_CONTROL:
> > + ep_desc->wMaxPacketSize = cpu_to_le16(512);
> > + ep_desc->bInterval = 0;
> > + break;
> > + case USB_ENDPOINT_XFER_BULK:
> > + ep_desc->wMaxPacketSize = cpu_to_le16(1024);
> > + ep_desc->bInterval = 0;
> > + break;
> > + case USB_ENDPOINT_XFER_INT:
> > + case USB_ENDPOINT_XFER_ISOC:
> > + break;
> > + }
> > + *tmp = mem;
> > + tmp++;
> > + mem += (*src)->bLength;
> > + /* add ep companion descriptor */
> > + memcpy(mem, &default_ep_comp_desc,
> > + default_ep_comp_desc.bLength);
> > + *tmp = mem;
> > + tmp++;
> > + /* Update wBytesPerInterval for periodic endpoints
*/
> > + ep_comp_desc = (struct usb_ss_ep_comp_descriptor
> *)mem;
> > + switch (ep_desc->bmAttributes &
> > + USB_ENDPOINT_XFERTYPE_MASK) {
> > + case USB_ENDPOINT_XFER_INT:
> > + case USB_ENDPOINT_XFER_ISOC:
> > + ep_comp_desc->wBytesPerInterval =
> > + ep_desc->wMaxPacketSize;
> > + break;
> > + }
> > + mem += default_ep_comp_desc.bLength;
> > + break;
> > + default:
> > + *tmp = mem;
> > + tmp++;
> > + mem += (*src)->bLength;
> > + break;
> > + }
> > + src++;
> > + }
> > + /*
> > + * The last (struct usb_descriptor_header *) in the descriptors
> > + * vector is NULL
> > + */
> > + *tmp = NULL;
> > + f->ss_desc_allocated = true;
> > +}
>
> I would rather expect gadget drivers to provide their own descriptors
> just like we do for full/low speed and high speed descriptors. Why you
> decided to take this approach ?
>
> > +
> > /*------------------------------------------------------------------
> -------*/
> > /**
> > * next_ep_desc() - advance to the next EP descriptor
> > @@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> > struct usb_endpoint_descriptor *chosen_desc = NULL;
> > struct usb_descriptor_header **speed_desc = NULL;
> >
> > + struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
> > + int want_comp_desc = 0;
> > +
> > struct usb_descriptor_header **d_spd; /* cursor for speed desc */
> >
> > if (!g || !f || !_ep)
> > @@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
> >
> > /* select desired speed */
> > switch (g->speed) {
> > + case USB_SPEED_SUPER:
> > + if (gadget_is_superspeed(g)) {
> > + speed_desc = f->ss_descriptors;
> > + want_comp_desc = 1;
> > + break;
> > + }
>
> same here, I would expect gadget drivers to be changed providing the
> correct descriptors. Just like I did when I sent my version.
>
> In my opinion, it would be easier that way. We could see the
> descriptors
> just by opening the gadget driver code. Besides, different gadget
> drivers might want different "sane defaults".
>
> > INFO(cdev, "%s speed config #%d: %s\n",
> > ({ char *speed;
> > switch (gadget->speed) {
> > - case USB_SPEED_LOW: speed = "low"; break;
> > - case USB_SPEED_FULL: speed = "full"; break;
> > - case USB_SPEED_HIGH: speed = "high"; break;
> > + case USB_SPEED_LOW:
> > + speed = "low";
> > + break;
> > + case USB_SPEED_FULL:
> > + speed = "full";
> > + break;
> > + case USB_SPEED_HIGH:
> > + speed = "high";
> > + break;
> > + case USB_SPEED_SUPER:
> > + speed = "super";
> > + break;
> > default: speed = "?"; break;
> > } ; speed; }), number, c ? c->label : "unconfigured");
>
> this part isn't part of this patch.
>
> --
> balbi
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-03-28 8:45 ` Tanya Brokhman
(?)
@ 2011-03-28 8:54 ` Felipe Balbi
2011-03-28 9:15 ` Tanya Brokhman
-1 siblings, 1 reply; 28+ messages in thread
From: Felipe Balbi @ 2011-03-28 8:54 UTC (permalink / raw)
To: Tanya Brokhman
Cc: balbi, gregkh, linux-arm-msm, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
On Mon, Mar 28, 2011 at 10:45:54AM +0200, Tanya Brokhman wrote:
> Hi
>
> >
> > On Wed, Mar 23, 2011 at 10:04:57AM +0200, Tatyana Brokhman wrote:
> > > This patch adds the SuperSpeed functionality to the gadget framework.
> > > In order not to force all the gadget drivers to supply SuperSpeed
> > > descriptors when operating in SuperSpeed mode the following approach
> > was
> > > taken:
> > > If we're operating in SuperSpeed mode and the gadget driver didn't
> > supply
> > > SuperSpeed descriptors, the composite layer will automatically create
> > > SuperSpeed descriptors with default values.
> > > Support for new SuperSpeed BOS descriptor was added.
> > > Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode
> > was
> > > added.
> >
> > IMHO, this patch tries to do too much in one huge patch. It needs major
> > splitting.
> >
> > Also, I don't get why you decided to go the path of letting composite.c
> > generate the super speed descriptors instead of expecting gadget
> > drivers
> > to pass those should they support super speed.
> >
> > I would like the other way much better: first, it's what we already do
> > for full/low speed and high speed. Second, we can easily see the
> > descriptors by opening the gadget driver's code.
> >
>
> Our goal was to provide a generic solution that will enable existing gadget
> drivers to operate in SS mode with minimum changes to their code. If a
> certain gadget driver wishes to operate in SS mode and the values in the
> descriptors are important to it, this FD should define its own SS
> descriptors as already done for HS/LS and those (SS) descriptors will be
> chosen.
and then we have two paths for achieving the same thing. And most
likely, what'll happen is that you're gonna one or two uses of the
composite.c layer for generating the descriptors and all the others will
prefer to provide their own descriptors.
I mean, depending on the function driver, we're gonna different
requirements WRT bandwidth, endpoints, etc. So, since you're gonna
little use of the generic solution, it's better not to have it. It won't
benefit many users.
> Please note that a gadget driver can also choose not to have SS descriptors
> generated for it all, even if it didn't provide any of its own and thus
> choosing not to operate in SS mode.
that's not what we want with the Linux gadget drivers. We want them to
be example (and productable) implementations working on all speeds. It's
like that because we want to, both, showcase the framework while also
letting those drivers be configurable enough so manufacturers can simply
re-use them.
Check nokia.c, that's a good example of a manufacturer re-using the open
source function drivers to provide a vendor specific solution :-)
> If you look at our implementation of the UAS gadget driver you'll see that
> we define the SS descriptors just as is done for HS/LS and those descriptors
> are chosen in enumeration.
then again, why do you need this Generic Solution ?
> I believe that in the future all gadget drivers will define their own SS
> descriptors with values that are suitable for that gadget driver and the
> create_ss_descriptors() will become obsolete. As I've already mentioned this
> is just a way to allow all the existing drivers to operate in SS mode with
> minimum changes to their code. So this patch doesn't contradict your
> approach, just extends it.
but that's not a good approach. You're adding bloat which will be
removed soon. Problem is that once it's there, we will have to support
it for a long long time as we won't know if there are any out of tree
gadget drivers depending on that, forcing us on publising of
Documentation/feature-removal-schedule.txt that we will remove that
layer in e.g. 4 major revisions. So, IMHO, better not to add it.
> > There are a few more comments below, but this patch needs major, major
> > splitting.
>
> I'll address your comments and split it up in the next version. I would like
> to give Greg and others a chance to comment as well before uploading another
> version.
Sure, makes sense :-)
--
balbi
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-03-28 8:54 ` Felipe Balbi
@ 2011-03-28 9:15 ` Tanya Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-03-28 9:15 UTC (permalink / raw)
To: balbi
Cc: gregkh, linux-arm-msm, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi
> > I believe that in the future all gadget drivers will define their own
> SS
> > descriptors with values that are suitable for that gadget driver and
> the
> > create_ss_descriptors() will become obsolete. As I've already
> mentioned this
> > is just a way to allow all the existing drivers to operate in SS mode
> with
> > minimum changes to their code. So this patch doesn't contradict your
> > approach, just extends it.
>
> but that's not a good approach. You're adding bloat which will be
> removed soon. Problem is that once it's there, we will have to support
> it for a long long time as we won't know if there are any out of tree
> gadget drivers depending on that, forcing us on publising of
> Documentation/feature-removal-schedule.txt that we will remove that
> layer in e.g. 4 major revisions. So, IMHO, better not to add it.
>
I see your point. I'll generate a different patch for this function
(create_ss_descriptors()) so it will be easy to take it out of the patch
series if we eventually decide against it. I think it's still useful to have
this ability since it can help developers at the beginning even if not
adding it to the formal kernel release and just have it in the mailing list
archives.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-03-28 9:15 ` Tanya Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-03-28 9:15 UTC (permalink / raw)
To: balbi
Cc: gregkh, linux-arm-msm, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi
> > I believe that in the future all gadget drivers will define their own
> SS
> > descriptors with values that are suitable for that gadget driver and
> the
> > create_ss_descriptors() will become obsolete. As I've already
> mentioned this
> > is just a way to allow all the existing drivers to operate in SS mode
> with
> > minimum changes to their code. So this patch doesn't contradict your
> > approach, just extends it.
>
> but that's not a good approach. You're adding bloat which will be
> removed soon. Problem is that once it's there, we will have to support
> it for a long long time as we won't know if there are any out of tree
> gadget drivers depending on that, forcing us on publising of
> Documentation/feature-removal-schedule.txt that we will remove that
> layer in e.g. 4 major revisions. So, IMHO, better not to add it.
>
I see your point. I'll generate a different patch for this function
(create_ss_descriptors()) so it will be easy to take it out of the patch
series if we eventually decide against it. I think it's still useful to have
this ability since it can help developers at the beginning even if not
adding it to the formal kernel release and just have it in the mailing list
archives.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-03-23 8:04 ` Tatyana Brokhman
@ 2011-04-11 17:59 ` Sebastian Andrzej Siewior
-1 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-11 17:59 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh-l3A5Bk7waGM, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
balbi-l0cyMroinI0, ablay-sgV2jX0FEOL9JmXXK+q4OQ,
open list:USB GADGET/PERIPH...,
open list
* Tatyana Brokhman | 2011-03-23 10:04:57 [+0200]:
>This patch adds the SuperSpeed functionality to the gadget framework.
>In order not to force all the gadget drivers to supply SuperSpeed
>descriptors when operating in SuperSpeed mode the following approach was
>taken:
>If we're operating in SuperSpeed mode and the gadget driver didn't supply
>SuperSpeed descriptors, the composite layer will automatically create
>SuperSpeed descriptors with default values.
>Support for new SuperSpeed BOS descriptor was added.
>Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
>added.
>
>Signed-off-by: Tatyana Brokhman <tlinder-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
>diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>index bc5123c..b4130bc 100644
>--- a/drivers/usb/gadget/Kconfig
>+++ b/drivers/usb/gadget/Kconfig
>@@ -605,6 +605,18 @@ config USB_GADGET_DUALSPEED
> Means that gadget drivers should include extra descriptors
> and code to handle dual-speed controllers.
>
>+config USB_GADGET_SUPERSPEED
>+ boolean "Gadget operating in Super Speed"
>+ depends on USB_GADGET
>+ depends on USB_GADGET_DUALSPEED
>+ default n
n is default. No need to set this.
>+ help
>+ Enabling this feature enables Super Speed support in the Gadget
>+ driver. It means that gadget drivers should provide extra (SuperSpeed)
>+ descriptors to the host.
>+ For composite devices: if SuperSpeed descriptors weren't supplied by
>+ the FD, they will be automatically generated with default values.
>+
> #
> # USB Gadget Drivers
> #
>diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>index fb7e488..d5fe1c2 100644
>--- a/drivers/usb/gadget/composite.c
>+++ b/drivers/usb/gadget/composite.c
>@@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
>
> static char composite_manufacturer[50];
>
>+/* Default endpoint companion descriptor */
>+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
>+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
>+ .bLength = 0x06,
sizeof() ?
>+ .bMaxBurst = 0, /* the default is we don't support bursting */
>+ .bmAttributes = 0, /* 2^0 streams supported */
>+ .wBytesPerInterval = 0,
It is already 0 :) It makes sense to set to 0 if you want to point out
something specific.
>+};
>+
>+/**
>+ * create_ss_descriptors() - Generate SuperSpeed descriptors
>+ * with default values
>+ * @f: pointer to usb_function to generate the descriptors for
>+ *
>+ * This function receives a pointer to usb_function and adds
>+ * missing super speed descriptors in the ss_descriptor field
>+ * according to its hs_descriptors field.
>+ *
>+ * This function copies f->hs_descriptors while updating the
>+ * endpoint descriptor and adding endpoint companion descriptor.
>+ */
>+static void create_ss_descriptors(struct usb_function *f)
>+{
>+ unsigned bytes; /* number of bytes to allocate */
>+ unsigned n_desc; /* number of descriptors */
>+ void *mem; /* allocated memory to copy to */
We don't put comments at the end. Here I think you should remove them
since the variable name should be enough.
>+ struct usb_descriptor_header **tmp;
>+ struct usb_endpoint_descriptor *ep_desc ;
>+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
>+ struct usb_descriptor_header **src = f->hs_descriptors;
It is better to read if you but the long types on top.
>+
>+ if (!f->hs_descriptors)
>+ return;
>+
>+ /*
>+ * Count number of EPs (in order to know how many SS_EP_COMPANION
>+ * descriptors to add), the total number of descriptors and the sum of
>+ * each descriptor bLength field in order to know how much memory to
>+ * allocate.
>+ */
>+ for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
>+ if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
>+ bytes += default_ep_comp_desc.bLength;
>+ n_desc++;
>+ }
>+ bytes += (*tmp)->bLength;
>+ }
>+
>+ bytes += (n_desc + 1) * sizeof(*tmp);
>+ mem = kmalloc(bytes, GFP_KERNEL);
>+ if (!mem)
>+ return;
>+
>+ /*
>+ * Fill in pointers starting at "tmp", to descriptors copied starting
>+ * at "mem" and return "ret"
>+ */
>+ tmp = mem;
>+ f->ss_descriptors = mem;
>+ mem += (n_desc + 1) * sizeof(*tmp);
>+ while (*src) {
>+ /* Copy the original descriptor */
>+ memcpy(mem, *src, (*src)->bLength);
>+ switch ((*src)->bDescriptorType) {
>+ case USB_DT_ENDPOINT:
>+ /* update ep descriptor */
>+ ep_desc = (struct usb_endpoint_descriptor *)mem;
>+ switch (ep_desc->bmAttributes &
>+ USB_ENDPOINT_XFERTYPE_MASK) {
>+ case USB_ENDPOINT_XFER_CONTROL:
>+ ep_desc->wMaxPacketSize = cpu_to_le16(512);
>+ ep_desc->bInterval = 0;
>+ break;
>+ case USB_ENDPOINT_XFER_BULK:
>+ ep_desc->wMaxPacketSize = cpu_to_le16(1024);
>+ ep_desc->bInterval = 0;
>+ break;
>+ case USB_ENDPOINT_XFER_INT:
>+ case USB_ENDPOINT_XFER_ISOC:
>+ break;
>+ }
>+ *tmp = mem;
>+ tmp++;
>+ mem += (*src)->bLength;
>+ /* add ep companion descriptor */
>+ memcpy(mem, &default_ep_comp_desc,
>+ default_ep_comp_desc.bLength);
>+ *tmp = mem;
>+ tmp++;
>+ /* Update wBytesPerInterval for periodic endpoints */
>+ ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
mem is void so that cast is not required.
>+ switch (ep_desc->bmAttributes &
>+ USB_ENDPOINT_XFERTYPE_MASK) {
>+ case USB_ENDPOINT_XFER_INT:
>+ case USB_ENDPOINT_XFER_ISOC:
>+ ep_comp_desc->wBytesPerInterval =
>+ ep_desc->wMaxPacketSize;
>+ break;
>+ }
>+ mem += default_ep_comp_desc.bLength;
>+ break;
>+ default:
>+ *tmp = mem;
>+ tmp++;
>+ mem += (*src)->bLength;
>+ break;
>+ }
>+ src++;
>+ }
>+ /*
>+ * The last (struct usb_descriptor_header *) in the descriptors
>+ * vector is NULL
>+ */
>+ *tmp = NULL;
>+ f->ss_desc_allocated = true;
>+}
>+
> /*-------------------------------------------------------------------------*/
> /**
> * next_ep_desc() - advance to the next EP descriptor
>@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> struct usb_endpoint_descriptor *chosen_desc = NULL;
> struct usb_descriptor_header **speed_desc = NULL;
>
>+ struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
>+ int want_comp_desc = 0;
>+
> struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>
> if (!g || !f || !_ep)
>@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
>
> /* select desired speed */
> switch (g->speed) {
>+ case USB_SPEED_SUPER:
>+ if (gadget_is_superspeed(g)) {
>+ speed_desc = f->ss_descriptors;
>+ want_comp_desc = 1;
>+ break;
>+ }
>+ /* else: Fall trough */
> case USB_SPEED_HIGH:
> if (gadget_is_dualspeed(g)) {
> speed_desc = f->hs_descriptors;
>@@ -147,7 +274,31 @@ ep_found:
> /* commit results */
> _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
> _ep->desc = chosen_desc;
>-
>+ _ep->comp_desc = NULL;
>+ _ep->maxburst = 0;
>+ _ep->mult = 0;
>+ if (want_comp_desc) {
>+ /*
>+ * Companion descriptor should follow EP descriptor
>+ * USB 3.0 spec, #9.6.7
>+ */
>+ comp_desc = (struct usb_ss_ep_comp_descriptor *)*(++d_spd);
>+ if (!comp_desc ||
>+ (comp_desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP))
>+ return -EIO;
>+ _ep->comp_desc = comp_desc;
>+ if (g->speed == USB_SPEED_SUPER) {
>+ int xfer_type = _ep->bEndpointAddress &
>+ USB_ENDPOINT_XFERTYPE_MASK ;
>+ if (xfer_type == USB_ENDPOINT_XFER_BULK)
>+ _ep->maxburst = comp_desc->bMaxBurst;
>+ else if (xfer_type == USB_ENDPOINT_XFER_INT)
>+ _ep->maxburst = comp_desc->bMaxBurst;
>+ else if (xfer_type == USB_ENDPOINT_XFER_ISOC)
>+ /* mult: bits 1:0 of bmAttributes */
>+ _ep->mult = comp_desc->bmAttributes & 0x3;
>+ }
>+ }
> return 0;
> }
>
>@@ -187,6 +338,14 @@ int usb_add_function(struct usb_configuration *config,
> list_del(&function->list);
> function->config = NULL;
> }
>+ /*
>+ * Add SS descriptors if there are any. This has to be done
>+ * after the bind since we need the hs_descriptors to be set in
>+ * usb_function and some of the FDs does it in the bind.
>+ */
>+ if ((gadget_is_superspeed(config->cdev->gadget)) &&
>+ (!function->ss_not_capable) && (!function->ss_descriptors))
>+ create_ss_descriptors(function);
> } else
> value = 0;
>
>@@ -199,6 +358,8 @@ int usb_add_function(struct usb_configuration *config,
> config->fullspeed = true;
> if (!config->highspeed && function->hs_descriptors)
> config->highspeed = true;
>+ if (!config->superspeed && function->ss_descriptors)
>+ config->superspeed = true;
>
> done:
> if (value)
>@@ -342,7 +503,9 @@ static int config_buf(struct usb_configuration *config,
> list_for_each_entry(f, &config->functions, list) {
> struct usb_descriptor_header **descriptors;
>
>- if (speed == USB_SPEED_HIGH)
>+ if (speed == USB_SPEED_SUPER)
>+ descriptors = f->ss_descriptors;
>+ else if (speed == USB_SPEED_HIGH)
> descriptors = f->hs_descriptors;
> else
> descriptors = f->descriptors;
>@@ -368,9 +531,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
> u8 type = w_value >> 8;
> enum usb_device_speed speed = USB_SPEED_UNKNOWN;
>
>- if (gadget_is_dualspeed(gadget)) {
>- int hs = 0;
>-
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ speed = gadget->speed;
>+ else if (gadget_is_dualspeed(gadget)) {
>+ int hs = 0;
> if (gadget->speed == USB_SPEED_HIGH)
> hs = 1;
> if (type == USB_DT_OTHER_SPEED_CONFIG)
>@@ -384,7 +548,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
> w_value &= 0xff;
> list_for_each_entry(c, &cdev->configs, list) {
> /* ignore configs that won't work at this speed */
>- if (speed == USB_SPEED_HIGH) {
>+ if (speed == USB_SPEED_SUPER) {
>+ if (!c->superspeed)
>+ continue;
>+ } else if (speed == USB_SPEED_HIGH) {
> if (!c->highspeed)
> continue;
> } else {
>@@ -404,16 +571,22 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
> struct usb_configuration *c;
> unsigned count = 0;
> int hs = 0;
>+ int ss = 0;
>
> if (gadget_is_dualspeed(gadget)) {
> if (gadget->speed == USB_SPEED_HIGH)
> hs = 1;
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ ss = 1;
> if (type == USB_DT_DEVICE_QUALIFIER)
> hs = !hs;
> }
> list_for_each_entry(c, &cdev->configs, list) {
> /* ignore configs that won't work at this speed */
>- if (hs) {
>+ if (ss) {
>+ if (!c->superspeed)
>+ continue;
>+ } else if (hs) {
> if (!c->highspeed)
> continue;
> } else {
>@@ -425,6 +598,73 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
> return count;
> }
>
>+/**
>+ * bos() - prepares the BOS descriptor.
>+ * @cdev: pointer to usb_composite device to generate the bos
>+ * descriptor for
>+ *
>+ * This function generates the BOS (Binary Device Object)
>+ * descriptor and its device capabilities descriptors. The BOS
>+ * descriptor should be supported by a SuperSpeed device.
>+ */
>+static int bos(struct usb_composite_dev *cdev)
>+{
>+ struct usb_bos_descriptor *bos = cdev->req->buf;
>+ struct usb_ext_cap_descriptor *usb_ext = NULL;
>+ struct usb_ss_cap_descriptor *ss_cap = NULL;
no need to NULL init. Please put the longer types before the short ones.
>+
>+ struct usb_dcd_config_params dcd_config_params;
>+
>+ bos->bLength = USB_DT_BOS_SIZE;
>+ bos->bDescriptorType = USB_DT_BOS;
>+
>+ bos->wTotalLength = USB_DT_BOS_SIZE;
cpu_to_le16(USB_DT_BOS_SIZE);
>+ bos->bNumDeviceCaps = 0;
>+
>+ /*
>+ * A SuperSpeed device shall include the USB2.0 extension descriptor
>+ * and shall support LPM when operating in USB2.0 HS mode.
>+ */
>+ usb_ext = (struct usb_ext_cap_descriptor *)
>+ (cdev->req->buf+bos->wTotalLength);
please run checkpatch.
>+ bos->bNumDeviceCaps++;
>+ bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
wTotalLength is __le16. Take a look at le16_add_cpu().
>+ usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
>+ usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>+ usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
>+ usb_ext->bmAttributes = USB_LPM_SUPPORT;
__le32
>+
>+ /*
>+ * The Superspeed USB Capability descriptor shall be implemented by all
>+ * SuperSpeed devices.
>+ */
>+ ss_cap = (struct usb_ss_cap_descriptor *)
>+ (cdev->req->buf+bos->wTotalLength);
>+ bos->bNumDeviceCaps++;
>+ bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
le16_add_cpu()
>+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
>+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
Who's fault is it? Isn't this always supported by the SS-udc?
>+ ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
>+ USB_FULL_SPEED_OPERATION |
>+ USB_HIGH_SPEED_OPERATION |
>+ USB_5GBPS_OPERATION;
__le16
>+ ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
>+
>+ /* Get Controller configuration */
>+ if (cdev->gadget->ops->get_config_params)
>+ cdev->gadget->ops->get_config_params(&dcd_config_params);
>+ else {
>+ dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
>+ dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
I guess this is __le16
>+ }
>+ ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
>+ ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
and I am sure that one is.
>+
>+ return bos->wTotalLength;
>+}
>+
> static void device_qual(struct usb_composite_dev *cdev)
> {
> struct usb_qualifier_descriptor *qual = cdev->req->buf;
>@@ -468,27 +708,42 @@ static int set_config(struct usb_composite_dev *cdev,
> unsigned power = gadget_is_otg(gadget) ? 8 : 100;
> int tmp;
>
>- if (cdev->config)
>- reset_config(cdev);
>-
> if (number) {
> list_for_each_entry(c, &cdev->configs, list) {
> if (c->bConfigurationValue == number) {
>+ /*
>+ * Need to disable the FDs of the previous
>+ * configuration
>+ */
>+ if (cdev->config)
>+ reset_config(cdev);
> result = 0;
> break;
> }
> }
> if (result < 0)
> goto done;
>- } else
>+ } else { /* Zero configuration value - need to reset the config */
>+ if (cdev->config)
>+ reset_config(cdev);
> result = 0;
>+ }
>
> INFO(cdev, "%s speed config #%d: %s\n",
> ({ char *speed;
> switch (gadget->speed) {
>- case USB_SPEED_LOW: speed = "low"; break;
>- case USB_SPEED_FULL: speed = "full"; break;
>- case USB_SPEED_HIGH: speed = "high"; break;
>+ case USB_SPEED_LOW:
>+ speed = "low";
>+ break;
>+ case USB_SPEED_FULL:
>+ speed = "full";
>+ break;
>+ case USB_SPEED_HIGH:
>+ speed = "high";
>+ break;
>+ case USB_SPEED_SUPER:
>+ speed = "super";
>+ break;
> default: speed = "?"; break;
> } ; speed; }), number, c ? c->label : "unconfigured");
>
>@@ -511,7 +766,9 @@ static int set_config(struct usb_composite_dev *cdev,
> * function's setup callback instead of the current
> * configuration's setup callback.
> */
>- if (gadget->speed == USB_SPEED_HIGH)
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ descriptors = f->ss_descriptors;
>+ else if (gadget->speed == USB_SPEED_HIGH)
> descriptors = f->hs_descriptors;
> else
> descriptors = f->descriptors;
>@@ -596,14 +853,14 @@ int usb_add_config(struct usb_composite_dev *cdev,
> } else {
> unsigned i;
>
>- DBG(cdev, "cfg %d/%p speeds:%s%s\n",
>+ DBG(cdev, "cfg %d/%p speeds:%s%s%s\n",
> config->bConfigurationValue, config,
>+ config->superspeed ? " super" : "",
> config->highspeed ? " high" : "",
> config->fullspeed
> ? (gadget_is_dualspeed(cdev->gadget)
> ? " full"
>- : " full/low")
>- : "");
>+ : " full/low") : "");
>
> for (i = 0; i < MAX_CONFIG_INTERFACES; i++) {
> struct usb_function *f = config->interface[i];
>@@ -721,7 +978,8 @@ static int get_string(struct usb_composite_dev *cdev,
> return s->bLength;
> }
>
>- /* Otherwise, look up and return a specified string. First
>+ /*
>+ * Otherwise, look up and return a specified string. First
> * check if the string has not been overridden.
> */
> if (cdev->manufacturer_override == id)
>@@ -876,6 +1134,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> struct usb_composite_dev *cdev = get_gadget_data(gadget);
> struct usb_request *req = cdev->req;
> int value = -EOPNOTSUPP;
>+ int status = 0;
> u16 w_index = le16_to_cpu(ctrl->wIndex);
> u8 intf = w_index & 0xFF;
> u16 w_value = le16_to_cpu(ctrl->wValue);
>@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> case USB_DT_DEVICE:
> cdev->desc.bNumConfigurations =
> count_configs(cdev, USB_DT_DEVICE);
>+ cdev->desc.bMaxPacketSize0 =
>+ cdev->gadget->ep0->maxpacket;
>+ if (gadget->speed >= USB_SPEED_SUPER)
there is something after super speed?
>+ cdev->desc.bcdUSB = cpu_to_le16(0x0300);
>+ else if ((gadget_is_superspeed(gadget)) &&
>+ (gadget->speed <= USB_SPEED_HIGH))
what about USB_SPEED_WIRELESS?
>+ cdev->desc.bcdUSB = cpu_to_le16(0x0210);
>+
> value = min(w_length, (u16) sizeof cdev->desc);
> memcpy(req->buf, &cdev->desc, value);
> break;
> case USB_DT_DEVICE_QUALIFIER:
>- if (!gadget_is_dualspeed(gadget))
>+ if (!gadget_is_dualspeed(gadget) ||
>+ gadget->speed >= USB_SPEED_SUPER)
> break;
> device_qual(cdev);
> value = min_t(int, w_length,
> sizeof(struct usb_qualifier_descriptor));
> break;
> case USB_DT_OTHER_SPEED_CONFIG:
>- if (!gadget_is_dualspeed(gadget))
>+ if (!gadget_is_dualspeed(gadget) ||
>+ gadget->speed >= USB_SPEED_SUPER)
> break;
> /* FALLTHROUGH */
> case USB_DT_CONFIG:
>@@ -928,6 +1197,12 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> if (value >= 0)
> value = min(w_length, (u16) value);
> break;
>+ case USB_DT_BOS:
>+ if (gadget_is_superspeed(gadget)) {
You return for <= USB_SPEED_HIGH bcdUSB with a value of 0x0210.
According to ch 9.2.6.6 you should handle GetDescriptor (BOS
Descriptor).
>+ value = bos(cdev);
>+ value = min(w_length, (u16) value);
>+ }
>+ break;
> }
> break;
>
>@@ -987,6 +1262,65 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> *((u8 *)req->buf) = value;
> value = min(w_length, (u16) 1);
> break;
>+
>+ /*
>+ * USB 3.0 additions:
>+ * Function driver should handle get_status request. If such cb
>+ * wasn't supplied we respond with default value = 0
>+ * Note: function driver should supply such cb only for the first
>+ * interface of the function
>+ */
>+ case USB_REQ_GET_STATUS:
>+ if (!gadget_is_superspeed(gadget))
>+ goto unknown;
>+ if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
>+ goto unknown;
>+ value = 2; /* This is the length of the get_status reply */
>+ *((u16 *)req->buf) = 0;
>+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
>+ break;
>+ f = cdev->config->interface[intf];
>+ if (!f)
>+ break;
>+ status = f->get_status ? f->get_status(f) : 0;
>+ if (status < 0)
>+ break;
>+ *((u16 *)req->buf) = status & 0x0000ffff;
My feeling is __le16 :)
>+ break;
>+ /*
>+ * Function drivers should handle SetFeature/ClearFeature
>+ * (FUNCTION_SUSPEND) request. function_suspend cb should be supplied
>+ * only for the first interface of the function
>+ */
>+ case USB_REQ_CLEAR_FEATURE:
>+ case USB_REQ_SET_FEATURE:
>+ if (!gadget_is_superspeed(gadget))
>+ goto unknown;
>+ if (ctrl->bRequestType != (USB_DIR_OUT | USB_RECIP_INTERFACE))
>+ goto unknown;
>+ switch (w_value) {
>+ case USB_INTRF_FUNC_SUSPEND:
>+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
>+ break;
>+ f = cdev->config->interface[intf];
>+ if (!f)
>+ break;
>+ value = f->func_suspend ?
>+ f->func_suspend(f,
>+ (u8)((w_index &
>+ USB_INTR_FUNC_SUSPEND_OPT_MASK)
>+ >> 8)) :
>+ 0;
What about something smaller like
value = 0;
if (f->func_suspend)
value = f->func_suspend(f, w_index >> 8);
>+ if (value < 0) {
>+ ERROR(cdev, "func_suspend() returned "
>+ "error %d\n", value);
>+ value = 0;
>+ }
>+ break;
>+ default:
>+ break;
>+ }
>+ break;
> default:
> unknown:
> VDBG(cdev,
>@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
> DBG(cdev, "unbind function '%s'/%p\n",
> f->name, f);
> f->unbind(c, f);
>- /* may free memory for "f" */
> }
>+ /* Free memory allocated for ss descriptors */
>+ if (f->ss_desc_allocated && f->ss_descriptors)
check for f->ss_descriptors is not required since kfree(null) is fine.
However this does no fly. f->unbind() will free the memmory of f. So
moving the comment does not delay the de-allocation :)
The comment is should probably use "will" instead of "may" since all
gadgets I checked do this.
The conditional allocation/free of the ss descriptor is kind of nasty.
Wouldn't it be better to add create_ss_descriptors() for every gadget
driver that needs it? This isn't that much. And in unbind you could
always free all three of them using one function like
free_all_descritpors().
>+ usb_free_descriptors(f->ss_descriptors);
>+ /* may free memory for "f" */
> }
> list_del(&c->list);
> if (c->unbind) {
>@@ -1346,6 +1683,9 @@ int usb_composite_probe(struct usb_composite_driver *driver,
> driver->iProduct = driver->name;
> composite_driver.function = (char *) driver->name;
> composite_driver.driver.name = driver->name;
>+#ifdef CONFIG_USB_GADGET_SUPERSPEED
>+ composite_driver.speed = USB_SPEED_SUPER;
>+#endif /* CONFIG_USB_GADGET_SUPERSPEED */
> composite = driver;
> composite_gadget_bind = bind;
>
>diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
>index de3461a..35c5818 100644
>--- a/drivers/usb/gadget/epautoconf.c
>+++ b/drivers/usb/gadget/epautoconf.c
>@@ -142,13 +142,13 @@ ep_matches (
> max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
> switch (type) {
> case USB_ENDPOINT_XFER_INT:
>- /* INT: limit 64 bytes full speed, 1024 high speed */
>+ /* INT: limit 64 bytes full speed, 1024 high/super speed */
> if (!gadget->is_dualspeed && max > 64)
> return 0;
> /* FALLTHROUGH */
>
> case USB_ENDPOINT_XFER_ISOC:
>- /* ISO: limit 1023 bytes full speed, 1024 high speed */
>+ /* ISO: limit 1023 bytes full speed, 1024 high/super speed */
> if (ep->maxpacket < max)
> return 0;
> if (!gadget->is_dualspeed && max > 1023)
>@@ -183,7 +183,7 @@ ep_matches (
> }
>
> /* report (variable) full speed bulk maxpacket */
>- if (USB_ENDPOINT_XFER_BULK == type) {
>+ if ((USB_ENDPOINT_XFER_BULK == type) & !gadget->is_dualspeed) {
> int size = ep->maxpacket;
>
> /* min() doesn't work on bitfields with gcc-3.5 */
>diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
>index 4f56e44..e9fa7d8 100644
>--- a/include/linux/usb/composite.h
>+++ b/include/linux/usb/composite.h
>@@ -51,6 +51,18 @@ struct usb_configuration;
> * @hs_descriptors: Table of high speed descriptors, using interface and
> * string identifiers assigned during @bind(). If this pointer is null,
> * the function will not be available at high speed.
>+ * @ss_descriptors: Table of super speed descriptors. If
>+ * wasnt supplied by the FD during @bind() and
>+ * !ss_not_capble, will be generated automaticly with
>+ * default values while working in superspeed mode. If this
>+ * pointer is null after initiation, the function will not
>+ * be available at super speed.
>+ * @ss_not_capable: This flag is used by the FD to indicate if
>+ * this function is SS capble. Meaning: if SS descriptors
>+ * weren't supplied by the FD, and the flag is set ss
>+ * descriptors will NOT be automatically generated
>+ * @ss_desc_allocated: This flag indicates whether the ss descriptors were
>+ * dynamically allocated (and needs to be released).
> * @config: assigned when @usb_add_function() is called; this is the
> * configuration with which this function is associated.
> * @bind: Before the gadget can register, all of its functions bind() to the
>@@ -69,6 +81,10 @@ struct usb_configuration;
> * @setup: Used for interface-specific control requests.
> * @suspend: Notifies functions when the host stops sending USB traffic.
> * @resume: Notifies functions when the host restarts USB traffic.
>+ * @get_status: Returns function status as a reply to
>+ * GetStatus() request when the recepient is Interface.
>+ * @func_suspend: callback to be called when
>+ * SetFeature(FUNCTION_SUSPEND) is reseived
> *
> * 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
>@@ -98,6 +114,10 @@ struct usb_function {
> struct usb_gadget_strings **strings;
> struct usb_descriptor_header **descriptors;
> struct usb_descriptor_header **hs_descriptors;
>+ struct usb_descriptor_header **ss_descriptors;
>+
>+ unsigned ss_desc_allocated:1;
>+ unsigned ss_not_capable:1;
>
> struct usb_configuration *config;
>
>@@ -124,6 +144,10 @@ struct usb_function {
> void (*suspend)(struct usb_function *);
> void (*resume)(struct usb_function *);
>
>+ /* USB 3.0 additions */
>+ int (*get_status)(struct usb_function *);
>+ int (*func_suspend)(struct usb_function *,
>+ u8 suspend_opt);
> /* private: */
> /* internals */
> struct list_head list;
>@@ -229,6 +253,7 @@ struct usb_configuration {
> struct list_head list;
> struct list_head functions;
> u8 next_interface_id;
>+ unsigned superspeed:1;
> unsigned highspeed:1;
> unsigned fullspeed:1;
> struct usb_function *interface[MAX_CONFIG_INTERFACES];
>diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>index 1bb7b3e..92c2d62 100644
>--- a/include/linux/usb/gadget.h
>+++ b/include/linux/usb/gadget.h
>@@ -131,11 +131,15 @@ struct usb_ep_ops {
> * @maxpacket:The maximum packet size used on this endpoint. The initial
> * value can sometimes be reduced (hardware allowing), according to
> * the endpoint descriptor used to configure the endpoint.
>+ * @mult: multiplier, 'mult' value for SS Isoc EPs
>+ * @maxburst: the maximum number of bursts supported by this EP (for usb3)
> * @driver_data:for use by the gadget driver.
> * @bEndpointAddress: used to identify the endpoint when finding
> * descriptor that matches connection speed
> * @desc: endpoint descriptor. This pointer is set before the endpoint is
> * enabled and remains valid until the endpoint is disabled.
>+ * @comp_desc: In case of SuperSpeed support, this is the endpoint companion
>+ * descriptor that is used to configure the endpoint
> *
> * the bus controller driver lists all the general purpose endpoints in
> * gadget->ep_list. the control endpoint (gadget->ep0) is not in that list,
>@@ -148,8 +152,12 @@ struct usb_ep {
> const struct usb_ep_ops *ops;
> struct list_head ep_list;
> unsigned maxpacket:16;
>+ unsigned mult:2;
>+ unsigned pad:1;
>+ unsigned maxburst:4;
> u8 bEndpointAddress;
> const struct usb_endpoint_descriptor *desc;
>+ const struct usb_ss_ep_comp_descriptor *comp_desc;
> };
>
> /*-------------------------------------------------------------------------*/
>@@ -417,6 +425,14 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
>
> /*-------------------------------------------------------------------------*/
>
>+struct usb_dcd_config_params {
>+ __u8 bU1devExitLat; /* U1 Device exit Latency */
>+#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
>+ __u16 bU2DevExitLat; /* U2 Device exit Latency */
isn't that __le16?
>+#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
>+};
>+
>+
> struct usb_gadget;
>
> /* the rest of the api to the controller hardware: device operations,
>@@ -431,6 +447,7 @@ struct usb_gadget_ops {
> int (*pullup) (struct usb_gadget *, int is_on);
> int (*ioctl)(struct usb_gadget *,
> unsigned code, unsigned long param);
>+ void (*get_config_params)(struct usb_dcd_config_params *);
> };
>
> /**
>@@ -522,6 +539,23 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
> }
>
> /**
>+ * gadget_is_superspeed() - return true if the hardware handles
>+ * supperspeed
>+ * @g: controller that might support supper speed
>+ */
>+static inline int gadget_is_superspeed(struct usb_gadget *g)
>+{
>+#ifdef CONFIG_USB_GADGET_SUPERSPEED
>+ /* runtime test would check "g->is_superspeed" ... that might be
>+ * useful to work around hardware bugs, but is mostly pointless
>+ */
>+ return 1;
>+#else
>+ return 0;
>+#endif
>+}
>+
>+/**
> * gadget_is_otg - return true iff the hardware is OTG-ready
> * @g: controller that might have a Mini-AB connector
> *
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-04-11 17:59 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-11 17:59 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay,
open list:USB GADGET/PERIPH...,
open list
* Tatyana Brokhman | 2011-03-23 10:04:57 [+0200]:
>This patch adds the SuperSpeed functionality to the gadget framework.
>In order not to force all the gadget drivers to supply SuperSpeed
>descriptors when operating in SuperSpeed mode the following approach was
>taken:
>If we're operating in SuperSpeed mode and the gadget driver didn't supply
>SuperSpeed descriptors, the composite layer will automatically create
>SuperSpeed descriptors with default values.
>Support for new SuperSpeed BOS descriptor was added.
>Support for SET_FEATURE and GET_STATUS requests in SuperSpeed mode was
>added.
>
>Signed-off-by: Tatyana Brokhman <tlinder@codeaurora.org>
>
>diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
>index bc5123c..b4130bc 100644
>--- a/drivers/usb/gadget/Kconfig
>+++ b/drivers/usb/gadget/Kconfig
>@@ -605,6 +605,18 @@ config USB_GADGET_DUALSPEED
> Means that gadget drivers should include extra descriptors
> and code to handle dual-speed controllers.
>
>+config USB_GADGET_SUPERSPEED
>+ boolean "Gadget operating in Super Speed"
>+ depends on USB_GADGET
>+ depends on USB_GADGET_DUALSPEED
>+ default n
n is default. No need to set this.
>+ help
>+ Enabling this feature enables Super Speed support in the Gadget
>+ driver. It means that gadget drivers should provide extra (SuperSpeed)
>+ descriptors to the host.
>+ For composite devices: if SuperSpeed descriptors weren't supplied by
>+ the FD, they will be automatically generated with default values.
>+
> #
> # USB Gadget Drivers
> #
>diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>index fb7e488..d5fe1c2 100644
>--- a/drivers/usb/gadget/composite.c
>+++ b/drivers/usb/gadget/composite.c
>@@ -73,6 +73,123 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
>
> static char composite_manufacturer[50];
>
>+/* Default endpoint companion descriptor */
>+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
>+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
>+ .bLength = 0x06,
sizeof() ?
>+ .bMaxBurst = 0, /* the default is we don't support bursting */
>+ .bmAttributes = 0, /* 2^0 streams supported */
>+ .wBytesPerInterval = 0,
It is already 0 :) It makes sense to set to 0 if you want to point out
something specific.
>+};
>+
>+/**
>+ * create_ss_descriptors() - Generate SuperSpeed descriptors
>+ * with default values
>+ * @f: pointer to usb_function to generate the descriptors for
>+ *
>+ * This function receives a pointer to usb_function and adds
>+ * missing super speed descriptors in the ss_descriptor field
>+ * according to its hs_descriptors field.
>+ *
>+ * This function copies f->hs_descriptors while updating the
>+ * endpoint descriptor and adding endpoint companion descriptor.
>+ */
>+static void create_ss_descriptors(struct usb_function *f)
>+{
>+ unsigned bytes; /* number of bytes to allocate */
>+ unsigned n_desc; /* number of descriptors */
>+ void *mem; /* allocated memory to copy to */
We don't put comments at the end. Here I think you should remove them
since the variable name should be enough.
>+ struct usb_descriptor_header **tmp;
>+ struct usb_endpoint_descriptor *ep_desc ;
>+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
>+ struct usb_descriptor_header **src = f->hs_descriptors;
It is better to read if you but the long types on top.
>+
>+ if (!f->hs_descriptors)
>+ return;
>+
>+ /*
>+ * Count number of EPs (in order to know how many SS_EP_COMPANION
>+ * descriptors to add), the total number of descriptors and the sum of
>+ * each descriptor bLength field in order to know how much memory to
>+ * allocate.
>+ */
>+ for (bytes = 0, n_desc = 0, tmp = src; *tmp; tmp++, n_desc++) {
>+ if ((*tmp)->bDescriptorType == USB_DT_ENDPOINT) {
>+ bytes += default_ep_comp_desc.bLength;
>+ n_desc++;
>+ }
>+ bytes += (*tmp)->bLength;
>+ }
>+
>+ bytes += (n_desc + 1) * sizeof(*tmp);
>+ mem = kmalloc(bytes, GFP_KERNEL);
>+ if (!mem)
>+ return;
>+
>+ /*
>+ * Fill in pointers starting at "tmp", to descriptors copied starting
>+ * at "mem" and return "ret"
>+ */
>+ tmp = mem;
>+ f->ss_descriptors = mem;
>+ mem += (n_desc + 1) * sizeof(*tmp);
>+ while (*src) {
>+ /* Copy the original descriptor */
>+ memcpy(mem, *src, (*src)->bLength);
>+ switch ((*src)->bDescriptorType) {
>+ case USB_DT_ENDPOINT:
>+ /* update ep descriptor */
>+ ep_desc = (struct usb_endpoint_descriptor *)mem;
>+ switch (ep_desc->bmAttributes &
>+ USB_ENDPOINT_XFERTYPE_MASK) {
>+ case USB_ENDPOINT_XFER_CONTROL:
>+ ep_desc->wMaxPacketSize = cpu_to_le16(512);
>+ ep_desc->bInterval = 0;
>+ break;
>+ case USB_ENDPOINT_XFER_BULK:
>+ ep_desc->wMaxPacketSize = cpu_to_le16(1024);
>+ ep_desc->bInterval = 0;
>+ break;
>+ case USB_ENDPOINT_XFER_INT:
>+ case USB_ENDPOINT_XFER_ISOC:
>+ break;
>+ }
>+ *tmp = mem;
>+ tmp++;
>+ mem += (*src)->bLength;
>+ /* add ep companion descriptor */
>+ memcpy(mem, &default_ep_comp_desc,
>+ default_ep_comp_desc.bLength);
>+ *tmp = mem;
>+ tmp++;
>+ /* Update wBytesPerInterval for periodic endpoints */
>+ ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
mem is void so that cast is not required.
>+ switch (ep_desc->bmAttributes &
>+ USB_ENDPOINT_XFERTYPE_MASK) {
>+ case USB_ENDPOINT_XFER_INT:
>+ case USB_ENDPOINT_XFER_ISOC:
>+ ep_comp_desc->wBytesPerInterval =
>+ ep_desc->wMaxPacketSize;
>+ break;
>+ }
>+ mem += default_ep_comp_desc.bLength;
>+ break;
>+ default:
>+ *tmp = mem;
>+ tmp++;
>+ mem += (*src)->bLength;
>+ break;
>+ }
>+ src++;
>+ }
>+ /*
>+ * The last (struct usb_descriptor_header *) in the descriptors
>+ * vector is NULL
>+ */
>+ *tmp = NULL;
>+ f->ss_desc_allocated = true;
>+}
>+
> /*-------------------------------------------------------------------------*/
> /**
> * next_ep_desc() - advance to the next EP descriptor
>@@ -118,6 +235,9 @@ int config_ep_by_speed(struct usb_gadget *g,
> struct usb_endpoint_descriptor *chosen_desc = NULL;
> struct usb_descriptor_header **speed_desc = NULL;
>
>+ struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
>+ int want_comp_desc = 0;
>+
> struct usb_descriptor_header **d_spd; /* cursor for speed desc */
>
> if (!g || !f || !_ep)
>@@ -125,6 +245,13 @@ int config_ep_by_speed(struct usb_gadget *g,
>
> /* select desired speed */
> switch (g->speed) {
>+ case USB_SPEED_SUPER:
>+ if (gadget_is_superspeed(g)) {
>+ speed_desc = f->ss_descriptors;
>+ want_comp_desc = 1;
>+ break;
>+ }
>+ /* else: Fall trough */
> case USB_SPEED_HIGH:
> if (gadget_is_dualspeed(g)) {
> speed_desc = f->hs_descriptors;
>@@ -147,7 +274,31 @@ ep_found:
> /* commit results */
> _ep->maxpacket = le16_to_cpu(chosen_desc->wMaxPacketSize);
> _ep->desc = chosen_desc;
>-
>+ _ep->comp_desc = NULL;
>+ _ep->maxburst = 0;
>+ _ep->mult = 0;
>+ if (want_comp_desc) {
>+ /*
>+ * Companion descriptor should follow EP descriptor
>+ * USB 3.0 spec, #9.6.7
>+ */
>+ comp_desc = (struct usb_ss_ep_comp_descriptor *)*(++d_spd);
>+ if (!comp_desc ||
>+ (comp_desc->bDescriptorType != USB_DT_SS_ENDPOINT_COMP))
>+ return -EIO;
>+ _ep->comp_desc = comp_desc;
>+ if (g->speed == USB_SPEED_SUPER) {
>+ int xfer_type = _ep->bEndpointAddress &
>+ USB_ENDPOINT_XFERTYPE_MASK ;
>+ if (xfer_type == USB_ENDPOINT_XFER_BULK)
>+ _ep->maxburst = comp_desc->bMaxBurst;
>+ else if (xfer_type == USB_ENDPOINT_XFER_INT)
>+ _ep->maxburst = comp_desc->bMaxBurst;
>+ else if (xfer_type == USB_ENDPOINT_XFER_ISOC)
>+ /* mult: bits 1:0 of bmAttributes */
>+ _ep->mult = comp_desc->bmAttributes & 0x3;
>+ }
>+ }
> return 0;
> }
>
>@@ -187,6 +338,14 @@ int usb_add_function(struct usb_configuration *config,
> list_del(&function->list);
> function->config = NULL;
> }
>+ /*
>+ * Add SS descriptors if there are any. This has to be done
>+ * after the bind since we need the hs_descriptors to be set in
>+ * usb_function and some of the FDs does it in the bind.
>+ */
>+ if ((gadget_is_superspeed(config->cdev->gadget)) &&
>+ (!function->ss_not_capable) && (!function->ss_descriptors))
>+ create_ss_descriptors(function);
> } else
> value = 0;
>
>@@ -199,6 +358,8 @@ int usb_add_function(struct usb_configuration *config,
> config->fullspeed = true;
> if (!config->highspeed && function->hs_descriptors)
> config->highspeed = true;
>+ if (!config->superspeed && function->ss_descriptors)
>+ config->superspeed = true;
>
> done:
> if (value)
>@@ -342,7 +503,9 @@ static int config_buf(struct usb_configuration *config,
> list_for_each_entry(f, &config->functions, list) {
> struct usb_descriptor_header **descriptors;
>
>- if (speed == USB_SPEED_HIGH)
>+ if (speed == USB_SPEED_SUPER)
>+ descriptors = f->ss_descriptors;
>+ else if (speed == USB_SPEED_HIGH)
> descriptors = f->hs_descriptors;
> else
> descriptors = f->descriptors;
>@@ -368,9 +531,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
> u8 type = w_value >> 8;
> enum usb_device_speed speed = USB_SPEED_UNKNOWN;
>
>- if (gadget_is_dualspeed(gadget)) {
>- int hs = 0;
>-
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ speed = gadget->speed;
>+ else if (gadget_is_dualspeed(gadget)) {
>+ int hs = 0;
> if (gadget->speed == USB_SPEED_HIGH)
> hs = 1;
> if (type == USB_DT_OTHER_SPEED_CONFIG)
>@@ -384,7 +548,10 @@ static int config_desc(struct usb_composite_dev *cdev, unsigned w_value)
> w_value &= 0xff;
> list_for_each_entry(c, &cdev->configs, list) {
> /* ignore configs that won't work at this speed */
>- if (speed == USB_SPEED_HIGH) {
>+ if (speed == USB_SPEED_SUPER) {
>+ if (!c->superspeed)
>+ continue;
>+ } else if (speed == USB_SPEED_HIGH) {
> if (!c->highspeed)
> continue;
> } else {
>@@ -404,16 +571,22 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
> struct usb_configuration *c;
> unsigned count = 0;
> int hs = 0;
>+ int ss = 0;
>
> if (gadget_is_dualspeed(gadget)) {
> if (gadget->speed == USB_SPEED_HIGH)
> hs = 1;
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ ss = 1;
> if (type == USB_DT_DEVICE_QUALIFIER)
> hs = !hs;
> }
> list_for_each_entry(c, &cdev->configs, list) {
> /* ignore configs that won't work at this speed */
>- if (hs) {
>+ if (ss) {
>+ if (!c->superspeed)
>+ continue;
>+ } else if (hs) {
> if (!c->highspeed)
> continue;
> } else {
>@@ -425,6 +598,73 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
> return count;
> }
>
>+/**
>+ * bos() - prepares the BOS descriptor.
>+ * @cdev: pointer to usb_composite device to generate the bos
>+ * descriptor for
>+ *
>+ * This function generates the BOS (Binary Device Object)
>+ * descriptor and its device capabilities descriptors. The BOS
>+ * descriptor should be supported by a SuperSpeed device.
>+ */
>+static int bos(struct usb_composite_dev *cdev)
>+{
>+ struct usb_bos_descriptor *bos = cdev->req->buf;
>+ struct usb_ext_cap_descriptor *usb_ext = NULL;
>+ struct usb_ss_cap_descriptor *ss_cap = NULL;
no need to NULL init. Please put the longer types before the short ones.
>+
>+ struct usb_dcd_config_params dcd_config_params;
>+
>+ bos->bLength = USB_DT_BOS_SIZE;
>+ bos->bDescriptorType = USB_DT_BOS;
>+
>+ bos->wTotalLength = USB_DT_BOS_SIZE;
cpu_to_le16(USB_DT_BOS_SIZE);
>+ bos->bNumDeviceCaps = 0;
>+
>+ /*
>+ * A SuperSpeed device shall include the USB2.0 extension descriptor
>+ * and shall support LPM when operating in USB2.0 HS mode.
>+ */
>+ usb_ext = (struct usb_ext_cap_descriptor *)
>+ (cdev->req->buf+bos->wTotalLength);
please run checkpatch.
>+ bos->bNumDeviceCaps++;
>+ bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
wTotalLength is __le16. Take a look at le16_add_cpu().
>+ usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
>+ usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>+ usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
>+ usb_ext->bmAttributes = USB_LPM_SUPPORT;
__le32
>+
>+ /*
>+ * The Superspeed USB Capability descriptor shall be implemented by all
>+ * SuperSpeed devices.
>+ */
>+ ss_cap = (struct usb_ss_cap_descriptor *)
>+ (cdev->req->buf+bos->wTotalLength);
>+ bos->bNumDeviceCaps++;
>+ bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
le16_add_cpu()
>+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
>+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
>+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
>+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
Who's fault is it? Isn't this always supported by the SS-udc?
>+ ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
>+ USB_FULL_SPEED_OPERATION |
>+ USB_HIGH_SPEED_OPERATION |
>+ USB_5GBPS_OPERATION;
__le16
>+ ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
>+
>+ /* Get Controller configuration */
>+ if (cdev->gadget->ops->get_config_params)
>+ cdev->gadget->ops->get_config_params(&dcd_config_params);
>+ else {
>+ dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
>+ dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
I guess this is __le16
>+ }
>+ ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
>+ ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
and I am sure that one is.
>+
>+ return bos->wTotalLength;
>+}
>+
> static void device_qual(struct usb_composite_dev *cdev)
> {
> struct usb_qualifier_descriptor *qual = cdev->req->buf;
>@@ -468,27 +708,42 @@ static int set_config(struct usb_composite_dev *cdev,
> unsigned power = gadget_is_otg(gadget) ? 8 : 100;
> int tmp;
>
>- if (cdev->config)
>- reset_config(cdev);
>-
> if (number) {
> list_for_each_entry(c, &cdev->configs, list) {
> if (c->bConfigurationValue == number) {
>+ /*
>+ * Need to disable the FDs of the previous
>+ * configuration
>+ */
>+ if (cdev->config)
>+ reset_config(cdev);
> result = 0;
> break;
> }
> }
> if (result < 0)
> goto done;
>- } else
>+ } else { /* Zero configuration value - need to reset the config */
>+ if (cdev->config)
>+ reset_config(cdev);
> result = 0;
>+ }
>
> INFO(cdev, "%s speed config #%d: %s\n",
> ({ char *speed;
> switch (gadget->speed) {
>- case USB_SPEED_LOW: speed = "low"; break;
>- case USB_SPEED_FULL: speed = "full"; break;
>- case USB_SPEED_HIGH: speed = "high"; break;
>+ case USB_SPEED_LOW:
>+ speed = "low";
>+ break;
>+ case USB_SPEED_FULL:
>+ speed = "full";
>+ break;
>+ case USB_SPEED_HIGH:
>+ speed = "high";
>+ break;
>+ case USB_SPEED_SUPER:
>+ speed = "super";
>+ break;
> default: speed = "?"; break;
> } ; speed; }), number, c ? c->label : "unconfigured");
>
>@@ -511,7 +766,9 @@ static int set_config(struct usb_composite_dev *cdev,
> * function's setup callback instead of the current
> * configuration's setup callback.
> */
>- if (gadget->speed == USB_SPEED_HIGH)
>+ if (gadget->speed == USB_SPEED_SUPER)
>+ descriptors = f->ss_descriptors;
>+ else if (gadget->speed == USB_SPEED_HIGH)
> descriptors = f->hs_descriptors;
> else
> descriptors = f->descriptors;
>@@ -596,14 +853,14 @@ int usb_add_config(struct usb_composite_dev *cdev,
> } else {
> unsigned i;
>
>- DBG(cdev, "cfg %d/%p speeds:%s%s\n",
>+ DBG(cdev, "cfg %d/%p speeds:%s%s%s\n",
> config->bConfigurationValue, config,
>+ config->superspeed ? " super" : "",
> config->highspeed ? " high" : "",
> config->fullspeed
> ? (gadget_is_dualspeed(cdev->gadget)
> ? " full"
>- : " full/low")
>- : "");
>+ : " full/low") : "");
>
> for (i = 0; i < MAX_CONFIG_INTERFACES; i++) {
> struct usb_function *f = config->interface[i];
>@@ -721,7 +978,8 @@ static int get_string(struct usb_composite_dev *cdev,
> return s->bLength;
> }
>
>- /* Otherwise, look up and return a specified string. First
>+ /*
>+ * Otherwise, look up and return a specified string. First
> * check if the string has not been overridden.
> */
> if (cdev->manufacturer_override == id)
>@@ -876,6 +1134,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> struct usb_composite_dev *cdev = get_gadget_data(gadget);
> struct usb_request *req = cdev->req;
> int value = -EOPNOTSUPP;
>+ int status = 0;
> u16 w_index = le16_to_cpu(ctrl->wIndex);
> u8 intf = w_index & 0xFF;
> u16 w_value = le16_to_cpu(ctrl->wValue);
>@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> case USB_DT_DEVICE:
> cdev->desc.bNumConfigurations =
> count_configs(cdev, USB_DT_DEVICE);
>+ cdev->desc.bMaxPacketSize0 =
>+ cdev->gadget->ep0->maxpacket;
>+ if (gadget->speed >= USB_SPEED_SUPER)
there is something after super speed?
>+ cdev->desc.bcdUSB = cpu_to_le16(0x0300);
>+ else if ((gadget_is_superspeed(gadget)) &&
>+ (gadget->speed <= USB_SPEED_HIGH))
what about USB_SPEED_WIRELESS?
>+ cdev->desc.bcdUSB = cpu_to_le16(0x0210);
>+
> value = min(w_length, (u16) sizeof cdev->desc);
> memcpy(req->buf, &cdev->desc, value);
> break;
> case USB_DT_DEVICE_QUALIFIER:
>- if (!gadget_is_dualspeed(gadget))
>+ if (!gadget_is_dualspeed(gadget) ||
>+ gadget->speed >= USB_SPEED_SUPER)
> break;
> device_qual(cdev);
> value = min_t(int, w_length,
> sizeof(struct usb_qualifier_descriptor));
> break;
> case USB_DT_OTHER_SPEED_CONFIG:
>- if (!gadget_is_dualspeed(gadget))
>+ if (!gadget_is_dualspeed(gadget) ||
>+ gadget->speed >= USB_SPEED_SUPER)
> break;
> /* FALLTHROUGH */
> case USB_DT_CONFIG:
>@@ -928,6 +1197,12 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> if (value >= 0)
> value = min(w_length, (u16) value);
> break;
>+ case USB_DT_BOS:
>+ if (gadget_is_superspeed(gadget)) {
You return for <= USB_SPEED_HIGH bcdUSB with a value of 0x0210.
According to ch 9.2.6.6 you should handle GetDescriptor (BOS
Descriptor).
>+ value = bos(cdev);
>+ value = min(w_length, (u16) value);
>+ }
>+ break;
> }
> break;
>
>@@ -987,6 +1262,65 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> *((u8 *)req->buf) = value;
> value = min(w_length, (u16) 1);
> break;
>+
>+ /*
>+ * USB 3.0 additions:
>+ * Function driver should handle get_status request. If such cb
>+ * wasn't supplied we respond with default value = 0
>+ * Note: function driver should supply such cb only for the first
>+ * interface of the function
>+ */
>+ case USB_REQ_GET_STATUS:
>+ if (!gadget_is_superspeed(gadget))
>+ goto unknown;
>+ if (ctrl->bRequestType != (USB_DIR_IN | USB_RECIP_INTERFACE))
>+ goto unknown;
>+ value = 2; /* This is the length of the get_status reply */
>+ *((u16 *)req->buf) = 0;
>+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
>+ break;
>+ f = cdev->config->interface[intf];
>+ if (!f)
>+ break;
>+ status = f->get_status ? f->get_status(f) : 0;
>+ if (status < 0)
>+ break;
>+ *((u16 *)req->buf) = status & 0x0000ffff;
My feeling is __le16 :)
>+ break;
>+ /*
>+ * Function drivers should handle SetFeature/ClearFeature
>+ * (FUNCTION_SUSPEND) request. function_suspend cb should be supplied
>+ * only for the first interface of the function
>+ */
>+ case USB_REQ_CLEAR_FEATURE:
>+ case USB_REQ_SET_FEATURE:
>+ if (!gadget_is_superspeed(gadget))
>+ goto unknown;
>+ if (ctrl->bRequestType != (USB_DIR_OUT | USB_RECIP_INTERFACE))
>+ goto unknown;
>+ switch (w_value) {
>+ case USB_INTRF_FUNC_SUSPEND:
>+ if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
>+ break;
>+ f = cdev->config->interface[intf];
>+ if (!f)
>+ break;
>+ value = f->func_suspend ?
>+ f->func_suspend(f,
>+ (u8)((w_index &
>+ USB_INTR_FUNC_SUSPEND_OPT_MASK)
>+ >> 8)) :
>+ 0;
What about something smaller like
value = 0;
if (f->func_suspend)
value = f->func_suspend(f, w_index >> 8);
>+ if (value < 0) {
>+ ERROR(cdev, "func_suspend() returned "
>+ "error %d\n", value);
>+ value = 0;
>+ }
>+ break;
>+ default:
>+ break;
>+ }
>+ break;
> default:
> unknown:
> VDBG(cdev,
>@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
> DBG(cdev, "unbind function '%s'/%p\n",
> f->name, f);
> f->unbind(c, f);
>- /* may free memory for "f" */
> }
>+ /* Free memory allocated for ss descriptors */
>+ if (f->ss_desc_allocated && f->ss_descriptors)
check for f->ss_descriptors is not required since kfree(null) is fine.
However this does no fly. f->unbind() will free the memmory of f. So
moving the comment does not delay the de-allocation :)
The comment is should probably use "will" instead of "may" since all
gadgets I checked do this.
The conditional allocation/free of the ss descriptor is kind of nasty.
Wouldn't it be better to add create_ss_descriptors() for every gadget
driver that needs it? This isn't that much. And in unbind you could
always free all three of them using one function like
free_all_descritpors().
>+ usb_free_descriptors(f->ss_descriptors);
>+ /* may free memory for "f" */
> }
> list_del(&c->list);
> if (c->unbind) {
>@@ -1346,6 +1683,9 @@ int usb_composite_probe(struct usb_composite_driver *driver,
> driver->iProduct = driver->name;
> composite_driver.function = (char *) driver->name;
> composite_driver.driver.name = driver->name;
>+#ifdef CONFIG_USB_GADGET_SUPERSPEED
>+ composite_driver.speed = USB_SPEED_SUPER;
>+#endif /* CONFIG_USB_GADGET_SUPERSPEED */
> composite = driver;
> composite_gadget_bind = bind;
>
>diff --git a/drivers/usb/gadget/epautoconf.c b/drivers/usb/gadget/epautoconf.c
>index de3461a..35c5818 100644
>--- a/drivers/usb/gadget/epautoconf.c
>+++ b/drivers/usb/gadget/epautoconf.c
>@@ -142,13 +142,13 @@ ep_matches (
> max = 0x7ff & le16_to_cpu(desc->wMaxPacketSize);
> switch (type) {
> case USB_ENDPOINT_XFER_INT:
>- /* INT: limit 64 bytes full speed, 1024 high speed */
>+ /* INT: limit 64 bytes full speed, 1024 high/super speed */
> if (!gadget->is_dualspeed && max > 64)
> return 0;
> /* FALLTHROUGH */
>
> case USB_ENDPOINT_XFER_ISOC:
>- /* ISO: limit 1023 bytes full speed, 1024 high speed */
>+ /* ISO: limit 1023 bytes full speed, 1024 high/super speed */
> if (ep->maxpacket < max)
> return 0;
> if (!gadget->is_dualspeed && max > 1023)
>@@ -183,7 +183,7 @@ ep_matches (
> }
>
> /* report (variable) full speed bulk maxpacket */
>- if (USB_ENDPOINT_XFER_BULK == type) {
>+ if ((USB_ENDPOINT_XFER_BULK == type) & !gadget->is_dualspeed) {
> int size = ep->maxpacket;
>
> /* min() doesn't work on bitfields with gcc-3.5 */
>diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
>index 4f56e44..e9fa7d8 100644
>--- a/include/linux/usb/composite.h
>+++ b/include/linux/usb/composite.h
>@@ -51,6 +51,18 @@ struct usb_configuration;
> * @hs_descriptors: Table of high speed descriptors, using interface and
> * string identifiers assigned during @bind(). If this pointer is null,
> * the function will not be available at high speed.
>+ * @ss_descriptors: Table of super speed descriptors. If
>+ * wasnt supplied by the FD during @bind() and
>+ * !ss_not_capble, will be generated automaticly with
>+ * default values while working in superspeed mode. If this
>+ * pointer is null after initiation, the function will not
>+ * be available at super speed.
>+ * @ss_not_capable: This flag is used by the FD to indicate if
>+ * this function is SS capble. Meaning: if SS descriptors
>+ * weren't supplied by the FD, and the flag is set ss
>+ * descriptors will NOT be automatically generated
>+ * @ss_desc_allocated: This flag indicates whether the ss descriptors were
>+ * dynamically allocated (and needs to be released).
> * @config: assigned when @usb_add_function() is called; this is the
> * configuration with which this function is associated.
> * @bind: Before the gadget can register, all of its functions bind() to the
>@@ -69,6 +81,10 @@ struct usb_configuration;
> * @setup: Used for interface-specific control requests.
> * @suspend: Notifies functions when the host stops sending USB traffic.
> * @resume: Notifies functions when the host restarts USB traffic.
>+ * @get_status: Returns function status as a reply to
>+ * GetStatus() request when the recepient is Interface.
>+ * @func_suspend: callback to be called when
>+ * SetFeature(FUNCTION_SUSPEND) is reseived
> *
> * 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
>@@ -98,6 +114,10 @@ struct usb_function {
> struct usb_gadget_strings **strings;
> struct usb_descriptor_header **descriptors;
> struct usb_descriptor_header **hs_descriptors;
>+ struct usb_descriptor_header **ss_descriptors;
>+
>+ unsigned ss_desc_allocated:1;
>+ unsigned ss_not_capable:1;
>
> struct usb_configuration *config;
>
>@@ -124,6 +144,10 @@ struct usb_function {
> void (*suspend)(struct usb_function *);
> void (*resume)(struct usb_function *);
>
>+ /* USB 3.0 additions */
>+ int (*get_status)(struct usb_function *);
>+ int (*func_suspend)(struct usb_function *,
>+ u8 suspend_opt);
> /* private: */
> /* internals */
> struct list_head list;
>@@ -229,6 +253,7 @@ struct usb_configuration {
> struct list_head list;
> struct list_head functions;
> u8 next_interface_id;
>+ unsigned superspeed:1;
> unsigned highspeed:1;
> unsigned fullspeed:1;
> struct usb_function *interface[MAX_CONFIG_INTERFACES];
>diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
>index 1bb7b3e..92c2d62 100644
>--- a/include/linux/usb/gadget.h
>+++ b/include/linux/usb/gadget.h
>@@ -131,11 +131,15 @@ struct usb_ep_ops {
> * @maxpacket:The maximum packet size used on this endpoint. The initial
> * value can sometimes be reduced (hardware allowing), according to
> * the endpoint descriptor used to configure the endpoint.
>+ * @mult: multiplier, 'mult' value for SS Isoc EPs
>+ * @maxburst: the maximum number of bursts supported by this EP (for usb3)
> * @driver_data:for use by the gadget driver.
> * @bEndpointAddress: used to identify the endpoint when finding
> * descriptor that matches connection speed
> * @desc: endpoint descriptor. This pointer is set before the endpoint is
> * enabled and remains valid until the endpoint is disabled.
>+ * @comp_desc: In case of SuperSpeed support, this is the endpoint companion
>+ * descriptor that is used to configure the endpoint
> *
> * the bus controller driver lists all the general purpose endpoints in
> * gadget->ep_list. the control endpoint (gadget->ep0) is not in that list,
>@@ -148,8 +152,12 @@ struct usb_ep {
> const struct usb_ep_ops *ops;
> struct list_head ep_list;
> unsigned maxpacket:16;
>+ unsigned mult:2;
>+ unsigned pad:1;
>+ unsigned maxburst:4;
> u8 bEndpointAddress;
> const struct usb_endpoint_descriptor *desc;
>+ const struct usb_ss_ep_comp_descriptor *comp_desc;
> };
>
> /*-------------------------------------------------------------------------*/
>@@ -417,6 +425,14 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
>
> /*-------------------------------------------------------------------------*/
>
>+struct usb_dcd_config_params {
>+ __u8 bU1devExitLat; /* U1 Device exit Latency */
>+#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
>+ __u16 bU2DevExitLat; /* U2 Device exit Latency */
isn't that __le16?
>+#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
>+};
>+
>+
> struct usb_gadget;
>
> /* the rest of the api to the controller hardware: device operations,
>@@ -431,6 +447,7 @@ struct usb_gadget_ops {
> int (*pullup) (struct usb_gadget *, int is_on);
> int (*ioctl)(struct usb_gadget *,
> unsigned code, unsigned long param);
>+ void (*get_config_params)(struct usb_dcd_config_params *);
> };
>
> /**
>@@ -522,6 +539,23 @@ static inline int gadget_is_dualspeed(struct usb_gadget *g)
> }
>
> /**
>+ * gadget_is_superspeed() - return true if the hardware handles
>+ * supperspeed
>+ * @g: controller that might support supper speed
>+ */
>+static inline int gadget_is_superspeed(struct usb_gadget *g)
>+{
>+#ifdef CONFIG_USB_GADGET_SUPERSPEED
>+ /* runtime test would check "g->is_superspeed" ... that might be
>+ * useful to work around hardware bugs, but is mostly pointless
>+ */
>+ return 1;
>+#else
>+ return 0;
>+#endif
>+}
>+
>+/**
> * gadget_is_otg - return true iff the hardware is OTG-ready
> * @g: controller that might have a Mini-AB connector
> *
Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-04-11 17:59 ` Sebastian Andrzej Siewior
(?)
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 2/5] usb/gadget: rename bos to get_bos_descr in composite Sebastian Andrzej Siewior
` (4 more replies)
-1 siblings, 5 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel
Hi Tatyana,
I tried to fixup this patch according to my review so you can focus on
the more imporant things :)
Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/5] usb/gadget: cleanup of "Add SuperSpeed support to the Gadget Framework"
2011-04-12 19:34 ` Sebastian Andrzej Siewior
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh-l3A5Bk7waGM, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
balbi-l0cyMroinI0, ablay-sgV2jX0FEOL9JmXXK+q4OQ,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sebastian Andrzej Siewior
style, some endianess fixups
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
drivers/usb/gadget/Kconfig | 1 -
drivers/usb/gadget/composite.c | 62 +++++++++++++++++----------------------
include/linux/usb/gadget.h | 2 +-
3 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b4130bc..21429c7 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -609,7 +609,6 @@ config USB_GADGET_SUPERSPEED
boolean "Gadget operating in Super Speed"
depends on USB_GADGET
depends on USB_GADGET_DUALSPEED
- default n
help
Enabling this feature enables Super Speed support in the Gadget
driver. It means that gadget drivers should provide extra (SuperSpeed)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index d5fe1c2..a94b7b7 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -76,10 +76,7 @@ static char composite_manufacturer[50];
/* Default endpoint companion descriptor */
static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
- .bLength = 0x06,
- .bMaxBurst = 0, /* the default is we don't support bursting */
- .bmAttributes = 0, /* 2^0 streams supported */
- .wBytesPerInterval = 0,
+ .bLength = sizeof(struct usb_ss_ep_comp_descriptor),
};
/**
@@ -96,13 +93,13 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
*/
static void create_ss_descriptors(struct usb_function *f)
{
- unsigned bytes; /* number of bytes to allocate */
- unsigned n_desc; /* number of descriptors */
- void *mem; /* allocated memory to copy to */
- struct usb_descriptor_header **tmp;
- struct usb_endpoint_descriptor *ep_desc ;
- struct usb_ss_ep_comp_descriptor *ep_comp_desc;
- struct usb_descriptor_header **src = f->hs_descriptors;
+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+ struct usb_endpoint_descriptor *ep_desc;
+ struct usb_descriptor_header **src = f->hs_descriptors;
+ struct usb_descriptor_header **tmp;
+ unsigned n_desc;
+ unsigned bytes;
+ void *mem;
if (!f->hs_descriptors)
return;
@@ -163,7 +160,7 @@ static void create_ss_descriptors(struct usb_function *f)
*tmp = mem;
tmp++;
/* Update wBytesPerInterval for periodic endpoints */
- ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
+ ep_comp_desc = mem;
switch (ep_desc->bmAttributes &
USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_INT:
@@ -609,47 +606,44 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
*/
static int bos(struct usb_composite_dev *cdev)
{
- struct usb_bos_descriptor *bos = cdev->req->buf;
- struct usb_ext_cap_descriptor *usb_ext = NULL;
- struct usb_ss_cap_descriptor *ss_cap = NULL;
-
+ struct usb_ext_cap_descriptor *usb_ext;
+ struct usb_ss_cap_descriptor *ss_cap;
struct usb_dcd_config_params dcd_config_params;
+ struct usb_bos_descriptor *bos = cdev->req->buf;
bos->bLength = USB_DT_BOS_SIZE;
bos->bDescriptorType = USB_DT_BOS;
- bos->wTotalLength = USB_DT_BOS_SIZE;
+ bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE);
bos->bNumDeviceCaps = 0;
/*
* A SuperSpeed device shall include the USB2.0 extension descriptor
* and shall support LPM when operating in USB2.0 HS mode.
*/
- usb_ext = (struct usb_ext_cap_descriptor *)
- (cdev->req->buf+bos->wTotalLength);
+ usb_ext = cdev->req->buf + bos->wTotalLength;
bos->bNumDeviceCaps++;
- bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
+ le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
- usb_ext->bmAttributes = USB_LPM_SUPPORT;
+ usb_ext->bmAttributes = cpu_to_le16(USB_LPM_SUPPORT);
/*
* The Superspeed USB Capability descriptor shall be implemented by all
* SuperSpeed devices.
*/
- ss_cap = (struct usb_ss_cap_descriptor *)
- (cdev->req->buf+bos->wTotalLength);
+ ss_cap = cdev->req->buf + bos->wTotalLength;
bos->bNumDeviceCaps++;
- bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
+ le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE);
ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
ss_cap->bmAttributes = 0; /* LTM is not supported yet */
- ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
+ ss_cap->wSpeedSupported = cpu_to_le16(USB_LOW_SPEED_OPERATION |
USB_FULL_SPEED_OPERATION |
USB_HIGH_SPEED_OPERATION |
- USB_5GBPS_OPERATION;
+ USB_5GBPS_OPERATION);
ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
/* Get Controller configuration */
@@ -657,12 +651,13 @@ static int bos(struct usb_composite_dev *cdev)
cdev->gadget->ops->get_config_params(&dcd_config_params);
else {
dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
- dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
+ dcd_config_params.bU2DevExitLat =
+ cpu_to_le16(USB_DEFULT_U2_DEV_EXIT_LAT);
}
ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
- return bos->wTotalLength;
+ return le16_to_cpu(bos->wTotalLength);
}
static void device_qual(struct usb_composite_dev *cdev)
@@ -1285,7 +1280,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
status = f->get_status ? f->get_status(f) : 0;
if (status < 0)
break;
- *((u16 *)req->buf) = status & 0x0000ffff;
+ *((u16 *)req->buf) = le16_to_cpu(status & 0x0000ffff);
break;
/*
* Function drivers should handle SetFeature/ClearFeature
@@ -1305,12 +1300,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
f = cdev->config->interface[intf];
if (!f)
break;
- value = f->func_suspend ?
- f->func_suspend(f,
- (u8)((w_index &
- USB_INTR_FUNC_SUSPEND_OPT_MASK)
- >> 8)) :
- 0;
+ value = 0;
+ if (f->func_suspend)
+ value = f->func_suspend(f, w_index >> 8);
if (value < 0) {
ERROR(cdev, "func_suspend() returned "
"error %d\n", value);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 92c2d62..9d496a4 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -428,7 +428,7 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
struct usb_dcd_config_params {
__u8 bU1devExitLat; /* U1 Device exit Latency */
#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
- __u16 bU2DevExitLat; /* U2 Device exit Latency */
+ __le16 bU2DevExitLat; /* U2 Device exit Latency */
#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
};
--
1.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/5] usb/gadget: cleanup of "Add SuperSpeed support to the Gadget Framework"
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel,
Sebastian Andrzej Siewior
style, some endianess fixups
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/usb/gadget/Kconfig | 1 -
drivers/usb/gadget/composite.c | 62 +++++++++++++++++----------------------
include/linux/usb/gadget.h | 2 +-
3 files changed, 28 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index b4130bc..21429c7 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -609,7 +609,6 @@ config USB_GADGET_SUPERSPEED
boolean "Gadget operating in Super Speed"
depends on USB_GADGET
depends on USB_GADGET_DUALSPEED
- default n
help
Enabling this feature enables Super Speed support in the Gadget
driver. It means that gadget drivers should provide extra (SuperSpeed)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index d5fe1c2..a94b7b7 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -76,10 +76,7 @@ static char composite_manufacturer[50];
/* Default endpoint companion descriptor */
static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
- .bLength = 0x06,
- .bMaxBurst = 0, /* the default is we don't support bursting */
- .bmAttributes = 0, /* 2^0 streams supported */
- .wBytesPerInterval = 0,
+ .bLength = sizeof(struct usb_ss_ep_comp_descriptor),
};
/**
@@ -96,13 +93,13 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
*/
static void create_ss_descriptors(struct usb_function *f)
{
- unsigned bytes; /* number of bytes to allocate */
- unsigned n_desc; /* number of descriptors */
- void *mem; /* allocated memory to copy to */
- struct usb_descriptor_header **tmp;
- struct usb_endpoint_descriptor *ep_desc ;
- struct usb_ss_ep_comp_descriptor *ep_comp_desc;
- struct usb_descriptor_header **src = f->hs_descriptors;
+ struct usb_ss_ep_comp_descriptor *ep_comp_desc;
+ struct usb_endpoint_descriptor *ep_desc;
+ struct usb_descriptor_header **src = f->hs_descriptors;
+ struct usb_descriptor_header **tmp;
+ unsigned n_desc;
+ unsigned bytes;
+ void *mem;
if (!f->hs_descriptors)
return;
@@ -163,7 +160,7 @@ static void create_ss_descriptors(struct usb_function *f)
*tmp = mem;
tmp++;
/* Update wBytesPerInterval for periodic endpoints */
- ep_comp_desc = (struct usb_ss_ep_comp_descriptor *)mem;
+ ep_comp_desc = mem;
switch (ep_desc->bmAttributes &
USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_INT:
@@ -609,47 +606,44 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
*/
static int bos(struct usb_composite_dev *cdev)
{
- struct usb_bos_descriptor *bos = cdev->req->buf;
- struct usb_ext_cap_descriptor *usb_ext = NULL;
- struct usb_ss_cap_descriptor *ss_cap = NULL;
-
+ struct usb_ext_cap_descriptor *usb_ext;
+ struct usb_ss_cap_descriptor *ss_cap;
struct usb_dcd_config_params dcd_config_params;
+ struct usb_bos_descriptor *bos = cdev->req->buf;
bos->bLength = USB_DT_BOS_SIZE;
bos->bDescriptorType = USB_DT_BOS;
- bos->wTotalLength = USB_DT_BOS_SIZE;
+ bos->wTotalLength = cpu_to_le16(USB_DT_BOS_SIZE);
bos->bNumDeviceCaps = 0;
/*
* A SuperSpeed device shall include the USB2.0 extension descriptor
* and shall support LPM when operating in USB2.0 HS mode.
*/
- usb_ext = (struct usb_ext_cap_descriptor *)
- (cdev->req->buf+bos->wTotalLength);
+ usb_ext = cdev->req->buf + bos->wTotalLength;
bos->bNumDeviceCaps++;
- bos->wTotalLength += USB_DT_USB_EXT_CAP_SIZE;
+ le16_add_cpu(&bos->wTotalLength, USB_DT_USB_EXT_CAP_SIZE);
usb_ext->bLength = USB_DT_USB_EXT_CAP_SIZE;
usb_ext->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
usb_ext->bDevCapabilityType = USB_CAP_TYPE_EXT;
- usb_ext->bmAttributes = USB_LPM_SUPPORT;
+ usb_ext->bmAttributes = cpu_to_le16(USB_LPM_SUPPORT);
/*
* The Superspeed USB Capability descriptor shall be implemented by all
* SuperSpeed devices.
*/
- ss_cap = (struct usb_ss_cap_descriptor *)
- (cdev->req->buf+bos->wTotalLength);
+ ss_cap = cdev->req->buf + bos->wTotalLength;
bos->bNumDeviceCaps++;
- bos->wTotalLength += USB_DT_USB_SS_CAP_SIZE;
+ le16_add_cpu(&bos->wTotalLength, USB_DT_USB_SS_CAP_SIZE);
ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
ss_cap->bmAttributes = 0; /* LTM is not supported yet */
- ss_cap->wSpeedSupported = USB_LOW_SPEED_OPERATION |
+ ss_cap->wSpeedSupported = cpu_to_le16(USB_LOW_SPEED_OPERATION |
USB_FULL_SPEED_OPERATION |
USB_HIGH_SPEED_OPERATION |
- USB_5GBPS_OPERATION;
+ USB_5GBPS_OPERATION);
ss_cap->bFunctionalitySupport = USB_LOW_SPEED_OPERATION;
/* Get Controller configuration */
@@ -657,12 +651,13 @@ static int bos(struct usb_composite_dev *cdev)
cdev->gadget->ops->get_config_params(&dcd_config_params);
else {
dcd_config_params.bU1devExitLat = USB_DEFULT_U1_DEV_EXIT_LAT;
- dcd_config_params.bU2DevExitLat = USB_DEFULT_U2_DEV_EXIT_LAT;
+ dcd_config_params.bU2DevExitLat =
+ cpu_to_le16(USB_DEFULT_U2_DEV_EXIT_LAT);
}
ss_cap->bU1devExitLat = dcd_config_params.bU1devExitLat;
ss_cap->bU2DevExitLat = dcd_config_params.bU2DevExitLat;
- return bos->wTotalLength;
+ return le16_to_cpu(bos->wTotalLength);
}
static void device_qual(struct usb_composite_dev *cdev)
@@ -1285,7 +1280,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
status = f->get_status ? f->get_status(f) : 0;
if (status < 0)
break;
- *((u16 *)req->buf) = status & 0x0000ffff;
+ *((u16 *)req->buf) = le16_to_cpu(status & 0x0000ffff);
break;
/*
* Function drivers should handle SetFeature/ClearFeature
@@ -1305,12 +1300,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
f = cdev->config->interface[intf];
if (!f)
break;
- value = f->func_suspend ?
- f->func_suspend(f,
- (u8)((w_index &
- USB_INTR_FUNC_SUSPEND_OPT_MASK)
- >> 8)) :
- 0;
+ value = 0;
+ if (f->func_suspend)
+ value = f->func_suspend(f, w_index >> 8);
if (value < 0) {
ERROR(cdev, "func_suspend() returned "
"error %d\n", value);
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 92c2d62..9d496a4 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -428,7 +428,7 @@ static inline void usb_ep_fifo_flush(struct usb_ep *ep)
struct usb_dcd_config_params {
__u8 bU1devExitLat; /* U1 Device exit Latency */
#define USB_DEFULT_U1_DEV_EXIT_LAT 0x01 /* Less then 1 microsec */
- __u16 bU2DevExitLat; /* U2 Device exit Latency */
+ __le16 bU2DevExitLat; /* U2 Device exit Latency */
#define USB_DEFULT_U2_DEV_EXIT_LAT 0x1F4 /* Less then 500 microsec */
};
--
1.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/5] usb/gadget: rename bos to get_bos_descr in composite
2011-04-12 19:34 ` Sebastian Andrzej Siewior
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel,
Sebastian Andrzej Siewior
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/usb/gadget/composite.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a94b7b7..d12e75a4 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -604,7 +604,7 @@ static int count_configs(struct usb_composite_dev *cdev, unsigned type)
* descriptor and its device capabilities descriptors. The BOS
* descriptor should be supported by a SuperSpeed device.
*/
-static int bos(struct usb_composite_dev *cdev)
+static int get_bos_descr(struct usb_composite_dev *cdev)
{
struct usb_ext_cap_descriptor *usb_ext;
struct usb_ss_cap_descriptor *ss_cap;
@@ -1194,7 +1194,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
break;
case USB_DT_BOS:
if (gadget_is_superspeed(gadget)) {
- value = bos(cdev);
+ value = get_bos_descr(cdev);
value = min(w_length, (u16) value);
}
break;
--
1.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors()
2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 2/5] usb/gadget: rename bos to get_bos_descr in composite Sebastian Andrzej Siewior
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled Sebastian Andrzej Siewior
` (2 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel,
Sebastian Andrzej Siewior
so have a namespace.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/usb/gadget/composite.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index d12e75a4..92cc238 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -80,7 +80,7 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
};
/**
- * create_ss_descriptors() - Generate SuperSpeed descriptors
+ * usb_create_ss_descriptors() - Generate SuperSpeed descriptors
* with default values
* @f: pointer to usb_function to generate the descriptors for
*
@@ -91,7 +91,7 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
* This function copies f->hs_descriptors while updating the
* endpoint descriptor and adding endpoint companion descriptor.
*/
-static void create_ss_descriptors(struct usb_function *f)
+void usb_create_ss_descriptors(struct usb_function *f)
{
struct usb_ss_ep_comp_descriptor *ep_comp_desc;
struct usb_endpoint_descriptor *ep_desc;
--
1.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled.
2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 2/5] usb/gadget: rename bos to get_bos_descr in composite Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-13 10:46 ` Sergei Shtylyov
[not found] ` <1302636896-12717-1-git-send-email-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2011-04-13 11:12 ` Tanya Brokhman
4 siblings, 1 reply; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel,
Sebastian Andrzej Siewior
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/usb/gadget/composite.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 92cc238..ac30e2f 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -74,10 +74,12 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
static char composite_manufacturer[50];
/* Default endpoint companion descriptor */
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
.bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
.bLength = sizeof(struct usb_ss_ep_comp_descriptor),
};
+#endif
/**
* usb_create_ss_descriptors() - Generate SuperSpeed descriptors
@@ -93,6 +95,7 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
*/
void usb_create_ss_descriptors(struct usb_function *f)
{
+#ifdef CONFIG_USB_GADGET_SUPERSPEED
struct usb_ss_ep_comp_descriptor *ep_comp_desc;
struct usb_endpoint_descriptor *ep_desc;
struct usb_descriptor_header **src = f->hs_descriptors;
@@ -185,6 +188,7 @@ void usb_create_ss_descriptors(struct usb_function *f)
*/
*tmp = NULL;
f->ss_desc_allocated = true;
+#endif
}
/*-------------------------------------------------------------------------*/
--
1.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5] usb/gadget: don't auto-create SS descriptors if HS are avilable
2011-04-12 19:34 ` Sebastian Andrzej Siewior
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh-l3A5Bk7waGM, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
balbi-l0cyMroinI0, ablay-sgV2jX0FEOL9JmXXK+q4OQ,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Sebastian Andrzej Siewior
Instead every gadget driver has to requests this to be done for him. A
gadget driver may use usb_create_ss_descriptors() if it wants a
descriptor based on its HS descriptor.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
drivers/usb/gadget/composite.c | 29 ++++++++++++++++-------------
drivers/usb/gadget/f_acm.c | 5 ++---
drivers/usb/gadget/f_audio.c | 4 ++--
drivers/usb/gadget/f_ecm.c | 11 ++++-------
drivers/usb/gadget/f_eem.c | 10 ++++------
drivers/usb/gadget/f_hid.c | 9 +++------
drivers/usb/gadget/f_loopback.c | 2 ++
drivers/usb/gadget/f_mass_storage.c | 9 +++++----
drivers/usb/gadget/f_ncm.c | 10 ++++------
drivers/usb/gadget/f_obex.c | 5 ++---
drivers/usb/gadget/f_phonet.c | 3 +++
drivers/usb/gadget/f_rndis.c | 14 ++++----------
drivers/usb/gadget/f_serial.c | 5 ++---
drivers/usb/gadget/f_sourcesink.c | 2 ++
drivers/usb/gadget/f_subset.c | 5 ++---
drivers/usb/gadget/f_uvc.c | 5 ++---
include/linux/usb/composite.h | 9 ---------
17 files changed, 59 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ac30e2f..c3c6cb1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -187,7 +187,6 @@ void usb_create_ss_descriptors(struct usb_function *f)
* vector is NULL
*/
*tmp = NULL;
- f->ss_desc_allocated = true;
#endif
}
@@ -304,6 +303,21 @@ ep_found:
}
/**
+ * usb_free_all_descriptors - free all allocated descriptors
+ * @f: function driver
+ *
+ * Simply remove all three usb descriptors which were dynamically allocated.
+ */
+void usb_free_all_descriptors(struct usb_function *f)
+{
+ usb_free_descriptors(f->ss_descriptors);
+ usb_free_descriptors(f->hs_descriptors);
+ usb_free_descriptors(f->descriptors);
+
+ f->ss_descriptors = f->hs_descriptors = f->descriptors = NULL;
+}
+
+/**
* usb_add_function() - add a function to a configuration
* @config: the configuration
* @function: the function being added
@@ -339,14 +353,6 @@ int usb_add_function(struct usb_configuration *config,
list_del(&function->list);
function->config = NULL;
}
- /*
- * Add SS descriptors if there are any. This has to be done
- * after the bind since we need the hs_descriptors to be set in
- * usb_function and some of the FDs does it in the bind.
- */
- if ((gadget_is_superspeed(config->cdev->gadget)) &&
- (!function->ss_not_capable) && (!function->ss_descriptors))
- create_ss_descriptors(function);
} else
value = 0;
@@ -1437,11 +1443,8 @@ composite_unbind(struct usb_gadget *gadget)
DBG(cdev, "unbind function '%s'/%p\n",
f->name, f);
f->unbind(c, f);
+ /* may free memory for "f" */
}
- /* Free memory allocated for ss descriptors */
- if (f->ss_desc_allocated && f->ss_descriptors)
- usb_free_descriptors(f->ss_descriptors);
- /* may free memory for "f" */
}
list_del(&c->list);
if (c->unbind) {
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 3f88493..aa2ceb4 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -642,6 +642,7 @@ acm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors */
f->hs_descriptors = usb_copy_descriptors(acm_hs_function);
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "acm ttyGS%d: %s speed IN/%s OUT/%s NOTIFY/%s\n",
@@ -673,9 +674,7 @@ acm_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct f_acm *acm = func_to_acm(f);
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
gs_free_req(acm->notify, acm->notify_req);
kfree(acm);
}
diff --git a/drivers/usb/gadget/f_audio.c b/drivers/usb/gadget/f_audio.c
index fdaa0a5..b9c054d 100644
--- a/drivers/usb/gadget/f_audio.c
+++ b/drivers/usb/gadget/f_audio.c
@@ -689,6 +689,7 @@ f_audio_bind(struct usb_configuration *c, struct usb_function *f)
if (gadget_is_dualspeed(c->cdev->gadget)) {
c->highspeed = true;
f->hs_descriptors = usb_copy_descriptors(f_audio_desc);
+ usb_create_ss_descriptors(f);
} else
f->descriptors = usb_copy_descriptors(f_audio_desc);
@@ -704,8 +705,7 @@ f_audio_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct f_audio *audio = func_to_audio(f);
- usb_free_descriptors(f->descriptors);
- usb_free_descriptors(f->hs_descriptors);
+ usb_free_all_descriptors(f);
kfree(audio);
}
diff --git a/drivers/usb/gadget/f_ecm.c b/drivers/usb/gadget/f_ecm.c
index ddedbc83..3899036 100644
--- a/drivers/usb/gadget/f_ecm.c
+++ b/drivers/usb/gadget/f_ecm.c
@@ -675,7 +675,8 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(ecm_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -694,8 +695,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (ecm->notify_req) {
kfree(ecm->notify_req->buf);
@@ -722,10 +722,7 @@ ecm_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "ecm unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
kfree(ecm->notify_req->buf);
usb_ep_free_request(ecm->notify, ecm->notify_req);
diff --git a/drivers/usb/gadget/f_eem.c b/drivers/usb/gadget/f_eem.c
index d28e61f..5a09eef 100644
--- a/drivers/usb/gadget/f_eem.c
+++ b/drivers/usb/gadget/f_eem.c
@@ -261,7 +261,8 @@ eem_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(eem_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -271,8 +272,7 @@ eem_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
/* we might as well release our claims on endpoints */
if (eem->port.out_ep->desc)
@@ -292,9 +292,7 @@ eem_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "eem unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(eem);
}
diff --git a/drivers/usb/gadget/f_hid.c b/drivers/usb/gadget/f_hid.c
index 403a48b..642a6bc 100644
--- a/drivers/usb/gadget/f_hid.c
+++ b/drivers/usb/gadget/f_hid.c
@@ -503,7 +503,7 @@ static int __init hidg_bind(struct usb_configuration *c, struct usb_function *f)
hidg_hs_in_ep_desc.bEndpointAddress =
hidg_fs_in_ep_desc.bEndpointAddress;
f->hs_descriptors = usb_copy_descriptors(hidg_hs_descriptors);
- if (!f->hs_descriptors)
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -531,9 +531,7 @@ fail:
usb_ep_free_request(hidg->in_ep, hidg->req);
}
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
return status;
}
@@ -551,8 +549,7 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
usb_ep_free_request(hidg->in_ep, hidg->req);
/* free descriptors copies */
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(hidg->report_desc);
kfree(hidg->set_report_buff);
diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 3756326..622a0df 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -173,6 +173,7 @@ autoconf_fail:
hs_loop_sink_desc.bEndpointAddress =
fs_loop_sink_desc.bEndpointAddress;
f->hs_descriptors = hs_loopback_descs;
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
@@ -184,6 +185,7 @@ autoconf_fail:
static void
loopback_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ usb_free_descriptors(f->ss_descriptors);
kfree(func_to_loop(f));
}
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index a688d04..a2da410 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -2989,8 +2989,8 @@ static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
}
fsg_common_put(common);
- usb_free_descriptors(fsg->function.descriptors);
- usb_free_descriptors(fsg->function.hs_descriptors);
+ usb_free_all_descriptors(f);
+
kfree(fsg);
}
@@ -3035,8 +3035,9 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
fsg_hs_bulk_out_desc.bEndpointAddress =
fsg_fs_bulk_out_desc.bEndpointAddress;
f->hs_descriptors = usb_copy_descriptors(fsg_hs_function);
- if (unlikely(!f->hs_descriptors)) {
- usb_free_descriptors(f->descriptors);
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors) {
+ usb_free_all_descriptors(f);
return -ENOMEM;
}
}
diff --git a/drivers/usb/gadget/f_ncm.c b/drivers/usb/gadget/f_ncm.c
index 681e5fc..f9ee06b 100644
--- a/drivers/usb/gadget/f_ncm.c
+++ b/drivers/usb/gadget/f_ncm.c
@@ -1237,7 +1237,8 @@ ncm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(ncm_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -1257,8 +1258,7 @@ ncm_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (ncm->notify_req) {
kfree(ncm->notify_req->buf);
@@ -1285,9 +1285,7 @@ ncm_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "ncm unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(ncm->notify_req->buf);
usb_ep_free_request(ncm->notify, ncm->notify_req);
diff --git a/drivers/usb/gadget/f_obex.c b/drivers/usb/gadget/f_obex.c
index 394502a..2aa8366 100644
--- a/drivers/usb/gadget/f_obex.c
+++ b/drivers/usb/gadget/f_obex.c
@@ -355,6 +355,7 @@ obex_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(hs_function);
+ usb_create_ss_descriptors(f);
}
/* Avoid letting this gadget enumerate until the userspace
@@ -390,9 +391,7 @@ fail:
static void
obex_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(func_to_obex(f));
}
diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index 0d6d260..47310d4 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -532,6 +532,7 @@ int pn_bind(struct usb_configuration *c, struct usb_function *f)
/* Do not try to bind Phonet twice... */
fp->function.descriptors = fs_pn_function;
fp->function.hs_descriptors = hs_pn_function;
+ usb_create_ss_descriptors(f);
/* Incoming USB requests */
status = -ENOMEM;
@@ -557,6 +558,7 @@ int pn_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
err:
+ usb_free_descriptors(f->ss_descriptors);
if (fp->out_ep)
fp->out_ep->driver_data = NULL;
if (fp->in_ep)
@@ -578,6 +580,7 @@ pn_unbind(struct usb_configuration *c, struct usb_function *f)
if (fp->out_reqv[i])
usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
+ usb_free_descriptors(f->ss_descriptors);
kfree(fp);
}
diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index 775a97d..46c859d 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -671,8 +671,8 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(eth_hs_function);
-
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -706,10 +706,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (gadget_is_dualspeed(c->cdev->gadget) && f->hs_descriptors)
- usb_free_descriptors(f->hs_descriptors);
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (rndis->notify_req) {
kfree(rndis->notify_req->buf);
@@ -737,10 +734,7 @@ rndis_unbind(struct usb_configuration *c, struct usb_function *f)
rndis_deregister(rndis->config);
rndis_exit();
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
kfree(rndis->notify_req->buf);
usb_ep_free_request(rndis->notify, rndis->notify_req);
diff --git a/drivers/usb/gadget/f_serial.c b/drivers/usb/gadget/f_serial.c
index 91fdf79..d9cb150 100644
--- a/drivers/usb/gadget/f_serial.c
+++ b/drivers/usb/gadget/f_serial.c
@@ -200,6 +200,7 @@ gser_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(gser_hs_function);
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "generic ttyGS%d: %s speed IN/%s OUT/%s\n",
@@ -223,9 +224,7 @@ fail:
static void
gser_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(func_to_gser(f));
}
diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c
index caf2f95..bf1c595 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -185,6 +185,7 @@ autoconf_fail:
hs_sink_desc.bEndpointAddress =
fs_sink_desc.bEndpointAddress;
f->hs_descriptors = hs_source_sink_descs;
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
@@ -196,6 +197,7 @@ autoconf_fail:
static void
sourcesink_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ usb_free_descriptors(f->ss_descriptors);
kfree(func_to_ss(f));
}
diff --git a/drivers/usb/gadget/f_subset.c b/drivers/usb/gadget/f_subset.c
index 93bf676..ada5b76 100644
--- a/drivers/usb/gadget/f_subset.c
+++ b/drivers/usb/gadget/f_subset.c
@@ -303,6 +303,7 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(hs_eth_function);
+ usb_create_ss_descriptors(f);
}
/* NOTE: all that is done without knowing or caring about
@@ -330,9 +331,7 @@ fail:
static void
geth_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
geth_string_defs[1].s = NULL;
kfree(func_to_geth(f));
}
diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index df74d03..f8c7673 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -481,9 +481,7 @@ uvc_function_unbind(struct usb_configuration *c, struct usb_function *f)
kfree(uvc->control_buf);
}
- kfree(f->descriptors);
- kfree(f->hs_descriptors);
-
+ usb_free_all_descriptors(f);
kfree(uvc);
}
@@ -530,6 +528,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
/* Copy descriptors. */
f->descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
+ usb_create_ss_descriptors(f);
/* Preallocate control endpoint request. */
uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index e9fa7d8..9f2254c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -57,12 +57,6 @@ struct usb_configuration;
* default values while working in superspeed mode. If this
* pointer is null after initiation, the function will not
* be available at super speed.
- * @ss_not_capable: This flag is used by the FD to indicate if
- * this function is SS capble. Meaning: if SS descriptors
- * weren't supplied by the FD, and the flag is set ss
- * descriptors will NOT be automatically generated
- * @ss_desc_allocated: This flag indicates whether the ss descriptors were
- * dynamically allocated (and needs to be released).
* @config: assigned when @usb_add_function() is called; this is the
* configuration with which this function is associated.
* @bind: Before the gadget can register, all of its functions bind() to the
@@ -116,9 +110,6 @@ struct usb_function {
struct usb_descriptor_header **hs_descriptors;
struct usb_descriptor_header **ss_descriptors;
- unsigned ss_desc_allocated:1;
- unsigned ss_not_capable:1;
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5] usb/gadget: don't auto-create SS descriptors if HS are avilable
@ 2011-04-12 19:34 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-12 19:34 UTC (permalink / raw)
To: Tatyana Brokhman
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel,
Sebastian Andrzej Siewior
Instead every gadget driver has to requests this to be done for him. A
gadget driver may use usb_create_ss_descriptors() if it wants a
descriptor based on its HS descriptor.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/usb/gadget/composite.c | 29 ++++++++++++++++-------------
drivers/usb/gadget/f_acm.c | 5 ++---
drivers/usb/gadget/f_audio.c | 4 ++--
drivers/usb/gadget/f_ecm.c | 11 ++++-------
drivers/usb/gadget/f_eem.c | 10 ++++------
drivers/usb/gadget/f_hid.c | 9 +++------
drivers/usb/gadget/f_loopback.c | 2 ++
drivers/usb/gadget/f_mass_storage.c | 9 +++++----
drivers/usb/gadget/f_ncm.c | 10 ++++------
drivers/usb/gadget/f_obex.c | 5 ++---
drivers/usb/gadget/f_phonet.c | 3 +++
drivers/usb/gadget/f_rndis.c | 14 ++++----------
drivers/usb/gadget/f_serial.c | 5 ++---
drivers/usb/gadget/f_sourcesink.c | 2 ++
drivers/usb/gadget/f_subset.c | 5 ++---
drivers/usb/gadget/f_uvc.c | 5 ++---
include/linux/usb/composite.h | 9 ---------
17 files changed, 59 insertions(+), 78 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index ac30e2f..c3c6cb1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -187,7 +187,6 @@ void usb_create_ss_descriptors(struct usb_function *f)
* vector is NULL
*/
*tmp = NULL;
- f->ss_desc_allocated = true;
#endif
}
@@ -304,6 +303,21 @@ ep_found:
}
/**
+ * usb_free_all_descriptors - free all allocated descriptors
+ * @f: function driver
+ *
+ * Simply remove all three usb descriptors which were dynamically allocated.
+ */
+void usb_free_all_descriptors(struct usb_function *f)
+{
+ usb_free_descriptors(f->ss_descriptors);
+ usb_free_descriptors(f->hs_descriptors);
+ usb_free_descriptors(f->descriptors);
+
+ f->ss_descriptors = f->hs_descriptors = f->descriptors = NULL;
+}
+
+/**
* usb_add_function() - add a function to a configuration
* @config: the configuration
* @function: the function being added
@@ -339,14 +353,6 @@ int usb_add_function(struct usb_configuration *config,
list_del(&function->list);
function->config = NULL;
}
- /*
- * Add SS descriptors if there are any. This has to be done
- * after the bind since we need the hs_descriptors to be set in
- * usb_function and some of the FDs does it in the bind.
- */
- if ((gadget_is_superspeed(config->cdev->gadget)) &&
- (!function->ss_not_capable) && (!function->ss_descriptors))
- create_ss_descriptors(function);
} else
value = 0;
@@ -1437,11 +1443,8 @@ composite_unbind(struct usb_gadget *gadget)
DBG(cdev, "unbind function '%s'/%p\n",
f->name, f);
f->unbind(c, f);
+ /* may free memory for "f" */
}
- /* Free memory allocated for ss descriptors */
- if (f->ss_desc_allocated && f->ss_descriptors)
- usb_free_descriptors(f->ss_descriptors);
- /* may free memory for "f" */
}
list_del(&c->list);
if (c->unbind) {
diff --git a/drivers/usb/gadget/f_acm.c b/drivers/usb/gadget/f_acm.c
index 3f88493..aa2ceb4 100644
--- a/drivers/usb/gadget/f_acm.c
+++ b/drivers/usb/gadget/f_acm.c
@@ -642,6 +642,7 @@ acm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors */
f->hs_descriptors = usb_copy_descriptors(acm_hs_function);
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "acm ttyGS%d: %s speed IN/%s OUT/%s NOTIFY/%s\n",
@@ -673,9 +674,7 @@ acm_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct f_acm *acm = func_to_acm(f);
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
gs_free_req(acm->notify, acm->notify_req);
kfree(acm);
}
diff --git a/drivers/usb/gadget/f_audio.c b/drivers/usb/gadget/f_audio.c
index fdaa0a5..b9c054d 100644
--- a/drivers/usb/gadget/f_audio.c
+++ b/drivers/usb/gadget/f_audio.c
@@ -689,6 +689,7 @@ f_audio_bind(struct usb_configuration *c, struct usb_function *f)
if (gadget_is_dualspeed(c->cdev->gadget)) {
c->highspeed = true;
f->hs_descriptors = usb_copy_descriptors(f_audio_desc);
+ usb_create_ss_descriptors(f);
} else
f->descriptors = usb_copy_descriptors(f_audio_desc);
@@ -704,8 +705,7 @@ f_audio_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct f_audio *audio = func_to_audio(f);
- usb_free_descriptors(f->descriptors);
- usb_free_descriptors(f->hs_descriptors);
+ usb_free_all_descriptors(f);
kfree(audio);
}
diff --git a/drivers/usb/gadget/f_ecm.c b/drivers/usb/gadget/f_ecm.c
index ddedbc83..3899036 100644
--- a/drivers/usb/gadget/f_ecm.c
+++ b/drivers/usb/gadget/f_ecm.c
@@ -675,7 +675,8 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(ecm_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -694,8 +695,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (ecm->notify_req) {
kfree(ecm->notify_req->buf);
@@ -722,10 +722,7 @@ ecm_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "ecm unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
kfree(ecm->notify_req->buf);
usb_ep_free_request(ecm->notify, ecm->notify_req);
diff --git a/drivers/usb/gadget/f_eem.c b/drivers/usb/gadget/f_eem.c
index d28e61f..5a09eef 100644
--- a/drivers/usb/gadget/f_eem.c
+++ b/drivers/usb/gadget/f_eem.c
@@ -261,7 +261,8 @@ eem_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(eem_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -271,8 +272,7 @@ eem_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
/* we might as well release our claims on endpoints */
if (eem->port.out_ep->desc)
@@ -292,9 +292,7 @@ eem_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "eem unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(eem);
}
diff --git a/drivers/usb/gadget/f_hid.c b/drivers/usb/gadget/f_hid.c
index 403a48b..642a6bc 100644
--- a/drivers/usb/gadget/f_hid.c
+++ b/drivers/usb/gadget/f_hid.c
@@ -503,7 +503,7 @@ static int __init hidg_bind(struct usb_configuration *c, struct usb_function *f)
hidg_hs_in_ep_desc.bEndpointAddress =
hidg_fs_in_ep_desc.bEndpointAddress;
f->hs_descriptors = usb_copy_descriptors(hidg_hs_descriptors);
- if (!f->hs_descriptors)
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -531,9 +531,7 @@ fail:
usb_ep_free_request(hidg->in_ep, hidg->req);
}
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
return status;
}
@@ -551,8 +549,7 @@ static void hidg_unbind(struct usb_configuration *c, struct usb_function *f)
usb_ep_free_request(hidg->in_ep, hidg->req);
/* free descriptors copies */
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(hidg->report_desc);
kfree(hidg->set_report_buff);
diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
index 3756326..622a0df 100644
--- a/drivers/usb/gadget/f_loopback.c
+++ b/drivers/usb/gadget/f_loopback.c
@@ -173,6 +173,7 @@ autoconf_fail:
hs_loop_sink_desc.bEndpointAddress =
fs_loop_sink_desc.bEndpointAddress;
f->hs_descriptors = hs_loopback_descs;
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
@@ -184,6 +185,7 @@ autoconf_fail:
static void
loopback_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ usb_free_descriptors(f->ss_descriptors);
kfree(func_to_loop(f));
}
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index a688d04..a2da410 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -2989,8 +2989,8 @@ static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
}
fsg_common_put(common);
- usb_free_descriptors(fsg->function.descriptors);
- usb_free_descriptors(fsg->function.hs_descriptors);
+ usb_free_all_descriptors(f);
+
kfree(fsg);
}
@@ -3035,8 +3035,9 @@ static int fsg_bind(struct usb_configuration *c, struct usb_function *f)
fsg_hs_bulk_out_desc.bEndpointAddress =
fsg_fs_bulk_out_desc.bEndpointAddress;
f->hs_descriptors = usb_copy_descriptors(fsg_hs_function);
- if (unlikely(!f->hs_descriptors)) {
- usb_free_descriptors(f->descriptors);
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors) {
+ usb_free_all_descriptors(f);
return -ENOMEM;
}
}
diff --git a/drivers/usb/gadget/f_ncm.c b/drivers/usb/gadget/f_ncm.c
index 681e5fc..f9ee06b 100644
--- a/drivers/usb/gadget/f_ncm.c
+++ b/drivers/usb/gadget/f_ncm.c
@@ -1237,7 +1237,8 @@ ncm_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(ncm_hs_function);
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -1257,8 +1258,7 @@ ncm_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (ncm->notify_req) {
kfree(ncm->notify_req->buf);
@@ -1285,9 +1285,7 @@ ncm_unbind(struct usb_configuration *c, struct usb_function *f)
DBG(c->cdev, "ncm unbind\n");
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(ncm->notify_req->buf);
usb_ep_free_request(ncm->notify, ncm->notify_req);
diff --git a/drivers/usb/gadget/f_obex.c b/drivers/usb/gadget/f_obex.c
index 394502a..2aa8366 100644
--- a/drivers/usb/gadget/f_obex.c
+++ b/drivers/usb/gadget/f_obex.c
@@ -355,6 +355,7 @@ obex_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(hs_function);
+ usb_create_ss_descriptors(f);
}
/* Avoid letting this gadget enumerate until the userspace
@@ -390,9 +391,7 @@ fail:
static void
obex_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(func_to_obex(f));
}
diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index 0d6d260..47310d4 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -532,6 +532,7 @@ int pn_bind(struct usb_configuration *c, struct usb_function *f)
/* Do not try to bind Phonet twice... */
fp->function.descriptors = fs_pn_function;
fp->function.hs_descriptors = hs_pn_function;
+ usb_create_ss_descriptors(f);
/* Incoming USB requests */
status = -ENOMEM;
@@ -557,6 +558,7 @@ int pn_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
err:
+ usb_free_descriptors(f->ss_descriptors);
if (fp->out_ep)
fp->out_ep->driver_data = NULL;
if (fp->in_ep)
@@ -578,6 +580,7 @@ pn_unbind(struct usb_configuration *c, struct usb_function *f)
if (fp->out_reqv[i])
usb_ep_free_request(fp->out_ep, fp->out_reqv[i]);
+ usb_free_descriptors(f->ss_descriptors);
kfree(fp);
}
diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
index 775a97d..46c859d 100644
--- a/drivers/usb/gadget/f_rndis.c
+++ b/drivers/usb/gadget/f_rndis.c
@@ -671,8 +671,8 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(eth_hs_function);
-
- if (!f->hs_descriptors)
+ usb_create_ss_descriptors(f);
+ if (!f->hs_descriptors || !f->ss_descriptors)
goto fail;
}
@@ -706,10 +706,7 @@ rndis_bind(struct usb_configuration *c, struct usb_function *f)
return 0;
fail:
- if (gadget_is_dualspeed(c->cdev->gadget) && f->hs_descriptors)
- usb_free_descriptors(f->hs_descriptors);
- if (f->descriptors)
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
if (rndis->notify_req) {
kfree(rndis->notify_req->buf);
@@ -737,10 +734,7 @@ rndis_unbind(struct usb_configuration *c, struct usb_function *f)
rndis_deregister(rndis->config);
rndis_exit();
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
-
+ usb_free_all_descriptors(f);
kfree(rndis->notify_req->buf);
usb_ep_free_request(rndis->notify, rndis->notify_req);
diff --git a/drivers/usb/gadget/f_serial.c b/drivers/usb/gadget/f_serial.c
index 91fdf79..d9cb150 100644
--- a/drivers/usb/gadget/f_serial.c
+++ b/drivers/usb/gadget/f_serial.c
@@ -200,6 +200,7 @@ gser_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(gser_hs_function);
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "generic ttyGS%d: %s speed IN/%s OUT/%s\n",
@@ -223,9 +224,7 @@ fail:
static void
gser_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
kfree(func_to_gser(f));
}
diff --git a/drivers/usb/gadget/f_sourcesink.c b/drivers/usb/gadget/f_sourcesink.c
index caf2f95..bf1c595 100644
--- a/drivers/usb/gadget/f_sourcesink.c
+++ b/drivers/usb/gadget/f_sourcesink.c
@@ -185,6 +185,7 @@ autoconf_fail:
hs_sink_desc.bEndpointAddress =
fs_sink_desc.bEndpointAddress;
f->hs_descriptors = hs_source_sink_descs;
+ usb_create_ss_descriptors(f);
}
DBG(cdev, "%s speed %s: IN/%s, OUT/%s\n",
@@ -196,6 +197,7 @@ autoconf_fail:
static void
sourcesink_unbind(struct usb_configuration *c, struct usb_function *f)
{
+ usb_free_descriptors(f->ss_descriptors);
kfree(func_to_ss(f));
}
diff --git a/drivers/usb/gadget/f_subset.c b/drivers/usb/gadget/f_subset.c
index 93bf676..ada5b76 100644
--- a/drivers/usb/gadget/f_subset.c
+++ b/drivers/usb/gadget/f_subset.c
@@ -303,6 +303,7 @@ geth_bind(struct usb_configuration *c, struct usb_function *f)
/* copy descriptors, and track endpoint copies */
f->hs_descriptors = usb_copy_descriptors(hs_eth_function);
+ usb_create_ss_descriptors(f);
}
/* NOTE: all that is done without knowing or caring about
@@ -330,9 +331,7 @@ fail:
static void
geth_unbind(struct usb_configuration *c, struct usb_function *f)
{
- if (gadget_is_dualspeed(c->cdev->gadget))
- usb_free_descriptors(f->hs_descriptors);
- usb_free_descriptors(f->descriptors);
+ usb_free_all_descriptors(f);
geth_string_defs[1].s = NULL;
kfree(func_to_geth(f));
}
diff --git a/drivers/usb/gadget/f_uvc.c b/drivers/usb/gadget/f_uvc.c
index df74d03..f8c7673 100644
--- a/drivers/usb/gadget/f_uvc.c
+++ b/drivers/usb/gadget/f_uvc.c
@@ -481,9 +481,7 @@ uvc_function_unbind(struct usb_configuration *c, struct usb_function *f)
kfree(uvc->control_buf);
}
- kfree(f->descriptors);
- kfree(f->hs_descriptors);
-
+ usb_free_all_descriptors(f);
kfree(uvc);
}
@@ -530,6 +528,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
/* Copy descriptors. */
f->descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
+ usb_create_ss_descriptors(f);
/* Preallocate control endpoint request. */
uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index e9fa7d8..9f2254c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -57,12 +57,6 @@ struct usb_configuration;
* default values while working in superspeed mode. If this
* pointer is null after initiation, the function will not
* be available at super speed.
- * @ss_not_capable: This flag is used by the FD to indicate if
- * this function is SS capble. Meaning: if SS descriptors
- * weren't supplied by the FD, and the flag is set ss
- * descriptors will NOT be automatically generated
- * @ss_desc_allocated: This flag indicates whether the ss descriptors were
- * dynamically allocated (and needs to be released).
* @config: assigned when @usb_add_function() is called; this is the
* configuration with which this function is associated.
* @bind: Before the gadget can register, all of its functions bind() to the
@@ -116,9 +110,6 @@ struct usb_function {
struct usb_descriptor_header **hs_descriptors;
struct usb_descriptor_header **ss_descriptors;
- unsigned ss_desc_allocated:1;
- unsigned ss_not_capable:1;
-
struct usb_configuration *config;
/* REVISIT: bind() functions can be marked __init, which
--
1.7.4
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] usb/gadget: cleanup of "Add SuperSpeed support to the Gadget Framework"
2011-04-12 19:34 ` Sebastian Andrzej Siewior
(?)
@ 2011-04-13 8:17 ` Felipe Balbi
-1 siblings, 0 replies; 28+ messages in thread
From: Felipe Balbi @ 2011-04-13 8:17 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Tatyana Brokhman, gregkh, linux-arm-msm, balbi, ablay, linux-usb,
linux-kernel
Hi,
On Tue, Apr 12, 2011 at 09:34:52PM +0200, Sebastian Andrzej Siewior wrote:
> style, some endianess fixups
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> drivers/usb/gadget/Kconfig | 1 -
> drivers/usb/gadget/composite.c | 62 +++++++++++++++++----------------------
> include/linux/usb/gadget.h | 2 +-
> 3 files changed, 28 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index b4130bc..21429c7 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -609,7 +609,6 @@ config USB_GADGET_SUPERSPEED
> boolean "Gadget operating in Super Speed"
> depends on USB_GADGET
> depends on USB_GADGET_DUALSPEED
> - default n
> help
> Enabling this feature enables Super Speed support in the Gadget
> driver. It means that gadget drivers should provide extra (SuperSpeed)
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index d5fe1c2..a94b7b7 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -76,10 +76,7 @@ static char composite_manufacturer[50];
> /* Default endpoint companion descriptor */
> static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> - .bLength = 0x06,
> - .bMaxBurst = 0, /* the default is we don't support bursting */
> - .bmAttributes = 0, /* 2^0 streams supported */
> - .wBytesPerInterval = 0,
> + .bLength = sizeof(struct usb_ss_ep_comp_descriptor),
actually, there's a define:
#define USB_DT_SS_EP_COMP_SIZE 6
--
balbi
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled.
2011-04-12 19:34 ` [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled Sebastian Andrzej Siewior
@ 2011-04-13 10:46 ` Sergei Shtylyov
[not found] ` <4DA57F0F.1090609-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
0 siblings, 1 reply; 28+ messages in thread
From: Sergei Shtylyov @ 2011-04-13 10:46 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Tatyana Brokhman, gregkh, linux-arm-msm, balbi, ablay, linux-usb,
linux-kernel
Hello.
On 12-04-2011 23:34, Sebastian Andrzej Siewior wrote:
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> drivers/usb/gadget/composite.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index 92cc238..ac30e2f 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -74,10 +74,12 @@ MODULE_PARM_DESC(iSerialNumber, "SerialNumber string");
> static char composite_manufacturer[50];
>
> /* Default endpoint companion descriptor */
> +#ifdef CONFIG_USB_GADGET_SUPERSPEED
> static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> .bLength = sizeof(struct usb_ss_ep_comp_descriptor),
> };
> +#endif
>
> /**
> * usb_create_ss_descriptors() - Generate SuperSpeed descriptors
> @@ -93,6 +95,7 @@ static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> */
> void usb_create_ss_descriptors(struct usb_function *f)
> {
> +#ifdef CONFIG_USB_GADGET_SUPERSPEED
> struct usb_ss_ep_comp_descriptor *ep_comp_desc;
> struct usb_endpoint_descriptor *ep_desc;
> struct usb_descriptor_header **src = f->hs_descriptors;
> @@ -185,6 +188,7 @@ void usb_create_ss_descriptors(struct usb_function *f)
> */
> *tmp = NULL;
> f->ss_desc_allocated = true;
> +#endif
Documentation/SubmittingPatches says this:
2) #ifdefs are ugly
Code cluttered with ifdefs is difficult to read and maintain. Don't do
it. Instead, put your ifdefs in a header, and conditionally define
'static inline' functions, or macros, which are used in the code.
Let the compiler optimize away the "no-op" case.
Simple example, of poor code:
dev = alloc_etherdev (sizeof(struct funky_private));
if (!dev)
return -ENODEV;
#ifdef CONFIG_NET_FUNKINESS
init_funky_net(dev);
#endif
Cleaned-up example:
(in header)
#ifndef CONFIG_NET_FUNKINESS
static inline void init_funky_net (struct net_device *d) {}
#endif
(in the code itself)
dev = alloc_etherdev (sizeof(struct funky_private));
if (!dev)
return -ENODEV;
init_funky_net(dev);
WBR, Sergei
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled.
2011-04-13 10:46 ` Sergei Shtylyov
@ 2011-04-13 10:56 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-13 10:56 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Tatyana Brokhman, gregkh-l3A5Bk7waGM,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, balbi-l0cyMroinI0,
ablay-sgV2jX0FEOL9JmXXK+q4OQ, linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Sergei Shtylyov wrote:
> Hello.
Hello Sergei,
> Cleaned-up example:
>
> (in header)
> #ifndef CONFIG_NET_FUNKINESS
> static inline void init_funky_net (struct net_device *d) {}
> #endif
>
> (in the code itself)
> dev = alloc_etherdev (sizeof(struct funky_private));
> if (!dev)
> return -ENODEV;
> init_funky_net(dev);
The problem here is that the code is included via
#include "composite.c
so we don't really use header files. The alternative would be to use the
gadget_is_super_speed() function but that one takes a gadget as argument.
Preferences?
>
> WBR, Sergei
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled.
@ 2011-04-13 10:56 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2011-04-13 10:56 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Tatyana Brokhman, gregkh, linux-arm-msm, balbi, ablay, linux-usb,
linux-kernel
Sergei Shtylyov wrote:
> Hello.
Hello Sergei,
> Cleaned-up example:
>
> (in header)
> #ifndef CONFIG_NET_FUNKINESS
> static inline void init_funky_net (struct net_device *d) {}
> #endif
>
> (in the code itself)
> dev = alloc_etherdev (sizeof(struct funky_private));
> if (!dev)
> return -ENODEV;
> init_funky_net(dev);
The problem here is that the code is included via
#include "composite.c
so we don't really use header files. The alternative would be to use the
gadget_is_super_speed() function but that one takes a gadget as argument.
Preferences?
>
> WBR, Sergei
Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled.
2011-04-13 10:56 ` Sebastian Andrzej Siewior
(?)
@ 2011-04-13 10:59 ` Michal Nazarewicz
-1 siblings, 0 replies; 28+ messages in thread
From: Michal Nazarewicz @ 2011-04-13 10:59 UTC (permalink / raw)
To: Sergei Shtylyov, Sebastian Andrzej Siewior
Cc: Tatyana Brokhman, gregkh, linux-arm-msm, balbi, ablay, linux-usb,
linux-kernel
On Wed, 13 Apr 2011 12:56:57 +0200, Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
> Sergei Shtylyov wrote:
>> Hello.
>
> Hello Sergei,
>
>> Cleaned-up example:
>> (in header)
>> #ifndef CONFIG_NET_FUNKINESS
>> static inline void init_funky_net (struct net_device *d) {}
>> #endif
>> (in the code itself)
>> dev = alloc_etherdev (sizeof(struct funky_private));
>> if (!dev)
>> return -ENODEV;
>> init_funky_net(dev);
>
> The problem here is that the code is included via
>
> #include "composite.c
>
> so we don't really use header files. The alternative would be to use the
> gadget_is_super_speed() function but that one takes a gadget as argument.
> Preferences?
#ifdef CONFIG_USB_GADGET_SUPERSPEED
void usb_create_ss_descriptors(struct usb_function *f)
{
/* ... */
}
#else
static inline void usb_create_ss_descriptors(struct usb_function *f) { }
#endif
Is usually preferred.
--
Best regards, _ _
.o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o
..o | Computer Science, Michal "mina86" Nazarewicz (o o)
ooo +-----<email/xmpp: mnazarewicz@google.com>-----ooO--(_)--Ooo--
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-04-12 19:34 ` Sebastian Andrzej Siewior
@ 2011-04-13 11:12 ` Tanya Brokhman
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
` (3 subsequent siblings)
4 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-04-13 11:12 UTC (permalink / raw)
To: 'Sebastian Andrzej Siewior'
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel
Hi Sebastian
Thanks you for your inputs and for taking the time to address them!
I got several other comments on this patch, one of which was to split it up.
I'm working on it at the moment and will address your comments as well in
the new version of this patch.
Again - thanks you for your time and work.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> -----Original Message-----
> From: Sebastian Andrzej Siewior [mailto:bigeasy@linutronix.de]
> Sent: Tuesday, April 12, 2011 10:35 PM
> To: Tatyana Brokhman
> Cc: gregkh@suse.de; linux-arm-msm@vger.kernel.org; balbi@ti.com;
> ablay@codeaurora.org; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the
> Gadget Framework
>
> Hi Tatyana,
>
> I tried to fixup this patch according to my review so you can focus on
> the more imporant things :)
>
> Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-04-13 11:12 ` Tanya Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-04-13 11:12 UTC (permalink / raw)
To: 'Sebastian Andrzej Siewior'
Cc: gregkh, linux-arm-msm, balbi, ablay, linux-usb, linux-kernel
Hi Sebastian
Thanks you for your inputs and for taking the time to address them!
I got several other comments on this patch, one of which was to split it up.
I'm working on it at the moment and will address your comments as well in
the new version of this patch.
Again - thanks you for your time and work.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
> -----Original Message-----
> From: Sebastian Andrzej Siewior [mailto:bigeasy@linutronix.de]
> Sent: Tuesday, April 12, 2011 10:35 PM
> To: Tatyana Brokhman
> Cc: gregkh@suse.de; linux-arm-msm@vger.kernel.org; balbi@ti.com;
> ablay@codeaurora.org; linux-usb@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the
> Gadget Framework
>
> Hi Tatyana,
>
> I tried to fixup this patch according to my review so you can focus on
> the more imporant things :)
>
> Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
2011-04-11 17:59 ` Sebastian Andrzej Siewior
@ 2011-04-14 7:36 ` Tanya Brokhman
-1 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-04-14 7:36 UTC (permalink / raw)
To: 'Sebastian Andrzej Siewior'
Cc: gregkh, linux-arm-msm, balbi, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi Sebastian,
> >+/* Default endpoint companion descriptor */
> >+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> >+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> >+ .bLength = 0x06,
> sizeof() ?
>
> >+ .bMaxBurst = 0, /* the default is we don't support bursting
> */
> >+ .bmAttributes = 0, /* 2^0 streams supported */
> >+ .wBytesPerInterval = 0,
> It is already 0 :) It makes sense to set to 0 if you want to point out
> something specific.
Just wanted all the values to be visible/specified. When reading the code
one can forget that wBytesPerInterval field is a part of this descriptor.
Since these are default values I thought that this "reminder" is better.
> >+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
> >+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> >+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
> >+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
>
> Who's fault is it? Isn't this always supported by the SS-udc?
I didn't find anything in the USB30 Spec that mentions that LTM support is
required from all SS-UDCs.
We're working on a SS-UDC at the moment and we decide that LTM support will
be added later on.
My guess is that when we do add this support we'll add some sort of a cb
from the UDC to set this value according to.
> >@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget,
> const struct usb_ctrlrequest *ctrl)
> > case USB_DT_DEVICE:
> > cdev->desc.bNumConfigurations =
> > count_configs(cdev, USB_DT_DEVICE);
> >+ cdev->desc.bMaxPacketSize0 =
> >+ cdev->gadget->ep0->maxpacket;
> >+ if (gadget->speed >= USB_SPEED_SUPER)
> there is something after super speed?
Not at the moment but I believe might be in the future :-)
> >@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
> > DBG(cdev, "unbind function '%s'/%p\n",
> > f->name, f);
> > f->unbind(c, f);
> >- /* may free memory for "f" */
> > }
> >+ /* Free memory allocated for ss descriptors */
> >+ if (f->ss_desc_allocated && f->ss_descriptors)
>
> check for f->ss_descriptors is not required since kfree(null) is fine.
> However this does no fly. f->unbind() will free the memmory of f. So
> moving the comment does not delay the de-allocation :)
> The comment is should probably use "will" instead of "may" since all
> gadgets I checked do this.
> The conditional allocation/free of the ss descriptor is kind of nasty.
> Wouldn't it be better to add create_ss_descriptors() for every gadget
> driver that needs it? This isn't that much. And in unbind you could
> always free all three of them using one function like
> free_all_descritpors().
You're right in your observation that the above is not the best way to
handle this. I've re-designed this a bit in the next version.
We don't want to create ss descriptors for all of the gadget drivers
automatically. First of all because this way all of the gadget drivers will
be forced to operate over SS connection if such is available. The purpose of
the function->ss_not_capable flag was to allow a gadget driver to force
HS/LS connection even if SS is available.
Anyway this two flags (ss_not_capable and ss_desc_allocated) were replaced
by another flag (function->generate_ss_desc) that should be set to true by
any gadget driver that wishes ss descriptors to be automatically generated
for it.
We discussed this with Felipe and came to a conclusion that this function
(create_ss_descriptors()) will be taken out to a different patch that won't
be merged into the mainline, just posted on the mailing list to help
developers in early stages of their work.
In the long run the better approach is that each gadget driver that wishes
to operate in ss connection speed should provide his own ss descriptors with
values that are suitable for it.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 28+ messages in thread
* RE: [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework
@ 2011-04-14 7:36 ` Tanya Brokhman
0 siblings, 0 replies; 28+ messages in thread
From: Tanya Brokhman @ 2011-04-14 7:36 UTC (permalink / raw)
To: 'Sebastian Andrzej Siewior'
Cc: gregkh, linux-arm-msm, balbi, ablay,
'open list:USB GADGET/PERIPH...', 'open list'
Hi Sebastian,
> >+/* Default endpoint companion descriptor */
> >+static struct usb_ss_ep_comp_descriptor default_ep_comp_desc = {
> >+ .bDescriptorType = USB_DT_SS_ENDPOINT_COMP,
> >+ .bLength = 0x06,
> sizeof() ?
>
> >+ .bMaxBurst = 0, /* the default is we don't support bursting
> */
> >+ .bmAttributes = 0, /* 2^0 streams supported */
> >+ .wBytesPerInterval = 0,
> It is already 0 :) It makes sense to set to 0 if you want to point out
> something specific.
Just wanted all the values to be visible/specified. When reading the code
one can forget that wBytesPerInterval field is a part of this descriptor.
Since these are default values I thought that this "reminder" is better.
> >+ ss_cap->bLength = USB_DT_USB_SS_CAP_SIZE;
> >+ ss_cap->bDescriptorType = USB_DT_DEVICE_CAPABILITY;
> >+ ss_cap->bDevCapabilityType = USB_SS_CAP_TYPE;
> >+ ss_cap->bmAttributes = 0; /* LTM is not supported yet */
>
> Who's fault is it? Isn't this always supported by the SS-udc?
I didn't find anything in the USB30 Spec that mentions that LTM support is
required from all SS-UDCs.
We're working on a SS-UDC at the moment and we decide that LTM support will
be added later on.
My guess is that when we do add this support we'll add some sort of a cb
from the UDC to set this value according to.
> >@@ -903,18 +1162,28 @@ composite_setup(struct usb_gadget *gadget,
> const struct usb_ctrlrequest *ctrl)
> > case USB_DT_DEVICE:
> > cdev->desc.bNumConfigurations =
> > count_configs(cdev, USB_DT_DEVICE);
> >+ cdev->desc.bMaxPacketSize0 =
> >+ cdev->gadget->ep0->maxpacket;
> >+ if (gadget->speed >= USB_SPEED_SUPER)
> there is something after super speed?
Not at the moment but I believe might be in the future :-)
> >@@ -1107,8 +1441,11 @@ composite_unbind(struct usb_gadget *gadget)
> > DBG(cdev, "unbind function '%s'/%p\n",
> > f->name, f);
> > f->unbind(c, f);
> >- /* may free memory for "f" */
> > }
> >+ /* Free memory allocated for ss descriptors */
> >+ if (f->ss_desc_allocated && f->ss_descriptors)
>
> check for f->ss_descriptors is not required since kfree(null) is fine.
> However this does no fly. f->unbind() will free the memmory of f. So
> moving the comment does not delay the de-allocation :)
> The comment is should probably use "will" instead of "may" since all
> gadgets I checked do this.
> The conditional allocation/free of the ss descriptor is kind of nasty.
> Wouldn't it be better to add create_ss_descriptors() for every gadget
> driver that needs it? This isn't that much. And in unbind you could
> always free all three of them using one function like
> free_all_descritpors().
You're right in your observation that the above is not the best way to
handle this. I've re-designed this a bit in the next version.
We don't want to create ss descriptors for all of the gadget drivers
automatically. First of all because this way all of the gadget drivers will
be forced to operate over SS connection if such is available. The purpose of
the function->ss_not_capable flag was to allow a gadget driver to force
HS/LS connection even if SS is available.
Anyway this two flags (ss_not_capable and ss_desc_allocated) were replaced
by another flag (function->generate_ss_desc) that should be set to true by
any gadget driver that wishes ss descriptors to be automatically generated
for it.
We discussed this with Felipe and came to a conclusion that this function
(create_ss_descriptors()) will be taken out to a different patch that won't
be merged into the mainline, just posted on the mailing list to help
developers in early stages of their work.
In the long run the better approach is that each gadget driver that wishes
to operate in ss connection speed should provide his own ss descriptors with
values that are suitable for it.
Best regards,
Tanya Brokhman
Consultant for Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-04-14 7:35 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-23 8:04 [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework Tatyana Brokhman
2011-03-23 8:04 ` Tatyana Brokhman
[not found] ` <1300867498-20997-1-git-send-email-tlinder-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2011-03-25 13:41 ` Felipe Balbi
2011-03-25 13:41 ` Felipe Balbi
2011-03-28 8:45 ` Tanya Brokhman
2011-03-28 8:45 ` Tanya Brokhman
2011-03-28 8:54 ` Felipe Balbi
2011-03-28 9:15 ` Tanya Brokhman
2011-03-28 9:15 ` Tanya Brokhman
2011-04-11 17:59 ` Sebastian Andrzej Siewior
2011-04-11 17:59 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 2/5] usb/gadget: rename bos to get_bos_descr in composite Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 3/5] usb/gadget: rename create_ss_descriptors() to usb_create_ss_descriptors() Sebastian Andrzej Siewior
2011-04-12 19:34 ` [PATCH 4/5] usb/gadget: don't deploy SS descriptors if SS is not enabled Sebastian Andrzej Siewior
2011-04-13 10:46 ` Sergei Shtylyov
[not found] ` <4DA57F0F.1090609-hkdhdckH98+B+jHODAdFcQ@public.gmane.org>
2011-04-13 10:56 ` Sebastian Andrzej Siewior
2011-04-13 10:56 ` Sebastian Andrzej Siewior
2011-04-13 10:59 ` Michal Nazarewicz
[not found] ` <1302636896-12717-1-git-send-email-bigeasy-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
2011-04-12 19:34 ` [PATCH 1/5] usb/gadget: cleanup of "Add SuperSpeed support to the Gadget Framework" Sebastian Andrzej Siewior
2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-13 8:17 ` Felipe Balbi
2011-04-12 19:34 ` [PATCH 5/5] usb/gadget: don't auto-create SS descriptors if HS are avilable Sebastian Andrzej Siewior
2011-04-12 19:34 ` Sebastian Andrzej Siewior
2011-04-13 11:12 ` [PATCH 5/5 v5] usb:gadget: Add SuperSpeed support to the Gadget Framework Tanya Brokhman
2011-04-13 11:12 ` Tanya Brokhman
2011-04-14 7:36 ` Tanya Brokhman
2011-04-14 7:36 ` Tanya Brokhman
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.