Re: OTG2.0 support for SNPS DWC3 core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> 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.

> 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 */
}

> 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.

> 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 ;-)

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux