Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753724AbbKCMyV (ORCPT ); Tue, 3 Nov 2015 07:54:21 -0500 Received: from mailout3.samsung.com ([203.254.224.33]:47147 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751011AbbKCMyS (ORCPT ); Tue, 3 Nov 2015 07:54:18 -0500 X-AuditID: cbfee61b-f79d56d0000048c5-57-5638ae788044 From: Robert Baldyga To: balbi@ti.com Cc: gregkh@linuxfoundation.org, andrzej.p@samsung.com, m.szyprowski@samsung.com, b.zolnierkie@samsung.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Robert Baldyga Subject: [PATCH 00/23] usb: gadget: composite: introduce new function API Date: Tue, 03 Nov 2015 13:53:39 +0100 Message-id: <1446555242-3733-1-git-send-email-r.baldyga@samsung.com> X-Mailer: git-send-email 1.9.1 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrPJMWRmVeSWpSXmKPExsVy+t9jQd2KdRZhBnte81jMetnOYrFxxnpW i4P36y2aF69ns7i8aw6bxaJlrcwWa4/cZbd4cHgnuwOHx/65a9g9+rasYvQ4fmM7k8fnTXIB LFFcNimpOZllqUX6dglcGRv/9DIWzM+q+LJrJ1MD4+GQLkZODgkBE4kpu2cwQdhiEhfurWfr YuTiEBKYxSgx5dQMRgjnJ6PEwx97WUGq2AR0JLZ8n8AIYosICEisf3GJHaSIWeAcUNGdNrCE sICnxNpnT9lBbBYBVYkzhy+B2bwCLhLb1r5mh1gnJ3Hy2GTWCYzcCxgZVjFKpBYkFxQnpeca 5aWW6xUn5haX5qXrJefnbmIEB8sz6R2Mh3e5H2IU4GBU4uFdsMQ8TIg1say4MvcQowQHs5II 7+65FmFCvCmJlVWpRfnxRaU5qcWHGKU5WJTEefU9jcKEBNITS1KzU1MLUotgskwcnFINjBnX rwUUnl5mZnv1ple10Y/px7KiGHbze9h3JTrr5E3+Wi+11umu8LVAo6+hC1YUWzNP0ro6fdWf D9KyBj9ru56sYDIzfh/wNtrpjv8Xy+gjb6w8eltm63E3vuVay8t4agXDItnON5OObpydHLj/ 2Qumf7/by0T9UpJWs10V25X2lENDLCA1QImlOCPRUIu5qDgRAA7pcc0SAgAA Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13506 Lines: 343 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/