hi, On Tue, Jun 28, 2011 at 08:37:58AM -0700, Greg KH wrote: > On Tue, Jun 28, 2011 at 04:33:45PM +0300, Felipe Balbi wrote: > > Hi Greg, > > > > here's the pull request for USB Gadget and MUSB. > > > > I just tested these changes with MUSB and thus far > > everything seems ok. I had a few last minute issues > > with MUSB but they are fixed on this pull request. > > > > Like noted before on the announcement I made [1] we > > have one small known regression with MUSB regarding > > a module parameter which shouldn't really exist. > > I don't like having known problems like this. Why do we now have to fair enough. > have a module parameter where we didn't before? And who is going to The module has always been there, what changed is its default value which is set based on an ugly architecture-based ifdef: 1012 #if defined(CONFIG_USB_MUSB_TUSB6010) || defined(CONFIG_USB_MUSB_OMAP2PLUS) \ 1013 || defined(CONFIG_USB_MUSB_AM35X) 1014 static ushort __initdata fifo_mode = 4; 1015 #elif defined(CONFIG_USB_MUSB_UX500) 1016 static ushort __initdata fifo_mode = 5; 1017 #else 1018 static ushort __initdata fifo_mode = 2; 1019 #endif 1020 1021 /* "modprobe ... fifo_mode=1" etc */ 1022 module_param(fifo_mode, ushort, 0); 1023 MODULE_PARM_DESC(fifo_mode, "initial endpoint configuration"); I decided it would be way uglier to add corresponding _MODULE version for each glue layer and left as is so that I force myself (and others) to make this work reliably. > properly set that thing? that thing has to vanish. The core (most of the versions that I know about, at least) has Dynamic FIFO Sizing support, which means we can defer FIFO allocation to usb_ep_enable() which is called when we actually know which configuration/interface was chosen. > > This regression will be patched during the -rc cycle, > > Why not "patch" it now? because to re-factor it properly, I need more time to test and to add yet another hack: diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index d53e616..6c096af 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1009,10 +1009,15 @@ static void musb_shutdown(struct platform_device *pdev) * We don't currently use dynamic fifo setup capability to do anything * more than selecting one of a bunch of predefined configurations. */ -#if defined(CONFIG_USB_MUSB_TUSB6010) || defined(CONFIG_USB_MUSB_OMAP2PLUS) \ - || defined(CONFIG_USB_MUSB_AM35X) +#if defined(CONFIG_USB_MUSB_TUSB6010) \ + || defined(CONFIG_USB_MUSB_TUSB6010_MODULE) \ + || defined(CONFIG_USB_MUSB_OMAP2PLUS) \ + || defined(CONFIG_USB_MUSB_OMAP2PLUS_MODULE) \ + || defined(CONFIG_USB_MUSB_AM35X) \ + || defined(CONFIG_USB_MUSB_AM35X_MODULE) static ushort __initdata fifo_mode = 4; -#elif defined(CONFIG_USB_MUSB_UX500) +#elif defined(CONFIG_USB_MUSB_UX500) \ + || defined(CONFIG_USB_MUSB_UX500) static ushort __initdata fifo_mode = 5; #else static ushort __initdata fifo_mode = 2; would just be perpetuating the wrong idiom. If you prefer I do that to avoid the default parameter value problem, I can fix that up in 2 minutes. > > but will only be properly fixed on the 3.2 merge window > > because it, again, requires a big re-work of how MUSB > > works (basically we need to change how FIFO space is > > allocated and how endpoints are setup). > > Ok, but adding a known problem isn't acceptable. Fair enough, I'll fix it up. -- balbi
Attachment:
signature.asc
Description: Digital signature
