Re: [PATCH v2] usb: dwc3: add disable u2mac linestate check quirk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Tue, Apr 18, 2017 at 8:59 PM, wlf <wulf@xxxxxxxxxxxxxx> wrote:
> Dear Guenter,
>
>
>
> 在 2017年04月18日 21:18, Guenter Roeck 写道:
>>
>> On Mon, Apr 17, 2017 at 10:17 PM, William Wu <william.wu@xxxxxxxxxxxxxx>
>> 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@xxxxxxxxxxxxxx>
>>> ---
>>> 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
>>>
>>>
>>
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux