Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL

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

 



On Sun, Sep 28, 2014 at 06:41:39PM -0500, Felipe Balbi wrote:
> Hi,
> 
> On Sun, Sep 28, 2014 at 11:11:23AM +0800, Huang Rui wrote:
> > On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> > > On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > > > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > index b0f4d52..6138c5d 100644
> > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > > > >  }
> > > > > >  
> > > > > >  /**
> > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > + */
> > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > +{
> > > > > > +	u32 reg = 0;
> > > > > > +
> > > > > > +	reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > +		| DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > +		| DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > 
> > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > > > erratum before I can accept it. You have also duplicated the bit in this
> > > > > initialization.
> > > > > 
> > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > > > 
> > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > > > only be proven to work with this bit after going through full USB
> > > > > certification.
> > > > > 
> > > > 
> > > > What do you mean of the faulty PHY?
> > > > We would use AMD PHY to talk with DWC3 controller.
> > > 
> > > Look at the description of each of those bits and you'll see it mentions
> > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> > > example:
> > > 
> > > 	"
> > > 	This bit is added for SS PHY workaround where SS PHY ...
> > > 	"
> > > 
> > 
> > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > the standards someplace.
> > 
> > For simulation board, we used vendor provided PHY. But on SOC, we
> > will use AMD PHY. I will re-check again to confirm which workarounds
> > need on simulation board and which workarounds need on SOC.
> 
> Thanks, that's great. I wonder if there's a way to detect that we're
> running on FPGA. Can you check with your HW designers ? I'll go dig on
> Synopsys databook myself too on Monday.
> 

I checked with HW designers, they can update the origin value of GUID
register of FPGA to add ASCII codes as prefix of "fpga". I can checked
it before driver re-writes it as kernel version (I see you latest
testing branch have this behavior).

> > > It's alright that AMD PHY needs this bit, but then, let's get
> > > confirmation from IP/SoC/SilVal team and add a proper comment stating
> > > why we need them. This is so we don't forget that $version of AMD's PHY
> > > needs workarounds for A, B, and C silicon errata.
> > > 
> > 
> > Yes, but currently, I needn't write AMD own phy driver. There isn't
> > any requirement from HW side to program the phy register. So I used
> > NOP USB transceiver driver till now. 
> 
> NOP is a perfectly valid use-case :-) We still want to know that version
> x of AMD's PHY is quirky on these or that case :-)
> 

I can use the SMBus revsion ID for different chips version of amd. You
can refer usb/host/pci-quirks.c, I already added the different chip
version macros there for xHC. If PHY version updates, the chip version
must update too.

> > > Also, I'd have to ask you to provide full boot logs of your platform
> > > booting with my testing/next (where all the latest patches are) plus
> > > your patches. 
> > 
> > I will provide the boot logs to you. Actually, I already did the
> > gadget mass storage and gadget zero testing with my patches under 3.14
> > before.
> 
> v3.14 is rather old, if you're sending patches upstream. I'd rather see
> patches tested with either next or latest Linus' tag. My testing/next
> branch is also usually fine too.
> 

Yes, I am doing these testing.

An issue meet your msc.c.

ray@hr-slim:~/felipe/usb-tools$ gcc -Wall -O2 -D_GNU_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -g -o msc msc.c
/tmp/ccUBcDC4.o: In function `do_write':
/home/ray/felipe/usb-tools/msc.c:275: undefined reference to `clock_gettime'
/home/ray/felipe/usb-tools/msc.c:308: undefined reference to `clock_gettime'
/tmp/ccUBcDC4.o: In function `do_read':
/home/ray/felipe/usb-tools/msc.c:332: undefined reference to `clock_gettime'
/home/ray/felipe/usb-tools/msc.c:349: undefined reference to `clock_gettime'
/tmp/ccUBcDC4.o: In function `do_writev':
/home/ray/felipe/usb-tools/msc.c:401: undefined reference to `clock_gettime'
/tmp/ccUBcDC4.o:/home/ray/felipe/usb-tools/msc.c:407: more undefined references to `clock_gettime' follow
collect2: ld returned 1 exit status

Any idea?

> If you want, you can definitely defer a v2 until v3.18-rc1 is tagged.

Do you mean: when kernel upgrade to v3.18-rc1, then I rebase my
patches to testing branch (v3.18-rc1+) to send V2, is that right?

> I'll send a few fixes I have pending when that happens too. All my
> latest fixes are on my testing/next branch, btw. I'll add Cc: stable to
> most of them, but you might want to cherry-pick a few that I don't to
> your vendor tree, if you have it.
> 

That's great. I expect to port to some LTS and late stable kernels
such as 3.10, 3.12, 3.14 and etc.

Thanks,
Rui

> > > Then, load g_mass_storage with a backing storage of your
> > > choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
> > > post the logs for that too. Last, but not least, please USB30CV (chapter
> > > 9 and Link Layer test, at least) just so we know there isn't anything
> > > new with your version of the core, which I suppose is 2.80a, based on
> > > the LPM Errata bits.
> > > 
> > 
> > OK, will post the logs to you.
> 
> thanks :-)
> 
> > > This is just because I don't have access to the HW myself, so I can't
> > > verify your patches. One thing I can tell you, with my testing/next,
> > > dwc3 is really stable. I have every test passing except for Halt
> > > Endpoint which I'm debugging right now.
> > > 
> > 
> > OK, I got it. Will rebase my patches to testing/next.
> 
> that'll help me, thanks
> 
> > Felipe, it's pleasure to leverage your dwc3 ip driver on AMD platform
> > for me.  Thanks to support. :)
> 
> hey, don't mention it. I'm happy to have several users on a single
> driver. Everybody can benefit from fixes ;-)
> 
> -- 
> 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




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

  Powered by Linux