Hello, On Mon, Sep 06, 2010 at 06:26:56PM +0200, Matthieu CASTET wrote: > Alan Stern a écrit : >> On Mon, 6 Sep 2010, Matthieu CASTET wrote: >> >>> tdi_reset is already taking care of setting host mode. Don't >>> duplicate code in platform driver. >>> >>> Also make ehci_halt a nop if the controller is not in ehci mode, >>> otherwise the halt will fail. >>> >>> >>> Signed-off-by: Matthieu CASTET <matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@xxxxxxxxxxxxxxxx> >> >> This looks like several different patches all rolled up into one. The >> patch description mentions two different things, and there are also >> those hcd->has_tt changes which aren't described at all. > We need all of them to work : > before setup, the controller could be in idle/host/device state. > ehci_halt work only when the controller is in host mode, so we need to > setup mode before ehci_halt. > > Need need to move hcd->has_tt up in order ehci_halt can use it. > > Note that after this patch orion/ixp4xx setup that does : > ehci_reset > (ehci_halt) > ehci_init > > could be rewrite to do the correct sequence. > >> >> The name "tdi_is_ehci" is confusing. Besides being very similar to >> "ehci_is_TDI", its meaning isn't clear. What other mode besides EHCI >> could the TDI/ARC hardware be in? Should the name be something more >> like "tdi_in_host_mode"? > Yes > > Is that better ? > > -------------------------- > tdi_reset is already taking care of setting host mode for tdi devices. > Don't duplicate code in platform driver. > > Make ehci_halt a nop if the controller is not in host mode (otherwise it > will fail), and let's ehci_reset do the tdi_reset. > We need to move hcd->has_tt flags before ehci_halt, in order ehci_halt > knows we are a tdi device. > > > Before the setup routine was doing : > - put controller in host mode > - ehci_halt > - ehci_init > - hcd->has_tt = 1; > - ehci_reset > > Now we do : > - hcd->has_tt = 1; > - ehci_halt > - ehci_init > - ehci_reset > > PS : now we handle correctly the device -> host transition. > > Signed-off-by: Matthieu CASTET <matthieu.castet-ITF29qwbsa/QT0dZR+AlfA@xxxxxxxxxxxxxxxx> This patch (i.e. 65fd42724aee31018b0bb53f4cb04971423be664) broke the build on mxc using the mx51_defconfig: CC drivers/usb/host/ehci-hcd.o In file included from drivers/usb/host/ehci-hcd.c:1166: drivers/usb/host/ehci-mxc.c: In function 'ehci_mxc_drv_probe': drivers/usb/host/ehci-mxc.c:192: error: 'ehci' undeclared (first use in this function) drivers/usb/host/ehci-mxc.c:192: error: (Each undeclared identifier is reported only once drivers/usb/host/ehci-mxc.c:192: error: for each function it appears in.) drivers/usb/host/ehci-mxc.c:117: warning: unused variable 'temp' make[3]: *** [drivers/usb/host/ehci-hcd.o] Error 1 make[2]: *** [drivers/usb/host/ehci-hcd.o] Error 2 make[1]: *** [sub-make] Error 2 make: *** [all] Error 2 It's not obvious for me how to fix it up. Does anyone care to help me? Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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