All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 00/43] usb: gadget: composite: introduce new function API
@ 2016-02-03 12:39 Robert Baldyga
  2016-02-03 12:39 ` [PATCH v4 01/43] usb: gadget: f_sourcesink: make ISO altset user-selectable Robert Baldyga
                   ` (42 more replies)
  0 siblings, 43 replies; 46+ messages in thread
From: Robert Baldyga @ 2016-02-03 12:39 UTC (permalink / raw)
  To: balbi
  Cc: gregkh, andrzej.p, m.szyprowski, b.zolnierkie, linux-usb,
	linux-kernel, Robert Baldyga

Hi Felipe,

Here is my new patch series doing a lot of 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 20 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 23 patches converts Functions: loopback, sourcesink, ecm, rndis,
hid, acm, eem, ncm, printer, serial, obex, phonet, ECM subset, uac1,
uac2, mass_storage and UVC to new API. Conversion of remaining Functions
will be provided 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

Changelog:

v4:
- Added 7 new patches
- Few minor fixes

v3: https://lkml.org/lkml/2015/12/11/311
- Fixed handling of vendor specific descriptor attached to endpoint
- Added 6 new patches converting Functions to new API

v2: https://lkml.org/lkml/2015/11/27/180
- Addressed comments from Sergei
- Added 6 new patches converting Functions to new API

v1: https://lkml.org/lkml/2015/11/3/288

Robert Baldyga (43):
  usb: gadget: f_sourcesink: make ISO altset user-selectable
  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: fix recursive spinlock locking
  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: usb_get_endpoint_address() 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
  usb: gadget: f_hid: handle requests lifetime properly
  usb: gadget: f_hid: conversion to new API
  usb: gadget: f_acm: conversion to new API
  usb: gadget: f_eem: conversion to new API
  usb: gadget: f_ncm: conversion to new API
  usb: gadget: f_printer: conversion to new API
  usb: gadget: f_serial: conversion to new API
  usb: gadget: f_obex: conversion to new API
  usb: gadget: f_phonet: conversion to new API
  usb: gadget: f_subset: conversion to new API
  usb: gadget: f_uac1: conversion to new API
  usb: gadget: f_uac2: conversion to new API
  usb: gadget: f_mass_storage: conversion to new API
  usb: gadget: u_serial: remove usb_ep_enable()/usb_ep_disable()
  usb: gadget: u_ether: remove usb_ep_enable()/usb_ep_disable()
  usb: gadget: uvc: fix typo in UVCG_OPTS_ATTR() macro
  usb: gadget: uvc: simplify descriptors generation
  Documentation: update uvc configfs interface description
  usb: gadget: f_uvc: conversion to new API

 Documentation/ABI/testing/configfs-usb-gadget-uvc |  39 +-
 Documentation/usb/gadget-testing.txt              |  18 +-
 drivers/usb/gadget/composite.c                    | 987 +++++++++++++++++++++-
 drivers/usb/gadget/configfs.c                     |  24 +-
 drivers/usb/gadget/function/f_acm.c               | 248 ++----
 drivers/usb/gadget/function/f_ecm.c               | 321 ++-----
 drivers/usb/gadget/function/f_eem.c               | 154 +---
 drivers/usb/gadget/function/f_hid.c               | 307 +++----
 drivers/usb/gadget/function/f_loopback.c          | 218 ++---
 drivers/usb/gadget/function/f_mass_storage.c      |  91 +-
 drivers/usb/gadget/function/f_ncm.c               | 320 +++----
 drivers/usb/gadget/function/f_obex.c              | 188 ++---
 drivers/usb/gadget/function/f_phonet.c            | 225 ++---
 drivers/usb/gadget/function/f_printer.c           | 300 ++-----
 drivers/usb/gadget/function/f_rndis.c             | 321 +++----
 drivers/usb/gadget/function/f_serial.c            | 122 +--
 drivers/usb/gadget/function/f_sourcesink.c        | 415 ++++-----
 drivers/usb/gadget/function/f_subset.c            | 165 ++--
 drivers/usb/gadget/function/f_uac1.c              | 134 ++-
 drivers/usb/gadget/function/f_uac2.c              | 360 +++-----
 drivers/usb/gadget/function/f_uvc.c               | 532 ++++--------
 drivers/usb/gadget/function/g_zero.h              |   6 +-
 drivers/usb/gadget/function/storage_common.c      |  29 -
 drivers/usb/gadget/function/storage_common.h      |   3 -
 drivers/usb/gadget/function/u_ether.c             |  28 +-
 drivers/usb/gadget/function/u_serial.c            |  16 -
 drivers/usb/gadget/function/u_uvc.h               |  14 +-
 drivers/usb/gadget/function/uvc.h                 |  10 +-
 drivers/usb/gadget/function/uvc_configfs.c        |  81 +-
 drivers/usb/gadget/legacy/webcam.c                |  43 +-
 drivers/usb/gadget/legacy/zero.c                  |  12 +
 include/linux/usb/composite.h                     | 204 +++++
 32 files changed, 2655 insertions(+), 3280 deletions(-)

-- 
1.9.1

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

end of thread, other threads:[~2016-02-04  5:40 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 12:39 [PATCH v4 00/43] usb: gadget: composite: introduce new function API Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 01/43] usb: gadget: f_sourcesink: make ISO altset user-selectable Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 02/43] usb: gadget: f_sourcesink: free requests in sourcesink_disable() Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 03/43] usb: gadget: f_loopback: free requests in loopback_disable() Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 04/43] usb: gadget: configfs: fix error path Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 05/43] usb: gadget: composite: fix recursive spinlock locking Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 06/43] usb: gadget: composite: introduce new descriptors format Robert Baldyga
2016-02-04  5:16   ` kbuild test robot
2016-02-03 12:39 ` [PATCH v4 07/43] usb: gadget: composite: add functions for descriptors handling Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 08/43] usb: gadget: composite: introduce new USB function ops Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 09/43] usb: gadget: composite: handle function bind Robert Baldyga
2016-02-04  5:42   ` kbuild test robot
2016-02-03 12:39 ` [PATCH v4 10/43] usb: gadget: composite: handle vendor descs Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 11/43] usb: gadget: composite: generate old descs for compatibility Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 12/43] usb: gadget: composite: disable eps before calling disable() callback Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 13/43] usb: gadget: composite: enable eps before calling set_alt() callback Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 14/43] usb: gadget: composite: introduce clear_alt() operation Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 15/43] usb: gadget: composite: handle get_alt() automatically Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 16/43] usb: gadget: composite: add usb_function_get_ep() function Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 17/43] usb: gadget: composite: add usb_get_interface_id() function Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 18/43] usb: gadget: composite: usb_get_endpoint_address() function Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 19/43] usb: gadget: composite: enable adding USB functions using new API Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 20/43] usb: gadget: configfs: add new composite API support Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 21/43] usb: gadget: f_loopback: convert to new API Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 22/43] usb: gadget: f_sourcesink: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 23/43] usb: gadget: f_ecm: conversion " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 24/43] usb: gadget: f_rndis: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 25/43] usb: gadget: f_hid: handle requests lifetime properly Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 26/43] usb: gadget: f_hid: conversion to new API Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 27/43] usb: gadget: f_acm: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 28/43] usb: gadget: f_eem: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 29/43] usb: gadget: f_ncm: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 30/43] usb: gadget: f_printer: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 31/43] usb: gadget: f_serial: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 32/43] usb: gadget: f_obex: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 33/43] usb: gadget: f_phonet: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 34/43] usb: gadget: f_subset: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 35/43] usb: gadget: f_uac1: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 36/43] usb: gadget: f_uac2: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 37/43] usb: gadget: f_mass_storage: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 38/43] usb: gadget: u_serial: remove usb_ep_enable()/usb_ep_disable() Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 39/43] usb: gadget: u_ether: " Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 40/43] usb: gadget: uvc: fix typo in UVCG_OPTS_ATTR() macro Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 41/43] usb: gadget: uvc: simplify descriptors generation Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 42/43] Documentation: update uvc configfs interface description Robert Baldyga
2016-02-03 12:39 ` [PATCH v4 43/43] usb: gadget: f_uvc: conversion to new API Robert Baldyga

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.