On Fri, Jan 13, 2023, Linyu Yuan wrote: > > On 1/13/2023 1:24 PM, Thinh Nguyen wrote: > > > > > > > > While some polling numbers seem large, we should not remove the tracing > > > > > > events during polling. There are useful info in the status register when > > > > > > there's a timeout. Also, the number of polls needed for certain state > > > > > > change is useful data point for debug. > > > > > > > > > > > > What we may want to update is not depend on the register read delay for > > > > > > the polling duration. Different setup will have different delay. > > > > > > > > > > > > If we want to disable it for debugging purpose, make sure to have the > > > > > > default option as enabled. > > > > > if so, do you accept define a new function and new trace event like, > > > > > > > > > > static inline u32 dwc3_readl_timeout(void __iomem *base, u32 offset) > > > > > > > > > > DEFINE_EVENT(dwc3_log_io, dwc3_readl_timeout, > > > > > TP_PROTO(void __iomem *base, u32 offset, u32 value), > > > > > TP_ARGS(base, offset, value) > > > > > ); > > > > > > > > > > let user enable it accordingly. > > > > > > > > > We can just redefine the event with an additional parameter for event > > > > filtering option. > > > > > > actually filtering option only work at event output time, it will show data > > > match filter, ignore data which not match. > > > > > > there is still no filter which run before event save buffer, event still > > > save into buffer, > > > > > > if read happen too many times, it will flush useful event like write > > > register operation. > > > > > Ok. What do you think of also reviving Jun's change to using > > readl_poll_timeout_atomic? It makes sense to create a new trace event > > in addition to that: > > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/87blmymwlz.fsf@xxxxxxxxxx/T/*t__;Iw!!A4F2R9G_pg!bgeN4Mp9hv2b33BpJ7QRAXAdm-vgafgurxXP-YSdQsdWMZkNFXnWV4qXNT4iCTz_0mLMHylw6Z84J5IVPYZnroj5eiNQnw$ > > > 1.if you review all places which read in this way, not all of them can > convert to > > readl_poll_timeout_atomic() Jun introduce. > which ones? > > 2. also his change block for more than two years, will it be approved ? > Previously this was done to resolve a separate issue. It was a combination of a fix and an enhancement that touched on different areas of the driver. It's better to keep them separated. IMHO it's a good change. It's better to keep the polling duration clear and determinable. BR, Thinh
