2010/12/13 Felipe Balbi <balbi@xxxxxx>:
> 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.
In fact many glue driver used the same default read_fifo/write_fifo, do
you want to duplicate the code many times in this glue drivers?
>
>>>>> 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
>
--
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
