On 09/05/2011 11:57 AM, Felipe Balbi wrote:
Hi,
Hi,
A few drivers, like ehci-tegra.c, will be more troublesome. For such
cases it might be better to have a private hc_driver structure with
most of the entries copied from the public ehci_hc_driver.
I don't agree here. If it's only a few fields, you can pass them on a
struct ehci_platform_ops structure by using platform_data pointer on
Now I understand why Alan is pointing over to tegra over and over again.
Looing at its hc_driver callbacks it has more non-standard callbacks
than the other platforms. So let me look
- reset: almost the same code comparing to others: no quirks, has tt,
offset +0x100
- *map_urb_for_dma(): looks like hc requires buffers to be 32byte
aligned and here it implements a bounce buffer if it is not. Now let
me look:
- It sets dma mask to 32bit which means it allows the subsystem(s)
to allocate memory somewhere between 0..4GiB-1. The copy code
does not use kmap() which means once it grabs pages from highmem
then it goes boom.
- The bounce buffer could be avoided to some degree if we would
expose the align mask in usb_hcd and let every user use it.
Something like that is done in crypto subsystem where proper
alignment is performance critical.
- bus_{suspend|resume}: PM code. For some reason everyone but PCI is
using ehci_bus_suspend() which calls ehci_pci_suspend(). tegra does
the same thing except is also powers down the phy. Due to this phy
related change it requires to change .shutdown so it does not play
with registers once the clock is off.
- hub_status_data: phy related suspend/resume together reset related
quirk.
So. The phy related stuff could go into its own subsystem. Could
bus_suspend/resume be moved somewhere into a pm-related struct? reset
can be probably shared with others. The *map_urb_for_dma could be moved
into generic code if we add an align_mask into the usb_hcd struct but
for now... And finally I think hub_status_data has to remain as it due
to the quirk in it not just the phy thingy but it makes it more
complicated.
I don't think how ehci_platform_ops would fit in here unless it is a
copy of of ehci_hc_driver. For the phy suspend it is a parent/child
relation where we first suspend the one then the other. The
hub_status_data is overwriting parts the generic code.
Sebastian
--
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
