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. > > 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. > > 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 checks.
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.
>
> Cc: <sta...@vger.kernel.org> # 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 <johny...@synopsys.com>
> ---
> drivers/usb/dwc3/core.c | 9 ++++++--
> drivers/usb/dwc3/core.h | 56
> +++++++++++++++++++++++++++++++------------------
> 2 files changed, 43 insertions(+), 22 deletions(-)
>
> 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)
>
> reg = dwc3_readl(dwc->regs, DWC3_GSNPSID);
> /* This should read as U3 followed by revision number */
> - if ((reg & DWC3_GSNPSID_MASK) != 0x55330000) {
> + if ((reg & DWC3_GSNPSID_MASK) == 0x55330000) {
> + /* Detected DWC_usb3 IP */
> + dwc->revision = reg & DWC3_GSNPSREV_MASK;
leave the mask out of it and ...
> + } else if ((reg & DWC3_GSNPSID_MASK) == 0x33310000) {
> + /* Detected DWC_usb31 IP */
> + dwc->revision = dwc3_readl(dwc->regs, DWC3_VER_NUMBER);
set a dwc->is_dwc31 = true flag here.
> + } else {
> dev_err(dwc->dev, "this is not a DesignWare USB3 DRD Core\n");
> ret = -ENODEV;
> goto err0;
> }
> - dwc->revision = reg;
>
> /*
> * 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
>
> +#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;
>
> -#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 ?
--
balbi
signature.asc
Description: PGP signature
