On Fri, Jan 13, 2023, Linyu Yuan wrote: > > On 1/13/2023 11:18 AM, Thinh Nguyen wrote: > > On Fri, Jan 13, 2023, Linyu Yuan wrote: > > > On 1/13/2023 9:08 AM, Thinh Nguyen wrote: > > > > Hi, > > > > > > > > On Thu, Jan 12, 2023, Linyu Yuan wrote: > > > > > On 1/11/2023 3:21 PM, Jun Li (OSS) wrote: > > > > > > > -----Original Message----- > > > > > > > From: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> > > > > > > > Sent: Monday, January 9, 2023 11:42 AM > > > > > > > To: Jun Li (OSS) <jun.li@xxxxxxxxxxx>; Greg Kroah-Hartman > > > > > > > <gregkh@xxxxxxxxxxxxxxxxxxx>; Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > > > > > > > Cc: linux-usb@xxxxxxxxxxxxxxx; Jack Pham <quic_jackp@xxxxxxxxxxx>; Wesley > > > > > > > Cheng <quic_wcheng@xxxxxxxxxxx> > > > > > > > Subject: Re: [PATCH 2/3] usb: dwc3: add dwc3_readl_notrace() function > > > > > > > > > > > > > > > > > > > > > On 1/9/2023 11:33 AM, Jun Li (OSS) wrote: > > > > > > > > > -----Original Message----- > > > > > > > > > From: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> > > > > > > > > > Sent: Friday, January 6, 2023 5:22 PM > > > > > > > > > To: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Thinh Nguyen > > > > > > > > > <Thinh.Nguyen@xxxxxxxxxxxx> > > > > > > > > > Cc: linux-usb@xxxxxxxxxxxxxxx; Jack Pham <quic_jackp@xxxxxxxxxxx>; > > > > > > > > > Wesley Cheng <quic_wcheng@xxxxxxxxxxx>; Linyu Yuan > > > > > > > > > <quic_linyyuan@xxxxxxxxxxx> > > > > > > > > > Subject: [PATCH 2/3] usb: dwc3: add dwc3_readl_notrace() function > > > > > > > > > > > > > > > > > > There are multiple places which will retry up to 10000 times to read > > > > > > > > > a register, > > > > > > > > It's "up to", I think at normal case, it's a small number, if a large > > > > > > > > Iteration number is observed, probably there is something wrong should > > > > > > > > be checked? > > > > > > > do you mean the original loop count can be reduced ? > > > > > > No. I mean the max loop number is not a problem at good HW. > > > > > > What's the actual loop number on you HW? > > > > > i didn't check it. how about you ? no matter what is good HW or bad HW, > > > > > current code define a big number. > > > > > > > > > > > > > > > actually i think we should not discuss is it a good number or not. > > > > > > > > > > what is purpose to use status register record ? do it give you any > > > > > information to understand the IP behavior ? > > > > > > > > > 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://lore.kernel.org/linux-usb/87blmymwlz.fsf@xxxxxxxxxx/T/#t Thanks, Thinh > > > > > Make sure not to change dwc3_readl() declaration so that the patch > > doesn't touch every part of the driver. > > > > BR, > > Thinh > > > > > > As for the inconsistent/large polling count, we can review and revisit > > > > the change Li Jun did a while ago to use readl_poll_timeout_atomic > > > > instead: > > > > https://urldefense.com/v3/__https://lore.kernel.org/linux-usb/87blmymwlz.fsf@xxxxxxxxxx/T/*t__;Iw!!A4F2R9G_pg!eZBcW78zLMkOtfPIhxNvIGjItv_W2IVvpLyOV-3eCrWRk7T1PVERSmz037HLx1nLOHgOsRTK1Cz8DHFeBXZ21WwY5OkJlw$ > > > > > > > > Thanks, > > > > Thinh
