From: Robert Baldyga <hidden> Date: 2015-07-31 14:01:23
Hello,
This patch series reworks endpoint matching and claiming mechanism in
epautoconf. From v2 there are couple of new patches adding 'ep_match'
to usb_gadget_ops and removing chip-specific quirk handling from generic
code of autoconfig.
I'm not sure if this patch set isn't too long, as it has 46 patches,
but I decided to send it as single series to avoid problems with patch
applying order.
The aim of whole patchset is to rework epautoconf code to get rid of
things like name-based endpoint matching and UDC name-based quirks in
generic code. These needed to do some modifications in framework like
adding 'endpoint capabilities flags' feature or adding 'match_ep'.
Following paragraphs contain brief description of what modifications are
done by particular parts of this patch set:
Patch (1) introduces new safer endpoint claiming method, basing on new
'claimed' flag. It was discussed here [1]. I proposed this solution over
year ago and it was accepted, but I apparently forgot to send the final
version of my patch.
Patches (2-3) add the 'capabilities flags' structure and helper macros.
This solution is inspired by the 'feature flags' originally proposed
by Felipe Balbi in 2013 [2], but unfortunately implementation of this
feature has never been completed.
Patches (4-36) add' capabilites flags' support to all UDC drivers present
in the kernel tree. It's needed to be done before replacing old endpoint
matching mechanism, otherwise UDC drivers which doesn't set 'capabilities
flags' won't work with new matching function.
Patch (37) finally replaces old endpoint matching method with the new
one basing on capabilities flags.
These changes aims to get rid of code, which guesses endpoint capabilities
basing on it's name, and introduce new better replacement. In result
we have better way to describe types and directions supported by each
endpoint.
For example the old name-based method didn't allow to have endpoint
supporing two types of transfers - there were only ability to support
one or all of endpoint types. The 'capabilities flags' feature supply
precise, flexible and extensible mechanism of description of endpoint
hardware limitations, which is desired for proper endpoint matching.
Patch (38) removes chip-specific quirk from ep_matches() function.
Patches (39-40) remove code modifying endpoint and descriptor structures
from ep_matches() function and cleans it up to make it simpler and more
readable.
Patch (41) add 'match_ep' callback to usb_gadget_ops and make use of
it in epautoconf. This callback allows UDC drivers to supply non-standard
endpoint matching algorithms.
Patches (42-43) move ep_matches() and find_ep() functions outside
epautoconf and rename them to usb_gadget_ep_match_desc() and
gadget_find_ep_by_name(). It's because they may be useful in 'match_ep'
callbacks in UDC drivers to avoid writing repetitive code.
Patches (44-46) move chip-specific enpoint matching algorithms from
generic code of usb_ep_autoconfig_ss() function to UDC controller drivers
using 'match_ep' callback.
In the result we have epautoconf source free of chip-specific code, plus
two new mechanisms allowing to handle non-standard hardware limitations.
[1] https://lkml.org/lkml/2014/6/16/94
[2] http://www.spinics.net/lists/linux-usb/msg99662.html
Best regards,
Robert Baldyga
Changelog:
v5:
- made code more grepable according to Felipe's suggestion
v4: https://lkml.org/lkml/2015/7/27/181
- addressed comments from Krzysztof Opasiak and Felipe Balbi
v3: https://lkml.org/lkml/2015/7/15/68
- addressed comments from Sergei Shtylyov
v2: https://lkml.org/lkml/2015/7/14/172
- remove PXA quirk from ep_matches() function without behaviour change
using ep capabilities flags
- separate ep and desc configuration code from ep_match() function
- add 'ep_match' to usb_gadget_ops and move chip-specific endpoint
matching algorithms from generic code to UDC controller drivers
v1: https://lkml.org/lkml/2015/7/8/436
Robert Baldyga (46):
usb: gadget: encapsulate endpoint claiming mechanism
usb: gadget: add endpoint capabilities flags
usb: gadget: add endpoint capabilities helper macros
staging: emxx_udc: add ep capabilities support
usb: chipidea: udc: add ep capabilities support
usb: dwc2: gadget: add ep capabilities support
usb: dwc3: gadget: add ep capabilities support
usb: gadget: amd5536udc: add ep capabilities support
usb: gadget: at91_udc: add ep capabilities support
usb: gadget: bcm63xx_udc: add ep capabilities support
usb: gadget: bdc: add ep capabilities support
usb: gadget: dummy-hcd: add ep capabilities support
usb: gadget: fotg210-udc: add ep capabilities support
usb: gadget: fsl_qe_udc: add ep capabilities support
usb: gadget: fsl_udc_core: add ep capabilities support
usb: gadget: fusb300_udc: add ep capabilities support
usb: gadget: goku_udc: add ep capabilities support
usb: gadget: gr_udc: add ep capabilities support
usb: gadget: lpc32xx_udc: add ep capabilities support
usb: gadget: m66592-udc: add ep capabilities support
usb: gadget: mv_u3d_core: add ep capabilities support
usb: gadget: mv_udc_core: add ep capabilities support
usb: gadget: net2272: add ep capabilities support
usb: gadget: net2280: add ep capabilities support
usb: gadget: omap_udc: add ep capabilities support
usb: gadget: pch_udc: add ep capabilities support
usb: gadget: pxa25x_udc: add ep capabilities support
usb: gadget: pxa27x_udc: add ep capabilities support
usb: gadget: r8a66597-udc: add ep capabilities support
usb: gadget: s3c-hsudc: add ep capabilities support
usb: gadget: s3c2410_udc: add ep capabilities support
usb: gadget: udc-xilinx: add ep capabilities support
usb: isp1760: udc: add ep capabilities support
usb: musb: gadget: add ep capabilities support
usb: renesas: gadget: add ep capabilities support
usb: gadget: atmel_usba_udc: add ep capabilities support
usb: gadget: epautoconf: add endpoint capabilities flags verification
usb: gadget: epautoconf: remove pxa quirk from ep_matches()
usb: gadget: epautoconf: remove ep and desc configuration from
ep_matches()
usb: gadget: epautoconf: rework ep_matches() function
usb: gadget: add 'ep_match' callback to usb_gadget_ops
usb: gadget: move ep_matches() from epautoconf to udc-core
usb: gadget: move find_ep() from epautoconf to gadget.h
usb: gadget: net2280: add net2280_match_ep() function
usb: gadget: goku_udc: add goku_match_ep() function
usb: musb: gadget: add musb_match_ep() function
drivers/staging/emxx_udc/emxx_udc.c | 73 +++++----
drivers/usb/chipidea/udc.c | 14 ++
drivers/usb/dwc2/gadget.c | 13 ++
drivers/usb/dwc3/gadget.c | 13 ++
drivers/usb/gadget/epautoconf.c | 280 +++++---------------------------
drivers/usb/gadget/udc/amd5536udc.c | 88 ++++++++--
drivers/usb/gadget/udc/at91_udc.c | 38 ++++-
drivers/usb/gadget/udc/atmel_usba_udc.c | 11 ++
drivers/usb/gadget/udc/bcm63xx_udc.c | 29 +++-
drivers/usb/gadget/udc/bdc/bdc_ep.c | 9 +
drivers/usb/gadget/udc/dummy_hcd.c | 95 +++++++++--
drivers/usb/gadget/udc/fotg210-udc.c | 11 ++
drivers/usb/gadget/udc/fsl_qe_udc.c | 11 ++
drivers/usb/gadget/udc/fsl_udc_core.c | 13 ++
drivers/usb/gadget/udc/fusb300_udc.c | 11 ++
drivers/usb/gadget/udc/goku_udc.c | 33 ++++
drivers/usb/gadget/udc/gr_udc.c | 11 ++
drivers/usb/gadget/udc/lpc32xx_udc.c | 32 ++++
drivers/usb/gadget/udc/m66592-udc.c | 11 ++
drivers/usb/gadget/udc/mv_u3d_core.c | 9 +
drivers/usb/gadget/udc/mv_udc_core.c | 9 +
drivers/usb/gadget/udc/net2272.c | 11 ++
drivers/usb/gadget/udc/net2280.c | 98 +++++++++--
drivers/usb/gadget/udc/omap_udc.c | 22 +++
drivers/usb/gadget/udc/pch_udc.c | 14 +-
drivers/usb/gadget/udc/pxa25x_udc.c | 29 ++++
drivers/usb/gadget/udc/pxa27x_udc.h | 40 +++--
drivers/usb/gadget/udc/r8a66597-udc.c | 10 ++
drivers/usb/gadget/udc/s3c-hsudc.c | 15 ++
drivers/usb/gadget/udc/s3c2410_udc.c | 10 ++
drivers/usb/gadget/udc/udc-core.c | 69 ++++++++
drivers/usb/gadget/udc/udc-xilinx.c | 9 +
drivers/usb/isp1760/isp1760-udc.c | 11 ++
drivers/usb/musb/musb_gadget.c | 46 ++++++
drivers/usb/renesas_usbhs/mod_gadget.c | 6 +
include/linux/usb/gadget.h | 71 ++++++++
36 files changed, 940 insertions(+), 335 deletions(-)
--
1.9.1
From: Robert Baldyga <hidden> Date: 2015-07-31 14:01:32
So far it was necessary for usb functions to set ep->driver_data in
endpoint obtained from autoconfig to non-null value, to indicate that
endpoint is claimed by function (in autoconfig it was checked if endpoint
has set this field to non-null value, and if it has, it was assumed that
it is claimed). It could cause bugs because if some function doesn't
set this field autoconfig could return the same endpoint more than one
time.
To help to avoid such bugs this patch adds claimed flag to struct usb_ep,
and encapsulates endpoint claiming mechanism inside usb_ep_autoconfig_ss()
and usb_ep_autoconfig_reset(), so now usb functions don't need to perform
any additional actions to mark endpoint obtained from autoconfig as claimed.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 11 ++++++-----
include/linux/usb/gadget.h | 1 +
2 files changed, 7 insertions(+), 5 deletions(-)
@@ -53,7 +53,7 @@ ep_matches (intnum_req_streams=0;/* endpoint already claimed? */-if(NULL!=ep->driver_data)+if(ep->claimed)return0;/* only support ep0 for portable CONTROL traffic */
From: Robert Baldyga <hidden> Date: 2015-07-31 14:01:37
Introduce struct usb_ep_caps which contains information about capabilities
of usb endpoints - supported transfer types and directions. This structure
should be filled by UDC driver for each of its endpoints, and will be
used in epautoconf in new ep matching mechanism which will replace ugly
guessing of endpoint capabilities basing on its name.
Signed-off-by: Robert Baldyga <redacted>
---
include/linux/usb/gadget.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
From: Robert Baldyga <hidden> Date: 2015-07-31 14:01:49
Add macros useful while initializing array of endpoint capabilities
structures. These macros makes structure initialization more compact
to decrease number of code lines and increase readability of code.
Signed-off-by: Robert Baldyga <redacted>
---
include/linux/usb/gadget.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
@@ -1952,12 +1952,18 @@ static int init_ep(struct bdc *bdc, u32 epnum, u32 dir)ep->bdc=bdc;ep->dir=dir;+if(dir)+ep->usb_ep.caps.dir_in=true;+else+ep->usb_ep.caps.dir_out=true;+/* ep->ep_num is the index inside bdc_ep */if(epnum==1){ep->ep_num=1;bdc->bdc_ep_array[ep->ep_num]=ep;snprintf(ep->name,sizeof(ep->name),"ep%d",epnum-1);usb_ep_set_maxpacket_limit(&ep->usb_ep,EP0_MAX_PKT_SIZE);+ep->usb_ep.caps.type_control=true;ep->comp_desc=NULL;bdc->gadget.ep0=&ep->usb_ep;}else{
@@ -127,23 +127,87 @@ static inline struct dummy_request *usb_request_to_dummy_requeststaticconstcharep0name[]="ep0";-staticconstchar*constep_name[]={-ep0name,/* everyone has ep0 */+staticconststruct{+constchar*name;+conststructusb_ep_capscaps;+}ep_info[]={+#define EP_INFO(_name, _caps) \+{\+.name=_name,\+.caps=_caps,\+}+/* everyone has ep0 */+EP_INFO(ep0name,+USB_EP_CAPS(USB_EP_CAPS_TYPE_CONTROL,USB_EP_CAPS_DIR_ALL)),/* act like a pxa250: fifteen fixed function endpoints */-"ep1in-bulk","ep2out-bulk","ep3in-iso","ep4out-iso","ep5in-int",-"ep6in-bulk","ep7out-bulk","ep8in-iso","ep9out-iso","ep10in-int",-"ep11in-bulk","ep12out-bulk","ep13in-iso","ep14out-iso",-"ep15in-int",-+EP_INFO("ep1in-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep2out-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep3in-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep4out-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep5in-int",+USB_EP_CAPS(USB_EP_CAPS_TYPE_INT,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep6in-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep7out-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep8in-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep9out-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep10in-int",+USB_EP_CAPS(USB_EP_CAPS_TYPE_INT,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep11in-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep12out-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep13in-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep14out-iso",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ISO,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep15in-int",+USB_EP_CAPS(USB_EP_CAPS_TYPE_INT,USB_EP_CAPS_DIR_IN)),/* or like sa1100: two fixed function endpoints */-"ep1out-bulk","ep2in-bulk",-+EP_INFO("ep1out-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep2in-bulk",+USB_EP_CAPS(USB_EP_CAPS_TYPE_BULK,USB_EP_CAPS_DIR_IN)),/* and now some generic EPs so we have enough in multi config */-"ep3out","ep4in","ep5out","ep6out","ep7in","ep8out","ep9in",-"ep10out","ep11out","ep12in","ep13out","ep14in","ep15out",+EP_INFO("ep3out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep4in",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep5out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep6out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep7in",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep8out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep9in",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep10out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep11out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep12in",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep13out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),+EP_INFO("ep14in",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_IN)),+EP_INFO("ep15out",+USB_EP_CAPS(USB_EP_CAPS_TYPE_ALL,USB_EP_CAPS_DIR_OUT)),++#undef EP_INFO};-#define DUMMY_ENDPOINTS ARRAY_SIZE(ep_name)++#define DUMMY_ENDPOINTS ARRAY_SIZE(ep_info)/*-------------------------------------------------------------------------*/
@@ -2313,6 +2313,19 @@ static int struct_ep_setup(struct fsl_udc *udc, unsigned char index,ep->ep.ops=&fsl_ep_ops;ep->stopped=0;+if(index==0){+ep->ep.caps.type_control=true;+}else{+ep->ep.caps.type_iso=true;+ep->ep.caps.type_bulk=true;+ep->ep.caps.type_int=true;+}++if(index&1)+ep->ep.caps.dir_in=true;+else+ep->ep.caps.dir_out=true;+/* for ep0: maxP defined in desc*forothereps,maxPissetbyepautoconfig()calledbygadgetlayer*/
@@ -2018,12 +2018,23 @@ static int gr_ep_init(struct gr_udc *dev, int num, int is_in, u32 maxplimit)usb_ep_set_maxpacket_limit(&ep->ep,MAX_CTRL_PL_SIZE);ep->bytes_per_buffer=MAX_CTRL_PL_SIZE;++ep->ep.caps.type_control=true;}else{usb_ep_set_maxpacket_limit(&ep->ep,(u16)maxplimit);list_add_tail(&ep->ep.ep_list,&dev->gadget.ep_list);++ep->ep.caps.type_iso=true;+ep->ep.caps.type_bulk=true;+ep->ep.caps.type_int=true;}list_add_tail(&ep->ep_list,&dev->ep_list);+if(is_in)+ep->ep.caps.dir_in=true;+else+ep->ep.caps.dir_out=true;+ep->tailbuf=dma_alloc_coherent(dev->dev,ep->ep.maxpacket_limit,&ep->tailbuf_paddr,GFP_ATOMIC);if(!ep->tailbuf)
@@ -2895,11 +2895,21 @@ static void pch_udc_pcd_reinit(struct pch_udc_dev *dev)ep->in=~i&1;ep->ep.name=ep_string[i];ep->ep.ops=&pch_udc_ep_ops;-if(ep->in)+if(ep->in){ep->offset_addr=ep->num*UDC_EP_REG_SHIFT;-else+ep->ep.caps.dir_in=true;+}else{ep->offset_addr=(UDC_EPINT_OUT_SHIFT+ep->num)*UDC_EP_REG_SHIFT;+ep->ep.caps.dir_out=true;+}+if(i==UDC_EP0IN_IDX||i==UDC_EP0OUT_IDX){+ep->ep.caps.type_control=true;+}else{+ep->ep.caps.type_iso=true;+ep->ep.caps.type_bulk=true;+ep->ep.caps.type_int=true;+}/* need to set ep->ep.maxpacket and set Default Configuration?*/usb_ep_set_maxpacket_limit(&ep->ep,UDC_BULK_MAX_PKT_SIZE);list_add_tail(&ep->ep.ep_list,&dev->gadget.ep_list);
From: Robert Baldyga <hidden> Date: 2015-07-31 14:05:53
Introduce endpoint matching mechanism basing on endpoint capabilities
flags. We check if endpoint supports transfer type and direction requested
in ep descriptor. Since we have this new endpoint matching mechanism
there is no need to have old code guessing endpoint capabilities basing
on its name, so we are getting rid of it. Remove also the obsolete comment.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 96 +++++++++++++----------------------------
1 file changed, 30 insertions(+), 66 deletions(-)
@@ -56,58 +39,39 @@ ep_matches (if(ep->claimed)return0;-/* only support ep0 for portable CONTROL traffic */type=usb_endpoint_type(desc);-if(USB_ENDPOINT_XFER_CONTROL==type)-return0;--/* some other naming convention */-if('e'!=ep->name[0])+switch(type){+caseUSB_ENDPOINT_XFER_CONTROL:+/* only support ep0 for portable CONTROL traffic */return0;+caseUSB_ENDPOINT_XFER_ISOC:+if(!ep->caps.type_iso)+return0;+break;+caseUSB_ENDPOINT_XFER_BULK:+if(!ep->caps.type_bulk)+return0;+break;+caseUSB_ENDPOINT_XFER_INT:+/* bulk endpoints handle interrupt transfers,+*exceptthetoggle-quirkyiso-synchkind+*/+if(!ep->caps.type_int&&!ep->caps.type_bulk)+return0;+/* for now, avoid PXA "interrupt-in";+*it'sdocumentedasneverusingDATA1.+*/+if(gadget_is_pxa(gadget)&&ep->caps.type_int)+return0;+break;+}-/* type-restriction: "-iso", "-bulk", or "-int".-*direction-restriction:"in","out".-*/-if('-'!=ep->name[2]){-tmp=strrchr(ep->name,'-');-if(tmp){-switch(type){-caseUSB_ENDPOINT_XFER_INT:-/* bulk endpoints handle interrupt transfers,-*exceptthetoggle-quirkyiso-synchkind-*/-if('s'==tmp[2])// == "-iso"-return0;-/* for now, avoid PXA "interrupt-in";-*it'sdocumentedasneverusingDATA1.-*/-if(gadget_is_pxa(gadget)-&&'i'==tmp[1])-return0;-break;-caseUSB_ENDPOINT_XFER_BULK:-if('b'!=tmp[1])// != "-bulk"-return0;-break;-caseUSB_ENDPOINT_XFER_ISOC:-if('s'!=tmp[2])// != "-iso"-return0;-}-}else{-tmp=ep->name+strlen(ep->name);-}--/* direction-restriction: "..in-..", "out-.." */-tmp--;-if(!isdigit(*tmp)){-if(desc->bEndpointAddress&USB_DIR_IN){-if('n'!=*tmp)-return0;-}else{-if('t'!=*tmp)-return0;-}-}+if(usb_endpoint_dir_in(desc)){+if(!ep->caps.dir_in)+return0;+}else{+if(!ep->caps.dir_out)+return0;}/*
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:09
As function ep_matches() is used to match endpoint with usb descriptor it's
highly unintuitive that it modifies endpoint and descriptor structures fields.
This patch moves code configuring ep and desc from ep_matches() to
usb_ep_autoconfig_ss(), so now function ep_matches() does nothing more than
its name suggests.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 66 +++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 32 deletions(-)
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:15
Rework ep_matches() function to make it shorter and more readable.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 87 +++++++++++++++++------------------------
1 file changed, 35 insertions(+), 52 deletions(-)
@@ -30,16 +30,29 @@ ep_matches (structusb_ss_ep_comp_descriptor*ep_comp){-u8type;-u16max;--intnum_req_streams=0;+u8type;+u16max;+intnum_req_streams=0;/* endpoint already claimed? */if(ep->claimed)return0;type=usb_endpoint_type(desc);+max=0x7ff&usb_endpoint_maxp(desc);++if(usb_endpoint_dir_in(desc)&&!ep->caps.dir_in)+return0;+elseif(!ep->caps.dir_out)+return0;++if(max>ep->maxpacket_limit)+return0;++/* "high bandwidth" works only at high speed */+if(!gadget_is_dualspeed(gadget)&&usb_endpoint_maxp(desc)&(3<<11))+return0;+switch(type){caseUSB_ENDPOINT_XFER_CONTROL:/* only support ep0 for portable CONTROL traffic */
@@ -47,66 +60,36 @@ ep_matches (caseUSB_ENDPOINT_XFER_ISOC:if(!ep->caps.type_iso)return0;+/* ISO: limit 1023 bytes full speed,+*1024high/superspeed+*/+if(!gadget_is_dualspeed(gadget)&&max>1023)+return0;break;caseUSB_ENDPOINT_XFER_BULK:if(!ep->caps.type_bulk)return0;+if(ep_comp&&gadget_is_superspeed(gadget)){+/* Get the number of required streams from the+*EPcompaniondescriptorandseeiftheEP+*matchesit+*/+num_req_streams=ep_comp->bmAttributes&0x1f;+if(num_req_streams>ep->max_streams)+return0;+}break;caseUSB_ENDPOINT_XFER_INT:-/* bulk endpoints handle interrupt transfers,+/* Bulk endpoints handle interrupt transfers,*exceptthetoggle-quirkyiso-synchkind*/if(!ep->caps.type_int&&!ep->caps.type_bulk)return0;-break;-}--if(usb_endpoint_dir_in(desc)){-if(!ep->caps.dir_in)-return0;-}else{-if(!ep->caps.dir_out)-return0;-}--/*-*GetthenumberofrequiredstreamsfromtheEPcompanion-*descriptorandseeiftheEPmatchesit-*/-if(usb_endpoint_xfer_bulk(desc)){-if(ep_comp&&gadget->max_speed>=USB_SPEED_SUPER){-num_req_streams=ep_comp->bmAttributes&0x1f;-if(num_req_streams>ep->max_streams)-return0;-}--}--/* endpoint maxpacket size is an input parameter, except for bulk-*whereit'sanoutputparameterrepresentingthefullspeedlimit.-*theusbspecfixeshighspeedbulkmaxpacketat512bytes.-*/-max=0x7ff&usb_endpoint_maxp(desc);-switch(type){-caseUSB_ENDPOINT_XFER_INT:-/* INT: limit 64 bytes full speed, 1024 high/super speed */+/* INT: limit 64 bytes full speed,+*1024high/superspeed+*/if(!gadget_is_dualspeed(gadget)&&max>64)return0;-/* FALLTHROUGH */--caseUSB_ENDPOINT_XFER_ISOC:-/* ISO: limit 1023 bytes full speed, 1024 high/super speed */-if(ep->maxpacket_limit<max)-return0;-if(!gadget_is_dualspeed(gadget)&&max>1023)-return0;--/* BOTH: "high bandwidth" works only at high speed */-if((desc->wMaxPacketSize&cpu_to_le16(3<<11))){-if(!gadget_is_dualspeed(gadget))-return0;-/* configure your hardware with enough buffering!! */-}break;}
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:22
Add callback that is called by epautoconf to allow UDC driver match the
best endpoint for specific descriptor. It's intended to supply mechanism
which allows to get rid of chip-specific endpoint matching code from
epautoconf.
If gadget has set 'ep_match' callback we prefer to call it first, and
if it fails to find matching endpoint, then we try to use default matching
algorithm.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 6 ++++++
include/linux/usb/gadget.h | 3 +++
2 files changed, 9 insertions(+)
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:31
Move ep_matches() function to udc-core and rename it to
usb_gadget_ep_match_desc(). This function can be used by UDC drivers
in 'match_ep' callback to avoid writing lots of repetitive code.
Replace all calls of ep_matches() with usb_gadget_ep_match_desc().
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 95 +++++----------------------------------
drivers/usb/gadget/udc/udc-core.c | 69 ++++++++++++++++++++++++++++
include/linux/usb/gadget.h | 8 ++++
3 files changed, 88 insertions(+), 84 deletions(-)
@@ -22,82 +22,6 @@#include"gadget_chips.h"-staticint-ep_matches(-structusb_gadget*gadget,-structusb_ep*ep,-structusb_endpoint_descriptor*desc,-structusb_ss_ep_comp_descriptor*ep_comp-)-{-u8type;-u16max;-intnum_req_streams=0;--/* endpoint already claimed? */-if(ep->claimed)-return0;--type=usb_endpoint_type(desc);-max=0x7ff&usb_endpoint_maxp(desc);--if(usb_endpoint_dir_in(desc)&&!ep->caps.dir_in)-return0;-elseif(!ep->caps.dir_out)-return0;--if(max>ep->maxpacket_limit)-return0;--/* "high bandwidth" works only at high speed */-if(!gadget_is_dualspeed(gadget)&&usb_endpoint_maxp(desc)&(3<<11))-return0;--switch(type){-caseUSB_ENDPOINT_XFER_CONTROL:-/* only support ep0 for portable CONTROL traffic */-return0;-caseUSB_ENDPOINT_XFER_ISOC:-if(!ep->caps.type_iso)-return0;-/* ISO: limit 1023 bytes full speed,-*1024high/superspeed-*/-if(!gadget_is_dualspeed(gadget)&&max>1023)-return0;-break;-caseUSB_ENDPOINT_XFER_BULK:-if(!ep->caps.type_bulk)-return0;-if(ep_comp&&gadget_is_superspeed(gadget)){-/* Get the number of required streams from the-*EPcompaniondescriptorandseeiftheEP-*matchesit-*/-num_req_streams=ep_comp->bmAttributes&0x1f;-if(num_req_streams>ep->max_streams)-return0;-}-break;-caseUSB_ENDPOINT_XFER_INT:-/* Bulk endpoints handle interrupt transfers,-*exceptthetoggle-quirkyiso-synchkind-*/-if(!ep->caps.type_int&&!ep->caps.type_bulk)-return0;-/* INT: limit 64 bytes full speed,-*1024high/superspeed-*/-if(!gadget_is_dualspeed(gadget)&&max>64)-return0;-break;-}--/* MATCH!! */--return1;-}-staticstructusb_ep*find_ep(structusb_gadget*gadget,constchar*name){
@@ -180,10 +104,12 @@ struct usb_ep *usb_ep_autoconfig_ss(if(type==USB_ENDPOINT_XFER_INT){/* ep-e, ep-f are PIO with only 64 byte fifos */ep=find_ep(gadget,"ep-e");-if(ep&&ep_matches(gadget,ep,desc,ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,+ep,desc,ep_comp))gotofound_ep;ep=find_ep(gadget,"ep-f");-if(ep&&ep_matches(gadget,ep,desc,ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,+ep,desc,ep_comp))gotofound_ep;}
@@ -191,20 +117,21 @@ struct usb_ep *usb_ep_autoconfig_ss(snprintf(name,sizeof(name),"ep%d%s",usb_endpoint_num(desc),usb_endpoint_dir_in(desc)?"in":"out");ep=find_ep(gadget,name);-if(ep&&ep_matches(gadget,ep,desc,ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;}elseif(gadget_is_goku(gadget)){if(USB_ENDPOINT_XFER_INT==type){/* single buffering is enough */ep=find_ep(gadget,"ep3-bulk");-if(ep&&ep_matches(gadget,ep,desc,ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,+ep,desc,ep_comp))gotofound_ep;}elseif(USB_ENDPOINT_XFER_BULK==type&&(USB_DIR_IN&desc->bEndpointAddress)){/* DMA may be available */ep=find_ep(gadget,"ep2-bulk");-if(ep&&ep_matches(gadget,ep,desc,-ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,+ep,desc,ep_comp))gotofound_ep;}
@@ -223,14 +150,14 @@ struct usb_ep *usb_ep_autoconfig_ss(ep=find_ep(gadget,"ep2out");}elseep=NULL;-if(ep&&ep_matches(gadget,ep,desc,ep_comp))+if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;#endif}/* Second, look at endpoints until an unclaimed one looks usable */list_for_each_entry(ep,&gadget->ep_list,ep_list){-if(ep_matches(gadget,ep,desc,ep_comp))+if(usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;}
@@ -131,6 +131,75 @@ EXPORT_SYMBOL_GPL(usb_gadget_giveback_request);/* ------------------------------------------------------------------------- */+intusb_gadget_ep_match_desc(structusb_gadget*gadget,+structusb_ep*ep,structusb_endpoint_descriptor*desc,+structusb_ss_ep_comp_descriptor*ep_comp)+{+u8type;+u16max;+intnum_req_streams=0;++/* endpoint already claimed? */+if(ep->claimed)+return0;++type=usb_endpoint_type(desc);+max=0x7ff&usb_endpoint_maxp(desc);++if(usb_endpoint_dir_in(desc)&&!ep->caps.dir_in)+return0;+elseif(!ep->caps.dir_out)+return0;++if(max>ep->maxpacket_limit)+return0;++/* "high bandwidth" works only at high speed */+if(!gadget_is_dualspeed(gadget)&&usb_endpoint_maxp(desc)&(3<<11))+return0;++switch(type){+caseUSB_ENDPOINT_XFER_CONTROL:+/* only support ep0 for portable CONTROL traffic */+return0;+caseUSB_ENDPOINT_XFER_ISOC:+if(!ep->caps.type_iso)+return0;+/* ISO: limit 1023 bytes full speed, 1024 high/super speed */+if(!gadget_is_dualspeed(gadget)&&max>1023)+return0;+break;+caseUSB_ENDPOINT_XFER_BULK:+if(!ep->caps.type_bulk)+return0;+if(ep_comp&&gadget_is_superspeed(gadget)){+/* Get the number of required streams from the+*EPcompaniondescriptorandseeiftheEP+*matchesit+*/+num_req_streams=ep_comp->bmAttributes&0x1f;+if(num_req_streams>ep->max_streams)+return0;+}+break;+caseUSB_ENDPOINT_XFER_INT:+/* Bulk endpoints handle interrupt transfers,+*exceptthetoggle-quirkyiso-synchkind+*/+if(!ep->caps.type_int&&!ep->caps.type_bulk)+return0;+/* INT: limit 64 bytes full speed, 1024 high/super speed */+if(!gadget_is_dualspeed(gadget)&&max>64)+return0;+break;+}++return1;+}+EXPORT_SYMBOL_GPL(usb_gadget_ep_match_desc);++/* ------------------------------------------------------------------------- */+staticvoidusb_gadget_state_work(structwork_struct*work){structusb_gadget*gadget=work_to_gadget(work);
@@ -1204,6 +1204,14 @@ extern void usb_gadget_giveback_request(struct usb_ep *ep,/*-------------------------------------------------------------------------*/+/* utility to check if endpoint caps match descriptor needs */++externintusb_gadget_ep_match_desc(structusb_gadget*gadget,+structusb_ep*ep,structusb_endpoint_descriptor*desc,+structusb_ss_ep_comp_descriptor*ep_comp);++/*-------------------------------------------------------------------------*/+/* utility to update vbus status for udc core, it may be scheduled */externvoidusb_udc_vbus_handler(structusb_gadget*gadget,boolstatus);
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:37
Move find_ep() function to gadget.h, rename it to gadget_find_ep_by_name()
and make it static inline. It can be used in UDC drivers, especially in
'match_ep' callback after moving chip-specific endpoint matching logic from
epautoconf to UDC drivers.
Replace all calls of find_ep() function with gadget_find_ep_by_name().
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 30 +++++++++---------------------
include/linux/usb/gadget.h | 18 ++++++++++++++++++
2 files changed, 27 insertions(+), 21 deletions(-)
@@ -103,11 +91,11 @@ struct usb_ep *usb_ep_autoconfig_ss(if(type==USB_ENDPOINT_XFER_INT){/* ep-e, ep-f are PIO with only 64 byte fifos */-ep=find_ep(gadget,"ep-e");+ep=gadget_find_ep_by_name(gadget,"ep-e");if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;-ep=find_ep(gadget,"ep-f");+ep=gadget_find_ep_by_name(gadget,"ep-f");if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;
@@ -116,20 +104,20 @@ struct usb_ep *usb_ep_autoconfig_ss(/* USB3380: use same address for usb and hardware endpoints */snprintf(name,sizeof(name),"ep%d%s",usb_endpoint_num(desc),usb_endpoint_dir_in(desc)?"in":"out");-ep=find_ep(gadget,name);+ep=gadget_find_ep_by_name(gadget,name);if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;}elseif(gadget_is_goku(gadget)){if(USB_ENDPOINT_XFER_INT==type){/* single buffering is enough */-ep=find_ep(gadget,"ep3-bulk");+ep=gadget_find_ep_by_name(gadget,"ep3-bulk");if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;}elseif(USB_ENDPOINT_XFER_BULK==type&&(USB_DIR_IN&desc->bEndpointAddress)){/* DMA may be available */-ep=find_ep(gadget,"ep2-bulk");+ep=gadget_find_ep_by_name(gadget,"ep2-bulk");if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:44
Add 'match_ep' callback to utilize chip-specific knowledge in endpoint matching
process. Function does the same that was done by chip-specific code inside
of epautoconf. Now this code can be removed from there to separate generic code
from platform specific logic.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 23 +----------------------
drivers/usb/gadget/udc/net2280.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 22 deletions(-)
@@ -86,28 +86,7 @@ struct usb_ep *usb_ep_autoconfig_ss(/* First, apply chip-specific "best usage" knowledge.*Thismightmakeagoodusb_gadget_opshook...*/-if(gadget_is_net2280(gadget)){-charname[8];--if(type==USB_ENDPOINT_XFER_INT){-/* ep-e, ep-f are PIO with only 64 byte fifos */-ep=gadget_find_ep_by_name(gadget,"ep-e");-if(ep&&usb_gadget_ep_match_desc(gadget,-ep,desc,ep_comp))-gotofound_ep;-ep=gadget_find_ep_by_name(gadget,"ep-f");-if(ep&&usb_gadget_ep_match_desc(gadget,-ep,desc,ep_comp))-gotofound_ep;-}--/* USB3380: use same address for usb and hardware endpoints */-snprintf(name,sizeof(name),"ep%d%s",usb_endpoint_num(desc),-usb_endpoint_dir_in(desc)?"in":"out");-ep=gadget_find_ep_by_name(gadget,name);-if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))-gotofound_ep;-}elseif(gadget_is_goku(gadget)){+if(gadget_is_goku(gadget)){if(USB_ENDPOINT_XFER_INT==type){/* single buffering is enough */ep=gadget_find_ep_by_name(gadget,"ep3-bulk");
@@ -1550,6 +1550,36 @@ static int net2280_pullup(struct usb_gadget *_gadget, int is_on)return0;}+staticstructusb_ep*net2280_match_ep(structusb_gadget*_gadget,+structusb_endpoint_descriptor*desc,+structusb_ss_ep_comp_descriptor*ep_comp)+{+charname[8];+structusb_ep*ep;+u8type;++type=usb_endpoint_type(desc);++if(type==USB_ENDPOINT_XFER_INT){+/* ep-e, ep-f are PIO with only 64 byte fifos */+ep=gadget_find_ep_by_name(_gadget,"ep-e");+if(ep&&usb_gadget_ep_match_desc(_gadget,ep,desc,ep_comp))+returnep;+ep=gadget_find_ep_by_name(_gadget,"ep-f");+if(ep&&usb_gadget_ep_match_desc(_gadget,ep,desc,ep_comp))+returnep;+}++/* USB3380: use same address for usb and hardware endpoints */+snprintf(name,sizeof(name),"ep%d%s",usb_endpoint_num(desc),+usb_endpoint_dir_in(desc)?"in":"out");+ep=gadget_find_ep_by_name(_gadget,name);+if(ep&&usb_gadget_ep_match_desc(_gadget,ep,desc,ep_comp))+returnep;++returnNULL;+}+staticintnet2280_start(structusb_gadget*_gadget,structusb_gadget_driver*driver);staticintnet2280_stop(structusb_gadget*_gadget);
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:50
Add 'match_ep' callback to utilize chip-specific knowledge in endpoint matching
process. Function does the same that was done by chip-specific code inside
of epautoconf. Now this code can be removed from there to separate generic code
from platform specific logic.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 20 ++------------------
drivers/usb/gadget/udc/goku_udc.c | 25 +++++++++++++++++++++++++
2 files changed, 27 insertions(+), 18 deletions(-)
@@ -86,24 +86,8 @@ struct usb_ep *usb_ep_autoconfig_ss(/* First, apply chip-specific "best usage" knowledge.*Thismightmakeagoodusb_gadget_opshook...*/-if(gadget_is_goku(gadget)){-if(USB_ENDPOINT_XFER_INT==type){-/* single buffering is enough */-ep=gadget_find_ep_by_name(gadget,"ep3-bulk");-if(ep&&usb_gadget_ep_match_desc(gadget,-ep,desc,ep_comp))-gotofound_ep;-}elseif(USB_ENDPOINT_XFER_BULK==type-&&(USB_DIR_IN&desc->bEndpointAddress)){-/* DMA may be available */-ep=gadget_find_ep_by_name(gadget,"ep2-bulk");-if(ep&&usb_gadget_ep_match_desc(gadget,-ep,desc,ep_comp))-gotofound_ep;-}-#ifdef CONFIG_BLACKFIN-}elseif(gadget_is_musbhdrc(gadget)){+if(gadget_is_musbhdrc(gadget)){if((USB_ENDPOINT_XFER_BULK==type)||(USB_ENDPOINT_XFER_ISOC==type)){if(USB_DIR_IN&desc->bEndpointAddress)
@@ -119,8 +103,8 @@ struct usb_ep *usb_ep_autoconfig_ss(ep=NULL;if(ep&&usb_gadget_ep_match_desc(gadget,ep,desc,ep_comp))gotofound_ep;-#endif}+#endif/* Second, look at endpoints until an unclaimed one looks usable */list_for_each_entry(ep,&gadget->ep_list,ep_list){
@@ -990,6 +990,30 @@ static int goku_get_frame(struct usb_gadget *_gadget)return-EOPNOTSUPP;}+staticstructusb_ep*goku_match_ep(structusb_gadget*g,+structusb_endpoint_descriptor*desc,+structusb_ss_ep_comp_descriptor*ep_comp)+{+structgoku_udc*dev=to_goku_udc(g);+structusb_ep*ep;+u8type=usb_endpoint_type(desc);++if(type==USB_ENDPOINT_XFER_INT){+/* single buffering is enough */+ep=&dev->ep[3].ep;+if(ep&&usb_gadget_ep_match_desc(g,ep,desc,ep_comp))+returnep;+}elseif(type==USB_ENDPOINT_XFER_BULK+&&usb_endpoint_dir_in(desc)){+/* DMA may be available */+ep=&dev->ep[2].ep;+if(ep&&usb_gadget_ep_match_desc(g,ep,desc,ep_comp))+returnep;+}++returnNULL;+}+staticintgoku_udc_start(structusb_gadget*g,structusb_gadget_driver*driver);staticintgoku_udc_stop(structusb_gadget*g);
@@ -998,6 +1022,7 @@ static const struct usb_gadget_ops goku_ops = {.get_frame=goku_get_frame,.udc_start=goku_udc_start,.udc_stop=goku_udc_stop,+.match_ep=goku_match_ep,// no remote wakeup// not selfpowered};
From: Robert Baldyga <hidden> Date: 2015-07-31 14:06:59
Add 'match_ep' callback to utilize chip-specific knowledge in endpoint matching
process. Function does the same that was done by chip-specific code inside
of epautoconf. Now this code can be removed from there to separate generic code
from platform specific logic.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 23 -----------------------
drivers/usb/musb/musb_gadget.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 34 insertions(+), 23 deletions(-)
From: Robert Baldyga <hidden> Date: 2015-07-31 14:09:12
The same effect can be achieved by using capabilities flags, so now we can
get rid of handling of hardware specific limitations in generic code.
Signed-off-by: Robert Baldyga <redacted>
---
drivers/usb/gadget/epautoconf.c | 5 -----
drivers/usb/gadget/udc/pxa25x_udc.c | 9 +++------
2 files changed, 3 insertions(+), 11 deletions(-)
From: Felipe Balbi <hidden> Date: 2015-07-31 15:59:13
Hi,
On Fri, Jul 31, 2015 at 03:51:52PM +0000, David Laight wrote:
From: Robert Baldyga
quoted
Sent: 31 July 2015 15:00
Introduce struct usb_ep_caps which contains information about capabilities
of usb endpoints - supported transfer types and directions. This structure
should be filled by UDC driver for each of its endpoints, and will be
used in epautoconf in new ep matching mechanism which will replace ugly
guessing of endpoint capabilities basing on its name.
Signed-off-by: Robert Baldyga <redacted>
---
include/linux/usb/gadget.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
With the way this is used (eg below from 13/46)
+
+ if (i == 0) {
+ ep->ep.caps.type_control = true;
+ } else {
+ ep->ep.caps.type_iso = true;
+ ep->ep.caps.type_bulk = true;
+ ep->ep.caps.type_int = true;
+ }
+
+ ep->ep.caps.dir_in = true;
+ ep->ep.caps.dir_out = true;
I think it would be more obvious if you used a u8 and explicit bitmasks.
The initialisation (as above) would the be explicitly assigning 'not supported'
to the other fields.
The compiler will also generate much better code...
compiler should convert single bit flags into u32 just fine. It's all
static data anyway. Besides, single bit flags allow us to have as many
as we need without ending up with stuff like:
u32 flags;
u32 flags1;
u32 flags2;
etc. Just let the compiler do those conversions for us.
--
balbi
From: Felipe Balbi <hidden> Date: 2015-08-04 17:30:25
Hi,
On Fri, Jul 31, 2015 at 04:00:12PM +0200, Robert Baldyga wrote:
Hello,
This patch series reworks endpoint matching and claiming mechanism in
epautoconf. From v2 there are couple of new patches adding 'ep_match'
to usb_gadget_ops and removing chip-specific quirk handling from generic
code of autoconfig.
I'm not sure if this patch set isn't too long, as it has 46 patches,
but I decided to send it as single series to avoid problems with patch
applying order.
The aim of whole patchset is to rework epautoconf code to get rid of
things like name-based endpoint matching and UDC name-based quirks in
generic code. These needed to do some modifications in framework like
adding 'endpoint capabilities flags' feature or adding 'match_ep'.
Following paragraphs contain brief description of what modifications are
done by particular parts of this patch set:
Patch (1) introduces new safer endpoint claiming method, basing on new
'claimed' flag. It was discussed here [1]. I proposed this solution over
year ago and it was accepted, but I apparently forgot to send the final
version of my patch.
Patches (2-3) add the 'capabilities flags' structure and helper macros.
This solution is inspired by the 'feature flags' originally proposed
by Felipe Balbi in 2013 [2], but unfortunately implementation of this
feature has never been completed.
Patches (4-36) add' capabilites flags' support to all UDC drivers present
in the kernel tree. It's needed to be done before replacing old endpoint
matching mechanism, otherwise UDC drivers which doesn't set 'capabilities
flags' won't work with new matching function.
Patch (37) finally replaces old endpoint matching method with the new
one basing on capabilities flags.
These changes aims to get rid of code, which guesses endpoint capabilities
basing on it's name, and introduce new better replacement. In result
we have better way to describe types and directions supported by each
endpoint.
For example the old name-based method didn't allow to have endpoint
supporing two types of transfers - there were only ability to support
one or all of endpoint types. The 'capabilities flags' feature supply
precise, flexible and extensible mechanism of description of endpoint
hardware limitations, which is desired for proper endpoint matching.
Patch (38) removes chip-specific quirk from ep_matches() function.
Patches (39-40) remove code modifying endpoint and descriptor structures
from ep_matches() function and cleans it up to make it simpler and more
readable.
Patch (41) add 'match_ep' callback to usb_gadget_ops and make use of
it in epautoconf. This callback allows UDC drivers to supply non-standard
endpoint matching algorithms.
Patches (42-43) move ep_matches() and find_ep() functions outside
epautoconf and rename them to usb_gadget_ep_match_desc() and
gadget_find_ep_by_name(). It's because they may be useful in 'match_ep'
callbacks in UDC drivers to avoid writing repetitive code.
Patches (44-46) move chip-specific enpoint matching algorithms from
generic code of usb_ep_autoconfig_ss() function to UDC controller drivers
using 'match_ep' callback.
In the result we have epautoconf source free of chip-specific code, plus
two new mechanisms allowing to handle non-standard hardware limitations.
[1] https://lkml.org/lkml/2014/6/16/94
[2] http://www.spinics.net/lists/linux-usb/msg99662.html
adding these to my testing/next after fixing a few build warnings and
the comment I had on dwc3 patch. Let's hope it just works :-)
--
balbi
From: Felipe Balbi <hidden> Date: 2015-08-04 18:34:26
On Fri, Jul 31, 2015 at 04:00:52PM +0200, Robert Baldyga wrote:
Rework ep_matches() function to make it shorter and more readable.
Signed-off-by: Robert Baldyga <redacted>
this regresses at least mass storage. How did you test it ? I'll keep
all patches up to this one, please fix the problem, rebase on
testing/next and resend the remaining patches.
cheers
--
balbi
From: Felipe Balbi <hidden> Date: 2015-08-20 15:36:44
Hi,
On Fri, Jul 31, 2015 at 04:00:13PM +0200, Robert Baldyga wrote:
So far it was necessary for usb functions to set ep->driver_data in
endpoint obtained from autoconfig to non-null value, to indicate that
endpoint is claimed by function (in autoconfig it was checked if endpoint
has set this field to non-null value, and if it has, it was assumed that
it is claimed). It could cause bugs because if some function doesn't
set this field autoconfig could return the same endpoint more than one
time.
To help to avoid such bugs this patch adds claimed flag to struct usb_ep,
and encapsulates endpoint claiming mechanism inside usb_ep_autoconfig_ss()
and usb_ep_autoconfig_reset(), so now usb functions don't need to perform
any additional actions to mark endpoint obtained from autoconfig as claimed.
Signed-off-by: Robert Baldyga <redacted>
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep->claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism") we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep->claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism")
Cc: Robert Baldyga [off-list ref]
Signed-off-by: Felipe Balbi [off-list ref]
From: Robert Baldyga <hidden> Date: 2015-08-20 16:28:51
Hi Felipe,
On 08/20/2015 05:35 PM, Felipe Balbi wrote:
[...]
quoted hunk
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep->claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism") we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep->claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism")
Cc: Robert Baldyga [off-list ref]
Signed-off-by: Felipe Balbi [off-list ref]
Removing this line causes autoconfig can return the same endpoint many
times. This probably causes problems with g_zero.
I will try to fix it ASAP.
Thanks,
Robert
From: Felipe Balbi <hidden> Date: 2015-08-20 16:48:59
On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote:
Hi Felipe,
On 08/20/2015 05:35 PM, Felipe Balbi wrote:
[...]
quoted
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep->claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism") we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep->claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism")
Cc: Robert Baldyga [off-list ref]
Signed-off-by: Felipe Balbi [off-list ref]
Removing this line causes autoconfig can return the same endpoint many
times. This probably causes problems with g_zero.
I will try to fix it ASAP.
I was considering the same thing, but the lifetime of ->claimed doesn't
look correct to me either way. Note that once the flag is enabled, it
won't get disabled by most gadget drivers.
--
balbi
From: Robert Baldyga <hidden> Date: 2015-08-20 17:17:14
On 08/20/2015 06:48 PM, Felipe Balbi wrote:
On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote:
quoted
Hi Felipe,
On 08/20/2015 05:35 PM, Felipe Balbi wrote:
[...]
quoted
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep->claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism") we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep->claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism")
Cc: Robert Baldyga [off-list ref]
Signed-off-by: Felipe Balbi [off-list ref]
Removing this line causes autoconfig can return the same endpoint many
times. This probably causes problems with g_zero.
I will try to fix it ASAP.
I was considering the same thing, but the lifetime of ->claimed doesn't
look correct to me either way. Note that once the flag is enabled, it
won't get disabled by most gadget drivers.
And it should not be. This flag is indicator, that endpoint is used by
some function. It should be set once by usb_ep_autoconfig() and cleared
by usb_ep_autoconfig_reset().
I wonder what is reason of this enable/disable regression. Maybe the
problem is that we don't set ep->driver_data to NULL in
usb_ep_autoconfig_reset() (so far it was done). Does this problem occur
while gadget is binded to UDC for the first time, or at any next time?
Unfortunately at this moment I don't have access to my hardware, so it
will take a moment before I will setup some testing environment.
Thanks,
Robert
From: Felipe Balbi <hidden> Date: 2015-08-20 17:45:03
Hi,
On Thu, Aug 20, 2015 at 07:16:48PM +0200, Robert Baldyga wrote:
On 08/20/2015 06:48 PM, Felipe Balbi wrote:
quoted
On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote:
quoted
Hi Felipe,
On 08/20/2015 05:35 PM, Felipe Balbi wrote:
[...]
quoted
just letting you know that this regresses all gadget drivers making them
try to disable previously disabled endpoints and enable previously
enabled endpoints.
I have a possible fix (see below) but then it shows a problem on the
host side when using with g_zero (see further below):
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00
Author: Felipe Balbi [off-list ref]
Date: Wed Aug 19 18:05:27 2015 -0500
usb: gadget: fix ep->claimed lifetime
In order to fix a regression introduced by commit
cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism") we have to introduce a simple
helper to check if a particular is enabled or not.
After that, we need to move ep->claimed lifetime to
usb_ep_enable() and usb_ep_disable() since those
are the only functions which actually enable and
disable endpoints.
A follow-up patch will come to drop all driver_data
checks from function drivers, since those are, now,
pointless.
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint
claiming mechanism")
Cc: Robert Baldyga [off-list ref]
Signed-off-by: Felipe Balbi [off-list ref]
Removing this line causes autoconfig can return the same endpoint many
times. This probably causes problems with g_zero.
I will try to fix it ASAP.
I was considering the same thing, but the lifetime of ->claimed doesn't
look correct to me either way. Note that once the flag is enabled, it
won't get disabled by most gadget drivers.
And it should not be. This flag is indicator, that endpoint is used by some
function. It should be set once by usb_ep_autoconfig() and cleared by
usb_ep_autoconfig_reset().
have you considered switching interfaces and/or alternate settings ?
I wonder what is reason of this enable/disable regression. Maybe the problem
is that we don't set ep->driver_data to NULL in usb_ep_autoconfig_reset()
(so far it was done). Does this problem occur while gadget is binded to UDC
for the first time, or at any next time? Unfortunately at this moment I
don't have access to my hardware, so it will take a moment before I will
setup some testing environment.
From: John Youn <hidden> Date: 2015-08-20 20:07:46
On 8/20/2015 10:45 AM, Felipe Balbi wrote:=0A=
Hi,=0A=
=0A=
On Thu, Aug 20, 2015 at 07:16:48PM +0200, Robert Baldyga wrote:=0A=
quoted
On 08/20/2015 06:48 PM, Felipe Balbi wrote:=0A=
quoted
On Thu, Aug 20, 2015 at 06:28:14PM +0200, Robert Baldyga wrote:=0A=
quoted
Hi Felipe,=0A=
=0A=
On 08/20/2015 05:35 PM, Felipe Balbi wrote:=0A=
[...]=0A=
quoted
just letting you know that this regresses all gadget drivers making t=
hem=0A=
quoted
quoted
quoted
quoted
try to disable previously disabled endpoints and enable previously=0A=
enabled endpoints.=0A=
=0A=
I have a possible fix (see below) but then it shows a problem on the=
=0A=
quoted
quoted
quoted
quoted
host side when using with g_zero (see further below):=0A=
=0A=
commit 3b8932100aacb6cfbffe288ca93025d8b8430c00=0A=
Author: Felipe Balbi [off-list ref]=0A=
Date: Wed Aug 19 18:05:27 2015 -0500=0A=
=0A=
usb: gadget: fix ep->claimed lifetime=0A=
=0A=
In order to fix a regression introduced by commit=0A=
cc476b42a39d ("usb: gadget: encapsulate endpoint=0A=
claiming mechanism") we have to introduce a simple=0A=
helper to check if a particular is enabled or not.=0A=
=0A=
After that, we need to move ep->claimed lifetime to=0A=
usb_ep_enable() and usb_ep_disable() since those=0A=
are the only functions which actually enable and=0A=
disable endpoints.=0A=
=0A=
A follow-up patch will come to drop all driver_data=0A=
checks from function drivers, since those are, now,=0A=
pointless.=0A=
=0A=
Fixes: cc476b42a39d ("usb: gadget: encapsulate endpoint=0A=
claiming mechanism")=0A=
Cc: Robert Baldyga [off-list ref]=0A=
Signed-off-by: Felipe Balbi [off-list ref]=0A=
=0A=
=0A=
Removing this line causes autoconfig can return the same endpoint many=
=0A=
quoted
quoted
quoted
times. This probably causes problems with g_zero.=0A=
=0A=
I will try to fix it ASAP.=0A=
=0A=
I was considering the same thing, but the lifetime of ->claimed doesn't=
=0A=
quoted
quoted
look correct to me either way. Note that once the flag is enabled, it=
=0A=
quoted
quoted
won't get disabled by most gadget drivers.=0A=
=0A=
And it should not be. This flag is indicator, that endpoint is used by s=
ome=0A=
quoted
function. It should be set once by usb_ep_autoconfig() and cleared by=0A=
usb_ep_autoconfig_reset().=0A=
=0A=
And the 'claimed' flag should be used for the ep autoconfig=0A=
mechanism alone. We may want to reset it during the autoconfig=0A=
phase for multiple configs and alt-interfaces. So there should be=0A=
separate 'claimed' and 'enabled' flags.=0A=
=0A=
=0A=
=0A=
have you considered switching interfaces and/or alternate settings ?=0A=
=0A=
We ran into similar issues before with this very scenario. Handling=0A=
of set_config(0 or N), in both addressed and configured states, and=0A=
set_interface requests.=0A=
=0A=
=0A=
John=0A=
=0A=