Hi Filip, On Tue, 2 May 2023 at 12:43, Filip Žaludek <filip.zalu...@oracle.com> wrote: > > > > Hi Simon, Michal, Marek, > > > > On 4/26/23 03:04, Simon Glass wrote: > > Hi Filip, > > > > On Tue, 25 Apr 2023 at 06:36, Filip Žaludek <filip.zalu...@oracle.com> > > wrote: > >> > >> > >> > >> Hi Simon, > >> > >> > >> On 4/19/23 03:49, Simon Glass wrote: > >>> Hi Filip, > >>> > >>> On Tue, 11 Apr 2023 at 14:24, Filip Žaludek <filip.zalu...@oracle.com> > >>> wrote: > >>>> > >>>> > >>>> > >>>> On 2/8/23 20:01, Mark Kettenis wrote: > >>>>>> Date: Wed, 8 Feb 2023 19:45:36 +0100 > >>>>>> From: Michal Suchánek <msucha...@suse.de> > >>>>>> > >>>>>> Hello, > >>>>>> > >>>>>> On Wed, Jan 18, 2023 at 05:01:12PM +0100, Filip Žaludek wrote: > >>>>>>> > >>>>>>> > >>>>>>> Hi Michal, > >>>>>>> > >>>>>>> thanks for testing! Do you consider keyboard as working once it > >>>>>>> is detected without > >>>>>>> 'usb_kbd usb_kbd: Timeout poll on interrupt endpoint', or judging > >>>>>>> from subsequent > >>>>>>> typing? Note that issue is reproducible only in about 20% of reboots. > >>>>>> > >>>>>> I rely on keyboard input to boot so if it was 20% broken I would > >>>>>> notice. > >>>>>> I don't use the rPi all that much so if it was broken only a few > >>>>>> % of the time there is a chance I would miss it. > >>>>>> > >>>>>> However, for me not typing on the keyboard during usb detection it is > >>>>>> 100% not detected, typing on it during usb detection it is 100% > >>>>>> detected. > >>>>>> > >>>>>> The timeout is limitation of the dwc2 controller handling of usb hubs. > >>>>>> > >>>>>> There might be a possibility to improve the driver so that it handles > >>>>>> the condition but it might be that the Linux driver relies on a > >>>>>> separate > >>>>>> thread handling the controller which is not acceptable for u-boot. > >>>>>> > >>>>>> I am not usb expert and definitely not dwc2 expert so I cannot do more > >>>>>> than workaround the current driver limitation. > >>>>>> > >>>>>>> For me I can always enter 'U-Boot>' shell, but then keyboard usually > >>>>>>> does not work. > >>>>>>> And yes, resetting the usb controller with pressing a key afterwards > >>>>>>> will > >>>>>>> finally break the keyboard. ('usb reset' typed from keyboard) > >>>>>>> If you are Prague located I am ready to demonstrate what I am talking > >>>>>>> about. > >>>>>>> > >>>>>>> Simon's keyboard detection is somewhat interfered by 'SanDisk USB > >>>>>>> Extreme Pro' detection, > >>>>>>> printed complaints but keyboard still works.. > >>>>>>> 'usb_kbd usb_kbd: Timeout poll on interrupt endpoint' and 'Failed to > >>>>>>> get keyboard state from device 0c40:8000' > >>>>>>> Btw. why from 0c40:8000 (ELMCU 2.4GHz receiver) when wired keyboard > >>>>>>> is 046d:c31c (Logitech Keyboard K120)? > >>>>>>> > >>>>>>> What is supposed scenario for RPi3/u-boot/grub usb keyboard > >>>>>>> equipped users wanting to boot non-default? > >>>>>>> Enter 'U-Boot>' shell to detect keyboard; type boot; select desired > >>>>>>> grub entry..? > >>>>>>> > >>>>>>> Reverting either from the two makes it non issue for me: > >>>>>>> 'dwc2: use the nonblock argument in submit_int_msg' > >>>>>>> commit 9dcab2c4d2cb50ab1864c818b82a72393c160236 > >>>>>> > >>>>>> Without this booting from USB is not feasible because reading every > >>>>>> block from the USB drive waits for the keyboard to time out. > >>>>>> > >>>>>>> 'console: usb: kbd: Limit poll frequency to improve performance' > >>>>>>> commit 96991e652f541323a03c5b7e075d54a117091618 > >>>>>> > >>>>>> No idea about this one, for me it doea not give any substantial > >>>>>> difference in behavior. > >>>>> > >>>>> Reverting that commit leads to a significant slowdown loading a kernel > >>>>> from disk with a usb keyboard connected. The slowdown is somewhat > >>>>> hardware dependent but on some systems loading the OpenBSD/arm64 > >>>>> kernel would take minutes instead of seconds. > > > > More updates to usb keyboard/RPi3/dwc2 controller issue: > > I was following my former observation about printing characters from semi > random places [usb.c, usb_hub.c, device.c, usb-uclass.c, dwc2.c] what > works as workaround. I realized this is only when printing to vidconsole, > not to serial. After disabling video_sync() and/or flush_dcache_range() > from corresponding vidconsole print functions, printing is no longer > workaround. This behavior seem to be due to cache coherency. > > > > Do you have any objections against elephant in porcelain proposal? > Not able to narrow it down more to single source code line. > With this keyboard works for me even when touching it only during 15s grub > timeout. > It is not for sure that cache coherency problem is from dwc2, but afaik there > are no other complaints to usb keyboard. > Performance degradation not observed.. > > > %< ------------------------------------------------------------- > diff --git a/drivers/usb/host/dwc2.c b/drivers/usb/host/dwc2.c > index 23060fc369..f95314ff1b 100644 > --- a/drivers/usb/host/dwc2.c > +++ b/drivers/usb/host/dwc2.c > @@ -814,6 +814,7 @@ static int dwc_otg_submit_rh_msg(struct dwc2_priv *priv, > struct usb_device *dev, > else > stat = dwc_otg_submit_rh_msg_out(priv, dev, buffer, txlen, > cmd); > > + flush_dcache_all(); > mdelay(1);
We have dma_map_single() and dma_unmap_single() which are designed for this. If you put these into the two functions that are called immediately above, perhaps that will solve the problem. > > return stat; > %< ------------------------------------------------------------- > > > > > > >>>> > >>>> > >>>> Hello, > >>>> I am about to dig more into this issue with proper tools, but > >>>> failed to > >>>> configure/compile trace functionality on RPi3 due to missing references > >>>> to timer_early_get_count() and timer_early_get_rate(). > >>> > >>> You could implement a proper timer driver for rpi. > >>> > >>>> > >>>> Is it possible/feasible to implement calls in CONFIG_SYS_ARCH_TIMER > >>>> and/or CONFIG_SP804_TIMER? > >>> > >>> Yes > >> > >> > >> I am little bit missing here secret sauce, timer_early_get_count() and > >> timer_early_get_rate() > >> are not supposed to be implemented in arch/arm/cpu/armv8/generic_timer.c? > >> But predestined for > >> drivers/timer/sp804_timer.c? > >> > >> TIMER is required for common/board_f.c and common/board_r.c but it > >> disables generic_timer.. > >> %< ----------------------------------------- > >> ifndef CONFIG_$(SPL_TPL_)TIMER > >> obj-$(CONFIG_SYS_ARCH_TIMER) += generic_timer.o > >> endif > >> %< ----------------------------------------- > >> And obviously multiple definition of get_tbclk and get_ticks when forced > >> to compile/link. > > > > It seems from the above that if TIMER is enabled for a particular > > U-Boot build phase, then generic_timer is not, and vice versa. I > > suppose that is fair enough. > > > Sorry, it was imprecisely formulated question from me. I was expecting > answer > confirming and advocating sp804 is superior to System Timer. Implementing > EARLY_TIMER for System Timer is trivial, sp804 requires research from my side. > Skimming TF-A project sp804 seems superior. Yes, implementing that as a timer driver seems fine to me. But we often have multiple drivers so I suppose some people will use the generic one if sp804 is not available. > >>>> > >>>> I would be grateful even for trace to generate function traces > >>>> without > >>>> timestamps. Is such nasty hack without timestamping supposed to work? > >>>> Basically my intention is to trace 'usb reset'. > >>>> > >>>> Appreciate any hints/outlines how to proceed. > >>> > >>> I assume you mean CONFIG_TRACE. Yes, you could update it to support > >>> writing a zero timestamp. See the add_ftrace() function. > >>> > >>> But better to add a driver if you can. It should not be difficult. > >>> > >>> Regards, > >>> Simon > >> > >> I am already happily timestamp tracing with borrowed functionality from > >> generic_timer.c, > >> albeit bypassing kbuild mechanism. It did not yet answered my usb polling > >> questions, > >> tracing report is polluted/overfilled. > >> > >> > >> > >> Instrumenting code thoughts: > >> * It would be handy to -finstrument-functions only for desired objects. > > > > The compiler probably doesn't support that, or at least we don't > > support passing different compiler args to each file in U-Boot, other > > than by manually hacking the Makefiles. > > > > But I wonder if we could have a list of wildcards to match against the > > function names? > > > >> * It would be handy to have macro inverse to 'notrace' to mark only > >> desired functions. Feasible? > >> * gcc -finstrument-functions-exclude-file-list still pollutes tracing > >> buffer. > > > > I wonder why? You could check whether the filename includes a full > > path, or something like that, so that it doesn't match. There will be > > a reason. > > > > > Please take my Instrumenting code comments with grain of salt, only as user > report. > Some doc pages track suggestions and whishlists how to make life easier.. Sure...people using a feature are always going to have ways to tidy it up / improve it. So I encourage you to take a look. > >> > >> More Tracing in U-Boot thoughts: > >> * There is proftool options discrepancy, documented {-c, -f, -m, -o, -t, > >> -v}, implemented {-m, -p, -t, -v}. > >> * Both types FUNCF_ENTRY and FUNCF_EXIT are marked as " <- " by proftool. > > > > Yes, please send a patch or two to clean these up. > > > > Regards, > > Simon > > > I will try to allocate spare time cycles to work on this, albeit with low > priority. OK Regards, Simon