Messages in this thread | | | Date | Thu, 9 Feb 2023 19:43:12 -0800 | Subject | Re: [PATCH v3 2/5] usb: dwc3: Add remote wakeup handling | From | Elson Serrao <> |
| |
On 2/9/2023 6:27 PM, Thinh Nguyen wrote: > On Thu, Feb 09, 2023, Elson Serrao wrote: >> >> >> On 2/7/2023 6:11 PM, Thinh Nguyen wrote: >>> On Tue, Feb 07, 2023, Elson Serrao wrote: >>>> >>>> >>>> On 2/7/2023 5:10 PM, Thinh Nguyen wrote: >>>>> On Tue, Feb 07, 2023, Elson Serrao wrote: >>>>>> On 2/6/2023 4:48 PM, Thinh Nguyen wrote: >>>>>>>> +static int __dwc3_gadget_wakeup(struct dwc3 *dwc, bool async) >>>>>>>> { >>>>>>>> int retries; >>>>>>>> @@ -2296,9 +2309,14 @@ static int __dwc3_gadget_wakeup(struct dwc3 *dwc) >>>>>>>> link_state = DWC3_DSTS_USBLNKST(reg); >>>>>>>> switch (link_state) { >>>>>>>> + case DWC3_LINK_STATE_U3: /* in HS, means SUSPEND */ >>>>>>> >>>>>>> It's also possible to do remote wakeup in L1 for highspeed. >>>>>>> >>>>>> >>>>>> The rw_configured flag here is in context of triggering remote wakeup from >>>>>> bus suspend only. >>>>>> >>>>>> The remote wakeup setting for l1 in HighSpeed is controlled through LPM >>>>>> token and overrides/ignores the config desc bmAttributes wakeup bit. >>>>>> >>>>>> Section 4.1 of USB2_LinkPowerMangement_ECN[final] spec "The host system sets the Remote Wake Flag parameter in this request to >>>>>> enable or disable the addressed device >>>>>> for remote wake from L1. The value of this flag will temporarily (while in >>>>>> L1) override the current setting of the >>>>>> Remote Wake feature settable by the standard Set/ClearFeature() commands >>>>>> defined in Universal Serial Bus Specification, revision 2.0, Chapter 9." >>>>>> >>>>>> Please let me know if I am missing something. >>>>>> >>>>> >>>>> It overrides the setting of the SetFeature request, not the device >>>>> configuration. >>>>> >>>>> The rw_configured reflects the user configuration. Whether the host >>>>> tries to enable the remote wakeup through SetFeature request or LPM >>>>> token, the device should operate within the user configuration >>>>> limitation. >>>>> >>>>> If the configuration indicates that it doesn't support remote wakeup, we >>>>> should prevent unexpected behavior from the device. For simplicity, we >>>>> can just return failure to wakeup for all states. >>>>> >>>>> Thanks, >>>>> Thinh >>>> >>>> L1 entry/exit is HW controlled and the remote wakeup is conditional.(Section >>>> 7.1/Table7.2 of dwc3 data book). Even though we block it from >>>> SW the l1 exit will still happen from HW point of view. >>>> >>>> To correlate the user configuration with LPM token, I experimented by >>>> disabling the wakeup bit in the bmAtrributes, but I still see remote wakeup >>>> bit being set in the LPM token. From the observation it seems like there is >>> >>> That's because the linux xhci driver enables remote wakeup bit in its >>> port without regard for the device configuration. >>> >>>> no correlation between the wakeup bit in the bmAtrributes and the wakeup bit >>>> in the LPM token. >>>> >>> >>> The host can bring the device out of L1, that's probably what you saw. >>> The controller doesn't initiate remote wakeup by itself. >>> >>> Thanks, >>> Thinh >> >> Actually it seems the controller is initiating a remote wakeup by itself to >> exit from l1 when we send a STARTTRANSFER command. I did below experiment >> when the device was in HighSpeed >> > > That's driven by the driver telling the controller to initiate remote > wakeup and not the controller itself. When we send the START_TRANSFER > command, the driver does remote wakeup so the host would bring the > device to ON state so that the command can go through. > > However you bring up a good point that if we prevent remote wakeup for > L1, then we have to delay sending START_TRANSFER command until the host > initiate resume. This would require additional enhancement to dwc3 to > handle this scenario. For now, can we ignore this specific case when > sending START_TRANSFER command and only check for the case when the user > trigger remote wakeup via gadget->ops->wakeup/func_wakeup. > > Thanks, > Thinh
Sure. I will upload v4 with the suggested feedback/comments.
Thanks Elson
| |