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]

 



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.

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

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

If you want, you can definitely defer a v2 until v3.18-rc1 is tagged.
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.

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

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