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?
> 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 ?!?
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.
--
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
