Chen Peter-B29397 <B29397@xxxxxxxxxxxxx> writes: >> -----Original Message----- >> From: linux-usb-owner@xxxxxxxxxxxxxxx [mailto:linux-usb- >> owner@xxxxxxxxxxxxxxx] On Behalf Of Alexander Shishkin >> Sent: Friday, May 11, 2012 8:06 AM >> To: Greg Kroah-Hartman >> Cc: linux-usb@xxxxxxxxxxxxxxx; Felipe Balbi; Alexander Shishkin; Alan >> Stern >> Subject: [PATCH v8 15/20] usb: chipidea: add host role >> >> This adds EHCI host support to the chipidea driver. We want it to be >> part of the hdrc driver and not a standalone (sub-)driver module, as >> the structure of ehci-hcd.c suggests, so for chipidea controller we >> hack it to not provide platform-related code, but only the ehci hcd. >> >> The ehci-platform driver won't work for us here too, because the >> controller uses the same registers for both device and host mode and >> also otg-related bits, so it's not really possible to put ehci registers >> into a separate resource. >> >> This is not a pretty solution, but the alternative is exporting symbols >> from the chipidea driver to a ehci-chipidea driver and doing all the >> module refcounting. >> > Hi Alexander, Hi, > First, it is really great work, and I hope we can all switch to it > in future. Thanks for your interest and sorry for not replying earlier. First, let me point out that my goal with this patchset was to lay the groundwork so that we could make it into something that's usable for everybody. I didn't try to implement every single corner case, because it would have turned into a much longer and even less maintainable patchset. > I can't find summery for this patchset, so I have added some > of my questions here, some of them I asked yesterday, but does > not get answers. Some of Freescale i.MX users would like to use > your structure, so I go through your code, and would like to > know your comments for below questions: > > - How to support OTG, you will give up struct usb_otg or not? I'm not sure what you mean by giving up usb_otg. What we were discussing with Heikki and Felipe is reworking the otg framework so that, for example, we have a common otg state machine instead of reimplementing it in every otg driver. We planned for this work to be based upon this chipidea driver. Maybe this topic should be brought up again. In any case, my plan is that the otg part of ci_hdrc should be based on those otg framework changes, which should come first. Heikki, do you have anything to add? > - When ID switch occurs, do udc_start/udc_stop is enough, seems no registers > operation? The ID switch will cause the stop() for current role and the start() for the new role, which at the very least resets the device and sets USBMODE_CM_{D,H}C. Also, when the gadget is stopped, UDC is deinitialized and queue heads are flushed. Does this answer your question? > - I have not found vbus interrupt code which is used to stand for > connect and disconnect at device mode. It's not there yet. I think it should be in the otg part of the driver. What do you think? > - Seems hcd will register interrupt 0 irq handler, if the system > has really interrupt 0 irq handler, it may mess up. The hcd core regards 0 irq line as "no irq" and doesn't set its own interrupt handler. But, since chipidea's core.c already has an irq handler on this line, I decided to reuse that and call usb_hcd_irq() to deliver interrupts to the ehci driver. Core irq handler will be installed on whatever line number is provided by the platform code as long as it's not negative (in which case the whole initialization will fail). So theoretically, it will work even if chipidea's irq is 0 and irq 0 is valid. > - Current, there is no PHY Layer code (I will begin to do it soon), > so your PHY operation is at udc code, once the PHY Layer code is ready, > do you will move it to core.c? I think of phy code as platform code, it doesn't directly concern the chipidea driver, except for the bits where chipidea might need explicit configuration of the phy type. But I think that is still separate from the actual phy code. My understanding is, usb controllers should be really decoupled from phy code and shoud only talk to one another through certain api. Again, Heikki might have more to say on this. > - What's your plan for low power mode and wakeup support? It needs to be implemented. As many other things on the original driver's todo list. > - For host driver, what's plan to add controller callback (override > struct hc_driver) for vendor special, like you have done for udc. I haven't really given it much thought yet and I don't have the whole picture in my mind. I will return to it shortly, when I have gone through the other thread. But certainly, we need to provide for enough flexibility for platforms to work. > Seems you have moved msm udc to chipidea folder, but have not changed > host and otg part, how msm user goes on using usb driver? Well, both ehci-msm.c and msm-otg.c are separate drivers, and nothing has changed from their prospective. With my patchset, they should still work as they did before it. I hope I did answer at least some of your questions. Regards, -- Alex -- 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