On Mon, Dec 13, 2010 at 09:21:48PM +0800, Ming Lei wrote:
2010/12/13 Felipe Balbi <balbi@xxxxxx>:
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.
Seems it is a low cost action, :-) Also not very much symbols are to be
exported.
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)
But where are .read_fifo from? we must set a symbol to it, and
where are the symbol from?
it's a static function in the glue layer. Just add it to struct
musb_platform_ops.
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 ?!?
Since read[bsl]/write[bsl] are called with very high frequency, it is better to
inline it. At least all read[bsl]/write[bsl] are inline or defined as
MACRO in other
arch or platform.
without numbers, we can't be sure it's really a big overhead.
--
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
