Re: [PATCH v3 2/5] usb: dwc3: Add remote wakeup handling

[Elson Serrao]

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

1.) Enabled l1.
2.) Disabled the remote wakeup software path (i.e avoid calling 
__gadget_wakeup() if link is in l1 in the gadget_ep_cmd() path).
3.) Sent an IN packet when the link was in l1.

From the lecroy logs it looks like the controller initiated a remote 
wakeup and sent the data.

Below are the events and the corresponding lecroy snippet
1.)Packet(55551) ------------> LPM token from Windows Host PC.

2.) Link in l1 for 2.445 secs

3. ) Send a ping data from device to host

4. )Packet(55554) ----------------> Resume

5.) IN data


Packet#
_______|_______________________________________________________________________
Transaction(26584) H(S) EXT(0x0F) LPM(0xC3) ADDR(11) ENDP(0) BESL(150 us)
_______| Link State(0x1) Rem Wake(0x1) ACK(0x4B) Time Stamp(27 . 204 671 
632)
_______|_______________________________________________________________________Ch0 

Packet(55550) Dir H(S) EXT(0x0F) ADDR(11) ENDP(0) CRC5(0x04) Pkt Len(8)
_______| Duration(133.330 ns) Idle(200.660 ns) Time Stamp(27 . 204 671 632)
_______|_______________________________________________________________________Ch0 

Packet(55551) Dir H(S) LPM(0xC3) BESL(150 us) Link State(0x1)
_______| Rem Wake(0x1) Rsvd(0x0) CRC5(0x04) Pkt Len(8) Duration(133.330 ns)
_______| Idle(182.660 ns) Time Stamp(27 . 204 671 966)
_______|_______________________________________________________________________Ch0 

Packet(55552) Dir H(S) ACK(0x4B) Pkt Len(6) Duration(100.000 ns)
_______| Idle( 11.450 us) Time Stamp(27 . 204 672 282)
_______|_______________________________________________________________________Ch0 

Packet(55553) Dir(?) Full Speed J (Suspend)( 2.445 sec)
_______| Time Stamp(27 . 204 683 832)
_______|_______________________________________________________________________Ch0 

Packet(55554) Dir(?) Full Speed K (Resume?)( 95.168 us) Time(165.134 us)
_______| Time Stamp(29 . 649 644 482)
_______|_______________________________________________________________________
Transfer(67) H(S) Bulk(IN) ADDR(11) ENDP(1) Bytes Transferred(142)
_______| Time(309.366 us) Time Stamp(29 . 649 809 616)
_______|_______________________________________________________________________
Transfer(68) H(S) Bulk(OUT) ADDR(11) ENDP(1) Bytes Transferred(142)
_______| Time(520.050 us) Time Stamp(29 . 650 118 982)
_______|_______________________________________________________________________
Transaction(26655) H(S) EXT(0x0F) LPM(0xC3) ADDR(11) ENDP(0) BESL(150 us)
_______| Link State(0x1) Rem Wake(0x1) ACK(0x4B) Time( 12.168 us)
_______| Time Stamp(29 . 650 639 032)
_______|_______________________________________________________________________

If software was solely responsible for waking up from l1, then there 
should be no reason why the host would exit l1 in this scenario.
I tried with different ping intervals and saw that the duration for 
which the link was in l1 correlates with the ping interval.

Thanks
Elson