MIME-Version: 1.0
References: <20211209072218.21651-1-chunfeng.yun@mediatek.com>
 <c448da77-f2f3-8b79-fc4b-b9dcff727c6e@linux.intel.com> <CAKgpwJXxtLwOjxjg3vFHiqS92j6rx_b1+C-bRwDnp+PBvXCMTg@mail.gmail.com>
 <e7a6e45e-68ce-54a5-9632-80244dd1e4c7@linux.intel.com>
In-Reply-To: <e7a6e45e-68ce-54a5-9632-80244dd1e4c7@linux.intel.com>
From:   Jun Li <lijun.kernel@gmail.com>
Date:   Wed, 15 Dec 2021 10:09:39 +0800
Message-ID: <CAKgpwJXdO8BZPcUH5Lq8AQUkQMDvLUW9h4HsgCm1XLcuP1G1HA@mail.gmail.com>
Subject: Re: [PATCH] usb: xhci: skip re-check pending port event if hibernated
To:     Mathias Nyman <mathias.nyman@linux.intel.com>
Cc:     Chunfeng Yun <chunfeng.yun@mediatek.com>,
        Mathias Nyman <mathias.nyman@intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Linux USB List <linux-usb@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        linux-mediatek@lists.infradead.org,
        lkml <linux-kernel@vger.kernel.org>,
        Eddie Hung <eddie.hung@mediatek.com>,
        Yun-Chien Yu <yun-chien.yu@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Mathias Nyman <mathias.nyman@linux.intel.com> =E4=BA=8E2021=E5=B9=B412=E6=
=9C=8814=E6=97=A5=E5=91=A8=E4=BA=8C 18:03=E5=86=99=E9=81=93=EF=BC=9A
>
> On 14.12.2021 10.00, Jun Li wrote:
> > Mathias Nyman <mathias.nyman@linux.intel.com> =E4=BA=8E2021=E5=B9=B412=
=E6=9C=8811=E6=97=A5=E5=91=A8=E5=85=AD 01:56=E5=86=99=E9=81=93=EF=BC=9A
> >>
> >> On 9.12.2021 9.22, Chunfeng Yun wrote:
> >>> When xHCI controller hibernated, the root hub lost power, if controll=
er
> >>> support Port Power Control (PPC), PP is not set at xhci_resume() and
> >>> set by hub_reset_resume() later, so no need check pending port event.
> >>> If PPC is not supported, device is disconneced, seems do not send out
> >>> U3 LFPS wake signal, no need re-check again and drop 120ms delay to
> >>> save resume time.
> >>>
> >>> Reported-by: Yun-Chien Yu <yun-chien.yu@mediatek.com>
> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> >>> ---
> >>
> >> Thanks, adding
> >
> > Hi Mathias, Chunfeng
> >
> > I have a question on this, if there is no any usb devices connected
> > before suspend, do we need this 120ms delay to check again?
> > So do we need one more condition to limit this like?
> > if (!pending_portevent && !hibernated && xhci_has_child_device())
>
> The 120ms delay was added to make sure we catch the second wake signal
> from a device in case host missed the first U3 exit LFPS wakeup signal.
>
> Even if no devices are connected this might be helpful if a device is
> connected while host is suspended.

Agree this may also help on this case.

> I haven't checked any timing for the link training during enumeration,
> but it also uses LFPS signalling, and connected device isn't visible to
> driver until link is successfully trained.
>
> So the original 120ms delay patch might as a positive side effect ensure
> driver doesn't suspend host mid device enumeration.

Is this unexpected suspend can be prevented by adding auto suspend
delay?

Thanks
Li Jun
>
> Could be looked into more, but I don't think we should this patch by
> Chunfeng
>
> Thanks
> Mathias