Jack Pham wrote: > Hi Felipe, > > On Wed, Apr 28, 2021 at 01:28:16PM +0300, Felipe Balbi wrote: >> Jack Pham <jackp@xxxxxxxxxxxxxx> writes: >>> The device event corresponding to End of Periodic Frame is only >>> found on older IP revisions (2.10a and prior, according to a >> >> you're reading databook for dwc3.1, right? Remember the original support >> for the driver was on dwc3. This was always called EOPF back then. We >> should maintain the name. > > I've looked through several revisions of the databook for both dwc3 and > dwc3.1. From what I can tell EOPF was nixed starting in DWC3 (not 3.1) > revision 2.20a. DWC3 revision 2.30a re-introduced event #6 for USB > suspend. And judging from the IP revision list in core.h, DWC3 is now > up to 3.30a (DWC3_REVISION_330A), so from number alone there are about > as many revisions that have this bit as EOPF as there are that use it > for SUSPEND. This carries over to DWC3.1 as well (not sure about DWC3.2) > so in fact there are probably more revisions of IP that no longer use > EOPF. > > Hi Thinh, I'm wondering if you could please help corroborate the history > of this bit, and confirm whether it is also used as Suspend entry in DWC > 3.2 IPs? DWC_usb32 IP is also the same as DWC_usb31 and uses it for suspend event. BR, Thinh > > But I don't want to make it seem that I'm using revision history as a > gauge of how many real devices out there support EOPF vs Suspend. That > figure we'll never truly know. > >>> cursory SNPS databook search). On revisions 2.30a and newer, >>> including DWC3.1, the same event value and corresponding DEVTEN >>> bit were repurposed to indicate that the link has gone into >>> suspend state (U3 or L2/L1). >>> >>> EOPF events had never been enabled before in this driver, and >>> going forward we expect current and future DWC3-based devices >>> won't likely to be using such old DWC3 IP revisions either. >> >> We still have original omap5 devices, running on revision 1.73a >> around. They'll remain supported for the time being. >> >>> Hence rather than keeping the deprecated EOPF macro names let's >>> rename them to indicate their usage for suspend events. >> >> what do we gain from this change? I mean, in practice, what changes? >> nothing realy, so why should we apply this? > > I'm saying since this macro has never really been used to enable any > kind of event handling specifically for "End Of Periodic Frame", that > there is not much utility in keeping the name as EOPF. Instead as I > explained in patch 1, the same bit/event is used on newer revisions for > USB Suspend entry so assuming you accept that, then the purpose of this > follow-on patch is simply to make the code more readable by renaming the > macro to fit its usage. > > Thanks, > Jack >