Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762619AbdDSLcV (ORCPT ); Wed, 19 Apr 2017 07:32:21 -0400 Received: from regular1.263xmail.com ([211.150.99.134]:55372 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761988AbdDSLcS (ORCPT ); Wed, 19 Apr 2017 07:32:18 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-RL-SENDER: wulf@rock-chips.com X-FST-TO: daniel.meng@rock-chips.com X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: wulf@rock-chips.com X-UNIQUE-TAG: <87e7f89a2a1dbf6bef8aa74b4f9cc641> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Subject: Re: [PATCH v2] usb: dwc3: add disable u2mac linestate check quirk To: Guenter Roeck References: <1492492659-19266-1-git-send-email-william.wu@rock-chips.com> <67e21e78-2f4e-47eb-38f5-734fe13dccab@rock-chips.com> 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 From: wlf Message-ID: <8e9e559b-7344-8d29-bd54-f2757325cfb0@rock-chips.com> Date: Wed, 19 Apr 2017 19:32:06 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7317 Lines: 178 Dear Guenter， 在 2017年04月19日 13:15, Guenter Roeck 写道: > 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 Thanks， looks good to me. I will fix it in patch v2. > >> 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 >>>> >>>> >>> >> > >