| Message ID | 67e21e78-2f4e-47eb-38f5-734fe13dccab@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 18, 2017 at 8:59 PM, wlf <wulf@rock-chips.com> wrote: > Dear Guenter, > > > > 在 2017年04月18日 21:18, Guenter Roeck 写道: >> >> On Mon, Apr 17, 2017 at 10:17 PM, William Wu <william.wu@rock-chips.com> >> 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 <william.wu@rock-chips.com> >>> --- >>> 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 >>> >>> >> >> > >
Dear Guenter， 在 2017年04月19日 13:15, Guenter Roeck 写道: > On Tue, Apr 18, 2017 at 8:59 PM, wlf <wulf@rock-chips.com> wrote: >> Dear Guenter, >> >> >> >> 在 2017年04月18日 21:18, Guenter Roeck 写道: >>> On Mon, Apr 17, 2017 at 10:17 PM, William Wu <william.wu@rock-chips.com> >>> 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 <william.wu@rock-chips.com> >>>> --- >>>> 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 >>>> >>>> >>> >> > >
--- 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); + } + Hi John & Felipe, Could you provide me some suggestion？