Hi Wesley, Wesley Cheng wrote: > Hi Thinh, > > On 2/15/2022 4:08 PM, Wesley Cheng wrote: >> While running the pullup disable sequence, if there are pending SETUP >> transfers stored in the controller, then the ENDTRANSFER commands on non >> control eps will fail w/ ETIMEDOUT. As a suggestion from SNPS, in order >> to drain potentially cached SETUP packets, SW needs to issue a >> STARTTRANSFER command. After issuing the STARTTRANSFER, and retrying the >> ENDTRANSFER, the command should succeed. Else, if the endpoints are not >> properly stopped, the controller halt sequence will fail as well. >> >> One limitation is that the current logic will drop the SETUP data >> being received (ie dropping the SETUP packet), however, it should be >> acceptable in the pullup disable case, as the device is eventually >> going to disconnect from the host. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx> >> --- > Just wondering if you had any inputs for this particular change? I > think on the dequeue path discussion you had some concerns with parts of > this change? I think the difficult part for the pullup disable path is > that we have this API running w/ interrupts disabled, so the EP0 state > won't be able to advance compared to the dequeue case. This doesn't sound right. The pullup disable (or device initiated disconnect) should wait for the EP0 state to be EP0_SETUP_PHASE before proceeding, which it does. > > Ideally, if there is a way to ensure that we avoid reading further setup > packets, that would be nice, but from our discussions with Synopsys, > this was not possible. (controller is always armed and ready to ACK > setup tokens) > > I did evaluate keeping IRQs enabled and periodically releasing/attaining > the lock between the stop active xfer calls, but that opened another can > of worms. If you think this is the approach we should take, I can take > a look at this implementation further. > This patch doesn't look right to me. The change I suggested before should address this (I believe Greg already picked it up). What other problem do you see, I'm not clear what's the problem here. One potential problem that I can see is that currently dwc3 driver doesn't wait for active endpoints to complete/end before clearing the run_stop bit on device initiated disconnect, but I'm not sure if that's what you're seeing. Please help clarify further. If there's trace points log, that'd help. Thanks, Thinh
