> Hi,
>
> On Fri, Aug 02, 2013 at 10:54:18AM +0000, Wang, Yu Y wrote:
> > Check my comments as follows.
>
> weird, you sent plain text email, but there are no quotation marks... it makes it
> very difficult to read. Please go through our netiquette, there is a copy of that
> at [1] and also lots of good information under Documentation, including tips on
> how to configure many email client.
[Yu:] Thanks for your help. Now I changed the configuration to add quotation marks :)
>
> > On Fri, Aug 02, 2013 at 09:25:39AM +0000, Wang, Yu Y wrote:
> > > Thanks the quick response.
> > > Actually, our solution is familiar with your thinking.
> > >
> > > Due to we have to do "Power-On or Soft Reset" for disconnect and
> > > re-connect case, so driver need re-initialized all hardware registers.
> >
> > you don't need a full Soft-reset when disconnecting the cable. Read
> > the driver and you'll we don't soft-reset the IP at every disconnect.
> > [Yu] I don't know what version of your spec. My spec version is
> > "Version 2.10a January 2012", in section 12.2.3.4.
> > " Configure the core as described in "Device
> > Power-On or Soft Reset". " Spec require the core
> > do soft-reset.
>
> As described in that, but it doesn't mean we need to do a full SW reset of the IP.
> It just means we need to setup DCTL and all those other registers again.
[Yu:] Correct. So we have to re-initialize all relevant registers for this requirement.
And we want to maximize to re-use original functions. But we find that we have to
modify some code to achieve this purposes.
>
> > > That's why we want to separate register initialization operation
> > > from software part (like sw structure init and endpoint
> > > initialization)
> >
> > register initialization is also SW, so I don't know what you want to split here.
> > [Yu] We just want to split the SW initialization and hardware settings.
>
> and what do you mean by that ? What are you calling "SW initialization"
> and what do you consider to be "hardware settings" ?
[Yu:] The SW initialization is mean that no relevant hardware registers access.
The hardware settings is mean set some configurations via write controller
registers.
>
> > > However, we find that the hardware registers initialization is
> > > located in dwc3_gadget_init and 'dwc->gadget_driver' initialization
> > > is handled in dwc3_gadget_start.() So we can't full re-use these two
> functions.
> >
> > this is wrong. dwc3_gadget_init() does nothing more than allocate memory.
> > [Yu] In kernel3.10. In dwc3_gadget_init(), We find some hardware setting.
> > For example, set clear suspend enable bit of GUSB2PHYCFG and
> > GUSB3PIPECFG registers. Maybe we need to check the new version?
>
> hmm, that code wouldn't even run in version 2.10a of the core, it's for versions
> before 1.94a as you can see below:
>
>
> | int dwc3_gadget_init(struct dwc3 *dwc) {
> | u32 reg;
> | int ret;
>
> [ ... ]
>
> | /* Enable USB2 LPM and automatic phy suspend only on recent versions */
> | if (dwc->revision >= DWC3_REVISION_194A) {
> | dwc3_gadget_usb2_phy_suspend(dwc, false);
> | dwc3_gadget_usb3_phy_suspend(dwc, false);
> | }
>
> [ ... ]
> | }
>
> and, recently, I've dropped support for manual PHY control, see [2].
[Yu:]This patch is good. But for hibernation mode, still need to set these
two PHY registers. Anyway, we can consider that to use other implementations
for them
.
>
> > > All device and host hardware initialization will be done in these API.
> > > So for connect/disconnect case, the driver will re-initialization
> > > the host or device stack.
> >
> > this wastes too much time and is usually unnecessary.
> > [Yu] Base on this design, the core will be reset during role
> > switching. Not only controller, also for the PHY. We will
> > try our design to confirm if working.
>
> that's way too much time wasted. You *really* don't need to reset the whole
> thing just to change roles. I doubt that's how Synopsys designed the IP, if they
> did then nobody will ever pass the tight timing requirements imposed by OTG
> specification.
>
> The only problem I know about in that area is that the IP mirrors some device
> registers to important xHCI register. We have a case with Synopsys to track
> that but I'd say all versions suffer from that bug all the way to 2.50a, IIRC 2.60a
> has that sorted out.
>
> BTW, it might very well be that because of this problem you're suggesting that
> we Soft-Reset the IP, perhaps Synopsys didn't give you the full gory details ? But
> in summary, writing to some of the DEPCMDPAR registers, mirrors the same
> value into the xHCI's Ring registers, thus destroying any functionality when
> switching from device to host and back.
>
> We need to *really* think how we want to support these older versions of the
> Synopsys IP since that will be a *really* invasive change on both
> dwc3 and xhci drivers. My *strong* suggestion is to focus on stabilizing role
> switching before trying to implement hibernation.
>
[Yu:] We are using 2.10a RTL now. But it will be upgrade to 2.50a in the near future.
I will verify the driver without do soft-reset every time. If it is working, then we will
try to follow your design. But I just concern the hibernation mode which is easy met
failures.
> > > I saw your design is do all hardware initialization during kernel
> > > boot. And just use GCTL.PORTCAP to do the role switch. I haven't try
> > > this solution on intel platform. And not sure if is working with
> > > hibernation feature.
> >
> > right, then test and let me know the results. Also, instead of
> > spending so much time telling me what kind of changes Intel has in
> > their own hidden tree, why don't you go ahead and send patches for
> > review ? That would be so much easier for both of us...
> > [Yu] Actually. The hibernation feature waste us lots of time. And we will
> > try your design to confirm if hibernation mode still working. And we
> > will also try to change our drivers to better upstream. After that, We
> > will provide the patch to review.
>
> sure, that'd be great. Patches are always welcome. Just make sure you
> properly test hibernation and make sure it doesn't break anything. As of today
> we have mainline Linux + mainline dwc3 passing the entire USB3 Peripheral side
> certification, all tests including Link Layer, USB30CV, interop and whatnot, so
> I'm very reluctant to let any features in which are not properly tested.
[Yu:] Sure, we are also do these test both for USB3 and USB3 components. And for hibernation
Mode, we did lots of test and also test with system power state(S0ix and Sx). Actually, our
driver base on K3.4 before. We are porting them to K3.10 now. And then consider to design our
codes with common architecture for upstream. After port done and stable with 2.50a RTL, we
will prepare our patches to review by community.
Thanks a lot for your information. I learned lots of thing from you. :)
>
> cheers
>
> [1] http://elinux.org/Netiquette
> [2]
> http://git.kernel.org/cgit/linux/kernel/git/balbi/usb.git/commit/?h=next&id=63
> 6e2a2caeafcb1b55b6799b29fe436039819eb9
>
> --
> balbi
--
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
