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
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
