From patchwork Sat Sep 27 04:30:44 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Felipe Balbi X-Patchwork-Id: 38037 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-la0-f69.google.com (mail-la0-f69.google.com [209.85.215.69]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id A6170202DB for ; Sat, 27 Sep 2014 04:31:11 +0000 (UTC) Received: by mail-la0-f69.google.com with SMTP id q1sf2436673lam.0 for ; Fri, 26 Sep 2014 21:31:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:cc:subject:message-id :reply-to:references:mime-version:content-type:content-disposition :in-reply-to:user-agent:sender:precedence:list-id:x-original-sender :x-original-authentication-results:mailing-list:list-post:list-help :list-archive:list-unsubscribe; bh=RsubsjvVxY0xpCcmFohvTpm5b+7nr8fd0j4EgwGp+c8=; b=MGJlO7LYOK91fR/WBUGmwgnylusfGDUe91wy7O4s8Sx7dk8wVBV5m+gZQZYdb9CCbb t7Wp57sb/EfZr+q3wzJDbA7iN+ZhLeQBaVNfAZzILNd3YvdDhfU70Cdkz63cRY1CvnzZ tUBNddrTM2cLVIJZ9pwIaXyr1LrTM7JwIO6ajNMDsHVCMebAo7sdRm+Wu8gNJlu4kKKi +SBqCmLPohLwfMQ3WPFYFHwY6e3QvMDOG8NQM1BAvGZ6M9SodVpP/ydbDJuzWIR+tdZT A1t5QMEKqhg/aeg0VU8C7tgpH9bdbJk88fWGqLEWMm0kiRmFp7UZGFbHjEPtIylQVxpx X2yA== X-Gm-Message-State: ALoCoQmfjPRtpF82393GW2faPenL1kURWwx8v1mehIjAjuXQvd+E1Nr2GgJtOnpjCUmyXyGr6YOc X-Received: by 10.181.8.226 with SMTP id dn2mr4173125wid.4.1411792270442; Fri, 26 Sep 2014 21:31:10 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.88.8 with SMTP id bc8ls418746lab.86.gmail; Fri, 26 Sep 2014 21:31:10 -0700 (PDT) X-Received: by 10.112.235.199 with SMTP id uo7mr23520408lbc.50.1411792270188; Fri, 26 Sep 2014 21:31:10 -0700 (PDT) Received: from mail-lb0-f182.google.com (mail-lb0-f182.google.com [209.85.217.182]) by mx.google.com with ESMTPS id t13si9604066lal.69.2014.09.26.21.31.09 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Fri, 26 Sep 2014 21:31:09 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) client-ip=209.85.217.182; Received: by mail-lb0-f182.google.com with SMTP id z11so3655171lbi.41 for ; Fri, 26 Sep 2014 21:31:09 -0700 (PDT) X-Received: by 10.112.55.7 with SMTP id n7mr23531085lbp.16.1411792269873; Fri, 26 Sep 2014 21:31:09 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.130.169 with SMTP id of9csp164730lbb; Fri, 26 Sep 2014 21:31:08 -0700 (PDT) X-Received: by 10.70.130.228 with SMTP id oh4mr45486879pdb.54.1411792267129; Fri, 26 Sep 2014 21:31:07 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id pk5si12242877pdb.182.2014.09.26.21.31.06 for ; Fri, 26 Sep 2014 21:31:07 -0700 (PDT) Received-SPF: none (google.com: linux-usb-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751156AbaI0EbE (ORCPT + 3 others); Sat, 27 Sep 2014 00:31:04 -0400 Received: from comal.ext.ti.com ([198.47.26.152]:35153 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750781AbaI0EbC (ORCPT ); Sat, 27 Sep 2014 00:31:02 -0400 Received: from dflxv15.itg.ti.com ([128.247.5.124]) by comal.ext.ti.com (8.13.7/8.13.7) with ESMTP id s8R4UmoO022385; Fri, 26 Sep 2014 23:30:48 -0500 Received: from DLEE71.ent.ti.com (dlee71.ent.ti.com [157.170.170.114]) by dflxv15.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8R4UmWa031614; Fri, 26 Sep 2014 23:30:48 -0500 Received: from dlep33.itg.ti.com (157.170.170.75) by DLEE71.ent.ti.com (157.170.170.114) with Microsoft SMTP Server id 14.3.174.1; Fri, 26 Sep 2014 23:30:48 -0500 Received: from localhost (ileax41-snat.itg.ti.com [10.172.224.153]) by dlep33.itg.ti.com (8.14.3/8.13.8) with ESMTP id s8R4UlUY006493; Fri, 26 Sep 2014 23:30:48 -0500 Date: Fri, 26 Sep 2014 23:30:44 -0500 From: Felipe Balbi To: Paul Zimmerman CC: "balbi@ti.com" , Huang Rui , Alan Stern , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" Subject: Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL Message-ID: <20140927043044.GA28474@saruman> Reply-To: References: <1411629707-7131-1-git-send-email-ray.huang@amd.com> <1411629707-7131-4-git-send-email-ray.huang@amd.com> <20140925145032.GE28045@saruman> <20140926214004.GA26533@saruman> <20140927005347.GA25432@saruman> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-usb-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: balbi@ti.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.182 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi, On Sat, Sep 27, 2014 at 01:05:46AM +0000, Paul Zimmerman wrote: > > From: Felipe Balbi [mailto:balbi@ti.com] > > Sent: Friday, September 26, 2014 5:54 PM > > > > On Fri, Sep 26, 2014 at 11:18:48PM +0000, Paul Zimmerman wrote: > > > > From: Felipe Balbi [mailto:balbi@ti.com] > > > > Sent: Friday, September 26, 2014 2:40 PM > > > > > > > > On Fri, Sep 26, 2014 at 08:57:19PM +0000, Paul Zimmerman wrote: > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > > > > > > index 0fcc0a3..8277065 100644 > > > > > > > --- a/drivers/usb/dwc3/gadget.c > > > > > > > +++ b/drivers/usb/dwc3/gadget.c > > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc) > > > > > > > */ > > > > > > > int dwc3_gadget_init(struct dwc3 *dwc) > > > > > > > { > > > > > > > + u32 reg; > > > > > > > int ret; > > > > > > > > > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req), > > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc) > > > > > > > if (ret) > > > > > > > goto err4; > > > > > > > > > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) { > > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL); > > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf); > > > > > > > > > > > > weird, why would Synopsys put this here ? It seems like this is only > > > > > > useful when LPM Errata is enabled and that's, apparently, a host-side > > > > > > thing. > > > > > > > > > > > > Paul, can you comment ? > > > > > > > > > > These bits contribute to how the device responds to an LPM transaction > > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above which > > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So > > > > > it's definitely a device-side thing, but only if the core is configured > > > > > with LPM Errata support enabled. > > > > > > > > right, and how can SW detect if LPM Errata was enabled ? From the host > > > > point of view, we can check bit 20 of xHCI capability register. What > > > > about device ? I can't seem to find anything :-s > > > > > > I just talked to one of our RTL designers. You're right, there is no > > > way to tell from the Device registers alone whether the controller is > > > configured with LPM Errata support or not. > > > > > > You can tell for sure if it is *not* enabled, by checking GSNPSID, and > > > if the version is earlier than 2.40a, the feature wasn't available. > > > > alright, we can use this for something like: > > > > WARN(rev < 2.40a && (flags & LPM_ERRATA || of_property_read_bool("lpm-errata")), > > "LPM Errata not available on dwc3 revisions <= 2.40a\n"); > > > > > So for Device-mode only controllers, I guess you will need a DT property > > > for this. > > > > right, DT property and platform_data feature flag, or something. I don't > > wanna call it a quirk though, although it _is_ an erratum WRT LPM > > support. Dunno. Any strong feelings ? > > Well, it's called LPM Errata because the feature was added to the USB > spec as an erratum. It's not an erratum to our controller. But I don't > have any strong feelings about how the driver support is implemented. how about adding a "has_lpm_erratum" to struct dwc3 which gets initialized either through pdata or DT ? Then above WARN() would become: WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum", "LPM Erratum not available on dwc3 revisisions < 2.40a\n"); Then we're not really calling it a quirk. In fact Huang, when respining your series, let's convert your quirk bits into single-bit fields inside struct dwc3 and struct platform_data. Like so: note that I have also moved DCTL access from gadget_init to gadget_start. cheers diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 4d4e854..e233ce1 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev) if (node) { dwc->maximum_speed = of_usb_get_maximum_speed(node); + dwc->has_lpm_erratum = of_property_read_bool(node, "snps,has-lpm-erratum"); dwc->needs_fifo_resize = of_property_read_bool(node, "tx-fifo-resize"); dwc->dr_mode = of_usb_get_dr_mode(node); } else if (pdata) { dwc->maximum_speed = pdata->maximum_speed; + dwc->has_lpm_erratum = pdata->has_lpm_erratum; dwc->needs_fifo_resize = pdata->tx_fifo_resize; dwc->dr_mode = pdata->dr_mode; diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index 66f6256..23e1504 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -661,6 +661,8 @@ struct dwc3_scratchpad_array { * @ep0_bounced: true when we used bounce buffer * @ep0_expect_in: true when we expect a DATA IN transfer * @has_hibernation: true when dwc3 was configured with Hibernation + * @has_lpm_erratum: true when core was configured with LPM Erratum. Note that + * there's now way for software to detect this in runtime. * @is_selfpowered: true when we are selfpowered * @needs_fifo_resize: not all users might want fifo resizing, flag it * @pullups_connected: true when Run/Stop bit is set @@ -764,6 +766,7 @@ struct dwc3 { unsigned ep0_bounced:1; unsigned ep0_expect_in:1; unsigned has_hibernation:1; + unsigned has_lpm_erratum:1; unsigned is_selfpowered:1; unsigned needs_fifo_resize:1; unsigned pullups_connected:1; diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index 68497b3..112352e 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -1577,6 +1577,13 @@ static int dwc3_gadget_start(struct usb_gadget *g, } dwc3_writel(dwc->regs, DWC3_DCFG, reg); + if (dwc->has_lpm_erratum) { + reg = dwc3_readl(dwc->regs, DWC3_DCTL); + /* REVISIT should this be configurable ? */ + reg |= DWC3_DCTL_LPM_ERRATA(0xf); + dwc3_writel(dwc->regs, DWC3_DCTL, reg); + } + dwc->start_config_issued = false; /* Start with SuperSpeed Default */ diff --git a/drivers/usb/dwc3/platform_data.h b/drivers/usb/dwc3/platform_data.h index 7db34f0..4bcec7c 100644 --- a/drivers/usb/dwc3/platform_data.h +++ b/drivers/usb/dwc3/platform_data.h @@ -24,4 +24,6 @@ struct dwc3_platform_data { enum usb_device_speed maximum_speed; enum usb_dr_mode dr_mode; bool tx_fifo_resize; + + unsigned has_lpm_erratum:1; };