Hi, On Thu, Aug 28, 2014 at 04:46:41PM +0800, Peter Chen wrote: > On Wed, Aug 27, 2014 at 02:22:30PM -0500, Felipe Balbi wrote: > > On Wed, Aug 20, 2014 at 01:30:40PM +0800, Peter Chen wrote: > > > On Tue, Aug 19, 2014 at 09:02:54PM +0000, Paul Zimmerman wrote: > > > > > From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb-owner@xxxxxxxxxxxxxxx] On Behalf Of Peter Chen > > > > > Sent: Tuesday, August 19, 2014 7:26 AM > > > > > > > > > > On Tue, Aug 19, 2014 at 01:46:17AM +0000, Paul Zimmerman wrote: > > > > > > > From: Peter Chen [mailto:peter.chen@xxxxxxxxxxxxx] > > > > > > > Sent: Sunday, August 17, 2014 9:14 PM > > > > > > > > > > > > > > Except for chipidea driver, all other udc drivers will tell the > > > > > > > gadget driver that they are ready to pullup dp at udc_start, it > > > > > > > is the default behaviour. > > > > > > > > > > > > > > The chipidea driver is ready to pullup dp when the vbus is there, > > > > > > > and isn't ready to pullup dp when the vbus is not there. Other > > > > > > > udc drivers which should not pull up when vbus is not there should > > > > > > > change like chipidea does to pass the Back-Voltage Testing at > > > > > > > www.usb.org/developers/docs/USB-IFTestProc1_3.pdf. > > > > > > > > > > > > > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> > > > > > > > --- > > > > > > > drivers/usb/chipidea/udc.c | 9 ++++----- > > > > > > > drivers/usb/dwc2/gadget.c | 2 ++ > > > > > > > drivers/usb/dwc3/gadget.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/amd5536udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/atmel_usba_udc.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/bcm63xx_udc.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/dummy_hcd.c | 1 + > > > > > > > drivers/usb/gadget/udc/fotg210-udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/fsl_qe_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/fsl_udc_core.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/fusb300_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/gr_udc.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/lpc32xx_udc.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/m66592-udc.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/mv_u3d_core.c | 1 + > > > > > > > drivers/usb/gadget/udc/mv_udc_core.c | 2 ++ > > > > > > > drivers/usb/gadget/udc/net2272.c | 1 + > > > > > > > drivers/usb/gadget/udc/net2280.c | 1 + > > > > > > > drivers/usb/gadget/udc/omap_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/pch_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/pxa25x_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/pxa27x_udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/r8a66597-udc.c | 1 + > > > > > > > drivers/usb/gadget/udc/s3c-hsudc.c | 1 + > > > > > > > drivers/usb/gadget/udc/s3c2410_udc.c | 1 + > > > > > > > drivers/usb/musb/musb_gadget.c | 2 ++ > > > > > > > drivers/usb/renesas_usbhs/mod_gadget.c | 7 ++++++- > > > > > > > 27 files changed, 45 insertions(+), 6 deletions(-) > > > > > > > > > > > > Instead of modifying all of the UDC drivers, how about adding a flag to > > > > > > 'struct usb_gadget' named 'needs_ready' or similar? If the UDC doesn't > > > > > > set the flag, udc-core would call usb_udc_ready_to_connect() on behalf > > > > > > of the UDC. If the UDC sets the flag (chipidea only?) then the UDC would > > > > > > be responsible for calling usb_udc_ready_to_connect(). > > > > > > > > > > > > > > > > USB spec requires dp is not pulled up when the vbus is not there, the dwc3 is > > > > > the newest core, I don't think other older udc cores all has similar capability > > > > > that does don't draw back voltage if software pullup bit is set and vbus is > > > > > not there. > > > > > > > > > > This patchset will delete the unconditional pullup dp operation at udc-core, > > > > > and we need to pullup dp at the end of hardware initialization (not consider > > > > > vbus case), then the end of .udc_start at udc driver is the old place. > > > > > > > > I think you misunderstood my suggestion. Since you are adding a call > > > > to usb_udc_ready_to_connect() at the end of almost every .udc_start > > > > function, why not have udc-core do it instead, immediately after the > > > > call to .udc_start? Unless the 'needs_ready' flag is set, which would > > > > only be set by the udc drivers for those controllers that need it. > > > > > > > > > > Thanks. > > > > > > Yes, we can do that, my original plan is usb_gadget_connect/usb_gadget_disconnect > > > are only called by gadget driver. If Felipe has no comment for it, I will > > > > not directly though. At least not for those using the composite layer. > > All functions using composite layer should use activate/deactivate. > > > > -- > > But the first usb_gadget_connect can't be called by most of function drivers, > it needs to be called after .udc_start. not true, even though that's how it is today it doesn't mean it's correct in all cases. Our current implementation leaves the possibility of us connecting to a host before e.g. obexd is ready to respond to host-side requests. At the end of the day, the gadget/function driver is the only one who really knows when everything it needs is ready to rock. cheers -- balbi
Attachment:
signature.asc
Description: Digital signature
