Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759599AbdDSFPv (ORCPT ); Wed, 19 Apr 2017 01:15:51 -0400 Received: from mail-qk0-f177.google.com ([209.85.220.177]:36706 "EHLO mail-qk0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759568AbdDSFPr (ORCPT ); Wed, 19 Apr 2017 01:15:47 -0400 MIME-Version: 1.0 In-Reply-To: <67e21e78-2f4e-47eb-38f5-734fe13dccab@rock-chips.com> References: <1492492659-19266-1-git-send-email-william.wu@rock-chips.com> <67e21e78-2f4e-47eb-38f5-734fe13dccab@rock-chips.com> From: Guenter Roeck Date: Tue, 18 Apr 2017 22:15:46 -0700 Message-ID: Subject: Re: [PATCH v2] usb: dwc3: add disable u2mac linestate check quirk To: wlf Cc: William Wu , Felipe Balbi , johnyoun@synopsys.com, gregkh@linuxfoundation.org, =?UTF-8?Q?Heiko_St=C3=BCbner?= , linux-kernel , linux-usb@vger.kernel.org, "open list:ARM/Rockchip SoC..." , devicetree@vger.kernel.org, Rob Herring , Frank Wang , Tao Huang , Doug Anderson , Brian Norris , daniel.meng@rock-chips.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v3J5FuA5019422 Content-Length: 6982 Lines: 181 On Tue, Apr 18, 2017 at 8:59 PM, wlf wrote: > Dear Guenter, > > > > 在 2017年04月18日 21:18, Guenter Roeck 写道: >> >> On Mon, Apr 17, 2017 at 10:17 PM, William Wu >> wrote: >>> >>> This patch adds a quirk to disable USB 2.0 MAC linestate check >>> during HS transmit. Refer the dwc3 databook, we can use it for >>> some special platforms if the linestate not reflect the expected >>> line state(J) during transmission. >>> >>> When use this quirk, the controller implements a fixed 40-bit >>> TxEndDelay after the packet is given on UTMI and ignores the >>> linestate during the transmit of a token (during token-to-token >>> and token-to-data IPGAP). >>> >>> On some rockchip platforms (e.g. rk3399), it requires to disable >>> the u2mac linestate check to decrease the SSPLIT token to SETUP >>> token inter-packet delay from 566ns to 466ns, and fix the issue >>> that FS/LS devices not recognized if inserted through USB 3.0 HUB. >>> >>> Signed-off-by: William Wu >>> --- >>> Changes in v2: >>> - fix coding style >>> >>> Documentation/devicetree/bindings/usb/dwc3.txt | 2 ++ >>> drivers/usb/dwc3/core.c | 14 ++++++++++---- >>> drivers/usb/dwc3/core.h | 4 ++++ >>> 3 files changed, 16 insertions(+), 4 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt >>> b/Documentation/devicetree/bindings/usb/dwc3.txt >>> index f658f39..6a89f0c 100644 >>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt >>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt >>> @@ -45,6 +45,8 @@ Optional properties: >>> a free-running PHY clock. >>> - snps,dis-del-phy-power-chg-quirk: when set core will change PHY >>> power >>> from P0 to P1/P2/P3 without delay. >>> + - snps,tx-ipgap-linecheck-dis-quirk: when set, disable u2mac linestate >>> check >>> + during HS transmit. >> >> All other disable-something quirks are named >> "snps,dis-something-quirk". Maybe use the same naming convention ? > > Yes, good idea！ I will fix it with "snps,dis-tx-ipgap-linecheck-quirk" in > next patch verison. > Thanks:-) >> >> >>> - snps,is-utmi-l1-suspend: true when DWC3 asserts output signal >>> utmi_l1_suspend_n, false when asserts >>> utmi_sleep_n >>> - snps,hird-threshold: HIRD threshold >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>> index 455d89a..03429c5 100644 >>> --- a/drivers/usb/dwc3/core.c >>> +++ b/drivers/usb/dwc3/core.c >>> @@ -796,15 +796,19 @@ static int dwc3_core_init(struct dwc3 *dwc) >>> dwc3_writel(dwc->regs, DWC3_GUCTL2, reg); >>> } >>> >>> + reg = dwc3_readl(dwc->regs, DWC3_GUCTL1); >>> + >> >> My understanding is that the register was only introduced with dwc3 >> revision 2.50a. Is it ok to read and write it unconditionally ? > > Yes, refer to dwc3 databook, the DWC3_GUCTL1 was introduced since 2.50a. > Maybe it's better > to read and write it only when we know our controller version. > > Is it good to fix it like the following patch? > But this patch has a problem that we need to read and write the register > twice if our controller verison > = 2.90a, and need this quirk. > > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -806,6 +806,12 @@ static int dwc3_core_init(struct dwc3 *dwc) > dwc3_writel(dwc->regs, DWC3_GUCTL1, reg); > } > > + if (dwc->dis_tx_ipgap_linecheck_quirk) { > + reg = dwc3_readl(dwc->regs, DWC3_GUCTL1); > + reg |= DWC3_GUCTL1_TX_IPGAP_LINECHECK_DIS; > + dwc3_writel(dwc->regs, DWC3_GUCTL1, reg); > + } > + > How about this ? if (dwc->revision >= DWC3_REVISION_250A) { reg = dwc3_readl(dwc->regs, DWC3_GUCTL1); if (dwc->revision >= DWC3_REVISION_290A) reg |= DWC3_GUCTL1_DEV_L1_EXIT_BY_HW; if (dwc->dis_tx_ipgap_linecheck_quirk) reg |= DWC3_GUCTL1_TX_IPGAP_LINECHECK_DIS; dwc3_writel(dwc->regs, DWC3_GUCTL1, reg); } Thanks, Guenter > Hi John & Felipe, > Could you provide me some suggestion？ > Thank you！ > >>> /* >>> * Enable hardware control of sending remote wakeup in HS when >>> * the device is in the L1 state. >>> */ >>> - if (dwc->revision >= DWC3_REVISION_290A) { >>> - reg = dwc3_readl(dwc->regs, DWC3_GUCTL1); >>> + if (dwc->revision >= DWC3_REVISION_290A) >>> reg |= DWC3_GUCTL1_DEV_L1_EXIT_BY_HW; >>> - dwc3_writel(dwc->regs, DWC3_GUCTL1, reg); >>> - } >>> + >>> + if (dwc->tx_ipgap_linecheck_dis_quirk) >>> + reg |= DWC3_GUCTL1_TX_IPGAP_LINECHECK_DIS; >>> + >>> + dwc3_writel(dwc->regs, DWC3_GUCTL1, reg); >>> >>> return 0; >>> >>> @@ -1023,6 +1027,8 @@ static void dwc3_get_properties(struct dwc3 *dwc) >>> "snps,dis-u2-freeclk-exists-quirk"); >>> dwc->dis_del_phy_power_chg_quirk = >>> device_property_read_bool(dev, >>> "snps,dis-del-phy-power-chg-quirk"); >>> + dwc->tx_ipgap_linecheck_dis_quirk = >>> device_property_read_bool(dev, >>> + "snps,tx-ipgap-linecheck-dis-quirk"); >>> >>> dwc->tx_de_emphasis_quirk = device_property_read_bool(dev, >>> "snps,tx_de_emphasis_quirk"); >>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h >>> index 981c77f..3c2537b 100644 >>> --- a/drivers/usb/dwc3/core.h >>> +++ b/drivers/usb/dwc3/core.h >>> @@ -204,6 +204,7 @@ >>> #define DWC3_GCTL_DSBLCLKGTNG BIT(0) >>> >>> /* Global User Control 1 Register */ >>> +#define DWC3_GUCTL1_TX_IPGAP_LINECHECK_DIS BIT(28) >>> #define DWC3_GUCTL1_DEV_L1_EXIT_BY_HW BIT(24) >>> >>> /* Global USB2 PHY Configuration Register */ >>> @@ -850,6 +851,8 @@ struct dwc3_scratchpad_array { >>> * provide a free-running PHY clock. >>> * @dis_del_phy_power_chg_quirk: set if we disable delay phy power >>> * change quirk. >>> + * @tx_ipgap_linecheck_dis_quirk: set if we disable u2mac linestate >>> + * check during HS transmit. >>> * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk >>> * @tx_de_emphasis: Tx de-emphasis value >>> * 0 - -6dB de-emphasis >>> @@ -1004,6 +1007,7 @@ struct dwc3 { >>> unsigned dis_rxdet_inp3_quirk:1; >>> unsigned dis_u2_freeclk_exists_quirk:1; >>> unsigned dis_del_phy_power_chg_quirk:1; >>> + unsigned tx_ipgap_linecheck_dis_quirk:1; >>> >>> unsigned tx_de_emphasis_quirk:1; >>> unsigned tx_de_emphasis:2; >>> -- >>> 2.0.0 >>> >>> >> >> > >