Re: [PATCH v2, 2/3] usb: dwc3: core: add p3p2tranok quirk

[joswang]

On Tue, Jun 4, 2024 at 8:02 AM Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> wrote:
>
> On Mon, Jun 03, 2024, joswang wrote:
> > From: joswang <joswang@xxxxxxxxxx>
> >
> > In the case of enable hibernation, there is an issue with
>
> I assume this is for host mode since we currently don't handle
> hibernation in device mode (please confirm).
Yes, your consideration is correct, hibernation is only handled in host mode
>
> > the DWC31 2.00a and earlier versions where the controller
> > link power state transition from P3/P3CPM/P4 to P2 may take
> > longer than expected, ultimately resulting in the hibernation
> > D3 entering time exceeding the expected 10ms.
>
> Can you provide more context where the 10ms requirement is from?
>
The P3/P3CPM/P4 to P2 power state change might take longer (maximum 10 ms).
If there is an impending D3 entry request, the controller does not
respond as long as the power state change is completed causing
unnecessary delays in D3 entry.
The above information is provided by your company.
STAR number 4236358
> >
> > Synopsys workaround:
> > If the PHY supports direct P3 to P2 transition, program
> > GUSB3PIPECTL.P3P2Tran0K=1.
> >
>
> Which STAR issue is this?
This is the solution provided by your company
STAR issue:  the DWC31 2.00a and earlier versions where the controller
link power state transition from P3/P3CPM/P4 to P2 may take longer
than expected.
>
> > Therefore, adding p3p2tranok quirk for workaround hibernation
> > D3 exceeded the expected entry time.
> >
> > Signed-off-by: joswang <joswang@xxxxxxxxxx>
> > ---
>
> Please provide change note for v1->v2 here (and the rest of the other
> patches).
>
> >  drivers/usb/dwc3/core.c | 5 +++++
> >  drivers/usb/dwc3/core.h | 4 ++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 7ee61a89520b..3a8fbc2d6b99 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -666,6 +666,9 @@ static int dwc3_ss_phy_setup(struct dwc3 *dwc, int index)
> >       if (dwc->dis_del_phy_power_chg_quirk)
> >               reg &= ~DWC3_GUSB3PIPECTL_DEPOCHANGE;
> >
> > +     if (dwc->p2p3tranok_quirk)
> > +             reg |= DWC3_GUSB3PIPECTL_P3P2TRANOK;
> > +
> >       dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(index), reg);
> >
> >       return 0;
> > @@ -1715,6 +1718,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
> >
> >       dwc->dis_split_quirk = device_property_read_bool(dev,
> >                               "snps,dis-split-quirk");
> > +     dwc->p2p3tranok_quirk = device_property_read_bool(dev,
> > +                             "snps,p2p3tranok-quirk");
> >
> >       dwc->lpm_nyet_threshold = lpm_nyet_threshold;
> >       dwc->tx_de_emphasis = tx_de_emphasis;
> > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> > index 3781c736c1a1..2810dce8b42e 100644
> > --- a/drivers/usb/dwc3/core.h
> > +++ b/drivers/usb/dwc3/core.h
> > @@ -327,6 +327,7 @@
> >  #define DWC3_GUSB3PIPECTL_DEP1P2P3_EN        DWC3_GUSB3PIPECTL_DEP1P2P3(1)
> >  #define DWC3_GUSB3PIPECTL_DEPOCHANGE BIT(18)
> >  #define DWC3_GUSB3PIPECTL_SUSPHY     BIT(17)
> > +#define DWC3_GUSB3PIPECTL_P3P2TRANOK BIT(11)
> >  #define DWC3_GUSB3PIPECTL_LFPSFILT   BIT(9)
> >  #define DWC3_GUSB3PIPECTL_RX_DETOPOLL        BIT(8)
> >  #define DWC3_GUSB3PIPECTL_TX_DEEPH_MASK      DWC3_GUSB3PIPECTL_TX_DEEPH(3)
> > @@ -1132,6 +1133,8 @@ struct dwc3_scratchpad_array {
> >   *                   instances in park mode.
> >   * @parkmode_disable_hs_quirk: set if we need to disable all HishSpeed
> >   *                   instances in park mode.
> > + * @p2p3tranok_quirk: set if Controller transitions directly from phy
> > + *                   power state P2 to P3 or from state P3 to P2.
> >   * @gfladj_refclk_lpm_sel: set if we need to enable SOF/ITP counter
> >   *                          running based on ref_clk
> >   * @tx_de_emphasis_quirk: set if we enable Tx de-emphasis quirk
> > @@ -1361,6 +1364,7 @@ struct dwc3 {
> >       unsigned                ulpi_ext_vbus_drv:1;
> >       unsigned                parkmode_disable_ss_quirk:1;
> >       unsigned                parkmode_disable_hs_quirk:1;
> > +     unsigned                p2p3tranok_quirk:1;
> >       unsigned                gfladj_refclk_lpm_sel:1;
> >
> >       unsigned                tx_de_emphasis_quirk:1;
> > --
> > 2.17.1
> >
>
> Thanks,
> Thinh