On Wed, Jan 18, 2023, Linyu Yuan wrote: > > On 1/14/2023 7:16 AM, Thinh Nguyen wrote: > > 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? > > > I try it before, but if strictly follow original code logic, it is hard to > convert all of them, e.g. dwc3_ulpi_busyloop(). > What's the problem with that one? Also, we're not exactly following the original code logic. The current logic doesn't ensure consistent/determinate polling duration between different setup, and keeping it consistent is a good thing. Just need to make sure to use the appropriate atomic/non-atomic version of the readl_poll_timeout where it can sleep or not. Thanks, Thinh
