Hi, On Fri, Apr 11, 2014 at 10:23:38AM +0800, Wang, Yu wrote: > Hi Balbi, > > At first, thank you to help give the response in patience. > > > Hi, > > > > On Wed, Apr 09, 2014 at 02:08:32PM +0800, Wang, Yu wrote: > > > Glad to see the OTG mode is prepare to support in your > > > dwc3-role-switch branch. But it is not fit for intel > > > Merrfield/Moorfield platforms. :( > > > > that's not what I heard when I asked some friends to test that branch > > on different platforms. It's certainly not perfect yet and that's why > > the code isn't merged in mainline, but claiming that it's not fit for > > Merrifield/Moorfield is just a lie. > > > > Can I get your friends' email address if they are in Intel? I would like to You already have that. Just look at Cc list. > contact them to make the branch work on my Merrifield/Moorefield board too. > Currently I can't see any link change event with the branch. how did you test it ? According to [1] Merrifield won't work for more than 20s or so with v3.14 and, since there is no resolution on the thread, I assume today's mainline won't work either. Anyway, if you really did test, test again with enabling verbose debug on dwc3 and show me the logs, I'm interested in what the IP is doing. > > > The reason is we implemented DRD mode instead of OTG mode. So the > > > GCTL.PortCapDir will be set as 01 for host mode, and 10 for device mode. > > > > from a SW perspective there is *no* difference. The only reason for > > using PortCapDir is to cope with platforms which want to switch roles > > but have screwed up ID pin reporting. And since most of the platforms > > actually have tha problem, it's just easier to implement it that way. > > > > Ok. Just confirm with you that you think it's not a matter to use > GCTL.PortCapDir or OCTL.PeriMode to switch role, right? As of today, I don't see a difference. Things *can* change though. We can find out more details about the HW which forces us to use one of the other. > > > If no cable connected, we will trigger D0i3cold which will power > > > gate every clock/power used by dwc3 controller. > > > > that's not in mainline though, why should I care ? Show me code adding > > support for D0i3cold for dwc3 then we can talk. Until then, I'll > > continue assuming there's no such PM state and continue with happy hacking > > sessions. > > > > > And entering D0i3hot with hibernation mode when acting as host mode > > > (micro A cable connected), also during device mode(micro B cable > > > connected to PC host). > > > > Current driver also doesn't support any runtime PM, so why should I > > care about your out-of-tree changes ? If you have any code which is > > not in mainline and I change something in mainline you have to deal to with it, > > not me. > > Get it. So we can treat this solution is working w/o PM implementation > so far. And maybe we need to re-design DRD/OTG when we consider to > support PM, right? We probably don't need to redesign anything when adding PM. What we need is that when we start to add PM support to this driver, in case there is any regression, we find out why there is a regression. Then we solve it properly, after looking at logs and fully understanding what's breaking the driver. In any case, that's something that won't happen in mainline for at least a couple more major releases since there are still quite a few changes pending. > > I just cannot spend time imagining all twisted scenarios Vendors > > (including my employer, for that matter) want to support with whatever > > hacked up version of mainline drivers they might have. > > > > My plan is to *not* add a lot of PM support until I know all other > > features are functional. When I'm happy with those features and know > > they have been thoroughly tested on *all* users of dwc3 then we can > > all get together in a conference (maybe have a DWC3 mini-summit or > > whatever) and discuss how we're gonna implement PM *together*. > > > > Since the driver is used by many different vendors, it would be unfair > > for me to treat Intel (or any vendor, really) differently just because > > someone dropped me an email commenting about all the out-of-tree changes > > they have. > > > > Understand. What are the other features you mean that need thorough testing > before adding PM support? DRD for one. Then there is a redesign of parts of the gadget API that I want to do before PM too. Then there is the whole Link Power Management (which has *nothing* to do with Linux PM - system or runtime; we can put the USB bus in low power mode even though we don't gate any of the USB IP's clocks or power rails). > > > For ID/VBus detection, we are using PMIC to do detect. So we will > > > also power gate the USB PHY for D0i3cold. > > > > yeah, everybody does that. Everybody I've seen so far have moved the > > analog comparators for VBUS and ID out of the PHY and into the PMIC. > > > > We even have a very nice framework to cope with that (see > > drivers/extcon/extcon-palmas.c for an example). > > > > Thanks for pointing it out. We will use it too. > > > > When we plug in micro A/B cable, the PMIC will report the ID/VBus > > > change event, then driver will force controller resume to D0 from > > > D0i3cold. Due to we haven't do any backup before entering D0i3cold, > > > so we have to re-initialize all host/device portion registers with > > > setting GCTL.PortCapDir to 01 or 10. > > > > yeah, but this is just how *you* decided to implement this for your > > employer and, guess what, all of that is out-of-tree and, by all > > practical means wrt mainline kernel, it's all non-existent. This means > > I'm free to implement all of this any way I feel it's best considering the diverse > > user-base we have on this driver. > > > > Quite frankly, resetting the IP is only necessary iff all power rails > > are cut, in that case, sure, we will reset the IP and start all over, > > but we don't want to cut all power while we're attached to host, which > > means we don't need to reset the IP, a simple context restore is enough in > > most cases. > > > > Also, when we implement hibernation (dwc3's hibernation, that is) we > > won't need to reset the IP even if we go all the way down to D0i3cold > > because the IP keeps all context in a scratch buffer we allocate during probe. > > Yes. I know what is your mean. Just I don't know how to implement > hibernation mode for DRD mode. The synopsys DWC3 spec haven't mentioned > it. one thing has nothing to do with the other. By the time hibernation kicks in, the IP is already set on one role and won't change unless you revert cable or HNP - for those which support it - kicks in. In both cases, dwc3 *must* first wake up before even figuring out what it is the user wants it to do. > And also for OTG hibernation mode. If we power gate the USB PHY during > OTG hibernation mode, and using PMIC to do ID/VBUS detection. I don't > know if there have any issues. Until we get there, we won't know. > > > So with this PM design and DRD mode, we can't use your OTG mode. :( > > > > that's not my problem though, is it ? Since *your* PM design isn't > > available in mainline anyway. Not to mention that *my* DRD mode wasn't > > merged either, even though I have good reports from some friends > > stating it's working in 90% of the cases. > > > > Maybe I should add more explanation on the purpose of this email. I > was not complaining you on your codes. Just wanted to discuss with you > on our solution in order to improve our codes acceptable in community. sure, that's always good; but you can't expect me to consider - or even know about - out-of-tree patches. > > > We add DRD support based on your otg.c or create new file drd.c? > > > > Well, since I've already been working on it (well, George Cherian - > > now in Cc - was the originator) I rather continue on it together with > > George. Thank you > > Do you have a first version that I can take a look? Since I'm working > on it too, I would like to avoid conflict at the beginning. Thanks. see my dwc3-role-switch branch in k.org. And since you're asking, it makes me wonder again how did you test my patches ? I mean, you _did_ claim they don't work and yet you don't know where to get them from ? [1] http://marc.info/?l=linux-kernel&m=139648140104265 -- balbi
Attachment:
signature.asc
Description: Digital signature
