Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1261473 > unrolled thread

[PATCH 00/23] usb: gadget: composite: introduce new function API

Started byRobert Baldyga <r.baldyga@samsung.com>
First post2015-11-03 14:00 +0100
Last post2015-11-03 14:10 +0100
Articles 18 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 00/23] usb: gadget: composite: introduce new function API Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 21/23] usb: gadget: f_sourcesink: convert to new API Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 09/23] usb: gadget: composite: handle function bind Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 22/23] usb: gadget: f_ecm: conversion to new API Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 19/23] usb: gadget: configfs: add new composite API support Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 15/23] usb: gadget: composite: handle get_alt() automatically Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
    [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset  user-selectable Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
      Re: [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset  user-selectable Robert Baldyga <r.baldyga@samsung.com> - 2015-11-06 08:30 +0100
    [PATCH 03/23] usb: gadget: f_sourcesink: free requests in  sourcesink_disable() Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:00 +0100
      Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in  sourcesink_disable() Robert Baldyga <r.baldyga@samsung.com> - 2015-11-06 10:00 +0100
        Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in  sourcesink_disable() Krzysztof Opasiak <k.opasiak@samsung.com> - 2015-11-06 11:00 +0100
    [PATCH 06/23] usb: gadget: composite: introduce new descriptors format Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100
    [PATCH 10/23] usb: gadget: composite: handle vendor descs Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100
    [PATCH 05/23] usb: gadget: configfs: fix error path Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100
      Re: [PATCH 05/23] usb: gadget: configfs: fix error path Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2015-11-03 14:50 +0100
    [PATCH 11/23] usb: gadget: composite: generate old descs for  compatibility Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100
    [PATCH 02/23] usb: gadget: f_sourcesink: compute req size once Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100
    [PATCH 07/23] usb: gadget: composite: add functions for descriptors  handling Robert Baldyga <r.baldyga@samsung.com> - 2015-11-03 14:10 +0100

#1261473 — [PATCH 00/23] usb: gadget: composite: introduce new function API

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 00/23] usb: gadget: composite: introduce new function API
Message-ID<qqJlU-2OU-5@gated-at.bofh.it>
Hi Felipe,

Here is my new patch series doing some changes in composite framework
and modifying USB Function API. Some of concepts changed significantly,
for example bind process is done automatically inside composite framework
after collecting descriptors from all Functions. Hence bind() operation
of USB Function has been replaced with prep_descs(). Besides the other
benefits, such as simple implementation of gadget-level autoconfig solver,
changes in API allowed to simplify code of USB Functions, which contain
lots of boilerplate code.

First five patches of this series are fixes or improvements, being
preparation for further changes. Patches from 6 to 19 add implementation
of new features. Some code allowing coexistence of both old and new API
is added. This code will be removed after converting all USB Functions
present in kernel to new API.
Last four patches converts Functions: loopback, sourcesink, ecm and rndis
to new API. Conversion of another Functions will be done soon.

*** What has changed? ***

The main changes are listed below:
A. Introduce new descriptors format. It makes descriptors creation process
   simpler plus creates good place to contain additional information such
   as result of automatic bind or actually selected altsetting.
B. Split descriptors creation process into two stages, implemented by
   two new operations:
   - prep_descs() provide entity descriptors (interfaces, altsettings
     and endpoints)
   - prep_vendor_descs() provide class and vendor specific descriptors.
   The first one is called before binding funciton to UDC and it's
   mandatory, because it provides information needed during bind process.
   The second one is optional and a Function can implement it if it wants
   to attach some class or vendor specific descriptors. It's called after
   bind process, so from it's context all information about interface
   numbers and endpoint addresses is accessible.
C. Perform bind automatically inside composite framework after collecting
   descriptors from all USB Functions. Besides removing lots of repetitive
   code from USB Functions, it gives us two main advantages:
   - We can have gadget-level autoconfig solver providing better endpoint
     resources usage. We can choose best endpoint configuration for all
     Functions in all configurations.
   - We have composite driver structure creation process separated from
     bind process which allows to modify configfs to operate directly
     on composite driver state - both legacy gadgets and configfs can
     use common composite driver creation process.
   Function allowing to obtain endpoints after bind process is provided,
   and it should be called in set_alt().
D. Replace disable() operation with more powerful clear_alt(). It is
   called when Function is being disabled or when altsetting being
   selected on interface which already has active altsetting. It makes
   API more symmetric, which greatly simplifies resource management.
E. Handle endpoint enable/disable automatically, which means, that in
   set_alt() we obtain set of already enabled endpoints for current
   altsetting. Likewise in clear_alt() endpoints are already disabled.
F. Change meaning of second parameter of set_alt() operation. Now it
   contains index of interface within desctiptors array of given USB
   Function instead of bInterfaceNumber of this interface, which
   simplifies altsetting handling (so far it was necessary to compare
   this value with bInterfaceNumber of each interface to find out which
   altsetting of which interface is being selected).
G. Handle get_alt() automatically. Currently selected altsetting number
   is stored for each interface.

*** How did it work before? ***

So far USB Functions had to handle bind process manually and deal with
endpoints state explicitly, which has been making code lengthy and
bug-prone. USB Functions contained lots of repetitive code which was
usually copied while creating new USB Function module. This resulted
with lots of boilerplate code scattered across all Functions present
in Linux kernel.

BIND:

During bind process we had to obtain interface id manually and assign
it to each interface descriptor (altsetting) of given interface. We also
had to obtain endpoints manually using usb_ep_autoconfig(). Beside its
verbosity, this solution resulted with suboptimal endpoints distribution,
because autoconfig algorithm was aware of requirements of only single
endpoint at a time.

udc_bind_to_driver() {
  composite_bind() {
    configuration1->bind() {
      function1->bind() {
        intf1_id = usb_interface_id(); // Obtain intf id manually
        ep1 = usb_ep_autoconfig(); // Endpoint-level autoconfig
        ep2 = usb_ep_autoconfig();
        intf2_id = usb_interface_id();
        ep3 = usb_ep_autoconfig();
        ep4 = usb_ep_autoconfig();
      }
      function2->bind() {
	intf1_id = usb_interface_id();
        ep1 = usb_ep_autoconfig();
        ep2 = usb_ep_autoconfig();
        ep3 = usb_ep_autoconfig();
      }
      function3->bind() {
        intf1_id = usb_interface_id();
        ep1 = usb_ep_autoconfig();
        intf2_id = usb_interface_id();
        ep2 = usb_ep_autoconfig();
      }
    }
    configuration2->bind() {
      function1->bind() {
        intf1_id = usb_interface_id();
        ep1 = usb_ep_autoconfig();
      }
      function2->bind() {
        intf1_id = usb_interface_id();
        ep1 = usb_ep_autoconfig();
      }
    }
  }
}

SET_ALT:

In set_alt() we had to guess if any altsetting for given interface had
been selected before or not. In fact many functions have been doing this
by storing some information in driver_data field of endpoints or simply
by doing interface reset regardless of previous state. We also needed
to guess for which interface set_alt() has been called, because interface
number passed to this callback was the interface index in configuration,
not index in function descriptors. It has been making set_alt() handling
quite problematic.

function1->set_alt() {
  id = detect_intf_id();
  if (id == intf1_id) {
    intf_specific_disable(); // Disable everything just in case
    usb_ep_disable(ep1); // Disable endpoint manually
    usb_ep_disable(ep2);
    config_ep_by_speed(ep1); // Configure endpoint manually
    config_ep_by_speed(ep2);
    usb_ep_enable(ep1); // Enable endpoint manually
    usb_ep_enable(ep2);
    intf_specific_enable();
  } else {
    intf_specific_disable();
    usb_ep_disable(ep3);
    usb_ep_disable(ep4);
    config_ep_by_speed(ep3);
    config_ep_by_speed(ep4);
    usb_ep_enable(ep3);
    usb_ep_enable(ep4);
    intf_specific_enable();
  }
}
function2->set_alt() {
  function_specific_disable();
  usb_ep_disable(ep1);
  usb_ep_disable(ep2);
  config_ep_by_speed(ep1);
  config_ep_by_speed(ep2);
  usb_ep_enable(ep1);
  usb_ep_enable(ep2);
  function_specific_enable();
}

DISABLE:

The disable() callback has been called only when entire Function had
being disabled. In this function we had to deactivate all functionality
and disable all endpoints manually.

function1->disable() {
  function_specific_disable();
  usb_ep_disable(ep1); // Disable endpoint manually
  usb_ep_disable(ep2);
  usb_ep_disable(ep3);
  usb_ep_disable(ep4);
}
function2->disable() {
  function_specific_disable();
  usb_ep_disable(ep1);
  usb_ep_disable(ep2);
}

*** How does it work now? ***

BIND:

Bind process is done entirely by composite framework internals. To
achieve this, composite needs to have a knowledge about interfaces and
endpoints which the Function is comprised of. This knowledge is provided
by prep_descs() callback which assigns descriptors needed for bind process
to Function. Having all descriptors collected allows to implement
configuration-level or even gadget-level autoconfig solver. This solver
could, basing on information from descriptors and endpoint capabilities
of UDC hardware, which gadget driver is being bound to, distribute
endpoints over interfaces in much more optimal way than it has been done
so far. At the end, after binding gadget to UDC hardware,
prep_vendor_descs() callback is invoked for each Function to allow it
to provide some class and vendor specific descriptors.

udc_bind_to_driver() {
  composite_bind() {
    configuration1->bind() {
      function1->prep_descs() { // Gather descriptors
        usb_function_set_descs(); // Set descs to Function
      }
      function2->prep_descs() {
        usb_function_set_descs();
      }
      function3->prep_descs() {
        usb_function_set_descs();
      }
    }
    usb_config_do_bind(); // Bind entire configuration

    configuration2->bind() {
      function1->prep_descs() {
        usb_function_set_descs();
      }
      function2->prep_descs() {
        usb_function_set_descs();
      }
    }
    usb_config_do_bind();

    composite_prep_vendor_descs(); // Gather class and vendor descs
  }
}

SET_ALT:

In set_alt() function we have to obtain endpoints for currently selected
altsetting. Endpoints are already configured and enabled. Interface number
passed to set_alt() is its index within Function descriptors array, which
simplifies set_alt() handling. We also don't need to remember if any
altsetting has been previously selected, because in such situation
clear_alt() is called before set_alt(), to allow function to cleanup
interface state.

function1->set_alt() {
  if (intf == 0) {
    ep1 = usb_function_get_ep();
    ep2 = usb_function_get_ep();
    intf_specific_enable();
  } else {
    ep3 = usb_function_get_ep();
    ep4 = usb_function_get_ep();
    intf_specific_enable();
  }
}
function2->set_alt() {
  ep1 = usb_function_get_ep();
  ep2 = usb_function_get_ep();
  function_specific_enable();
}

CLEAR_ALT:

In clear_alt() callback function can clear interface state and free
resources allocated in set_alt(). It's called before selecting altsetting
in interface which has already selected active altsetting, or when
function is being disabled. Thanks to this clear_alt() can be used as
an enhanced replacement of disable() operation.

function1->clear_alt() {
  if (intf == 0) {
    intf_specific_disable();
  } else {
    intf_specific_disable();
  }
}
function2->clear_alt() {
  function_specific_disable();
}

*** What's next? ***

The next step is to convert all Functions to new API and cleanup composite
code. Then it will be possible to implement intelligent configuration-level
autoconfig solver. We can also try to implement gadget-level autoconfig
solver which could be capable to reconfigure UDC hardware according to
requirements of specific gadget driver.

Thanks to separation of bind process from composite driver creation
process (adding function to configuration doesn't involve its bind, so
it can be done before hardware is available), we can also simplify
configfs gadget implementation. We can make legacy gadgets and configfs
using common composite driver creation process.

I believe it's also possible to make descriptors creation process less
verbose by providing set of macros/functions dedicated for this purpose
(now descriptors take at average over 200 lines of code per Function).

I have some WIP version of patches in which I'm doing part of changes
mentioned above. I can send them as RFC to show what is final result
which I want to achieve.

Best regards,
Robert Baldyga

Robert Baldyga (23):
  usb: gadget: f_sourcesink: make ISO altset user-selectable
  usb: gadget: f_sourcesink: compute req size once
  usb: gadget: f_sourcesink: free requests in sourcesink_disable()
  usb: gadget: f_loopback: free requests in loopback_disable()
  usb: gadget: configfs: fix error path
  usb: gadget: composite: introduce new descriptors format
  usb: gadget: composite: add functions for descriptors handling
  usb: gadget: composite: introduce new USB function ops
  usb: gadget: composite: handle function bind
  usb: gadget: composite: handle vendor descs
  usb: gadget: composite: generate old descs for compatibility
  usb: gadget: composite: disable eps before calling disable() callback
  usb: gadget: composite: enable eps before calling set_alt() callback
  usb: gadget: composite: introduce clear_alt() operation
  usb: gadget: composite: handle get_alt() automatically
  usb: gadget: composite: add usb_function_get_ep() function
  usb: gadget: composite: add usb_get_interface_id() function
  usb: gadget: composite: enable adding USB functions using new API
  usb: gadget: configfs: add new composite API support
  usb: gadget: f_loopback: convert to new API
  usb: gadget: f_sourcesink: convert to new API
  usb: gadget: f_ecm: conversion to new API
  usb: gadget: f_rndis: conversion to new API

 drivers/usb/gadget/composite.c             | 947 ++++++++++++++++++++++++++++-
 drivers/usb/gadget/configfs.c              |  24 +-
 drivers/usb/gadget/function/f_ecm.c        | 321 +++-------
 drivers/usb/gadget/function/f_loopback.c   | 218 ++-----
 drivers/usb/gadget/function/f_rndis.c      | 321 ++++------
 drivers/usb/gadget/function/f_sourcesink.c | 433 +++++--------
 drivers/usb/gadget/function/g_zero.h       |   6 +-
 drivers/usb/gadget/legacy/zero.c           |  12 +
 include/linux/usb/composite.h              | 194 ++++++
 9 files changed, 1569 insertions(+), 907 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1261474 — [PATCH 21/23] usb: gadget: f_sourcesink: convert to new API

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 21/23] usb: gadget: f_sourcesink: convert to new API
Message-ID<qqJlW-2OU-41@gated-at.bofh.it>
In reply to#1261473
Generate descriptors in new format and attach them to USB function in
prep_descs(). Change set_alt() implementation and implement clear_alt()
operation. Get rid of get_alt() callback, as now USB_REQ_GET_INTERFACE
is handled automatically by composite framwework. Remove unnecessary
boilerplate code.

Call usb_config_do_bind() in legacy gadget zero, because it uses
usb_add_config_only() instead of usb_add_config() and prepares
configuration manually.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_sourcesink.c | 314 ++++++-----------------------
 drivers/usb/gadget/function/g_zero.h       |   3 -
 drivers/usb/gadget/legacy/zero.c           |   3 +
 3 files changed, 65 insertions(+), 255 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index d8f5f56..6e3d3df 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -51,7 +51,6 @@ struct f_sourcesink {
 	struct usb_ep		*out_ep;
 	struct usb_ep		*iso_in_ep;
 	struct usb_ep		*iso_out_ep;
-	int			cur_alt;
 
 	struct usb_request	*in_req;
 	struct usb_request	*out_req;
@@ -132,19 +131,6 @@ static struct usb_endpoint_descriptor fs_iso_sink_desc = {
 	.bInterval =		4,
 };
 
-static struct usb_descriptor_header *fs_source_sink_descs[] = {
-	(struct usb_descriptor_header *) &source_sink_intf_alt0,
-	(struct usb_descriptor_header *) &fs_sink_desc,
-	(struct usb_descriptor_header *) &fs_source_desc,
-	(struct usb_descriptor_header *) &source_sink_intf_alt1,
-#define FS_ALT_IFC_1_OFFSET	3
-	(struct usb_descriptor_header *) &fs_sink_desc,
-	(struct usb_descriptor_header *) &fs_source_desc,
-	(struct usb_descriptor_header *) &fs_iso_sink_desc,
-	(struct usb_descriptor_header *) &fs_iso_source_desc,
-	NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_source_desc = {
@@ -181,19 +167,6 @@ static struct usb_endpoint_descriptor hs_iso_sink_desc = {
 	.bInterval =		4,
 };
 
-static struct usb_descriptor_header *hs_source_sink_descs[] = {
-	(struct usb_descriptor_header *) &source_sink_intf_alt0,
-	(struct usb_descriptor_header *) &hs_source_desc,
-	(struct usb_descriptor_header *) &hs_sink_desc,
-	(struct usb_descriptor_header *) &source_sink_intf_alt1,
-#define HS_ALT_IFC_1_OFFSET	3
-	(struct usb_descriptor_header *) &hs_source_desc,
-	(struct usb_descriptor_header *) &hs_sink_desc,
-	(struct usb_descriptor_header *) &hs_iso_source_desc,
-	(struct usb_descriptor_header *) &hs_iso_sink_desc,
-	NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_source_desc = {
@@ -266,24 +239,24 @@ static struct usb_ss_ep_comp_descriptor ss_iso_sink_comp_desc = {
 	.wBytesPerInterval =	cpu_to_le16(1024),
 };
 
-static struct usb_descriptor_header *ss_source_sink_descs[] = {
-	(struct usb_descriptor_header *) &source_sink_intf_alt0,
-	(struct usb_descriptor_header *) &ss_source_desc,
-	(struct usb_descriptor_header *) &ss_source_comp_desc,
-	(struct usb_descriptor_header *) &ss_sink_desc,
-	(struct usb_descriptor_header *) &ss_sink_comp_desc,
-	(struct usb_descriptor_header *) &source_sink_intf_alt1,
-#define SS_ALT_IFC_1_OFFSET	5
-	(struct usb_descriptor_header *) &ss_source_desc,
-	(struct usb_descriptor_header *) &ss_source_comp_desc,
-	(struct usb_descriptor_header *) &ss_sink_desc,
-	(struct usb_descriptor_header *) &ss_sink_comp_desc,
-	(struct usb_descriptor_header *) &ss_iso_source_desc,
-	(struct usb_descriptor_header *) &ss_iso_source_comp_desc,
-	(struct usb_descriptor_header *) &ss_iso_sink_desc,
-	(struct usb_descriptor_header *) &ss_iso_sink_comp_desc,
-	NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_source, &fs_source_desc, &hs_source_desc,
+		&ss_source_desc, &ss_source_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_sink, &fs_sink_desc, &hs_sink_desc,
+		&ss_sink_desc, &ss_sink_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_iso_source, &fs_iso_source_desc, &hs_iso_source_desc,
+		&ss_iso_source_desc, &ss_iso_source_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_iso_sink, &fs_iso_sink_desc, &hs_iso_sink_desc,
+		&ss_iso_sink_desc, &ss_iso_sink_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(altset0, &source_sink_intf_alt0, &ep_source, &ep_sink);
+USB_COMPOSITE_ALTSETTING(altset1, &source_sink_intf_alt1, &ep_source, &ep_sink,
+		&ep_iso_source, &ep_iso_sink);
+
+USB_COMPOSITE_INTERFACE(intf0, &altset0, &altset1);
+USB_COMPOSITE_INTERFACE(intf0_no_iso, &altset0);
+
+USB_COMPOSITE_DESCRIPTORS(source_sink_descs, &intf0);
+USB_COMPOSITE_DESCRIPTORS(source_sink_descs_no_iso, &intf0_no_iso);
 
 /* function-specific strings: */
 
@@ -317,65 +290,12 @@ void free_ep_req(struct usb_ep *ep, struct usb_request *req)
 	usb_ep_free_request(ep, req);
 }
 
-static void disable_ep(struct usb_composite_dev *cdev, struct usb_ep *ep)
-{
-	int			value;
-
-	value = usb_ep_disable(ep);
-	if (value < 0)
-		DBG(cdev, "disable %s --> %d\n", ep->name, value);
-}
-
-void disable_endpoints(struct usb_composite_dev *cdev,
-		struct usb_ep *in, struct usb_ep *out,
-		struct usb_ep *iso_in, struct usb_ep *iso_out)
-{
-	disable_ep(cdev, in);
-	disable_ep(cdev, out);
-	if (iso_in)
-		disable_ep(cdev, iso_in);
-	if (iso_out)
-		disable_ep(cdev, iso_out);
-}
-
-static int
-sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
+static int sourcesink_prep_descs(struct usb_function *f)
 {
-	struct usb_composite_dev *cdev = c->cdev;
 	struct f_sourcesink	*ss = func_to_ss(f);
-	int	id;
-	int ret;
 
-	/* allocate interface ID(s) */
-	id = usb_interface_id(c, f);
-	if (id < 0)
-		return id;
-	source_sink_intf_alt0.bInterfaceNumber = id;
-	source_sink_intf_alt1.bInterfaceNumber = id;
-
-	/* allocate bulk endpoints */
-	ss->in_ep = usb_ep_autoconfig(cdev->gadget, &fs_source_desc);
-	if (!ss->in_ep)
-		goto autoconf_fail;
-
-	ss->out_ep = usb_ep_autoconfig(cdev->gadget, &fs_sink_desc);
-	if (!ss->out_ep)
-		goto autoconf_fail;
-
-	/* support high speed hardware */
-	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
-	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
-
-	/* support super speed hardware */
-	ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
-	ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
-
-	if (!ss->isoc_enabled) {
-		fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
-		hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
-		ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
-		goto no_iso;
-	}
+	if (!ss->isoc_enabled)
+		return usb_function_set_descs(f, &source_sink_descs_no_iso);
 
 	/* sanity check the isoc module parameters */
 	if (ss->isoc_interval < 1)
@@ -395,15 +315,6 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 						1023 : ss->isoc_maxpacket;
 	fs_iso_sink_desc.bInterval = ss->isoc_interval;
 
-	/* allocate iso endpoints */
-	ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
-	if (!ss->iso_in_ep)
-		goto autoconf_fail;
-
-	ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_sink_desc);
-	if (!ss->iso_out_ep)
-		goto autoconf_fail;
-
 	if (ss->isoc_maxpacket > 1024)
 		ss->isoc_maxpacket = 1024;
 	/*
@@ -414,14 +325,10 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 	hs_iso_source_desc.wMaxPacketSize = ss->isoc_maxpacket;
 	hs_iso_source_desc.wMaxPacketSize |= ss->isoc_mult << 11;
 	hs_iso_source_desc.bInterval = ss->isoc_interval;
-	hs_iso_source_desc.bEndpointAddress =
-		fs_iso_source_desc.bEndpointAddress;
 
 	hs_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
 	hs_iso_sink_desc.wMaxPacketSize |= ss->isoc_mult << 11;
 	hs_iso_sink_desc.bInterval = ss->isoc_interval;
-	hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
-
 	/*
 	 * Fill in the SS isoc descriptors from the module parameters.
 	 * We assume that the user knows what they are doing and won't
@@ -433,8 +340,6 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 	ss_iso_source_comp_desc.bMaxBurst = ss->isoc_maxburst;
 	ss_iso_source_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
 		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
-	ss_iso_source_desc.bEndpointAddress =
-		fs_iso_source_desc.bEndpointAddress;
 
 	ss_iso_sink_desc.wMaxPacketSize = ss->isoc_maxpacket;
 	ss_iso_sink_desc.bInterval = ss->isoc_interval;
@@ -442,26 +347,8 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 	ss_iso_sink_comp_desc.bMaxBurst = ss->isoc_maxburst;
 	ss_iso_sink_comp_desc.wBytesPerInterval = ss->isoc_maxpacket *
 		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
-	ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
 
-no_iso:
-	ret = usb_assign_descriptors(f, fs_source_sink_descs,
-			hs_source_sink_descs, ss_source_sink_descs);
-	if (ret)
-		return ret;
-
-	DBG(cdev, "%s speed %s: IN/%s, OUT/%s, ISO-IN/%s, ISO-OUT/%s\n",
-	    (gadget_is_superspeed(c->cdev->gadget) ? "super" :
-	     (gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full")),
-			f->name, ss->in_ep->name, ss->out_ep->name,
-			ss->iso_in_ep ? ss->iso_in_ep->name : "<none>",
-			ss->iso_out_ep ? ss->iso_out_ep->name : "<none>");
-	return 0;
-
-autoconf_fail:
-	ERROR(cdev, "%s: can't autoconfigure on %s\n",
-			f->name, cdev->gadget->name);
-	return -ENODEV;
+	return usb_function_set_descs(f, &source_sink_descs);
 }
 
 static void
@@ -475,7 +362,6 @@ sourcesink_free_func(struct usb_function *f)
 	opts->refcnt--;
 	mutex_unlock(&opts->lock);
 
-	usb_free_all_descriptors(f);
 	kfree(func_to_ss(f));
 }
 
@@ -658,136 +544,61 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
 	return status;
 }
 
-static void disable_source_sink(struct f_sourcesink *ss)
-{
-	struct usb_composite_dev	*cdev;
-
-	cdev = ss->function.config->cdev;
-	disable_endpoints(cdev, ss->in_ep, ss->out_ep, ss->iso_in_ep,
-			ss->iso_out_ep);
-	VDBG(cdev, "%s disabled\n", ss->function.name);
-}
-
-static int
-enable_source_sink(struct usb_composite_dev *cdev, struct f_sourcesink *ss,
-		int alt)
-{
-	int					result = 0;
-	int					speed = cdev->gadget->speed;
-	struct usb_ep				*ep;
-
-	/* one bulk endpoint writes (sources) zeroes IN (to the host) */
-	ep = ss->in_ep;
-	result = config_ep_by_speed(cdev->gadget, &(ss->function), ep);
-	if (result)
-		return result;
-	result = usb_ep_enable(ep);
-	if (result < 0)
-		return result;
-	ep->driver_data = ss;
-
-	result = source_sink_start_ep(ss, true, false, speed);
-	if (result < 0) {
-fail:
-		ep = ss->in_ep;
-		usb_ep_disable(ep);
-		return result;
-	}
-
-	/* one bulk endpoint reads (sinks) anything OUT (from the host) */
-	ep = ss->out_ep;
-	result = config_ep_by_speed(cdev->gadget, &(ss->function), ep);
-	if (result)
-		goto fail;
-	result = usb_ep_enable(ep);
-	if (result < 0)
-		goto fail;
-	ep->driver_data = ss;
-
-	result = source_sink_start_ep(ss, false, false, speed);
-	if (result < 0) {
-fail2:
-		ep = ss->out_ep;
-		usb_ep_disable(ep);
-		goto fail;
-	}
-
-	if (alt == 0)
-		goto out;
-
-	/* one iso endpoint writes (sources) zeroes IN (to the host) */
-	ep = ss->iso_in_ep;
-	if (ep) {
-		result = config_ep_by_speed(cdev->gadget, &(ss->function), ep);
-		if (result)
-			goto fail2;
-		result = usb_ep_enable(ep);
-		if (result < 0)
-			goto fail2;
-		ep->driver_data = ss;
-
-		result = source_sink_start_ep(ss, true, true, speed);
-		if (result < 0) {
-fail3:
-			ep = ss->iso_in_ep;
-			if (ep)
-				usb_ep_disable(ep);
-			goto fail2;
-		}
-	}
-
-	/* one iso endpoint reads (sinks) anything OUT (from the host) */
-	ep = ss->iso_out_ep;
-	if (ep) {
-		result = config_ep_by_speed(cdev->gadget, &(ss->function), ep);
-		if (result)
-			goto fail3;
-		result = usb_ep_enable(ep);
-		if (result < 0)
-			goto fail3;
-		ep->driver_data = ss;
-
-		result = source_sink_start_ep(ss, false, true, speed);
-		if (result < 0) {
-			usb_ep_disable(ep);
-			goto fail3;
-		}
-	}
-out:
-	ss->cur_alt = alt;
-
-	DBG(cdev, "%s enabled, alt intf %d\n", ss->function.name, alt);
-	return result;
-}
-
 static int sourcesink_set_alt(struct usb_function *f,
 		unsigned intf, unsigned alt)
 {
 	struct f_sourcesink		*ss = func_to_ss(f);
 	struct usb_composite_dev	*cdev = f->config->cdev;
+	int				speed = cdev->gadget->speed;
+	int				ret;
 
-	disable_source_sink(ss);
-	return enable_source_sink(cdev, ss, alt);
-}
+	ss->in_ep = usb_function_get_ep(f, intf, 0);
+	if (!ss->in_ep)
+		return -ENODEV;
+	ss->in_ep->driver_data = ss;
+	ret = source_sink_start_ep(ss, true, false, speed);
+	if (ret < 0)
+		return ret;
 
-static int sourcesink_get_alt(struct usb_function *f, unsigned intf)
-{
-	struct f_sourcesink		*ss = func_to_ss(f);
+	ss->out_ep = usb_function_get_ep(f, intf, 1);
+	if (!ss->out_ep)
+		return -ENODEV;
+	ss->out_ep->driver_data = ss;
+	ret = source_sink_start_ep(ss, false, false, speed);
+	if (ret < 0)
+		return ret;
+
+	if (alt == 1) {
+		ss->iso_in_ep = usb_function_get_ep(f, intf, 2);
+		if (!ss->iso_in_ep)
+			return -ENODEV;
+		ss->iso_in_ep->driver_data = ss;
+		ret = source_sink_start_ep(ss, true, true, speed);
+		if (ret < 0)
+			return ret;
+
+		ss->iso_out_ep = usb_function_get_ep(f, intf, 3);
+		if (!ss->iso_out_ep)
+			return -ENODEV;
+		ss->iso_out_ep->driver_data = ss;
+		ret = source_sink_start_ep(ss, false, true, speed);
+		if (ret < 0)
+			return ret;
+	}
 
-	return ss->cur_alt;
+	return 0;
 }
 
-static void sourcesink_disable(struct usb_function *f)
+static void sourcesink_clear_alt(struct usb_function *f,
+		unsigned intf, unsigned alt)
 {
 	struct f_sourcesink	*ss = func_to_ss(f);
 	int i;
 
-	disable_source_sink(ss);
-
 	free_ep_req(ss->in_ep, ss->in_req);
 	free_ep_req(ss->out_ep, ss->out_req);
 
-	if (ss->iso_in_ep) {
+	if (alt == 1) {
 		for (i = 0; i < REQ_CNT; i++) {
 			free_ep_req(ss->iso_in_ep, ss->iso_in_req[i]);
 			free_ep_req(ss->iso_out_ep, ss->iso_out_req[i]);
@@ -894,10 +705,9 @@ static struct usb_function *source_sink_alloc_func(
 	ss->buflen = ss_opts->bulk_buflen;
 
 	ss->function.name = "source/sink";
-	ss->function.bind = sourcesink_bind;
+	ss->function.prep_descs = sourcesink_prep_descs;
 	ss->function.set_alt = sourcesink_set_alt;
-	ss->function.get_alt = sourcesink_get_alt;
-	ss->function.disable = sourcesink_disable;
+	ss->function.clear_alt = sourcesink_clear_alt;
 	ss->function.setup = sourcesink_setup;
 	ss->function.strings = sourcesink_strings;
 
diff --git a/drivers/usb/gadget/function/g_zero.h b/drivers/usb/gadget/function/g_zero.h
index 8a99071..34857e9 100644
--- a/drivers/usb/gadget/function/g_zero.h
+++ b/drivers/usb/gadget/function/g_zero.h
@@ -63,8 +63,5 @@ int lb_modinit(void);
 
 /* common utilities */
 void free_ep_req(struct usb_ep *ep, struct usb_request *req);
-void disable_endpoints(struct usb_composite_dev *cdev,
-		struct usb_ep *in, struct usb_ep *out,
-		struct usb_ep *iso_in, struct usb_ep *iso_out);
 
 #endif /* __G_ZERO_H */
diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
index 097bdaf..76fc97d 100644
--- a/drivers/usb/gadget/legacy/zero.c
+++ b/drivers/usb/gadget/legacy/zero.c
@@ -361,6 +361,9 @@ static int zero_bind(struct usb_composite_dev *cdev)
 	status = usb_add_function(&sourcesink_driver, func_ss);
 	if (status)
 		goto err_free_otg_desc;
+	status = usb_config_do_bind(&sourcesink_driver);
+	if (status)
+		goto err_free_otg_desc;
 
 	usb_ep_autoconfig_reset(cdev->gadget);
 	status = usb_add_function(&loopback_driver, func_lb);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261475 — [PATCH 09/23] usb: gadget: composite: handle function bind

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 09/23] usb: gadget: composite: handle function bind
Message-ID<qqJlW-2OU-45@gated-at.bofh.it>
In reply to#1261473
As now USB function supplies entity descriptors to composite in
prep_descs() callback, we can perform bind inside composite framework
without involving bind() callback (which now is unused and will be
removed after converting all functions in kernel to new API).

For now we bind each configuration when it's added, because we have
to support functions based on old API, but after completing conversion
of functions, we will be able to do bind after adding all configurations.
Also more sophisticated autoconfig solver will be provided to improve
utilization of available hardware endpoints.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c | 162 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/composite.h  |   3 +
 2 files changed, 165 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 3ecfaca..f4189b1 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -327,6 +327,21 @@ int usb_function_activate(struct usb_function *function)
 EXPORT_SYMBOL_GPL(usb_function_activate);
 
 /**
+ * usb_function_is_new_api - checks if USB function uses new API
+ * @f: USB function
+ *
+ * This function is added temporarily to allow both old and new function API
+ * to coexist. It function will be removed after converting all USB functions
+ * in kernel to new API.
+ *
+ * Returns true if function uses new API.
+ */
+static inline bool usb_function_is_new_api(struct usb_function *f)
+{
+	return !!f->prep_descs;
+}
+
+/**
  * usb_function_set_descs - assing descriptors to USB function
  * @f: USB function
  * @descs: USB descriptors to be assigned to function
@@ -1109,6 +1124,12 @@ int usb_add_config(struct usb_composite_dev *cdev,
 		goto done;
 
 	status = bind(config);
+	if (status < 0)
+		goto out;
+
+	status = usb_config_do_bind(config);
+
+out:
 	if (status < 0) {
 		while (!list_empty(&config->functions)) {
 			struct usb_function		*f;
@@ -2404,6 +2425,147 @@ void composite_dev_cleanup(struct usb_composite_dev *cdev)
 	device_remove_file(&cdev->gadget->dev, &dev_attr_suspended);
 }
 
+/**
+ * usb_cmp_ep_descs - compare descriptors of two endpoints
+ *
+ * As currently during autoconfig procedure we take into consideration only
+ * FullSpeed and SuperSpeed Companion descriptors, we need to compare only
+ * these descriptors. It they are the same, endpoints are identical from
+ * autoconfig point of view.
+ */
+static int usb_cmp_ep_descs(struct usb_composite_ep *ep1,
+		struct usb_composite_ep *ep2)
+{
+	if (ep1->fs.desc->bLength != ep2->fs.desc->bLength)
+		return 0;
+	if (usb_endpoint_dir_in(ep1->fs.desc) ^
+			usb_endpoint_dir_in(ep2->fs.desc))
+		return 0;
+	if (ep1->fs.desc->bmAttributes != ep2->fs.desc->bmAttributes)
+		return 0;
+	if (ep1->fs.desc->wMaxPacketSize != ep2->fs.desc->wMaxPacketSize)
+		return 0;
+	if (ep1->fs.desc->bInterval != ep2->fs.desc->bInterval)
+		return 0;
+
+	if (ep1->fs.desc->bLength != USB_DT_ENDPOINT_AUDIO_SIZE)
+		goto ss_comp;
+
+	if (ep1->fs.desc->bRefresh != ep2->fs.desc->bRefresh)
+		return 0;
+	if (ep1->fs.desc->bSynchAddress != ep2->fs.desc->bSynchAddress)
+		return 0;
+
+ss_comp:
+	if (!ep1->ss_comp.desc ^ !ep2->ss_comp.desc)
+		return 0;
+	if (!ep1->ss_comp.desc)
+		return 1;
+
+	if (ep1->ss_comp.desc->bMaxBurst != ep2->ss_comp.desc->bMaxBurst)
+		return 0;
+	if (ep1->ss_comp.desc->bmAttributes != ep2->ss_comp.desc->bmAttributes)
+		return 0;
+	if (ep1->ss_comp.desc->wBytesPerInterval !=
+			ep2->ss_comp.desc->wBytesPerInterval)
+		return 0;
+
+	return 1;
+}
+
+/**
+ * ep_update_address() - update endpoint address in descriptors
+ * @ep: composite endpoint with assigned hardware ep
+ *
+ * This function should be called after setting ep->ep to endpoint obtained
+ * from usb_ep_autoconfig_ss(), to update endpoint address in descriptors for
+ * all supported speeds.
+ */
+static inline void ep_update_address(struct usb_composite_ep *ep)
+{
+	if (ep->fs.desc)
+		ep->hs.desc->bEndpointAddress = ep->ep->address;
+	if (ep->hs.desc)
+		ep->hs.desc->bEndpointAddress = ep->ep->address;
+	if (ep->ss.desc)
+		ep->ss.desc->bEndpointAddress = ep->ep->address;
+}
+
+/**
+ * interface_do_bind() - bind interface to UDC
+ * @c: USB configuration
+ * @f: USB function in configuration c
+ * @intf: USB interface in function f
+ *
+ * For now we use only simple interface-level ep aucoconfig solver.
+ * We share endpoints between altsettings where it's possible.
+ */
+static int interface_do_bind(struct usb_configuration *c,
+		struct usb_function *f, struct usb_composite_intf *intf)
+{
+	struct usb_composite_altset *alt, *altx;
+	struct usb_composite_ep *ep, *epx;
+	int a, e, ax, ex;
+
+	intf->id = usb_interface_id(c, f);
+	intf->cur_altset = -1;
+
+	for (a = 0; a < intf->altsets_num; ++a) {
+		alt = intf->altsets[a];
+		alt->alt.desc->bInterfaceNumber = intf->id;
+		for (e = 0; e < alt->eps_num; ++e) {
+			ep = alt->eps[e];
+			if (ep->ep)
+				continue;
+			ep->ep = usb_ep_autoconfig_ss(c->cdev->gadget,
+					ep->fs.desc, ep->ss_comp.desc);
+			if (!ep->ep)
+				return -ENODEV;
+			ep_update_address(ep);
+			/* Try endpoint for other altsets */
+			for (ax = a + 1; ax < intf->altsets_num; ++ax) {
+				altx = intf->altsets[ax];
+				for (ex = 0; ex < altx->eps_num; ++ex) {
+					epx = altx->eps[ex];
+					if (usb_cmp_ep_descs(ep, epx)) {
+						epx->ep = ep->ep;
+						ep_update_address(epx);
+					}
+				}
+			}
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * config_do_bind() - bind configuration to UDC
+ * @c: USB configuration
+ *
+ * Bind the all functions in configuration to UDC.
+ */
+int usb_config_do_bind(struct usb_configuration *c)
+{
+	struct usb_function *f;
+	struct usb_composite_intf *intf;
+	int i, ret;
+
+	list_for_each_entry(f, &c->functions, list) {
+		if (!usb_function_is_new_api(f))
+			continue;
+		for (i = 0; i < f->descs->intfs_num; ++i) {
+			intf = f->descs->intfs[i];
+			ret = interface_do_bind(c, f, intf);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_config_do_bind);
+
 static int composite_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *gdriver)
 {
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 58d2929..a92da38 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -413,6 +413,7 @@ int usb_altset_add_vendor_desc(struct usb_function *f, int i, int a,
 int usb_ep_add_vendor_desc(struct usb_function *f, int i, int a, int e,
 		struct usb_descriptor_header *desc);
 
+
 int usb_interface_id(struct usb_configuration *, struct usb_function *);
 
 int config_ep_by_speed(struct usb_gadget *g, struct usb_function *f,
@@ -503,6 +504,8 @@ int usb_add_config(struct usb_composite_dev *,
 void usb_remove_config(struct usb_composite_dev *,
 		struct usb_configuration *);
 
+int usb_config_do_bind(struct usb_configuration *c);
+
 /* predefined index for usb_composite_driver */
 enum {
 	USB_GADGET_MANUFACTURER_IDX	= 0,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261476 — [PATCH 22/23] usb: gadget: f_ecm: conversion to new API

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 22/23] usb: gadget: f_ecm: conversion to new API
Message-ID<qqJlW-2OU-47@gated-at.bofh.it>
In reply to#1261473
Generate descriptors in new format and attach them to USB function in
prep_descs(). Implement prep_vendor_descs() to supply class specific
descriptors. Change set_alt() implementation and implement clear_alt()
operation. Get rid of get_alt() which now is handled automatically.
Remove boilerplate code. Change USB request lifetime management - now
it's allocated in set_alt() and freed in clear_alt().

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_ecm.c | 321 +++++++++++-------------------------
 1 file changed, 92 insertions(+), 229 deletions(-)

diff --git a/drivers/usb/gadget/function/f_ecm.c b/drivers/usb/gadget/function/f_ecm.c
index 4abca70..6e1da78 100644
--- a/drivers/usb/gadget/function/f_ecm.c
+++ b/drivers/usb/gadget/function/f_ecm.c
@@ -214,25 +214,6 @@ static struct usb_endpoint_descriptor fs_ecm_out_desc = {
 	.bmAttributes =		USB_ENDPOINT_XFER_BULK,
 };
 
-static struct usb_descriptor_header *ecm_fs_function[] = {
-	/* CDC ECM control descriptors */
-	(struct usb_descriptor_header *) &ecm_iad_descriptor,
-	(struct usb_descriptor_header *) &ecm_control_intf,
-	(struct usb_descriptor_header *) &ecm_header_desc,
-	(struct usb_descriptor_header *) &ecm_union_desc,
-	(struct usb_descriptor_header *) &ecm_desc,
-
-	/* NOTE: status endpoint might need to be removed */
-	(struct usb_descriptor_header *) &fs_ecm_notify_desc,
-
-	/* data interface, altsettings 0 and 1 */
-	(struct usb_descriptor_header *) &ecm_data_nop_intf,
-	(struct usb_descriptor_header *) &ecm_data_intf,
-	(struct usb_descriptor_header *) &fs_ecm_in_desc,
-	(struct usb_descriptor_header *) &fs_ecm_out_desc,
-	NULL,
-};
-
 /* high speed support: */
 
 static struct usb_endpoint_descriptor hs_ecm_notify_desc = {
@@ -263,25 +244,6 @@ static struct usb_endpoint_descriptor hs_ecm_out_desc = {
 	.wMaxPacketSize =	cpu_to_le16(512),
 };
 
-static struct usb_descriptor_header *ecm_hs_function[] = {
-	/* CDC ECM control descriptors */
-	(struct usb_descriptor_header *) &ecm_iad_descriptor,
-	(struct usb_descriptor_header *) &ecm_control_intf,
-	(struct usb_descriptor_header *) &ecm_header_desc,
-	(struct usb_descriptor_header *) &ecm_union_desc,
-	(struct usb_descriptor_header *) &ecm_desc,
-
-	/* NOTE: status endpoint might need to be removed */
-	(struct usb_descriptor_header *) &hs_ecm_notify_desc,
-
-	/* data interface, altsettings 0 and 1 */
-	(struct usb_descriptor_header *) &ecm_data_nop_intf,
-	(struct usb_descriptor_header *) &ecm_data_intf,
-	(struct usb_descriptor_header *) &hs_ecm_in_desc,
-	(struct usb_descriptor_header *) &hs_ecm_out_desc,
-	NULL,
-};
-
 /* super speed support: */
 
 static struct usb_endpoint_descriptor ss_ecm_notify_desc = {
@@ -331,27 +293,21 @@ static struct usb_ss_ep_comp_descriptor ss_ecm_bulk_comp_desc = {
 	/* .bmAttributes =	0, */
 };
 
-static struct usb_descriptor_header *ecm_ss_function[] = {
-	/* CDC ECM control descriptors */
-	(struct usb_descriptor_header *) &ecm_iad_descriptor,
-	(struct usb_descriptor_header *) &ecm_control_intf,
-	(struct usb_descriptor_header *) &ecm_header_desc,
-	(struct usb_descriptor_header *) &ecm_union_desc,
-	(struct usb_descriptor_header *) &ecm_desc,
-
-	/* NOTE: status endpoint might need to be removed */
-	(struct usb_descriptor_header *) &ss_ecm_notify_desc,
-	(struct usb_descriptor_header *) &ss_ecm_intr_comp_desc,
-
-	/* data interface, altsettings 0 and 1 */
-	(struct usb_descriptor_header *) &ecm_data_nop_intf,
-	(struct usb_descriptor_header *) &ecm_data_intf,
-	(struct usb_descriptor_header *) &ss_ecm_in_desc,
-	(struct usb_descriptor_header *) &ss_ecm_bulk_comp_desc,
-	(struct usb_descriptor_header *) &ss_ecm_out_desc,
-	(struct usb_descriptor_header *) &ss_ecm_bulk_comp_desc,
-	NULL,
-};
+USB_COMPOSITE_ENDPOINT(ep_notify, &fs_ecm_notify_desc, &hs_ecm_notify_desc,
+		&ss_ecm_notify_desc, &ss_ecm_intr_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_in, &fs_ecm_in_desc, &hs_ecm_in_desc,
+		&ss_ecm_in_desc, &ss_ecm_bulk_comp_desc);
+USB_COMPOSITE_ENDPOINT(ep_out, &fs_ecm_out_desc, &hs_ecm_out_desc,
+		&ss_ecm_out_desc, &ss_ecm_bulk_comp_desc);
+
+USB_COMPOSITE_ALTSETTING(intf0alt0, &ecm_control_intf, &ep_notify);
+USB_COMPOSITE_ALTSETTING(intf1alt0, &ecm_data_nop_intf);
+USB_COMPOSITE_ALTSETTING(intf1alt1, &ecm_data_intf, &ep_in, &ep_out);
+
+USB_COMPOSITE_INTERFACE(intf0, &intf0alt0);
+USB_COMPOSITE_INTERFACE(intf1, &intf1alt0, &intf1alt1);
+
+USB_COMPOSITE_DESCRIPTORS(ecm_descs, &intf0, &intf1);
 
 /* string descriptors: */
 
@@ -537,47 +493,43 @@ static int ecm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 
 	/* Control interface has only altsetting 0 */
-	if (intf == ecm->ctrl_id) {
-		if (alt != 0)
-			goto fail;
+	if (intf == 0) {
+		/* NOTE:  a status/notification endpoint is *OPTIONAL* but we
+		 * don't treat it that way.  It's simpler, and some newer CDC
+		 * profiles (wireless handsets) no longer treat it as optional.
+		 */
 
 		VDBG(cdev, "reset ecm control %d\n", intf);
-		usb_ep_disable(ecm->notify);
-		if (!(ecm->notify->desc)) {
-			VDBG(cdev, "init ecm ctrl %d\n", intf);
-			if (config_ep_by_speed(cdev->gadget, f, ecm->notify))
-				goto fail;
-		}
-		usb_ep_enable(ecm->notify);
-
-	/* Data interface has two altsettings, 0 and 1 */
-	} else if (intf == ecm->data_id) {
-		if (alt > 1)
-			goto fail;
 
-		if (ecm->port.in_ep->enabled) {
-			DBG(cdev, "reset ecm\n");
-			gether_disconnect(&ecm->port);
+		ecm->notify = usb_function_get_ep(f, intf, 0);
+		if (!ecm->notify)
+			return -ENODEV;
+
+		/* allocate notification request and buffer */
+		ecm->notify_req = usb_ep_alloc_request(ecm->notify, GFP_KERNEL);
+		if (!ecm->notify_req)
+			return -ENOMEM;
+		ecm->notify_req->buf = kmalloc(ECM_STATUS_BYTECOUNT,
+				GFP_KERNEL);
+		if (!ecm->notify_req->buf) {
+			usb_ep_free_request(ecm->notify, ecm->notify_req);
+			return -ENOMEM;
 		}
-
-		if (!ecm->port.in_ep->desc ||
-		    !ecm->port.out_ep->desc) {
-			DBG(cdev, "init ecm\n");
-			if (config_ep_by_speed(cdev->gadget, f,
-					       ecm->port.in_ep) ||
-			    config_ep_by_speed(cdev->gadget, f,
-					       ecm->port.out_ep)) {
-				ecm->port.in_ep->desc = NULL;
-				ecm->port.out_ep->desc = NULL;
-				goto fail;
-			}
-		}
-
+		ecm->notify_req->context = ecm;
+		ecm->notify_req->complete = ecm_notify_complete;
+	/* Data interface has two altsettings, 0 and 1 */
+	} else if (intf == 1) {
 		/* CDC Ethernet only sends data in non-default altsettings.
 		 * Changing altsettings resets filters, statistics, etc.
 		 */
 		if (alt == 1) {
 			struct net_device	*net;
+			ecm->port.in_ep = usb_function_get_ep(f, intf, 0);
+			if (!ecm->port.in_ep)
+				return -ENODEV;
+			ecm->port.out_ep = usb_function_get_ep(f, intf, 1);
+			if (!ecm->port.out_ep)
+				return -ENODEV;
 
 			/* Enable zlps by default for ECM conformance;
 			 * override for musb_hdrc (avoids txdma ovhead).
@@ -598,38 +550,24 @@ static int ecm_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 		 * just guarantee that a speed notification is always sent.
 		 */
 		ecm_notify(ecm);
-	} else
-		goto fail;
+	}
 
 	return 0;
-fail:
-	return -EINVAL;
 }
 
-/* Because the data interface supports multiple altsettings,
- * this ECM function *MUST* implement a get_alt() method.
- */
-static int ecm_get_alt(struct usb_function *f, unsigned intf)
+static void ecm_clear_alt(struct usb_function *f, unsigned intf, unsigned alt)
 {
-	struct f_ecm		*ecm = func_to_ecm(f);
-
-	if (intf == ecm->ctrl_id)
-		return 0;
-	return ecm->port.in_ep->enabled ? 1 : 0;
-}
-
-static void ecm_disable(struct usb_function *f)
-{
-	struct f_ecm		*ecm = func_to_ecm(f);
+	struct f_ecm            *ecm = func_to_ecm(f);
 	struct usb_composite_dev *cdev = f->config->cdev;
 
 	DBG(cdev, "ecm deactivated\n");
 
-	if (ecm->port.in_ep->enabled)
+	if (intf == 0) {
+		kfree(ecm->notify_req->buf);
+		usb_ep_free_request(ecm->notify, ecm->notify_req);
+	} else if (intf == 1 && alt == 1) {
 		gether_disconnect(&ecm->port);
-
-	usb_ep_disable(ecm->notify);
-	ecm->notify->desc = NULL;
+	}
 }
 
 /*-------------------------------------------------------------------------*/
@@ -676,20 +614,33 @@ static void ecm_close(struct gether *geth)
 
 /* ethernet function driver setup/binding */
 
-static int
-ecm_bind(struct usb_configuration *c, struct usb_function *f)
+static int ecm_prep_descs(struct usb_function *f)
 {
-	struct usb_composite_dev *cdev = c->cdev;
-	struct f_ecm		*ecm = func_to_ecm(f);
+	struct usb_composite_dev *cdev = f->config->cdev;
 	struct usb_string	*us;
-	int			status;
-	struct usb_ep		*ep;
-
-	struct f_ecm_opts	*ecm_opts;
 
 	if (!can_support_ecm(cdev->gadget))
 		return -EINVAL;
 
+	us = usb_gstrings_attach(cdev, ecm_strings,
+			ARRAY_SIZE(ecm_string_defs));
+	if (IS_ERR(us))
+		return PTR_ERR(us);
+	ecm_control_intf.iInterface = us[0].id;
+	ecm_data_intf.iInterface = us[2].id;
+	ecm_desc.iMACAddress = us[1].id;
+	ecm_iad_descriptor.iFunction = us[3].id;
+
+	return usb_function_set_descs(f, &ecm_descs);
+}
+
+static int ecm_prep_vendor_descs(struct usb_function *f)
+{
+	struct f_ecm		*ecm = func_to_ecm(f);
+	struct f_ecm_opts	*ecm_opts;
+	struct usb_composite_dev *cdev = f->config->cdev;
+	int			status, intf0_id, intf1_id;
+
 	ecm_opts = container_of(f->fi, struct f_ecm_opts, func_inst);
 
 	/*
@@ -709,86 +660,26 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 		ecm_opts->bound = true;
 	}
 
-	us = usb_gstrings_attach(cdev, ecm_strings,
-				 ARRAY_SIZE(ecm_string_defs));
-	if (IS_ERR(us))
-		return PTR_ERR(us);
-	ecm_control_intf.iInterface = us[0].id;
-	ecm_data_intf.iInterface = us[2].id;
-	ecm_desc.iMACAddress = us[1].id;
-	ecm_iad_descriptor.iFunction = us[3].id;
+	intf0_id = usb_get_interface_id(f, 0);
+	intf1_id = usb_get_interface_id(f, 1);
 
-	/* allocate instance-specific interface IDs */
-	status = usb_interface_id(c, f);
-	if (status < 0)
-		goto fail;
-	ecm->ctrl_id = status;
-	ecm_iad_descriptor.bFirstInterface = status;
-
-	ecm_control_intf.bInterfaceNumber = status;
-	ecm_union_desc.bMasterInterface0 = status;
-
-	status = usb_interface_id(c, f);
-	if (status < 0)
-		goto fail;
-	ecm->data_id = status;
-
-	ecm_data_nop_intf.bInterfaceNumber = status;
-	ecm_data_intf.bInterfaceNumber = status;
-	ecm_union_desc.bSlaveInterface0 = status;
-
-	status = -ENODEV;
-
-	/* allocate instance-specific endpoints */
-	ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_in_desc);
-	if (!ep)
-		goto fail;
-	ecm->port.in_ep = ep;
-
-	ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_out_desc);
-	if (!ep)
-		goto fail;
-	ecm->port.out_ep = ep;
-
-	/* NOTE:  a status/notification endpoint is *OPTIONAL* but we
-	 * don't treat it that way.  It's simpler, and some newer CDC
-	 * profiles (wireless handsets) no longer treat it as optional.
-	 */
-	ep = usb_ep_autoconfig(cdev->gadget, &fs_ecm_notify_desc);
-	if (!ep)
-		goto fail;
-	ecm->notify = ep;
-
-	status = -ENOMEM;
-
-	/* allocate notification request and buffer */
-	ecm->notify_req = usb_ep_alloc_request(ep, GFP_KERNEL);
-	if (!ecm->notify_req)
-		goto fail;
-	ecm->notify_req->buf = kmalloc(ECM_STATUS_BYTECOUNT, GFP_KERNEL);
-	if (!ecm->notify_req->buf)
-		goto fail;
-	ecm->notify_req->context = ecm;
-	ecm->notify_req->complete = ecm_notify_complete;
-
-	/* support all relevant hardware speeds... we expect that when
-	 * hardware is dual speed, all bulk-capable endpoints work at
-	 * both speeds
-	 */
-	hs_ecm_in_desc.bEndpointAddress = fs_ecm_in_desc.bEndpointAddress;
-	hs_ecm_out_desc.bEndpointAddress = fs_ecm_out_desc.bEndpointAddress;
-	hs_ecm_notify_desc.bEndpointAddress =
-		fs_ecm_notify_desc.bEndpointAddress;
+	ecm->ctrl_id = intf0_id;
+	ecm->data_id = intf1_id;
+
+	ecm_iad_descriptor.bFirstInterface = intf0_id;
+
+	ecm_union_desc.bMasterInterface0 = intf0_id;
+	ecm_union_desc.bSlaveInterface0 = intf1_id;
 
-	ss_ecm_in_desc.bEndpointAddress = fs_ecm_in_desc.bEndpointAddress;
-	ss_ecm_out_desc.bEndpointAddress = fs_ecm_out_desc.bEndpointAddress;
-	ss_ecm_notify_desc.bEndpointAddress =
-		fs_ecm_notify_desc.bEndpointAddress;
+	usb_function_add_vendor_desc(f,
+			(struct usb_descriptor_header *)&ecm_iad_descriptor);
 
-	status = usb_assign_descriptors(f, ecm_fs_function, ecm_hs_function,
-			ecm_ss_function);
-	if (status)
-		goto fail;
+	usb_altset_add_vendor_desc(f, 0, 0,
+			(struct usb_descriptor_header *)&ecm_header_desc);
+	usb_altset_add_vendor_desc(f, 0, 0,
+			(struct usb_descriptor_header *)&ecm_union_desc);
+	usb_altset_add_vendor_desc(f, 0, 0,
+			(struct usb_descriptor_header *)&ecm_desc);
 
 	/* NOTE:  all that is done without knowing or caring about
 	 * the network link ... which is unavailable to this code
@@ -798,22 +689,7 @@ ecm_bind(struct usb_configuration *c, struct usb_function *f)
 	ecm->port.open = ecm_open;
 	ecm->port.close = ecm_close;
 
-	DBG(cdev, "CDC Ethernet: %s speed IN/%s OUT/%s NOTIFY/%s\n",
-			gadget_is_superspeed(c->cdev->gadget) ? "super" :
-			gadget_is_dualspeed(c->cdev->gadget) ? "dual" : "full",
-			ecm->port.in_ep->name, ecm->port.out_ep->name,
-			ecm->notify->name);
 	return 0;
-
-fail:
-	if (ecm->notify_req) {
-		kfree(ecm->notify_req->buf);
-		usb_ep_free_request(ecm->notify, ecm->notify_req);
-	}
-
-	ERROR(cdev, "%s: can't bind, err %d\n", f->name, status);
-
-	return status;
 }
 
 static inline struct f_ecm_opts *to_f_ecm_opts(struct config_item *item)
@@ -897,18 +773,6 @@ static void ecm_free(struct usb_function *f)
 	mutex_unlock(&opts->lock);
 }
 
-static void ecm_unbind(struct usb_configuration *c, struct usb_function *f)
-{
-	struct f_ecm		*ecm = func_to_ecm(f);
-
-	DBG(c->cdev, "ecm unbind\n");
-
-	usb_free_all_descriptors(f);
-
-	kfree(ecm->notify_req->buf);
-	usb_ep_free_request(ecm->notify, ecm->notify_req);
-}
-
 static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 {
 	struct f_ecm	*ecm;
@@ -940,12 +804,11 @@ static struct usb_function *ecm_alloc(struct usb_function_instance *fi)
 
 	ecm->port.func.name = "cdc_ethernet";
 	/* descriptors are per-instance copies */
-	ecm->port.func.bind = ecm_bind;
-	ecm->port.func.unbind = ecm_unbind;
+	ecm->port.func.prep_descs = ecm_prep_descs;
+	ecm->port.func.prep_vendor_descs = ecm_prep_vendor_descs;
 	ecm->port.func.set_alt = ecm_set_alt;
-	ecm->port.func.get_alt = ecm_get_alt;
+	ecm->port.func.clear_alt = ecm_clear_alt;
 	ecm->port.func.setup = ecm_setup;
-	ecm->port.func.disable = ecm_disable;
 	ecm->port.func.free_func = ecm_free;
 
 	return &ecm->port.func;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261477 — [PATCH 19/23] usb: gadget: configfs: add new composite API support

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 19/23] usb: gadget: configfs: add new composite API support
Message-ID<qqJlW-2OU-49@gated-at.bofh.it>
In reply to#1261473
Handle functions using new API properly.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/configfs.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index d2101d8..fa5334f 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1424,6 +1424,11 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 				goto err_purge_funcs;
 			}
 		}
+
+		ret = usb_config_do_bind(c);
+		if (ret)
+			goto err_purge_funcs;
+
 		usb_ep_autoconfig_reset(cdev->gadget);
 	}
 	if (cdev->use_os_string) {
@@ -1432,10 +1437,23 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 			goto err_purge_funcs;
 	}
 
+	ret = composite_prep_vendor_descs(cdev);
+	if (ret < 0)
+		goto err_free_vendor_descs;
+
+	ret = composite_generate_old_descs(cdev);
+	if (ret < 0)
+		goto err_free_old_descs;
+
 	usb_ep_autoconfig_reset(cdev->gadget);
 	return 0;
 
+err_free_old_descs:
+	composite_free_old_descs(cdev);
+err_free_vendor_descs:
+	composite_free_vendor_descs(cdev);
 err_purge_funcs:
+	composite_free_descs(cdev);
 	purge_configs_funcs(gi);
 err_comp_cleanup:
 	composite_dev_cleanup(cdev);
@@ -1452,6 +1470,10 @@ static void configfs_composite_unbind(struct usb_gadget *gadget)
 	cdev = get_gadget_data(gadget);
 	gi = container_of(cdev, struct gadget_info, cdev);
 
+	composite_free_old_descs(cdev);
+	composite_free_vendor_descs(cdev);
+	composite_free_descs(cdev);
+
 	kfree(otg_desc[0]);
 	otg_desc[0] = NULL;
 	purge_configs_funcs(gi);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261479 — [PATCH 15/23] usb: gadget: composite: handle get_alt() automatically

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 15/23] usb: gadget: composite: handle get_alt() automatically
Message-ID<qqJlW-2OU-53@gated-at.bofh.it>
In reply to#1261473
As now we store current altsetting number for each interface, we can
handle USB_REQ_GET_INTERFACE automatically.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 5ce95d2..21f8c51 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2075,7 +2075,13 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		if (!f)
 			break;
 		/* lots of interfaces only need altsetting zero... */
-		value = f->get_alt ? f->get_alt(f, w_index) : 0;
+		if (usb_function_is_new_api(f)) {
+			value = usb_interface_id_to_index(f, intf);
+			if (value >= 0)
+				value = f->descs->intfs[value]->cur_altset;
+		} else {
+			value = f->get_alt ? f->get_alt(f, w_index) : 0;
+		}
 		if (value < 0)
 			break;
 		*((u8 *)req->buf) = value;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261480 — [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable
Message-ID<qqJlX-2OU-57@gated-at.bofh.it>
In reply to#1261473
So far it was decided during the bind process whether is iso altsetting
included to f_sourcesink function or not. This decision was based on
availability of isochronous endpoints.

Since we can assemble gadget driver using composite framework and configfs
from many different functions, availability of given type of endpoint
can depend on selected components or even on their order in given
configuration.

This can result with non-obvious behavior - even small, seemingly unrelated
change in gadget configuration can decide if we have second altsetting with
iso endpoints in given sourcesink function instance or not.

Because of this it's way better to have additional parameter allowing user
to decide if he/she wants to have iso altsetting, and if iso altsetting is
included, and there are no iso endpoints available, function bind will fail
instead of silently allowing to have non-complete function bound.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_sourcesink.c | 99 ++++++++++++++++++++----------
 drivers/usb/gadget/function/g_zero.h       |  3 +
 drivers/usb/gadget/legacy/zero.c           |  6 ++
 3 files changed, 77 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index d7646d3..1d6ec88 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -56,6 +56,7 @@ struct f_sourcesink {
 	unsigned isoc_maxpacket;
 	unsigned isoc_mult;
 	unsigned isoc_maxburst;
+	unsigned isoc_enabled;
 	unsigned buflen;
 };
 
@@ -347,17 +348,28 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
 
 	/* allocate bulk endpoints */
 	ss->in_ep = usb_ep_autoconfig(cdev->gadget, &fs_source_desc);
-	if (!ss->in_ep) {
-autoconf_fail:
-		ERROR(cdev, "%s: can't autoconfigure on %s\n",
-			f->name, cdev->gadget->name);
-		return -ENODEV;
-	}
+	if (!ss->in_ep)
+		goto autoconf_fail;
 
 	ss->out_ep = usb_ep_autoconfig(cdev->gadget, &fs_sink_desc);
 	if (!ss->out_ep)
 		goto autoconf_fail;
 
+	/* support high speed hardware */
+	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
+	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
+
+	/* support super speed hardware */
+	ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
+	ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
+
+	if (!ss->isoc_enabled) {
+		fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
+		hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
+		ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
+		goto no_iso;
+	}
+
 	/* sanity check the isoc module parameters */
 	if (ss->isoc_interval < 1)
 		ss->isoc_interval = 1;
@@ -379,30 +391,14 @@ autoconf_fail:
 	/* allocate iso endpoints */
 	ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
 	if (!ss->iso_in_ep)
-		goto no_iso;
+		goto autoconf_fail;
 
 	ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_sink_desc);
-	if (!ss->iso_out_ep) {
-		usb_ep_autoconfig_release(ss->iso_in_ep);
-		ss->iso_in_ep = NULL;
-no_iso:
-		/*
-		 * We still want to work even if the UDC doesn't have isoc
-		 * endpoints, so null out the alt interface that contains
-		 * them and continue.
-		 */
-		fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
-		hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
-		ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
-	}
+	if (!ss->iso_out_ep)
+		goto autoconf_fail;
 
 	if (ss->isoc_maxpacket > 1024)
 		ss->isoc_maxpacket = 1024;
-
-	/* support high speed hardware */
-	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
-	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
-
 	/*
 	 * Fill in the HS isoc descriptors from the module parameters.
 	 * We assume that the user knows what they are doing and won't
@@ -419,12 +415,6 @@ no_iso:
 	hs_iso_sink_desc.bInterval = ss->isoc_interval;
 	hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
 
-	/* support super speed hardware */
-	ss_source_desc.bEndpointAddress =
-		fs_source_desc.bEndpointAddress;
-	ss_sink_desc.bEndpointAddress =
-		fs_sink_desc.bEndpointAddress;
-
 	/*
 	 * Fill in the SS isoc descriptors from the module parameters.
 	 * We assume that the user knows what they are doing and won't
@@ -447,6 +437,7 @@ no_iso:
 		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
 	ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
 
+no_iso:
 	ret = usb_assign_descriptors(f, fs_source_sink_descs,
 			hs_source_sink_descs, ss_source_sink_descs);
 	if (ret)
@@ -459,6 +450,11 @@ no_iso:
 			ss->iso_in_ep ? ss->iso_in_ep->name : "<none>",
 			ss->iso_out_ep ? ss->iso_out_ep->name : "<none>");
 	return 0;
+
+autoconf_fail:
+	ERROR(cdev, "%s: can't autoconfigure on %s\n",
+			f->name, cdev->gadget->name);
+	return -ENODEV;
 }
 
 static void
@@ -868,6 +864,7 @@ static struct usb_function *source_sink_alloc_func(
 	ss->isoc_maxpacket = ss_opts->isoc_maxpacket;
 	ss->isoc_mult = ss_opts->isoc_mult;
 	ss->isoc_maxburst = ss_opts->isoc_maxburst;
+	ss->isoc_enabled = ss_opts->isoc_enabled;
 	ss->buflen = ss_opts->bulk_buflen;
 
 	ss->function.name = "source/sink";
@@ -1125,6 +1122,45 @@ static struct f_ss_opts_attribute f_ss_opts_isoc_maxburst =
 			f_ss_opts_isoc_maxburst_show,
 			f_ss_opts_isoc_maxburst_store);
 
+static ssize_t f_ss_opts_isoc_enabled_show(struct f_ss_opts *opts, char *page)
+{
+	int result;
+
+	mutex_lock(&opts->lock);
+	result = sprintf(page, "%u\n", opts->isoc_enabled);
+	mutex_unlock(&opts->lock);
+
+	return result;
+}
+
+static ssize_t f_ss_opts_isoc_enabled_store(struct f_ss_opts *opts,
+				       const char *page, size_t len)
+{
+	int ret;
+	bool enabled;
+
+	mutex_lock(&opts->lock);
+	if (opts->refcnt) {
+		ret = -EBUSY;
+		goto end;
+	}
+
+	ret = strtobool(page, &enabled);
+	if (ret)
+		goto end;
+
+	opts->isoc_enabled = enabled;
+	ret = len;
+end:
+	mutex_unlock(&opts->lock);
+	return ret;
+}
+
+static struct f_ss_opts_attribute f_ss_opts_isoc_enabled =
+	__CONFIGFS_ATTR(isoc_enabled, S_IRUGO | S_IWUSR,
+			f_ss_opts_isoc_enabled_show,
+			f_ss_opts_isoc_enabled_store);
+
 static ssize_t f_ss_opts_bulk_buflen_show(struct f_ss_opts *opts, char *page)
 {
 	int result;
@@ -1170,6 +1206,7 @@ static struct configfs_attribute *ss_attrs[] = {
 	&f_ss_opts_isoc_maxpacket.attr,
 	&f_ss_opts_isoc_mult.attr,
 	&f_ss_opts_isoc_maxburst.attr,
+	&f_ss_opts_isoc_enabled.attr,
 	&f_ss_opts_bulk_buflen.attr,
 	NULL,
 };
diff --git a/drivers/usb/gadget/function/g_zero.h b/drivers/usb/gadget/function/g_zero.h
index 15f1809..8a99071 100644
--- a/drivers/usb/gadget/function/g_zero.h
+++ b/drivers/usb/gadget/function/g_zero.h
@@ -10,6 +10,7 @@
 #define GZERO_QLEN		32
 #define GZERO_ISOC_INTERVAL	4
 #define GZERO_ISOC_MAXPACKET	1024
+#define GZERO_ISOC_ENABLED	1
 
 struct usb_zero_options {
 	unsigned pattern;
@@ -17,6 +18,7 @@ struct usb_zero_options {
 	unsigned isoc_maxpacket;
 	unsigned isoc_mult;
 	unsigned isoc_maxburst;
+	unsigned isoc_enabled;
 	unsigned bulk_buflen;
 	unsigned qlen;
 };
@@ -28,6 +30,7 @@ struct f_ss_opts {
 	unsigned isoc_maxpacket;
 	unsigned isoc_mult;
 	unsigned isoc_maxburst;
+	unsigned isoc_enabled;
 	unsigned bulk_buflen;
 
 	/*
diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
index 37a4100..3579310 100644
--- a/drivers/usb/gadget/legacy/zero.c
+++ b/drivers/usb/gadget/legacy/zero.c
@@ -66,6 +66,7 @@ module_param(loopdefault, bool, S_IRUGO|S_IWUSR);
 static struct usb_zero_options gzero_options = {
 	.isoc_interval = GZERO_ISOC_INTERVAL,
 	.isoc_maxpacket = GZERO_ISOC_MAXPACKET,
+	.isoc_enabled = GZERO_ISOC_ENABLED,
 	.bulk_buflen = GZERO_BULK_BUFLEN,
 	.qlen = GZERO_QLEN,
 };
@@ -249,6 +250,10 @@ module_param_named(isoc_maxburst, gzero_options.isoc_maxburst, uint,
 		S_IRUGO|S_IWUSR);
 MODULE_PARM_DESC(isoc_maxburst, "0 - 15 (ss only)");
 
+module_param_named(isoc_enabled, gzero_options.isoc_enabled, uint,
+		S_IRUGO|S_IWUSR);
+MODULE_PARM_DESC(isoc_enabled, "0 - disabled, 1 - enabled");
+
 static struct usb_function *func_lb;
 static struct usb_function_instance *func_inst_lb;
 
@@ -284,6 +289,7 @@ static int zero_bind(struct usb_composite_dev *cdev)
 	ss_opts->isoc_maxpacket = gzero_options.isoc_maxpacket;
 	ss_opts->isoc_mult = gzero_options.isoc_mult;
 	ss_opts->isoc_maxburst = gzero_options.isoc_maxburst;
+	ss_opts->isoc_enabled = gzero_options.isoc_enabled;
 	ss_opts->bulk_buflen = gzero_options.bulk_buflen;
 
 	func_ss = usb_get_function(func_inst_ss);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1263802 — Re: [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-06 08:30 +0100
SubjectRe: [PATCH 01/23] usb: gadget: f_sourcesink: make ISO altset user-selectable
Message-ID<qrJDb-1sw-1@gated-at.bofh.it>
In reply to#1261480
On 11/06/2015 04:05 AM, Peter Chen wrote:
> On Tue, Nov 03, 2015 at 01:53:40PM +0100, Robert Baldyga wrote:
>> So far it was decided during the bind process whether is iso altsetting
>> included to f_sourcesink function or not. This decision was based on
>> availability of isochronous endpoints.
>>
>> Since we can assemble gadget driver using composite framework and configfs
>> from many different functions, availability of given type of endpoint
>> can depend on selected components or even on their order in given
>> configuration.
>>
>> This can result with non-obvious behavior - even small, seemingly unrelated
>> change in gadget configuration can decide if we have second altsetting with
>> iso endpoints in given sourcesink function instance or not.
>>
>> Because of this it's way better to have additional parameter allowing user
>> to decide if he/she wants to have iso altsetting, and if iso altsetting is
>> included, and there are no iso endpoints available, function bind will fail
>> instead of silently allowing to have non-complete function bound.
> 
> Hi Robert,
> 
> Why another isoc_enabled parameter is needed instead of judging if it
> is supported through gadget framework? Any use cases can't be supported
> by current way?
> 

It's because guessing during bind process leads to non-obvious behavior
- we can have iso altsetting included into function or not depending on
many seemingly unrelated factors.

Moreover SourceSink, which is in fact testing Function, is the only
Function which implements conditional altsetting enabling, and we
definitely don't want testing function to behave that much differently
from another USB Functions.

The third reason is that modifying descriptors set depending on
availability of given endpoint type during bind process complicates bind
process automatization, which I implement in this patch set. After all,
I don't know any real USB device doing such strange thing.

Thanks,
Robert

>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> ---
>>  drivers/usb/gadget/function/f_sourcesink.c | 99 ++++++++++++++++++++----------
>>  drivers/usb/gadget/function/g_zero.h       |  3 +
>>  drivers/usb/gadget/legacy/zero.c           |  6 ++
>>  3 files changed, 77 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
>> index d7646d3..1d6ec88 100644
>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>> @@ -56,6 +56,7 @@ struct f_sourcesink {
>>  	unsigned isoc_maxpacket;
>>  	unsigned isoc_mult;
>>  	unsigned isoc_maxburst;
>> +	unsigned isoc_enabled;
>>  	unsigned buflen;
>>  };
>>  
>> @@ -347,17 +348,28 @@ sourcesink_bind(struct usb_configuration *c, struct usb_function *f)
>>  
>>  	/* allocate bulk endpoints */
>>  	ss->in_ep = usb_ep_autoconfig(cdev->gadget, &fs_source_desc);
>> -	if (!ss->in_ep) {
>> -autoconf_fail:
>> -		ERROR(cdev, "%s: can't autoconfigure on %s\n",
>> -			f->name, cdev->gadget->name);
>> -		return -ENODEV;
>> -	}
>> +	if (!ss->in_ep)
>> +		goto autoconf_fail;
>>  
>>  	ss->out_ep = usb_ep_autoconfig(cdev->gadget, &fs_sink_desc);
>>  	if (!ss->out_ep)
>>  		goto autoconf_fail;
>>  
>> +	/* support high speed hardware */
>> +	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> +	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
>> +
>> +	/* support super speed hardware */
>> +	ss_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> +	ss_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
>> +
>> +	if (!ss->isoc_enabled) {
>> +		fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
>> +		hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
>> +		ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>> +		goto no_iso;
>> +	}
>> +
>>  	/* sanity check the isoc module parameters */
>>  	if (ss->isoc_interval < 1)
>>  		ss->isoc_interval = 1;
>> @@ -379,30 +391,14 @@ autoconf_fail:
>>  	/* allocate iso endpoints */
>>  	ss->iso_in_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_source_desc);
>>  	if (!ss->iso_in_ep)
>> -		goto no_iso;
>> +		goto autoconf_fail;
>>  
>>  	ss->iso_out_ep = usb_ep_autoconfig(cdev->gadget, &fs_iso_sink_desc);
>> -	if (!ss->iso_out_ep) {
>> -		usb_ep_autoconfig_release(ss->iso_in_ep);
>> -		ss->iso_in_ep = NULL;
>> -no_iso:
>> -		/*
>> -		 * We still want to work even if the UDC doesn't have isoc
>> -		 * endpoints, so null out the alt interface that contains
>> -		 * them and continue.
>> -		 */
>> -		fs_source_sink_descs[FS_ALT_IFC_1_OFFSET] = NULL;
>> -		hs_source_sink_descs[HS_ALT_IFC_1_OFFSET] = NULL;
>> -		ss_source_sink_descs[SS_ALT_IFC_1_OFFSET] = NULL;
>> -	}
>> +	if (!ss->iso_out_ep)
>> +		goto autoconf_fail;
>>  
>>  	if (ss->isoc_maxpacket > 1024)
>>  		ss->isoc_maxpacket = 1024;
>> -
>> -	/* support high speed hardware */
>> -	hs_source_desc.bEndpointAddress = fs_source_desc.bEndpointAddress;
>> -	hs_sink_desc.bEndpointAddress = fs_sink_desc.bEndpointAddress;
>> -
>>  	/*
>>  	 * Fill in the HS isoc descriptors from the module parameters.
>>  	 * We assume that the user knows what they are doing and won't
>> @@ -419,12 +415,6 @@ no_iso:
>>  	hs_iso_sink_desc.bInterval = ss->isoc_interval;
>>  	hs_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>>  
>> -	/* support super speed hardware */
>> -	ss_source_desc.bEndpointAddress =
>> -		fs_source_desc.bEndpointAddress;
>> -	ss_sink_desc.bEndpointAddress =
>> -		fs_sink_desc.bEndpointAddress;
>> -
>>  	/*
>>  	 * Fill in the SS isoc descriptors from the module parameters.
>>  	 * We assume that the user knows what they are doing and won't
>> @@ -447,6 +437,7 @@ no_iso:
>>  		(ss->isoc_mult + 1) * (ss->isoc_maxburst + 1);
>>  	ss_iso_sink_desc.bEndpointAddress = fs_iso_sink_desc.bEndpointAddress;
>>  
>> +no_iso:
>>  	ret = usb_assign_descriptors(f, fs_source_sink_descs,
>>  			hs_source_sink_descs, ss_source_sink_descs);
>>  	if (ret)
>> @@ -459,6 +450,11 @@ no_iso:
>>  			ss->iso_in_ep ? ss->iso_in_ep->name : "<none>",
>>  			ss->iso_out_ep ? ss->iso_out_ep->name : "<none>");
>>  	return 0;
>> +
>> +autoconf_fail:
>> +	ERROR(cdev, "%s: can't autoconfigure on %s\n",
>> +			f->name, cdev->gadget->name);
>> +	return -ENODEV;
>>  }
>>  
>>  static void
>> @@ -868,6 +864,7 @@ static struct usb_function *source_sink_alloc_func(
>>  	ss->isoc_maxpacket = ss_opts->isoc_maxpacket;
>>  	ss->isoc_mult = ss_opts->isoc_mult;
>>  	ss->isoc_maxburst = ss_opts->isoc_maxburst;
>> +	ss->isoc_enabled = ss_opts->isoc_enabled;
>>  	ss->buflen = ss_opts->bulk_buflen;
>>  
>>  	ss->function.name = "source/sink";
>> @@ -1125,6 +1122,45 @@ static struct f_ss_opts_attribute f_ss_opts_isoc_maxburst =
>>  			f_ss_opts_isoc_maxburst_show,
>>  			f_ss_opts_isoc_maxburst_store);
>>  
>> +static ssize_t f_ss_opts_isoc_enabled_show(struct f_ss_opts *opts, char *page)
>> +{
>> +	int result;
>> +
>> +	mutex_lock(&opts->lock);
>> +	result = sprintf(page, "%u\n", opts->isoc_enabled);
>> +	mutex_unlock(&opts->lock);
>> +
>> +	return result;
>> +}
>> +
>> +static ssize_t f_ss_opts_isoc_enabled_store(struct f_ss_opts *opts,
>> +				       const char *page, size_t len)
>> +{
>> +	int ret;
>> +	bool enabled;
>> +
>> +	mutex_lock(&opts->lock);
>> +	if (opts->refcnt) {
>> +		ret = -EBUSY;
>> +		goto end;
>> +	}
>> +
>> +	ret = strtobool(page, &enabled);
>> +	if (ret)
>> +		goto end;
>> +
>> +	opts->isoc_enabled = enabled;
>> +	ret = len;
>> +end:
>> +	mutex_unlock(&opts->lock);
>> +	return ret;
>> +}
>> +
>> +static struct f_ss_opts_attribute f_ss_opts_isoc_enabled =
>> +	__CONFIGFS_ATTR(isoc_enabled, S_IRUGO | S_IWUSR,
>> +			f_ss_opts_isoc_enabled_show,
>> +			f_ss_opts_isoc_enabled_store);
>> +
>>  static ssize_t f_ss_opts_bulk_buflen_show(struct f_ss_opts *opts, char *page)
>>  {
>>  	int result;
>> @@ -1170,6 +1206,7 @@ static struct configfs_attribute *ss_attrs[] = {
>>  	&f_ss_opts_isoc_maxpacket.attr,
>>  	&f_ss_opts_isoc_mult.attr,
>>  	&f_ss_opts_isoc_maxburst.attr,
>> +	&f_ss_opts_isoc_enabled.attr,
>>  	&f_ss_opts_bulk_buflen.attr,
>>  	NULL,
>>  };
>> diff --git a/drivers/usb/gadget/function/g_zero.h b/drivers/usb/gadget/function/g_zero.h
>> index 15f1809..8a99071 100644
>> --- a/drivers/usb/gadget/function/g_zero.h
>> +++ b/drivers/usb/gadget/function/g_zero.h
>> @@ -10,6 +10,7 @@
>>  #define GZERO_QLEN		32
>>  #define GZERO_ISOC_INTERVAL	4
>>  #define GZERO_ISOC_MAXPACKET	1024
>> +#define GZERO_ISOC_ENABLED	1
>>  
>>  struct usb_zero_options {
>>  	unsigned pattern;
>> @@ -17,6 +18,7 @@ struct usb_zero_options {
>>  	unsigned isoc_maxpacket;
>>  	unsigned isoc_mult;
>>  	unsigned isoc_maxburst;
>> +	unsigned isoc_enabled;
>>  	unsigned bulk_buflen;
>>  	unsigned qlen;
>>  };
>> @@ -28,6 +30,7 @@ struct f_ss_opts {
>>  	unsigned isoc_maxpacket;
>>  	unsigned isoc_mult;
>>  	unsigned isoc_maxburst;
>> +	unsigned isoc_enabled;
>>  	unsigned bulk_buflen;
>>  
>>  	/*
>> diff --git a/drivers/usb/gadget/legacy/zero.c b/drivers/usb/gadget/legacy/zero.c
>> index 37a4100..3579310 100644
>> --- a/drivers/usb/gadget/legacy/zero.c
>> +++ b/drivers/usb/gadget/legacy/zero.c
>> @@ -66,6 +66,7 @@ module_param(loopdefault, bool, S_IRUGO|S_IWUSR);
>>  static struct usb_zero_options gzero_options = {
>>  	.isoc_interval = GZERO_ISOC_INTERVAL,
>>  	.isoc_maxpacket = GZERO_ISOC_MAXPACKET,
>> +	.isoc_enabled = GZERO_ISOC_ENABLED,
>>  	.bulk_buflen = GZERO_BULK_BUFLEN,
>>  	.qlen = GZERO_QLEN,
>>  };
>> @@ -249,6 +250,10 @@ module_param_named(isoc_maxburst, gzero_options.isoc_maxburst, uint,
>>  		S_IRUGO|S_IWUSR);
>>  MODULE_PARM_DESC(isoc_maxburst, "0 - 15 (ss only)");
>>  
>> +module_param_named(isoc_enabled, gzero_options.isoc_enabled, uint,
>> +		S_IRUGO|S_IWUSR);
>> +MODULE_PARM_DESC(isoc_enabled, "0 - disabled, 1 - enabled");
>> +
>>  static struct usb_function *func_lb;
>>  static struct usb_function_instance *func_inst_lb;
>>  
>> @@ -284,6 +289,7 @@ static int zero_bind(struct usb_composite_dev *cdev)
>>  	ss_opts->isoc_maxpacket = gzero_options.isoc_maxpacket;
>>  	ss_opts->isoc_mult = gzero_options.isoc_mult;
>>  	ss_opts->isoc_maxburst = gzero_options.isoc_maxburst;
>> +	ss_opts->isoc_enabled = gzero_options.isoc_enabled;
>>  	ss_opts->bulk_buflen = gzero_options.bulk_buflen;
>>  
>>  	func_ss = usb_get_function(func_inst_ss);
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261482 — [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:00 +0100
Subject[PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()
Message-ID<qqJlX-2OU-59@gated-at.bofh.it>
In reply to#1261473
USB requests in SourceSink function are allocated in sourcesink_get_alt()
function, so we prefer to free them rather in sourcesink_disable() than
in source_sink_complete() when request is completed with error. It provides
better symetry in resource management and improves code readability.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_sourcesink.c | 33 +++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index a8b68c6..d8f5f56 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -22,6 +22,8 @@
 #include "g_zero.h"
 #include "u_f.h"
 
+#define REQ_CNT	8
+
 /*
  * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
  * controller drivers.
@@ -51,6 +53,11 @@ struct f_sourcesink {
 	struct usb_ep		*iso_out_ep;
 	int			cur_alt;
 
+	struct usb_request	*in_req;
+	struct usb_request	*out_req;
+	struct usb_request	*iso_in_req[REQ_CNT];
+	struct usb_request	*iso_out_req[REQ_CNT];
+
 	unsigned pattern;
 	unsigned isoc_interval;
 	unsigned isoc_maxpacket;
@@ -561,7 +568,6 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
 				req->actual, req->length);
 		if (ep == ss->out_ep)
 			check_read_data(ss, req);
-		free_ep_req(ep, req);
 		return;
 
 	case -EOVERFLOW:		/* buffer overrun on read means that
@@ -613,7 +619,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
 		ep = is_in ? ss->in_ep : ss->out_ep;
 	}
 
-	for (i = 0; i < 8; i++) {
+	for (i = 0; i < REQ_CNT; i++) {
 		req = ss_alloc_ep_req(ep, size);
 		if (!req)
 			return -ENOMEM;
@@ -635,8 +641,18 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
 			free_ep_req(ep, req);
 		}
 
-		if (!is_iso)
+		if (is_iso) {
+			if (is_in)
+				ss->iso_in_req[i] = req;
+			else
+				ss->iso_out_req[i] = req;
+		} else {
+			if (is_in)
+				ss->in_req = req;
+			else
+				ss->out_req = req;
 			break;
+		}
 	}
 
 	return status;
@@ -764,8 +780,19 @@ static int sourcesink_get_alt(struct usb_function *f, unsigned intf)
 static void sourcesink_disable(struct usb_function *f)
 {
 	struct f_sourcesink	*ss = func_to_ss(f);
+	int i;
 
 	disable_source_sink(ss);
+
+	free_ep_req(ss->in_ep, ss->in_req);
+	free_ep_req(ss->out_ep, ss->out_req);
+
+	if (ss->iso_in_ep) {
+		for (i = 0; i < REQ_CNT; i++) {
+			free_ep_req(ss->iso_in_ep, ss->iso_in_req[i]);
+			free_ep_req(ss->iso_out_ep, ss->iso_out_req[i]);
+		}
+	}
 }
 
 /*-------------------------------------------------------------------------*/
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1263849 — Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-06 10:00 +0100
SubjectRe: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()
Message-ID<qrL2h-2eY-7@gated-at.bofh.it>
In reply to#1261482
On 11/06/2015 09:15 AM, Peter Chen wrote:
> On Tue, Nov 03, 2015 at 01:53:42PM +0100, Robert Baldyga wrote:
>> USB requests in SourceSink function are allocated in sourcesink_get_alt()
>> function, so we prefer to free them rather in sourcesink_disable() than
>> in source_sink_complete() when request is completed with error. It provides
>> better symetry in resource management and improves code readability.
>>
>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>> ---
>>  drivers/usb/gadget/function/f_sourcesink.c | 33 +++++++++++++++++++++++++++---
>>  1 file changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
>> index a8b68c6..d8f5f56 100644
>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>> @@ -22,6 +22,8 @@
>>  #include "g_zero.h"
>>  #include "u_f.h"
>>  
>> +#define REQ_CNT	8
>> +
> 
> It would be buffer if we can have module parameter for this like
> qlen at f_loopback.
> 
>>  /*
>>   * SOURCE/SINK FUNCTION ... a primary testing vehicle for USB peripheral
>>   * controller drivers.
>> @@ -51,6 +53,11 @@ struct f_sourcesink {
>>  	struct usb_ep		*iso_out_ep;
>>  	int			cur_alt;
>>  
>> +	struct usb_request	*in_req;
>> +	struct usb_request	*out_req;
>> +	struct usb_request	*iso_in_req[REQ_CNT];
>> +	struct usb_request	*iso_out_req[REQ_CNT];
>> +
>>  	unsigned pattern;
>>  	unsigned isoc_interval;
>>  	unsigned isoc_maxpacket;
>> @@ -561,7 +568,6 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
>>  				req->actual, req->length);
>>  		if (ep == ss->out_ep)
>>  			check_read_data(ss, req);
>> -		free_ep_req(ep, req);
> 
> I find the current code may cause memory leak, since above code
> is only called one time.
> 

I don't see what you mean. Can you explain a bit more in which situation
can memory leak take place?

I've only changed place where requests are freed. Now they are freed in
sourcesink_disable(). The current code is even more memory leak
resistant, because requests will be freed even in case of failure of
usb_ep_queue() in source_sink_complete(), which was not provided so far.

>>  		return;
>>  
>>  	case -EOVERFLOW:		/* buffer overrun on read means that
>> @@ -613,7 +619,7 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
>>  		ep = is_in ? ss->in_ep : ss->out_ep;
>>  	}
>>  
>> -	for (i = 0; i < 8; i++) {
>> +	for (i = 0; i < REQ_CNT; i++) {
>>  		req = ss_alloc_ep_req(ep, size);
>>  		if (!req)
>>  			return -ENOMEM;
>> @@ -635,8 +641,18 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
>>  			free_ep_req(ep, req);
>>  		}
>>  
>> -		if (!is_iso)
>> +		if (is_iso) {
>> +			if (is_in)
>> +				ss->iso_in_req[i] = req;
>> +			else
>> +				ss->iso_out_req[i] = req;
>> +		} else {
>> +			if (is_in)
>> +				ss->in_req = req;
>> +			else
>> +				ss->out_req = req;
> 
> The req will be different for both bulk and iso, why you only
> have array for iso requests?

Because only for iso endpoints there is allocated more than one request.
I don't know why, but I've decided to not change this.

> 
>>  			break;
>> +		}
>>  	}
>>  
>>  	return status;
>> @@ -764,8 +780,19 @@ static int sourcesink_get_alt(struct usb_function *f, unsigned intf)
>>  static void sourcesink_disable(struct usb_function *f)
>>  {
>>  	struct f_sourcesink	*ss = func_to_ss(f);
>> +	int i;
>>  
>>  	disable_source_sink(ss);
>> +
>> +	free_ep_req(ss->in_ep, ss->in_req);
>> +	free_ep_req(ss->out_ep, ss->out_req);
>> +
>> +	if (ss->iso_in_ep) {
>> +		for (i = 0; i < REQ_CNT; i++) {
>> +			free_ep_req(ss->iso_in_ep, ss->iso_in_req[i]);
>> +			free_ep_req(ss->iso_out_ep, ss->iso_out_req[i]);
>> +		}
>> +	}
>>  }
>>  
>>  /*-------------------------------------------------------------------------*/
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1263876 — Re: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()

FromKrzysztof Opasiak <k.opasiak@samsung.com>
Date2015-11-06 11:00 +0100
SubjectRe: [PATCH 03/23] usb: gadget: f_sourcesink: free requests in sourcesink_disable()
Message-ID<qrLYm-2Oc-19@gated-at.bofh.it>
In reply to#1263849

On 11/06/2015 10:48 AM, Peter Chen wrote:
> On Fri, Nov 06, 2015 at 09:50:11AM +0100, Robert Baldyga wrote:
>> On 11/06/2015 09:15 AM, Peter Chen wrote:
>>> On Tue, Nov 03, 2015 at 01:53:42PM +0100, Robert Baldyga wrote:
>>>> USB requests in SourceSink function are allocated in sourcesink_get_alt()
>>>> function, so we prefer to free them rather in sourcesink_disable() than
>>>> in source_sink_complete() when request is completed with error. It provides
>>>> better symetry in resource management and improves code readability.
>>>>
>>>> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
>>>> ---
>>>>   drivers/usb/gadget/function/f_sourcesink.c | 33 +++++++++++++++++++++++++++---
>>>>   1 file changed, 30 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
>>>> index a8b68c6..d8f5f56 100644
>>>> --- a/drivers/usb/gadget/function/f_sourcesink.c
>>>> +++ b/drivers/usb/gadget/function/f_sourcesink.c
>>>> @@ -22,6 +22,8 @@
>>>>   #include "g_zero.h"
>>>>   #include "u_f.h"
>>>>
>>>> +#define REQ_CNT	8
>>>> +
>>>
>>> It would be buffer if we can have module parameter for this like
>>> qlen at f_loopback.
>>>> @@ -561,7 +568,6 @@ static void source_sink_complete(struct usb_ep *ep, struct usb_request *req)
>>>>   				req->actual, req->length);
>>>>   		if (ep == ss->out_ep)
>>>>   			check_read_data(ss, req);
>>>> -		free_ep_req(ep, req);
>>>
>>> I find the current code may cause memory leak, since above code
>>> is only called one time.
>>>
>>
>> I don't see what you mean. Can you explain a bit more in which situation
>> can memory leak take place?
>
> Oh, sorry, I have not seen there is a "break;" at the source_sink_start_ep for
> none-iso endpoint, so, I am wrong, no memory leak here.
>
> I have tried changing bulk for 8 requests, the performance improves
> greatly, but still a little problem for OUT,  I will see what's the matter.
> Besides, it will be better we can have a qlen parameters like f_loopback,
> in that case, we can improve the performance for gadget, and get the best performance
> when testing with usbtest, I will do it later.
>

Moreover, I would suggest to add qlen and iso_qlen params not only qlen 
as even now we are using different qlen for bulk and iso.

Best regards,
-- 
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261483 — [PATCH 06/23] usb: gadget: composite: introduce new descriptors format

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 06/23] usb: gadget: composite: introduce new descriptors format
Message-ID<qqJvz-37I-9@gated-at.bofh.it>
In reply to#1261473
Introduce new structures designed to contain information about
descriptors. It splits descriptors in two categories:
1. Entity descs - interface and endpoint descriptors
2. Vendor descs - all other vendor and class specific descriptors

Entity descriptors are embedded in hierarchy of structures while vendor
descriptors are contained in linked lists. This distinction is caused
by fact, that entity descriptors are needed during gadget bind procedure,
while vendor descriptors can be supplied later, which is usually desired,
as these descriptors may need to be filled with interface numbers and
endpoint addresses which are assigned during gadget bind.

In result we can split descriptors creation process in two steps - first
collecs entity descriptors, perform the bind and then update and attach
all other descriptors. This process can be done this way not only for
each function separately, but also for entire gadget at once, which means
we can first gather descriptors from all functions in gadget, next perform
bind procedure, and then allow functions to supply additional descriptors.

It allows us to have autoconfig solver capable to better distibute ep
resources, and additionally, because we now store information about
endpoints, allows us to handle endpoint state inside composite framework,
and in result remove lots of boilerplate code from USB functions.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 include/linux/usb/composite.h | 119 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 1074b89..686c5f7 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -57,6 +57,121 @@
 struct usb_configuration;
 
 /**
+ * struct usb_composite_vendor_desc - vendor specific descriptor
+ * @desc: pointer to vendor specific descriptor
+ * @list: descriptor list element
+ *
+ * It's designed to be element of vendor specific descriptor list,
+ * which can be attached to function, interface (per altsetting) or
+ * endpoint.
+ */
+struct usb_composite_vendor_desc {
+	struct usb_descriptor_header *desc;
+	struct list_head list;
+};
+
+/**
+ * struct usb_composite_ep - representation of USB endpoint
+ * @fs.desc: FullSpeed descriptor
+ * @hs.desc: HighSpeed descriptor
+ * @ss.desc: SuperSpeed descriptor
+ * @ss_comp.desc: SuperSpeed Companion descriptor
+ * @vendor_descs: list of vendor specific descriptors
+ * @vendor_descs_num: count of vendor specific descriptors
+ * @ep: pointer to endpoint obtained during bind process
+ *
+ * We have pointer to each descriptor in union with pointer to descriptor
+ * header in order to avoid casting in many places in code, because in
+ * some situations we want to have access to fields of particular type
+ * of descriptor, while in other situations we want to treat all types
+ * of descriptors in the same way.
+ */
+struct usb_composite_ep {
+	union {
+		struct usb_descriptor_header *header;
+		struct usb_endpoint_descriptor *desc;
+	} fs;
+
+	union {
+		struct usb_descriptor_header *header;
+		struct usb_endpoint_descriptor *desc;
+	} hs;
+
+	union {
+		struct usb_descriptor_header *header;
+		struct usb_endpoint_descriptor *desc;
+	} ss;
+
+	union {
+		struct usb_descriptor_header *header;
+		struct usb_ss_ep_comp_descriptor *desc;
+	} ss_comp;
+
+	struct list_head vendor_descs;
+	int vendor_descs_num;
+
+	struct usb_ep *ep;
+};
+
+/**
+ * struct usb_composite_altset - representation of USB altsetting
+ * @alt.desc: interface (altsetting) descriptor
+ * @eps: array of endpoints in altsetting
+ * @eps_num: number of endpoints
+ * @vendor_descs: list of vendor specific descriptors
+ * @vendor_descs_num: count of vendor specific descriptors
+ *
+ * We have pointer to alt descriptor in union with pointer to descriptor
+ * header in order to avoid casting in many places in code, because in
+ * some situations we want to have access to fields of particular type
+ * of descriptor, while in other situations we want to treat all types
+ * of descriptors in the same way.
+ */
+struct usb_composite_altset {
+	union {
+		struct usb_descriptor_header *header;
+		struct usb_interface_descriptor *desc;
+	} alt;
+
+	struct usb_composite_ep **eps;
+	int eps_num;
+
+	struct list_head vendor_descs;
+	int vendor_descs_num;
+};
+
+/**
+ * struct usb_composite_intf - representation of USB interface
+ * @altsets: array of altsettings in interface
+ * @altsets_num: number of altsettings
+ * @cur_altset: number of currently selected altsetting
+ * @id: id number of interface in configuraion (value of
+ *	bInterfaceNumber in interface descriptor)
+ */
+struct usb_composite_intf {
+	struct usb_composite_altset **altsets;
+	int altsets_num;
+
+	int cur_altset;
+	u8 id;
+};
+
+/**
+ * struct usb_composite_descs - representation of USB descriptors
+ * @intfs: array of interfaces in function
+ * @intfs_num: number of interfaces
+ * @vendor_descs: list of vendor specific descriptors
+ * @vendor_descs_num: count of vendor specific descriptors
+ */
+struct usb_composite_descs {
+	struct usb_composite_intf **intfs;
+	int intfs_num;
+
+	struct list_head vendor_descs;
+	int vendor_descs_num;
+};
+
+/**
  * struct usb_os_desc_ext_prop - describes one "Extended Property"
  * @entry: used to keep a list of extended properties
  * @type: Extended Property type
@@ -126,6 +241,8 @@ struct usb_os_desc_table {
  *	string identifiers assigned during @bind(). If this
  *	pointer is null after initiation, the function will not
  *	be available at super speed.
+ * @descs: structure containing information about descriptors and endpoints
+ *	assigned during gadget bind.
  * @config: assigned when @usb_add_function() is called; this is the
  *	configuration with which this function is associated.
  * @os_desc_table: Table of (interface id, os descriptors) pairs. The function
@@ -187,6 +304,8 @@ struct usb_function {
 	struct usb_descriptor_header	**hs_descriptors;
 	struct usb_descriptor_header	**ss_descriptors;
 
+	struct usb_composite_descs	*descs;
+
 	struct usb_configuration	*config;
 
 	struct usb_os_desc_table	*os_desc_table;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261484 — [PATCH 10/23] usb: gadget: composite: handle vendor descs

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 10/23] usb: gadget: composite: handle vendor descs
Message-ID<qqJvz-37I-13@gated-at.bofh.it>
In reply to#1261473
After binding all configurations in gadget, call prep_vendor_descs()
for each function which uses new API and implements this callback.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c | 31 +++++++++++++++++++++++++++++++
 include/linux/usb/composite.h  |  2 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index f4189b1..9b4fcfe 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2566,6 +2566,33 @@ int usb_config_do_bind(struct usb_configuration *c)
 }
 EXPORT_SYMBOL_GPL(usb_config_do_bind);
 
+/**
+ * composite_prep_vendor_descs - for each function in each configuration call
+ *	prep_vendor_descs() callback.
+ * @cdev: composite device
+ */
+int composite_prep_vendor_descs(struct usb_composite_dev *cdev)
+{
+	struct usb_configuration *c;
+	struct usb_function *f;
+	int ret;
+
+	list_for_each_entry(c, &cdev->configs, list)
+		list_for_each_entry(f, &c->functions, list) {
+			if (!usb_function_is_new_api(f))
+				continue;
+			if (f->prep_vendor_descs) {
+				ret = f->prep_vendor_descs(f);
+				if (ret)
+					return ret;
+			}
+
+		}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(composite_prep_vendor_descs);
+
 static int composite_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *gdriver)
 {
@@ -2595,6 +2622,10 @@ static int composite_bind(struct usb_gadget *gadget,
 	if (status < 0)
 		goto fail;
 
+	status = composite_prep_vendor_descs(cdev);
+	if (status < 0)
+		goto fail;
+
 	if (cdev->use_os_string) {
 		status = composite_os_desc_req_prepare(cdev, gadget->ep0);
 		if (status)
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index a92da38..dc0ac28c 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -592,6 +592,8 @@ extern int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
 					 struct usb_ep *ep0);
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
 
+int composite_prep_vendor_descs(struct usb_composite_dev *cdev);
+
 void composite_free_descs(struct usb_composite_dev *cdev);
 void composite_free_vendor_descs(struct usb_composite_dev *cdev);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261485 — [PATCH 05/23] usb: gadget: configfs: fix error path

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 05/23] usb: gadget: configfs: fix error path
Message-ID<qqJvA-37I-17@gated-at.bofh.it>
In reply to#1261473
As usb_gstrings_attach() fail can happen when some USB functions are
are already added to some configurations (in prevoius loop iterations),
we should always call purge_configs_funcs() to be sure that failure is
be handled properly.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/configfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
index 294eb74..d2101d8 100644
--- a/drivers/usb/gadget/configfs.c
+++ b/drivers/usb/gadget/configfs.c
@@ -1411,7 +1411,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
 			s = usb_gstrings_attach(&gi->cdev, cfg->gstrings, 1);
 			if (IS_ERR(s)) {
 				ret = PTR_ERR(s);
-				goto err_comp_cleanup;
+				goto err_purge_funcs;
 			}
 			c->iConfiguration = s[0].id;
 		}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261506 — Re: [PATCH 05/23] usb: gadget: configfs: fix error path

FromSergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date2015-11-03 14:50 +0100
SubjectRe: [PATCH 05/23] usb: gadget: configfs: fix error path
Message-ID<qqK8j-3mL-13@gated-at.bofh.it>
In reply to#1261485
Hello.

On 11/3/2015 3:53 PM, Robert Baldyga wrote:

> As usb_gstrings_attach() fail can happen when some USB functions are

    Failure?

> are already added to some configurations (in prevoius loop iterations),

    Previous?

> we should always call purge_configs_funcs() to be sure that failure is
> be handled properly.
>
> Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261486 — [PATCH 11/23] usb: gadget: composite: generate old descs for compatibility

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 11/23] usb: gadget: composite: generate old descs for compatibility
Message-ID<qqJvA-37I-19@gated-at.bofh.it>
In reply to#1261473
For now we generate descriptor arrays for each speed as it is done by old
API functions, to allow use mixed new and old API based functions in single
configurations.

This will be removed after complete switch to new API.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c | 175 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/composite.h  |   2 +
 2 files changed, 177 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 9b4fcfe..b50ebdb 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2255,6 +2255,30 @@ void composite_free_vendor_descs(struct usb_composite_dev *cdev)
 }
 EXPORT_SYMBOL_GPL(composite_free_vendor_descs);
 
+/**
+ * composite_free_old_descs - for all functions implementing new API free old
+ *	descriptors arrays.
+ * @cdev: composite device
+ */
+void composite_free_old_descs(struct usb_composite_dev *cdev)
+{
+	struct usb_configuration *c;
+	struct usb_function *f;
+
+	list_for_each_entry(c, &cdev->configs, list)
+		list_for_each_entry(f, &c->functions, list) {
+			if (!usb_function_is_new_api(f))
+				continue;
+			kfree(f->fs_descriptors);
+			kfree(f->hs_descriptors);
+			kfree(f->ss_descriptors);
+			f->fs_descriptors = NULL;
+			f->hs_descriptors = NULL;
+			f->ss_descriptors = NULL;
+		}
+}
+EXPORT_SYMBOL_GPL(composite_free_old_descs);
+
 static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 {
 	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
@@ -2266,6 +2290,7 @@ static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 	 */
 	WARN_ON(cdev->config);
 
+	composite_free_old_descs(cdev);
 	composite_free_vendor_descs(cdev);
 	composite_free_descs(cdev);
 
@@ -2593,6 +2618,152 @@ int composite_prep_vendor_descs(struct usb_composite_dev *cdev)
 }
 EXPORT_SYMBOL_GPL(composite_prep_vendor_descs);
 
+/*
+ * function_add_desc() - Add given descriptor to descriptor arrays for
+ *	each supported speed, in proper version for each speed.
+ *
+ * @f - USB function
+ * @idx - pointer to index of descriptor in fs and hs array
+ * @idx_ss - pointer to index of descriptor in ss array
+ * @fs - descriptor for FS
+ * @hs - descriptor for HS
+ * @ss - descriptor for SS
+ *
+ * Indexes are automatically incremented.
+ *
+ * This function will be removed after converting all USB functions
+ * in kernel to new API.
+ */
+static inline void function_add_desc(struct usb_function *f,
+		int *idx, int *idx_ss,
+		struct usb_descriptor_header *fs,
+		struct usb_descriptor_header *hs,
+		struct usb_descriptor_header *ss) {
+	if (f->config->fullspeed)
+		f->fs_descriptors[*idx] = fs;
+	if (f->config->highspeed)
+		f->hs_descriptors[*idx] = hs;
+	if (f->config->superspeed)
+		f->ss_descriptors[*idx_ss] = ss;
+	++(*idx);
+	++(*idx_ss);
+}
+
+/*
+ * function_generate_old_descs() - generate descriptors array for each speed
+ *
+ * Allocate arrays of needed size and assign to them USB descriptors.
+ *
+ * This is temporary solution allowing coexistence to both old and new function
+ * API. It will be removed after converting all functions in kernel to new API.
+ */
+static int function_generate_old_descs(struct usb_function *f)
+{
+	struct usb_composite_intf *intf;
+	struct usb_composite_altset *alt;
+	struct usb_composite_ep *ep;
+	struct usb_composite_vendor_desc *vd;
+	int cnt, eps, i, a, e, idx, idx_ss;
+
+	cnt = f->descs->vendor_descs_num;
+	eps = 0;
+
+	for (i = 0; i < f->descs->intfs_num; ++i) {
+		intf = f->descs->intfs[i];
+		for (a = 0; a < intf->altsets_num; ++a) {
+			alt = intf->altsets[a];
+			cnt += alt->vendor_descs_num + 1;
+			eps += alt->eps_num;
+			for (e = 0; e < alt->eps_num; ++e)
+				cnt += alt->eps[e]->vendor_descs_num + 1;
+		}
+	}
+
+	if (f->config->fullspeed) {
+		f->fs_descriptors = kzalloc((cnt + 1) *
+				sizeof(*f->fs_descriptors), GFP_KERNEL);
+		if (!f->fs_descriptors)
+			return -ENOMEM;
+	}
+	if (f->config->highspeed) {
+		f->hs_descriptors = kzalloc((cnt + 1) *
+				sizeof(*f->hs_descriptors), GFP_KERNEL);
+		if (!f->hs_descriptors)
+			goto err;
+	}
+	if (f->config->superspeed) {
+		f->ss_descriptors = kzalloc((cnt + eps + 1) *
+				sizeof(*f->ss_descriptors), GFP_KERNEL);
+		if (!f->ss_descriptors)
+			goto err;
+	}
+
+	idx = 0;
+	idx_ss = 0;
+	list_for_each_entry(vd, &f->descs->vendor_descs, list)
+		function_add_desc(f, &idx, &idx_ss,
+				vd->desc, vd->desc, vd->desc);
+	for (i = 0; i < f->descs->intfs_num; ++i) {
+		intf = f->descs->intfs[i];
+		for (a = 0; a < intf->altsets_num; ++a) {
+			alt = intf->altsets[a];
+			function_add_desc(f, &idx, &idx_ss, alt->alt.header,
+					alt->alt.header, alt->alt.header);
+			list_for_each_entry(vd, &alt->vendor_descs, list)
+				function_add_desc(f, &idx, &idx_ss,
+						vd->desc, vd->desc, vd->desc);
+			for (e = 0; e < alt->eps_num; ++e) {
+				ep = alt->eps[e];
+				function_add_desc(f, &idx, &idx_ss,
+						ep->fs.header, ep->hs.header,
+						ep->ss.header);
+				if (f->config->superspeed)
+					f->ss_descriptors[idx_ss++] =
+							ep->ss_comp.header;
+				list_for_each_entry(vd,
+						&ep->vendor_descs, list)
+					function_add_desc(f, &idx, &idx_ss,
+						vd->desc, vd->desc, vd->desc);
+			}
+		}
+	}
+
+	return 0;
+
+err:
+	kfree(f->ss_descriptors);
+	f->ss_descriptors = NULL;
+	kfree(f->hs_descriptors);
+	f->hs_descriptors = NULL;
+	kfree(f->fs_descriptors);
+	f->fs_descriptors = NULL;
+
+	return -ENOMEM;
+}
+
+/*
+ * composite_generate_old_descs() - generate descriptors arrays for each
+ *	function in each configuraion
+ */
+int composite_generate_old_descs(struct usb_composite_dev *cdev)
+{
+	struct usb_configuration *c;
+	struct usb_function *f;
+	int ret;
+
+	list_for_each_entry(c, &cdev->configs, list)
+		list_for_each_entry(f, &c->functions, list) {
+			if (!usb_function_is_new_api(f))
+				continue;
+			ret = function_generate_old_descs(f);
+			if (ret)
+				return ret;
+		}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(composite_generate_old_descs);
+
 static int composite_bind(struct usb_gadget *gadget,
 		struct usb_gadget_driver *gdriver)
 {
@@ -2626,6 +2797,10 @@ static int composite_bind(struct usb_gadget *gadget,
 	if (status < 0)
 		goto fail;
 
+	status = composite_generate_old_descs(cdev);
+	if (status < 0)
+		goto fail;
+
 	if (cdev->use_os_string) {
 		status = composite_os_desc_req_prepare(cdev, gadget->ep0);
 		if (status)
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index dc0ac28c..7cef8c0 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -593,9 +593,11 @@ extern int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
 
 int composite_prep_vendor_descs(struct usb_composite_dev *cdev);
+int composite_generate_old_descs(struct usb_composite_dev *cdev);
 
 void composite_free_descs(struct usb_composite_dev *cdev);
 void composite_free_vendor_descs(struct usb_composite_dev *cdev);
+void composite_free_old_descs(struct usb_composite_dev *cdev);
 
 static inline struct usb_composite_driver *to_cdriver(
 		struct usb_gadget_driver *gdrv)
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261487 — [PATCH 02/23] usb: gadget: f_sourcesink: compute req size once

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 02/23] usb: gadget: f_sourcesink: compute req size once
Message-ID<qqJvB-37I-29@gated-at.bofh.it>
In reply to#1261473
Compute request size once before the loop instead of computing it in each
loop iteration.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/function/f_sourcesink.c | 45 +++++++++++++++---------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/drivers/usb/gadget/function/f_sourcesink.c b/drivers/usb/gadget/function/f_sourcesink.c
index 1d6ec88..a8b68c6 100644
--- a/drivers/usb/gadget/function/f_sourcesink.c
+++ b/drivers/usb/gadget/function/f_sourcesink.c
@@ -591,31 +591,30 @@ static int source_sink_start_ep(struct f_sourcesink *ss, bool is_in,
 {
 	struct usb_ep		*ep;
 	struct usb_request	*req;
-	int			i, size, status;
-
-	for (i = 0; i < 8; i++) {
-		if (is_iso) {
-			switch (speed) {
-			case USB_SPEED_SUPER:
-				size = ss->isoc_maxpacket *
-						(ss->isoc_mult + 1) *
-						(ss->isoc_maxburst + 1);
-				break;
-			case USB_SPEED_HIGH:
-				size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
-				break;
-			default:
-				size = ss->isoc_maxpacket > 1023 ?
-						1023 : ss->isoc_maxpacket;
-				break;
-			}
-			ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
-			req = ss_alloc_ep_req(ep, size);
-		} else {
-			ep = is_in ? ss->in_ep : ss->out_ep;
-			req = ss_alloc_ep_req(ep, 0);
+	int			i, size = 0, status;
+
+	if (is_iso) {
+		switch (speed) {
+		case USB_SPEED_SUPER:
+			size = ss->isoc_maxpacket *
+				(ss->isoc_mult + 1) *
+				(ss->isoc_maxburst + 1);
+			break;
+		case USB_SPEED_HIGH:
+			size = ss->isoc_maxpacket * (ss->isoc_mult + 1);
+			break;
+		default:
+			size = ss->isoc_maxpacket > 1023 ?
+				1023 : ss->isoc_maxpacket;
+			break;
 		}
+		ep = is_in ? ss->iso_in_ep : ss->iso_out_ep;
+	} else {
+		ep = is_in ? ss->in_ep : ss->out_ep;
+	}
 
+	for (i = 0; i < 8; i++) {
+		req = ss_alloc_ep_req(ep, size);
 		if (!req)
 			return -ENOMEM;
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1261488 — [PATCH 07/23] usb: gadget: composite: add functions for descriptors handling

FromRobert Baldyga <r.baldyga@samsung.com>
Date2015-11-03 14:10 +0100
Subject[PATCH 07/23] usb: gadget: composite: add functions for descriptors handling
Message-ID<qqJvB-37I-27@gated-at.bofh.it>
In reply to#1261473
Introduce functions and macros allowing to create and assign descriptors
to function easily. Macros build structure hierarchy using pointers to
USB descriptors, while functions assigning them to gadget make a deep
copy. It allows for easy conversion of USB functions to make them using
new descriptors format.

Signed-off-by: Robert Baldyga <r.baldyga@samsung.com>
---
 drivers/usb/gadget/composite.c | 344 +++++++++++++++++++++++++++++++++++++++++
 include/linux/usb/composite.h  |  52 +++++++
 2 files changed, 396 insertions(+)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 8b14c2a..3ecfaca 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -327,6 +327,315 @@ int usb_function_activate(struct usb_function *function)
 EXPORT_SYMBOL_GPL(usb_function_activate);
 
 /**
+ * usb_function_set_descs - assing descriptors to USB function
+ * @f: USB function
+ * @descs: USB descriptors to be assigned to function
+ *
+ * This function is to be called from prep_desc() callback to provide
+ * descriptors needed during bind process. It does a deep copy of
+ * descriptors hierarchy.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_function_set_descs(struct usb_function *f,
+		struct usb_composite_descs *descs)
+{
+	struct usb_composite_descs *descs_c;
+	struct usb_composite_intf *intf, *intf_c;
+	struct usb_composite_altset *altset, *altset_c;
+	struct usb_composite_ep *ep, *ep_c;
+	int i, a, e;
+	size_t size;
+	void *mem;
+
+	size = sizeof(*descs);
+
+	if (!descs->intfs_num)
+		return -EINVAL;
+
+	if (!f->config)
+		return -ENODEV;
+
+	size += descs->intfs_num *
+		(sizeof(*descs->intfs) + sizeof(**descs->intfs));
+	for (i = 0; i < descs->intfs_num; ++i) {
+		intf = descs->intfs[i];
+		if (!intf->altsets_num)
+			return -EINVAL;
+		size += intf->altsets_num *
+			(sizeof(*intf->altsets) + sizeof(**intf->altsets));
+		for (a = 0; a < intf->altsets_num; ++a) {
+			altset = intf->altsets[a];
+			size += sizeof(*altset->alt.desc);
+			size += altset->eps_num *
+				(sizeof(*altset->eps) + sizeof(**altset->eps));
+			for (e = 0; e < altset->eps_num; ++e) {
+				ep = altset->eps[e];
+				if (ep->fs.desc) {
+					size += sizeof(*ep->fs.desc);
+					f->config->fullspeed = true;
+				}
+				if (ep->hs.desc) {
+					size += sizeof(*ep->hs.desc);
+					f->config->highspeed = true;
+				}
+				if (ep->ss.desc) {
+					size += sizeof(*ep->ss.desc);
+					f->config->superspeed = true;
+				}
+				if (ep->ss_comp.desc)
+					size += sizeof(*ep->ss_comp.desc);
+			}
+		}
+	}
+
+	mem = kzalloc(size, GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	f->descs = descs_c = mem;
+	mem += sizeof(*descs_c);
+	INIT_LIST_HEAD(&descs_c->vendor_descs);
+	descs_c->intfs_num = descs->intfs_num;
+	descs_c->intfs = mem;
+	mem += descs_c->intfs_num * sizeof(*descs_c->intfs);
+
+	for (i = 0; i < f->descs->intfs_num; ++i) {
+		intf = descs->intfs[i];
+		descs_c->intfs[i] = intf_c = mem;
+		mem += sizeof(*intf_c);
+		intf_c->altsets_num = intf->altsets_num;
+		intf_c->altsets = mem;
+		mem += intf_c->altsets_num * sizeof(*intf_c->altsets);
+
+		for (a = 0; a < intf->altsets_num; ++a) {
+			altset = intf->altsets[a];
+			intf_c->altsets[a] = altset_c = mem;
+			mem += sizeof(*altset_c);
+			INIT_LIST_HEAD(&altset_c->vendor_descs);
+			altset_c->alt.desc = mem;
+			mem += sizeof(*altset->alt.desc);
+			memcpy(altset_c->alt.desc, altset->alt.desc,
+				sizeof(*altset->alt.desc));
+			altset_c->eps_num = altset->eps_num;
+			altset_c->eps = mem;
+			mem += altset_c->eps_num * sizeof(*altset_c->eps);
+
+			for (e = 0; e < altset->eps_num; ++e) {
+				ep = altset->eps[e];
+				altset_c->eps[e] = ep_c = mem;
+				mem += sizeof(*ep_c);
+				INIT_LIST_HEAD(&ep_c->vendor_descs);
+				if (ep->fs.desc) {
+					ep_c->fs.desc = mem;
+					mem += sizeof(*ep_c->fs.desc);
+					memcpy(ep_c->fs.desc, ep->fs.desc,
+						sizeof(*ep_c->fs.desc));
+				}
+				if (ep->hs.desc) {
+					ep_c->hs.desc = mem;
+					mem += sizeof(*ep_c->hs.desc);
+					memcpy(ep_c->hs.desc, ep->hs.desc,
+						sizeof(*ep_c->hs.desc));
+				}
+				if (ep->ss.desc) {
+					ep_c->ss.desc = mem;
+					mem += sizeof(*ep_c->ss.desc);
+					memcpy(ep_c->ss.desc, ep->ss.desc,
+						sizeof(*ep_c->ss.desc));
+				}
+				if (ep->ss_comp.desc) {
+					ep_c->ss_comp.desc = mem;
+					mem += sizeof(*ep_c->ss_comp.desc);
+					memcpy(ep_c->ss_comp.desc,
+						ep->ss_comp.desc,
+						sizeof(*ep_c->ss_comp.desc));
+				}
+			}
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_function_set_descs);
+
+/**
+ * usb_function_free_descs - frees descriptors assinged to function
+ * @f: USB function
+ */
+static inline void usb_function_free_descs(struct usb_function *f)
+{
+	kfree(f->descs);
+	f->descs = NULL;
+}
+
+/**
+ * usb_function_add_vendor_desc - add vendor specific descriptor to USB
+ *	function
+ * @f: USB function
+ * @desc: descriptor to be attached
+ *
+ * Descriptor is copied and attached at the end of linked list.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_function_add_vendor_desc(struct usb_function *f,
+		struct usb_descriptor_header *desc)
+{
+	struct usb_composite_vendor_desc *vd;
+	void *mem;
+
+	if (!f->descs)
+		return -ENODEV;
+
+	mem = kmalloc(sizeof(*vd) + desc->bLength, GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	vd = mem;
+	vd->desc = mem + sizeof(*vd);
+
+	memcpy(vd->desc, desc, desc->bLength);
+
+	list_add_tail(&vd->list, &f->descs->vendor_descs);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_function_add_vendor_desc);
+
+/**
+ * usb_ep_add_vendor_desc - add vendor specific descriptor to altsetting
+ * @f: USB function
+ * @i: index of interface in function
+ * @a: index of altsetting in interface
+ * @desc: descriptor to be attached
+ *
+ * Descriptor is copied and attached at the end of linked list.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_altset_add_vendor_desc(struct usb_function *f, int i, int a,
+		struct usb_descriptor_header *desc)
+{
+	struct usb_composite_vendor_desc *vd;
+	struct usb_composite_altset *alt;
+	void *mem;
+
+	if (!f->descs)
+		return -ENODEV;
+	if (f->descs->intfs_num <= i)
+		return -ENODEV;
+	if (f->descs->intfs[i]->altsets_num <= a)
+		return -ENODEV;
+
+	mem = kmalloc(sizeof(*vd) + desc->bLength, GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	vd = mem;
+	vd->desc = mem + sizeof(*vd);
+
+	memcpy(vd->desc, desc, desc->bLength);
+
+	alt = f->descs->intfs[i]->altsets[a];
+	list_add_tail(&vd->list, &alt->vendor_descs);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_altset_add_vendor_desc);
+
+/**
+ * usb_ep_add_vendor_desc - add vendor specific descriptor to endpoint
+ * @f: USB function
+ * @i: index of interface in function
+ * @a: index of altsetting in interface
+ * @e: index of endpoint in altsetting
+ * @desc: descriptor to be attached
+ *
+ * Descriptor is copied and attached at the end of linked list.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_ep_add_vendor_desc(struct usb_function *f, int i, int a, int e,
+		struct usb_descriptor_header *desc)
+{
+	struct usb_composite_vendor_desc *vd;
+	struct usb_composite_ep *ep;
+	void *mem;
+
+	if (!f->descs)
+		return -ENODEV;
+	if (f->descs->intfs_num <= i)
+		return -ENODEV;
+	if (f->descs->intfs[i]->altsets_num <= a)
+		return -ENODEV;
+	if (f->descs->intfs[i]->altsets[e]->eps_num <= e)
+		return -ENODEV;
+
+	mem = kmalloc(sizeof(*vd) + desc->bLength, GFP_KERNEL);
+	if (!mem)
+		return -ENOMEM;
+
+	vd = mem;
+	vd->desc = mem + sizeof(*vd);
+
+	memcpy(vd->desc, desc, desc->bLength);
+
+	ep = f->descs->intfs[i]->altsets[a]->eps[e];
+	list_add_tail(&vd->list, &ep->vendor_descs);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(usb_ep_add_vendor_desc);
+
+/**
+ * free_vendor_descs - removes and frees all vendor descriptors from
+ *	given list
+ * @vendor_descs: handle to the list of descriptors
+ */
+static inline void free_vendor_descs(struct list_head *vendor_descs)
+{
+	while (!list_empty(vendor_descs)) {
+		struct usb_composite_vendor_desc *d;
+
+		d = list_first_entry(vendor_descs,
+				struct usb_composite_vendor_desc, list);
+		list_del(&d->list);
+		kfree(d);
+	}
+}
+
+/**
+ * usb_function_free_vendor_descs - frees vendor specific descriptors
+ *	assinged to function
+ * @f: USB function
+ */
+static void usb_function_free_vendor_descs(struct usb_function *f)
+{
+	struct usb_composite_intf *intf;
+	struct usb_composite_altset *alt;
+	int i, a, e;
+
+	if (!f->descs)
+		return;
+
+	free_vendor_descs(&f->descs->vendor_descs);
+	f->descs->vendor_descs_num = 0;
+	for (i = 0; i < f->descs->intfs_num; ++i) {
+		intf = f->descs->intfs[i];
+		for (a = 0; a < intf->altsets_num; ++a) {
+			alt = intf->altsets[a];
+			free_vendor_descs(&alt->vendor_descs);
+			alt->vendor_descs_num = 0;
+			for (e = 0; e < alt->eps_num; ++e) {
+				free_vendor_descs(&alt->eps[e]->vendor_descs);
+				alt->eps[e]->vendor_descs_num = 0;
+			}
+		}
+	}
+}
+
+/**
  * usb_interface_id() - allocate an unused interface ID
  * @config: configuration associated with the interface
  * @function: function handling the interface
@@ -1893,6 +2202,38 @@ static ssize_t suspended_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(suspended);
 
+/**
+ * composite_free_descs - free entity descriptors for all functions in all
+ *	configurations, allocated with usb_function_set_descs()
+ * @cdev: composite device
+ */
+void composite_free_descs(struct usb_composite_dev *cdev)
+{
+	struct usb_configuration *c;
+	struct usb_function *f;
+
+	list_for_each_entry(c, &cdev->configs, list)
+		list_for_each_entry(f, &c->functions, list)
+			usb_function_free_descs(f);
+}
+EXPORT_SYMBOL_GPL(composite_free_descs);
+
+/**
+ * composite_free_descs - free vendor and class specific descriptors for all
+ *	functions in all configurations.
+ * @cdev: composite device
+ */
+void composite_free_vendor_descs(struct usb_composite_dev *cdev)
+{
+	struct usb_configuration *c;
+	struct usb_function *f;
+
+	list_for_each_entry(c, &cdev->configs, list)
+		list_for_each_entry(f, &c->functions, list)
+			usb_function_free_vendor_descs(f);
+}
+EXPORT_SYMBOL_GPL(composite_free_vendor_descs);
+
 static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 {
 	struct usb_composite_dev	*cdev = get_gadget_data(gadget);
@@ -1904,6 +2245,9 @@ static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
 	 */
 	WARN_ON(cdev->config);
 
+	composite_free_vendor_descs(cdev);
+	composite_free_descs(cdev);
+
 	while (!list_empty(&cdev->configs)) {
 		struct usb_configuration	*c;
 		c = list_first_entry(&cdev->configs,
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index 686c5f7..b778d4d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -171,6 +171,43 @@ struct usb_composite_descs {
 	int vendor_descs_num;
 };
 
+/*
+ * Macros to be used to create USB descriptors hierarchy.
+ */
+
+#define USB_COMPOSITE_ENDPOINT(_name, _fs_desc, _hs_desc, _ss_desc, _ss_comp) \
+	static struct usb_composite_ep _name = { \
+		.fs = { .desc = _fs_desc, }, \
+		.hs = { .desc = _hs_desc, }, \
+		.ss = { .desc = _ss_desc, }, \
+		.ss_comp = { .desc = _ss_comp, }, \
+		.vendor_descs = LIST_HEAD_INIT(_name.vendor_descs), \
+	}
+
+#define __EP_ARRAY(...) ((struct usb_composite_ep*[]){ __VA_ARGS__ })
+#define USB_COMPOSITE_ALTSETTING(_name, _desc, ...) \
+	static struct usb_composite_altset _name = { \
+		.alt = { .desc = _desc, }, \
+		.eps = __EP_ARRAY(__VA_ARGS__), \
+		.eps_num = ARRAY_SIZE(__EP_ARRAY(__VA_ARGS__)), \
+		.vendor_descs = LIST_HEAD_INIT(_name.vendor_descs), \
+	}
+
+#define __ALTSET_ARRAY(...) ((struct usb_composite_altset*[]){ __VA_ARGS__ })
+#define USB_COMPOSITE_INTERFACE(_name, ...) \
+	static struct usb_composite_intf _name = { \
+		.altsets = __ALTSET_ARRAY(__VA_ARGS__), \
+		.altsets_num = ARRAY_SIZE(__ALTSET_ARRAY(__VA_ARGS__)), \
+	}
+
+#define __INTF_ARRAY(...) ((struct usb_composite_intf*[]){ __VA_ARGS__ })
+#define USB_COMPOSITE_DESCRIPTORS(_name, ...) \
+	static struct usb_composite_descs _name = { \
+		.intfs = __INTF_ARRAY(__VA_ARGS__), \
+		.intfs_num = ARRAY_SIZE(__INTF_ARRAY(__VA_ARGS__)), \
+		.vendor_descs = LIST_HEAD_INIT(_name.vendor_descs), \
+	}
+
 /**
  * struct usb_os_desc_ext_prop - describes one "Extended Property"
  * @entry: used to keep a list of extended properties
@@ -356,6 +393,18 @@ int usb_add_function(struct usb_configuration *, struct usb_function *);
 int usb_function_deactivate(struct usb_function *);
 int usb_function_activate(struct usb_function *);
 
+int usb_function_set_descs(struct usb_function *f,
+		struct usb_composite_descs *descs);
+
+int usb_function_add_vendor_desc(struct usb_function *f,
+		struct usb_descriptor_header *desc);
+
+int usb_altset_add_vendor_desc(struct usb_function *f, int i, int a,
+		struct usb_descriptor_header *desc);
+
+int usb_ep_add_vendor_desc(struct usb_function *f, int i, int a, int e,
+		struct usb_descriptor_header *desc);
+
 int usb_interface_id(struct usb_configuration *, struct usb_function *);
 
 int config_ep_by_speed(struct usb_gadget *g, struct usb_function *f,
@@ -532,6 +581,9 @@ extern int composite_os_desc_req_prepare(struct usb_composite_dev *cdev,
 					 struct usb_ep *ep0);
 void composite_dev_cleanup(struct usb_composite_dev *cdev);
 
+void composite_free_descs(struct usb_composite_dev *cdev);
+void composite_free_vendor_descs(struct usb_composite_dev *cdev);
+
 static inline struct usb_composite_driver *to_cdriver(
 		struct usb_gadget_driver *gdrv)
 {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web