From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from comal.ext.ti.com ([198.47.26.152]:56492 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750868AbbJBCD6 (ORCPT ); Thu, 1 Oct 2015 22:03:58 -0400 Date: Thu, 1 Oct 2015 21:03:55 -0500 From: Felipe Balbi To: John Youn CC: , , , Subject: Re: [PATCH 1/6] usb: dwc3: Support Synopsys USB 3.1 IP Message-ID: <20151002020355.GB2534@saruman.tx.rr.com> Reply-To: References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="oC1+HKm2/end4ao3" Content-Disposition: inline In-Reply-To: Sender: stable-owner@vger.kernel.org List-ID: --oC1+HKm2/end4ao3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi, On Fri, Sep 04, 2015 at 07:15:10PM -0700, John Youn wrote: > This patch allows the dwc3 driver to run on the new Synopsys USB 3.1 > IP core, albeit in USB 3.0 mode only. >=20 > The Synopsys USB 3.1 IP (DWC_usb31) retains mostly the same register > interface and programming model as the existing USB 3.0 controller IP > (DWC_usb3). However, the underlying IP is different and the GSNPSID > and version numbers are different. >=20 > The DWC_usb31 version register is actually lower in value than the > full GSNPSID of the DWC_usb3 IP. So if we are on DWC_usb3 IP just > store the lower word of the GSNPSID instead of the full register. Then > adjust the revision constants to match. This will allow existing > revision checks to continue to work when running on the new IP. I would rather not touch those constants at all. We can have the driver set a is_dwc31 flag and use if !is_dwc31 && rev < XYZ for the revision chec= ks. It's more work, but it seems better IMO. > Finally add a documentation note about the revision numbering and > checking with regards to the old and new IP. Because these are > different IPs which will both continue to be supported, feature sets > and revisions checks may not sync-up across future versions. and this is why I prefer to have the flag :-) We can run different revision check methods depending if we're running on dwc3 or dwc31. > From now, any check based on a revision (for STARS, workarounds, and > new features) should take into consideration how it applies to both > the 3.1/3.0 IP and make the check accordingly. >=20 > Cc: # v3.18+ I really fail to how any of these patches in this series apply for stable. = Care to explain ? > Signed-off-by: John Youn > --- > drivers/usb/dwc3/core.c | 9 ++++++-- > drivers/usb/dwc3/core.h | 56 +++++++++++++++++++++++++++++++------------= ------ > 2 files changed, 43 insertions(+), 22 deletions(-) >=20 > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index c72c8c5..566cca1 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -534,12 +534,17 @@ static int dwc3_core_init(struct dwc3 *dwc) > =20 > reg =3D dwc3_readl(dwc->regs, DWC3_GSNPSID); > /* This should read as U3 followed by revision number */ > - if ((reg & DWC3_GSNPSID_MASK) !=3D 0x55330000) { > + if ((reg & DWC3_GSNPSID_MASK) =3D=3D 0x55330000) { > + /* Detected DWC_usb3 IP */ > + dwc->revision =3D reg & DWC3_GSNPSREV_MASK; leave the mask out of it and ... > + } else if ((reg & DWC3_GSNPSID_MASK) =3D=3D 0x33310000) { > + /* Detected DWC_usb31 IP */ > + dwc->revision =3D dwc3_readl(dwc->regs, DWC3_VER_NUMBER); set a dwc->is_dwc31 =3D true flag here. > + } else { > dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n"); > ret =3D -ENODEV; > goto err0; > } > - dwc->revision =3D reg; > =20 > /* > * Write Linux Version Code to our GUID register so it's easy to figure > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index 9188745..7446467 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -108,6 +108,9 @@ > #define DWC3_GPRTBIMAP_FS0 0xc188 > #define DWC3_GPRTBIMAP_FS1 0xc18c > =20 > +#define DWC3_VER_NUMBER 0xc1a0 > +#define DWC3_VER_TYPE 0xc1a4 what is this VER_TYPE register ? > @@ -661,7 +664,8 @@ struct dwc3_scratchpad_array { > * @num_event_buffers: calculated number of event buffers > * @u1u2: only used on revisions <1.83a for workaround > * @maximum_speed: maximum speed requested (mainly for testing purposes) > - * @revision: revision register contents > + * @revision: the core revision. the contents will depend on the whether > + * this is a usb3 or usb31 core. > * @dr_mode: requested mode of operation > * @usb2_phy: pointer to USB2 PHY > * @usb3_phy: pointer to USB3 PHY > @@ -771,27 +775,39 @@ struct dwc3 { > u32 num_event_buffers; > u32 u1u2; > u32 maximum_speed; > + > + /* > + * All 3.1 IP version constants are greater than the 3.0 IP > + * version constants. This works for most version checks in > + * dwc3. However, in the future, this may not apply as > + * features may be developed on newer versions of the 3.0 IP > + * that are not in the 3.1 IP. > + */ > u32 revision; > =20 > -#define DWC3_REVISION_173A 0x5533173a > -#define DWC3_REVISION_175A 0x5533175a > -#define DWC3_REVISION_180A 0x5533180a > -#define DWC3_REVISION_183A 0x5533183a > -#define DWC3_REVISION_185A 0x5533185a > -#define DWC3_REVISION_187A 0x5533187a > -#define DWC3_REVISION_188A 0x5533188a > -#define DWC3_REVISION_190A 0x5533190a > -#define DWC3_REVISION_194A 0x5533194a > -#define DWC3_REVISION_200A 0x5533200a > -#define DWC3_REVISION_202A 0x5533202a > -#define DWC3_REVISION_210A 0x5533210a > -#define DWC3_REVISION_220A 0x5533220a > -#define DWC3_REVISION_230A 0x5533230a > -#define DWC3_REVISION_240A 0x5533240a > -#define DWC3_REVISION_250A 0x5533250a > -#define DWC3_REVISION_260A 0x5533260a > -#define DWC3_REVISION_270A 0x5533270a > -#define DWC3_REVISION_280A 0x5533280a yeah, I'd rather not do all this. > +/* DWC_usb3 revisions */ > +#define DWC3_REVISION_173A 0x173a > +#define DWC3_REVISION_175A 0x175a > +#define DWC3_REVISION_180A 0x180a > +#define DWC3_REVISION_183A 0x183a > +#define DWC3_REVISION_185A 0x185a > +#define DWC3_REVISION_187A 0x187a > +#define DWC3_REVISION_188A 0x188a > +#define DWC3_REVISION_190A 0x190a > +#define DWC3_REVISION_194A 0x194a > +#define DWC3_REVISION_200A 0x200a > +#define DWC3_REVISION_202A 0x202a > +#define DWC3_REVISION_210A 0x210a > +#define DWC3_REVISION_220A 0x220a > +#define DWC3_REVISION_230A 0x230a > +#define DWC3_REVISION_240A 0x240a > +#define DWC3_REVISION_250A 0x250a > +#define DWC3_REVISION_260A 0x260a > +#define DWC3_REVISION_270A 0x270a > +#define DWC3_REVISION_280A 0x280a > + > +/* DWC_usb31 revisions */ > +#define DWC3_USB31_REVISION_110A 0x3131302a are you sure you tested this ? Above you check for 0x33310000 but here you = use 0x3131 ? What gives ? Also, it seems odd that revision 1.10a is actually 3.= 02a in HW, is this really correct ? --=20 balbi --oC1+HKm2/end4ao3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWDeYLAAoJEIaOsuA1yqREkVQP/RBGEDSkvjYGKPMCzH5ulkHW itwUH1GyYuxhdy9fDV8lEXwhfuDHNsaU0kketHaaY4iUoWI+m36IFR3ymgMTxDMZ N+IpjJwkPgWQMgQwK0JogFOPF7sLpytFBk+Pr87bjOoHGhtb/4L2mR8phT09sg8M DASbcG+H17xx7cys0TWDTq4PsH9Ggl452jigQSXQiUXnRf2Yh+BPaQMUsuzzgqb0 XQOLDmbfZ3u4CfHfxbwZEdcRf24ZWACQiSdJ2Mmz76qvmibKtiK4Pnw2z5KsIXZ2 nkpqCqbiT3AQvH0mr/vUp+CbiVtE1fgFxu01VMYlEuAxNzq9EvuJogAAMj5DnNCP bkT3R1SVgQrcjx4rbSFp4QLCuFowv5P2vfk8tQbWRjjAtEN7vfu/SOypOdsvTbIY GoQe4EGwcJqoLFWLFXQ/hdtryww6N69Kju/HOrKt0yw1GeOBwlSwxSmJZ8m5Fp1G WCuxtVt8ftJxoMIsAmgzWszBJmz7oKaiuZPBOX3kGW9kH6jRYK+r+nIZSsrxaZwy YSsFCxjkQmMZCw8VPhv8kbDA//+AMz99po4KD8YejQ+UtXkMESch7E60rUah/XUN oguq6FvFzkoPfjLO6K0mMt84N5QUbEBCtDxUz/iyBB8c5z5LPgDu3SkeS7k0ltdR 6cS7inxAJ1zxt0NDaJsD =senq -----END PGP SIGNATURE----- --oC1+HKm2/end4ao3--