Hi,
On Mon, Dec 13, 2010 at 08:11:41PM +0800, Ming Lei wrote:
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.
exactly, and that's why we pass function pointers of our hardware
specific implementatations to the more generic musb_hdrc.
It is very reasonable that specific drivers depends on more generic drivers,
for example, ehci/ohci/uhci drivers depend on usbcore.
but ehci-omap doesn't depend on ehci, it sets things up for ehci.
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?
you're increasing the amount of exported symbols.
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.
I rather use something like:
static int musb_read_fifo(...)
{
if (musb->ops->read_fifo)
return musb->ops->read_fifo(...);
/* otherwise fallback to generic implementation. */
[...]
}
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.
Without some numbers I can't see the real performance impact. I doubt a
pointer dereference is that expensive, is it ?!?
--
balbi
--
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
