| Message ID | 1356200860-3241-2-git-send-email-mikedunn@newsguy.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Dear Mike Dunn, > This patch fixes some bad behaviour from the usb gadget during machine > initialization by changing the management of the D+ pull-up gpio from the > gpio-vbus driver to the pxa27x-udc driver. Also, code that drives the > pull-up high is removed. (The gpio-vbus driver can optionally manage the > D+ line pull-up, but the pxa27x-udc driver does this itself.) > > Without this patch, the host senses the presence of the usb gadget during > machine initialization (when palm27x_udc_init() runs), at which point it > tries to enumerate the newly detected usb gadget. But because the > pxa27x-udc driver has not been initialized yet, there's no gadget driver > to respond to the host, and enumeration fails. Tested on my Palm Treo680. [...] I think it was the whole big idea to let gpio-vbus manage this kind of stuff. But it's been a while, Ccing Haojian to review these. Best regards, Marek Vasut
Marek Vasut <marex@denx.de> writes: > Dear Mike Dunn, > >> This patch fixes some bad behaviour from the usb gadget during machine >> initialization by changing the management of the D+ pull-up gpio from the >> gpio-vbus driver to the pxa27x-udc driver. Also, code that drives the >> pull-up high is removed. (The gpio-vbus driver can optionally manage the >> D+ line pull-up, but the pxa27x-udc driver does this itself.) >> >> Without this patch, the host senses the presence of the usb gadget during >> machine initialization (when palm27x_udc_init() runs), at which point it >> tries to enumerate the newly detected usb gadget. But because the >> pxa27x-udc driver has not been initialized yet, there's no gadget driver >> to respond to the host, and enumeration fails. Tested on my Palm Treo680. > [...] > > I think it was the whole big idea to let gpio-vbus manage this kind of stuff. > But it's been a while, Ccing Haojian to review these. Actually gpio-vbus was designed to : - detect and handle VBus - and handle following D+ pullup if peripheral controller *can't* But pxa27x_udc is a peripheral controller which does handle D+ pullup, and expects to handle it by himself (ie. is not fully compatible with a D+ pullup handling by gpio-vbus, one not working case being Mike's one). So Mike's patch makes sense IMHO. He could have left the VBUS handling to gpio-vbus and D+ to pxa27x_udc, or let pxa27x_udc handle both, that's up to you. Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>
Hi Robert, On 12/24/2012 07:34 AM, Robert Jarzmik wrote: [..] Actually gpio-vbus was designed to : > - detect and handle VBus > - and handle following D+ pullup if peripheral controller *can't* > > But pxa27x_udc is a peripheral controller which does handle D+ pullup, and > expects to handle it by himself (ie. is not fully compatible with a D+ pullup > handling by gpio-vbus, one not working case being Mike's one). > > So Mike's patch makes sense IMHO. He could have left the VBUS handling to > gpio-vbus and D+ to pxa27x_udc, or let pxa27x_udc handle both, that's up to you. I didn't realize the pxa27x-udc driver could handle the vbus as well. > Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> Thanks Robert. Should have CC'd you, since you're the udc driver author. Glad you saw this. Thanks again, MIke
diff --git a/arch/arm/mach-pxa/palm27x.c b/arch/arm/mach-pxa/palm27x.c index 17d4c53..298a8a9 100644 --- a/arch/arm/mach-pxa/palm27x.c +++ b/arch/arm/mach-pxa/palm27x.c @@ -167,7 +167,7 @@ void __init palm27x_lcd_init(int power, struct pxafb_mode_info *mode) #if defined(CONFIG_USB_PXA27X) || \ defined(CONFIG_USB_PXA27X_MODULE) static struct gpio_vbus_mach_info palm27x_udc_info = { - .gpio_vbus_inverted = 1, + .gpio_pullup = -1, /* pxa27x-udc driver handles D+ pull-up, not vbus */ }; static struct platform_device palm27x_gpio_vbus = { @@ -180,17 +180,22 @@ static struct platform_device palm27x_gpio_vbus = { void __init palm27x_udc_init(int vbus, int pullup, int vbus_inverted) { - palm27x_udc_info.gpio_vbus = vbus; - palm27x_udc_info.gpio_pullup = pullup; + struct pxa2xx_udc_mach_info udc_mach_info = { + .udc_is_connected = NULL, + .udc_command = NULL, + .gpio_pullup_inverted = false, + }; + udc_mach_info.gpio_pullup = pullup; + palm27x_udc_info.gpio_vbus = vbus; palm27x_udc_info.gpio_vbus_inverted = vbus_inverted; - if (!gpio_request(pullup, "USB Pullup")) { - gpio_direction_output(pullup, - palm27x_udc_info.gpio_vbus_inverted); - gpio_free(pullup); - } else + /* driver will quietly ignore an invalid gpio */ + if (!gpio_is_valid(pullup)) { + pr_err("Palm27x: USB D+ pull-up gpio is invalid!\n"); return; + } + pxa_set_udc_info(&udc_mach_info); platform_device_register(&palm27x_gpio_vbus); }
This patch fixes some bad behaviour from the usb gadget during machine initialization by changing the management of the D+ pull-up gpio from the gpio-vbus driver to the pxa27x-udc driver. Also, code that drives the pull-up high is removed. (The gpio-vbus driver can optionally manage the D+ line pull-up, but the pxa27x-udc driver does this itself.) Without this patch, the host senses the presence of the usb gadget during machine initialization (when palm27x_udc_init() runs), at which point it tries to enumerate the newly detected usb gadget. But because the pxa27x-udc driver has not been initialized yet, there's no gadget driver to respond to the host, and enumeration fails. Tested on my Palm Treo680. Signed-off-by: Mike Dunn <mikedunn@newsguy.com> --- arch/arm/mach-pxa/palm27x.c | 21 +++++++++++++-------- 1 files changed, 13 insertions(+), 8 deletions(-)