Re: [PATCH 2/2] usb: dwc3: gadget: Rename EOPF event macros to Suspend

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

Jack Pham wrote:
> Hi Felipe,
> 
> On Wed, Apr 28, 2021 at 01:28:16PM +0300, Felipe Balbi wrote:
>> Jack Pham <jackp@codeaurora.org> 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
>