2010/12/13 Felipe Balbi <balbi@xxxxxx>: > On Mon, Dec 13, 2010 at 07:21:42PM +0800, Ming Lei wrote: >> >> 2010/12/13 Felipe Balbi <balbi@xxxxxx>: >>> >>> On Mon, Dec 13, 2010 at 01:01:47AM +0800, tom.leiming@xxxxxxxxx wrote: >>>> >>>> From: Ming Lei <tom.leiming@xxxxxxxxx> >>>> >>>> Since hw glue drivers may depend on musb_hdrc and musb_init will return >>>> failure if no glue device is registered, this patch fixes the issue >>>> by registering musb_hdrc driver first. >>> >>> don't do that. We want the glue layer to be registered first as it will >>> setup the platform_device for musb and setup the platform_data, enable >>> clocks, etc. >>> >>> NAK >> >> If we want to compile glue layer as module, we must load musb_hdrc >> first since glue layers depend on musb_hdrc. (It is a very reasonable >> model, that specific/concrete device modules depend on musb core driver, >> which is more generic) >> >> I change platform_driver_probe to platform_driver_register, which not >> cause .probe of musb_hdrc called until 'musb_hdrc' device is created. >> >> So the patch doesn't change the running order between .probe of glue and >> .probe of musb_hdrc, and first is .probe of glue always, musb_hdrc .probe >> is second. >> >> So I think the patch is correct, and does make sense. > > could be, but ideally, glue layer would not depend on symbols exported > by musb, it would be the other way around. Glue layer sets things up for > musb to use. So any exported symbols from musb used in glue layer should > be studied and converted into a more generic thing. All DBG() calls on In fact, we can think glue device as specific device and musb device (musb_hdrc) as more generic device, so glue driver should focus on specific hardware details and musb hdrc should be more generic to handle all standard things. It is very reasonable that specific drivers depends on more generic drivers, for example, ehci/ohci/uhci drivers depend on usbcore. > glue layer can be converted into dev_dbg(), for musb_interrupt() we can > add a 'irq' field on musb_platform_ops, similarly for read/write fifo EXPORT is simpler and reasonable, also no obvious side effect, right? For example, musb_read_fifo/musb_write_fifo in musb_core.c should be enough for most of glue devices, except for AM35x(read), bfin and tusb. So we have to put the generic musb_read_fifo/musb_write_fifo into one place, no doubt the ideal place is musb_hdrc. But how do other glue drivers see the symbols? export_symbol is OK. Also same with above for musb_interrupt too. > and the read[bsl]/write[bsl] calls, although those I think should go to > some musb_platform_io structure. As said before, for read[bsl]/write[bsl], we should avoid to access this via function pointer as far as possible for performance reason. It is best to access this helpers by inline function or macro always. thanks, -- Lei Ming -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
