Dear Lucas Stach,
On Tuesday, November 6, 2012 8:43:43 AM, Lucas Stach wrote:
Am Dienstag, den 06.11.2012, 00:56 +0100 schrieb Marek Vasut:
Dear Benoît Thébaudeau,
Dear Marek Vasut,
On Monday, November 5, 2012 11:54:12 PM, Marek Vasut wrote:
Dear Benoît Thébaudeau,
Hi Marek,
Thanks to Lucas' series coming with commits c7e3b2b and 676ae06, I'd like to use the multi-controller feature on MXC since most of these SoCs come with a USB IP supporting an OTG controller and multiple host-only controllers.
Currently the MXC code in ehci-mx{c|5|6}.c just ignores the index passed to ehci_hcd_init() and the like, and there are 3 port-specific configs (CONFIG_MXC_USB_PORT, CONFIG_MXC_USB_FLAGS and CONFIG_MXC_USB_PORTSC).
Not all USB ports from the USB IP will be available on each board for a given SoC, so we need a logical to physical USB port mapping.
I would suggest something like the following.
board.h: #define CONFIG_MXC_USB { \
{ \
0, \ MXC_EHCI_INTERNAL_PHY, \ MXC_EHCI_UTMI_16BIT | MXC_EHCI_MODE_UTMI \}, { \
1, \ MXC_EHCI_POWER_PINS_ENABLED | MXC_EHCI_PWR_PIN_ACTIVE_HIGH | \ MXC_EHCI_OC_PIN_ACTIVE_LOW, \ MXC_EHCI_MODE_ULPI \}, \
}
ehci-fsl.h: struct mxc_ehci_cfg {
int port; u32 flags; u32 portsc;
};
ehci-mx{c|5|6}.c: static const struct mxc_ehci_cfg cfg[CONFIG_USB_MAX_CONTROLLER_COUNT] =
CONFIG_MXC_USB;Then, in ehci_hcd_init(), there would be the following
replacements:
- CONFIG_MXC_USB_PORT -> cfg[index].port,
- CONFIG_MXC_USB_FLAGS -> cfg[index].flags,
- CONFIG_MXC_USB_PORTSC -> cfg[index].portsc.
What do you think?
What about passing port private / platform data instead of ID ?
The ID is already passed to ehci_hcd_init(), so we have to live with it if we don't want to change the newly introduced multi-controller infrastructure.
Let's change it .... remove the ID and pass some generic pdata.
I don't like the idea of passing around data at this level. It's breaking the abstraction, as we have to pass low-level usb information around in the higher USB stack levels.
The USB driver code should be able to do the virt-to-phys controller mapping on it's own. In the Tegra world we use the information we get from device tree to do so, but I don't see a reason why your USB host driver code wouldn't be able to just require an array with configuration data from the board file.
There is really no need to pass this information through all the USB stack interfaces.
I agree, all the more ehci_hcd_init() is called from cmd_usb.c, completely outside of any board init context, so collecting the platform data would be a real pain, without bringing much. And moving usb_init() calls to board init context would also not be good because of the added boot time.
IMHO, the best solutions here are either a CONFIG_MXC_USB as I suggested, or the same structure passed to some init function specific to these EHCI drivers (which would add more code for little benefit).
Marek? Stefano?
Best regards, Benoît