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().
but if you prefer read_poll_timeout_atomic() implementation,
@jun can you review my change and covert all places into it ?
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.
@Jun can you review this comment and update a new patch ?
BR,
Thinh
