Hi, > Hi, > > On Mon, Jan 09, 2012 at 05:50:41AM -0800, Ido Shayevitz wrote: >> > On Mon, Jan 02, 2012 at 08:29:07AM -0800, Ido Shayevitz wrote: >> >> > On Sun, Jan 01, 2012 at 12:44:59PM -0800, idos@xxxxxxxxxxxxxx >> wrote: >> >> >> I wonder if there is a work being done for OTG2.0 driver support >> over >> >> >> the Synopsys dwc3 core? If so, which git/branch contain this >> driver? >> >> > >> >> > Sure there is :-) But in order to get there we must first stabilize >> >> the >> >> > host side and that we haven't achieved yet. >> >> > >> >> > Still, the newest code for this driver lies in my master branch. >> Just >> >> > grab a hold of that one. >> >> > >> >> > -- >> >> > balbi >> >> > >> >> >> >> Thanks Felipe, >> >> >> >> I see that in the dwc3_probe also xhci host device is constructed >> with >> >> the >> >> same irq and address base(of dwc3 core), but I couldn't see any >> > >> > true, but we still have some pending patches on the xHCI stack to get >> > the whole thing working out of the box with mainline kernel. >> > Unfortunately we won't have that on 3.3 merge window. But on 3.4 it's >> > likely that it will be there. >> > >> >> configuration done to the OTG registers nor a read of OSTS.ConIDSts >> for >> >> determining A or B device. >> > >> > That OTG address space isn't implemented by all cores. So you need to >> > find a way to implement that address space without breaking other >> users. >> > >> > The best way would be to check if GHWPARAMS6[10] is 1. In that case >> you >> > will have SRP enabled, which means DWC_USB3_EN_OTG is also enabled. If >> > that's true, then you can access the OTG address space. >> > >> >> I guess that the otg driver should implement the programming model >> >> starting from figure 12-4 in the dwc3 controller spec. >> > >> > Indeed. See that you _must_ be sure that GHWPARAMS6 says you support >> > OTG. Otherwise this will not work for everybody. >> > >> >> If this implementation exists, can you give me a pointer, if not, >> what >> >> are the plans? >> > >> > We _do_ have plans, but not before mid-2012. Patches are welcome of >> > course. >> > >> > -- >> > balbi >> > >> >> Hi Felipe, all, >> >> I was thinking on some kind of architecture to the dwc3 driver that will >> support also otg and use this opportunity, which the dwc3 driver is >> still >> clean and neat, to keep it that way and let us better maintenance in the >> future. >> >> First of all, I noticed that Heikki already sent a patch on separating >> the >> otg_transceiver struct to usb_otg and usb_phy. >> IMO this is important if we want to create neat otg drivers, that will >> implement only otg logic, as I want to do now. >> http://marc.info/?l=linux-usb&m=132342667512493&w=2 >> >> I currently see the following as the dwc3 driver design: >> >> 1. SoC unique logic is implemented at the level of dwc3-xxx, as for >> example dwc3-omap. > > that has always been the case. The whole idea of creating those dwc3-*.c > parent drivers/devices is exactly the hide platform-specific details and > keep the core IP driver "pure" and reusable. > >> All platform specific initializations, logic that is related to the HW >> core which wrap the dwc3 core in some specific SoC solution is >> implemented >> in this layer. > > yeah, but let's not get carried away. dwc3-omap.ko will only have code > for the particular wrapper around the DWC3 core and it will not access > any of DWC3's address space directly, or any other address which doesn't > belong to the core. I will be EXTREMELY pedantic about it. > Of course, that what I meant :) >> This operates as a standalone kernel module which will activate the >> core.c >> kernel module by adding the appropriate platform device. > > this is how me and Sebastian designed it. > >> 2. core.c implements all logic done against the global register space of >> the dwc3 IP core. This kernel module also responsible for common tasks >> that are in use of the dwc3 different modes (peripheral/host/drd/otg) >> such >> as allocating the event buffers. This layer activates the appropriate >> software stack according to the DWC3_MODE (gadget/host) > > This is how it is designed. > >> 3. gadget.c (and ep0.c) implement the gadget/peripheral mode and all >> logic >> done against the device register space of the dwc3 IP core. This is not >> a >> kernel module of its own, but as part of core.c kernel module. > > no changes there. > >> 4. xhci host: using "standard" xhci driver kernel module. > > no changes there. > >> I was thinking about adding new otg.c driver under the dwc3 directory, >> which will implement only otg related logic (no phy/transceiver logic) >> as >> explained in chapter 12.1 in Synopsys spec. This driver will be >> activated >> ("probed") by core.c when the DWC3_MODE is DRD. In Felipe's master >> branch, > > not only mode is DRD, that's not enough to check. Like I said before, > you need to check one of the GHWPARAMS registers to see if SRP is > supported. Only then you know this version of the IP is using the > internal OTG facilities created by Synopsys. You also need to check if > HNP is supported and flag that accordingly, so you can tell standard > host you support HNP later. > Understood. >> the core.c activates the gadget and host in case the DWC3_MODE is DRD. >> Instead I suggest that the core.c will activate the new otg.c driver >> which >> will implement the programming model as started in figure 12-4 in >> Synopsys >> spec. >> After the activation of the otg.c, it will check the >> GHWPARAMS6[10](=SRPSupport), for determining if OTG register space is >> available (as also illustrated in the figure 12-4) and if the otg >> interrupt handler can be settled, otg registers can be initialized, >> etc... > > Well, it would look something like: > > switch (mode) { > case DWC3_MODE_DRD: > ret = dwc3_otg_init(dwc); > /* error check */ > > ret = dwc3_device_init(dwc); > /* error check */ > > ret = dwc3_host_init(dwc); > /* error check */ > break; > ... > } > > > dwc3_otg_init would look like: > > int dwc3_otg_init(struct dwc3 *dwc) > { > u32 reg; > > reg = dwc3_readl(dwc->regs, DWC3_GHWPARAMS6); > > if (!(reg & BIT(10))) > return -ENODEV; > > /* initialize OTG here */ > } > Let's say that we have SRPSupport (means OTG_EN=1). Therefore wouldn't we like to activate the device or host according to the ID pin? I am saying that in the above code, the call to dwc3_otg_init will generate the calls to dwc3_device_init _OR_ dwc3_host_init in case that the SRPSupport is on and according to the ID pin. But later, returning to core probe, it will call again to the dwc3_device_init and dwc3_host_init. So one of the drivers will be initlized twice... Did I misunderstood you? Maybe something like this: switch (mode) { case DWC3_MODE_DRD: ret = dwc3_otg_init(dwc); /* error check */ break; case DWC3_MODE_DEVICE: ret = dwc3_device_init(dwc); /* error check */ break; case DWC3_MODE_HOST: ret = dwc3_host_init(dwc); /* error check */ break; ... } dwc3_otg_init would look like: int dwc3_otg_init(struct dwc3 *dwc) { u32 reg; reg = dwc3_readl(dwc->regs, DWC3_GHWPARAMS6); if (!(reg & BIT(10))) { /* No OTG support, just activating device+host drivers*/ ret = dwc3_device_init(dwc); /* error check */ ret = dwc3_host_init(dwc); /* error check */ return -ENODEV; } /* initialize OTG here and init host/device according to ID pin*/ } >> As for the phy logic, I am not sure where it's goes in this design, I >> tried to follow the "OTG rework" loop and I understand that this in >> check >> by Kishon? > > Yes, we will have something to show soonish, right now we are really, > really busy with some internal stuff. > OK, I just wanted to mention that I am not sure about this. >> Please correct or comment me on the above. >> If it is ok with you, I think I will be able to supply some initial >> patch to review in a few days. > > That's ok, but before sending anything be sure to run checkpatch.pl and > sparse on each and every single patch. Also make sure to compile on your > architecture and on x86. Do *NOT* introduce any compile warnings/errors > to that driver and make sure to follow CodingStyle correctly. Take a > look at SubmittingPatches and SubmitChecklist and I need to know how you > tested the patches. It should be ok to add a note about testing on your > series' cover letter. > > (for ease compile testing: > > $ make allmodconfig > $ make -j8 drivers/usb > /dev/null > > $ make ARCH=arm my_device_defconfig > $ make ARCH=arm menuconfig > (enable everything USB as modules and I really mean everything) > $ make ARCH=arm =j8 drivers/usb > /dev/null) > > If these aren't followed, I will go into rant mode hehe ;-) > Cool, no problem, I will ask you if I get trouble with something. > -- > balbi > Thanks, Ido -- Consultant for Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum -- 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
