Hi Igor Grinberg <grinberg@xxxxxxxxxxxxxx> writes: [...] > Thanks for the information. Now I am finally getting the picture of what's is > going on there... > >> On the Smartbook at least both USB host ports (H1 and H2) on the board >> (one port each) are connected directly to 4-port USB hubs (SMSC2514). >> We don't have anything on there except that connection: the hub should >> handle VBUS properly. Both ports use an SMSC3317 (just a 3311 with a >> built in 3.3V supply so the id and behavior should be identical). > > OK, so the SMSC331x ulpi transceiver is connected to the SMSC2514 usb hub, > so all the peripheral devices (ethernet/wifi/bt/hid) are connected to the > downstream ports of that hub and thus get their Vbus as expected. This is fine. > >> On the Smarttop H1 is connected to a 4-port USB hub (Terminus FE1.1) >> with the same configuration. Same PHY. The DR port is connected >> directly to an ASIX ethernet controller. VBUS seems routed to a test >> point. >> >> I'm curious exactly what the real problem here is: that VBUS is >> basically not being handled correctly? It should be driven or not? I'm >> not entirely familiar with the specification. > > SMSC2514 usb hub will not provide power to its D+ and D- pull-up resistors > until it detects a Vbus enabled on the upstream port. This is totally fine. > > SMSC331x ulpi transceiver does not have either an integrated Vbus switch or > an integrated charge pump - this means that it cannot provide a Vbus to the hub. > > The hub in its turn does not power the pull-up resistors and peripheral devices > are not being connected to the usb subsystem. > > With this patch applied, SMSC331x ulpi transceiver issues an SRP pulses on the Vbus > and hub senses that there is something that looks like Vbus and then enables the > pull-up resistors -> peripheral devices are being connected to the usb subsystem. > This behavior violates the ULPI (and mostly certain OTG and may be also USB 2.0) > specification and SMSC2514 usb hub datasheet. > > According to the SMSC2514 usb hub datasheet, VBUS_DET pin has to be connected > to a valid Vbus from the upstream port, but SMSC331x does not provide this Vbus. > This means that you should have add a Vbus switch or a charge pump to the > VBUS_DET pin of the usb hub and provide means (like GPIO) to enable/disable that > switch or charge pump. > This is h/w design bug we are dealing with. > > The best solution to this would be to add a missing h/w component. > Now, I understand that it can be kind a problematic ;) > But, we cannot violate the ULPI spec and the generic driver to workaround > some h/w problem that is existing in some specific configuration and hopefully will > be fixed in the next h/w revisions. Therefore, as I said before, this patch is NAK. > > What we can do is: > 1) implement the int (*start_srp)(struct otg_transceiver *otg); > method as defined by the ULPI spec. > > 2) and then add a call (along with huge comment explaining this workaround) > to otg_start_srp(). I'd recommend to restrict this call to that specific board somehow, > but it is up to Sascha to decide where to put it. What about the following patch ?
ulpi: provide start_srp Add support for setting CHRGVBUS thanks to start_srp and use it to workaround a hardware bug on efika mx/sb boards. See http://lists.infradead.org/pipermail/linux-arm-kernel/2011-January/037341.html Signed-off-by: Arnaud Patard <arnaud.patard@xxxxxxxxxxx> Index: linux-2.6-submit/drivers/usb/otg/ulpi.c =================================================================== --- linux-2.6-submit.orig/drivers/usb/otg/ulpi.c 2011-01-04 17:12:26.000000000 +0100 +++ linux-2.6-submit/drivers/usb/otg/ulpi.c 2011-01-06 14:44:50.000000000 +0100 @@ -247,6 +247,14 @@ return otg_io_write(otg, flags, ULPI_OTG_CTRL); } +static int ulpi_start_srp(struct otg_transceiver *otg, bool on) +{ + unsigned int flags = otg_io_read(otg, ULPI_OTG_CTRL); + + flags |= ULPI_OTG_CTRL_CHRGVBUS; + return otg_io_write(otg, flags, ULPI_OTG_CTRL); +} + struct otg_transceiver * otg_ulpi_create(struct otg_io_access_ops *ops, unsigned int flags) @@ -263,6 +271,7 @@ otg->init = ulpi_init; otg->set_host = ulpi_set_host; otg->set_vbus = ulpi_set_vbus; + otg->start_srp = ulpi_start_srp; return otg; } Index: linux-2.6-submit/drivers/usb/host/ehci-mxc.c =================================================================== --- linux-2.6-submit.orig/drivers/usb/host/ehci-mxc.c 2011-01-06 14:45:14.000000000 +0100 +++ linux-2.6-submit/drivers/usb/host/ehci-mxc.c 2011-01-06 15:48:16.000000000 +0100 @@ -25,6 +25,8 @@ #include <mach/mxc_ehci.h> +#include <asm/mach-types.h> + #define ULPI_VIEWPORT_OFFSET 0x170 struct ehci_mxc_priv { @@ -239,6 +241,13 @@ dev_err(dev, "unable to enable vbus on transceiver\n"); goto err_add; } + if (machine_is_mx51_efikamx() || machine_is_mx51_efikasb()) { + ret = otg_start_srp(pdata->otg); + if (ret) { + dev_err(dev, "unable to start srp\n"); + goto err_add; + } + } } return 0;
