[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hello,
  change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
When reverted, usb keyboard works as expected.

Anybody sitting front of RPi3B+ care to confirm?


Regards,
Filip





Patch:
https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618


Debug:
USB KBD: found interrupt EP: 0x81
USB KBD: set boot protocol
dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
udev->dev='usb_kbd', portnr=3
USB KBD: set idle interval...
dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
udev->dev='usb_kbd', portnr=3
USB KBD: enable interrupt pipe...
usb_kbd usb_kbd: Timeout poll on interrupt endpoint


Tested:
SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
HW: USB 1.1 and 2.0 keyboards, RPi3B+
JeOS:
http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz
(u-boot-rpiarm64-2022.10-1.1.aarch64)

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Sat, 17 Dec 2022 at 05:50, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>
>
>
> Hello,
>   change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
> When reverted, usb keyboard works as expected.
>
> Anybody sitting front of RPi3B+ care to confirm?
>
>
> Regards,
> Filip
>
>
>
>
>
> Patch:
> https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618
>
>
> Debug:
> USB KBD: found interrupt EP: 0x81
> USB KBD: set boot protocol
> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> udev->dev='usb_kbd', portnr=3
> USB KBD: set idle interval...
> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> udev->dev='usb_kbd', portnr=3
> USB KBD: enable interrupt pipe...
> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
>
>
> Tested:
> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
> HW: USB 1.1 and 2.0 keyboards, RPi3B+
> JeOS:
> http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz
> (u-boot-rpiarm64-2022.10-1.1.aarch64)
>
>

This seems to work on current master:

do-try-int.sh rpi3
Revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee, board rpi3

Checking revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee
/vid/software/devel/ubtest
tbot starting ...
├─Parameters:
│     rev        = '9bd3d354a1a0712ac27c717df9ad60566b0406ee'
│     clean      = False
├─Calling uboot_build_and_flash ...
│   ├─Calling uboot_build ...
│   │   ├─Calling uboot_checkout ...
│   │   │   ├─Builder: rpi3
│   │   │   └─Done. (0.181s)
│   │   ├─Configuring build ...
│   │   ├─Calling uboot_make ...
│   │   │   └─Done. (1.504s)
│   │   └─Done. (2.178s)
│   ├─Calling uboot_flash ...
│   │   ├─Calling copy ...
│   │   │   └─Done. (0.004s)
│   │   └─Done. (1.119s)
│   └─Done. (3.663s)
├─────────────────────────────────────────
└─SUCCESS (3.799s)
tbot starting ...
├─Calling interactive_board ...
│   ├─POWERON (rpi3)
│   ├─Entering interactive shell (CTRL+D to exit) ...


U-Boot 2023.01-rc3-00057-g9bd3d354a1a (Dec 17 2022 - 15:23:14 -0700)

DRAM:  992 MiB
RPI 3 Model B (0xa22082)
Core:  66 devices, 14 uclasses, devicetree: embed
MMC:   mmc@7e202000: 0, mmc@7e300000: 1
Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1...
In:    serial
Out:   vidconsole
Err:   vidconsole
Net:   No ethernet found.
starting USB...
Bus usb@7e980000: USB DWC2
scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll
on interrupt endpoint
Failed to get keyboard state from device 0c40:8000
6 USB Device(s) found
       scanning usb for storage devices... 1 Storage Device(s) found
Hit any key to stop autoboot:  0
U-Boot> usb info
1: Hub,  USB Revision 1.10
 -  U-Boot Root Hub
 - Class: Hub
 - PacketSize: 8  Configurations: 1
 - Vendor: 0x0000  Product 0x0000 Version 0.0
   Configuration: 1
   - Interfaces: 1 Self Powered 0mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 1
     - Class Hub
     - Endpoint 1 In Interrupt MaxPacket 2 Interval 255ms

2: Hub,  USB Revision 2.0
 - Class: Hub
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x0424  Product 0x9514 Version 2.0
   Configuration: 1
   - Interfaces: 1 Self Powered Remote Wakeup 2mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 1
     - Class Hub
     - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
     - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms

3: Human Interface,  USB Revision 1.10
 - Logitech USB Keyboard
 - Class: (from Interface) Human Interface
 - PacketSize: 8  Configurations: 1
 - Vendor: 0x046d  Product 0xc31c Version 100.0
   Configuration: 1
   - Interfaces: 2 Bus Powered Remote Wakeup 90mA
   - String: "U64.00_B0001"
     Interface: 0
     - Alternate Setting 0, Endpoints: 1
     - Class Human Interface, Subclass: Boot Keyboard
     - String: "USB Keyboard"
     - Endpoint 1 In Interrupt MaxPacket 8 Interval 10ms
     Interface: 1
     - Alternate Setting 0, Endpoints: 1
     - Class Human Interface, Subclass: None
     - String: "USB Keyboard"
     - Endpoint 2 In Interrupt MaxPacket 4 Interval 255ms

4: Human Interface,  USB Revision 0.1
 - QDtech MPI5001 ?
 - Class: (from Interface) Human Interface
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x0483  Product 0x5750 Version 2.0
   Configuration: 1
   - Interfaces: 1 Bus Powered 64mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 1
     - Class Human Interface, Subclass: None
     - Endpoint 2 In Interrupt MaxPacket 64 Interval 1ms
     - Endpoint 2 Out Interrupt MaxPacket 64 Interval 1ms

5: Vendor specific,  USB Revision 2.0
 - Class: Vendor specific
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x0424  Product 0xec00 Version 2.0
   Configuration: 1
   - Interfaces: 1 Self Powered Remote Wakeup 2mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 3
     - Class Vendor specific
     - Endpoint 1 In Bulk MaxPacket 512
     - Endpoint 2 Out Bulk MaxPacket 512
     - Endpoint 3 In Interrupt MaxPacket 16 Interval 4ms

6: Mass Storage,  USB Revision 2.10
 - SanDisk USB Extreme Pro 513456791960
 - Class: (from Interface) Mass Storage
 - PacketSize: 64  Configurations: 1
 - Vendor: 0x0781  Product 0x5588 Version 1.0
   Configuration: 1
   - Interfaces: 1 Self Powered 0mA
     Interface: 0
     - Alternate Setting 0, Endpoints: 2
     - Class Mass Storage, Transp. SCSI, Bulk only
     - Endpoint 1 In Bulk MaxPacket 512
     - Endpoint 2 Out Bulk MaxPacket 512

U-Boot> hello
Unknown command 'hello' - try 'help'
U-Boot>
│   ├─POWEROFF (rpi3)
│   ├─Exiting poweroff
│   └─Done. (34.381s)
├─────────────────────────────────────────
└─SUCCESS (34.481s)

(the 'hello' is typed on the keyboard)

Regards,
Simon

Re: [External] : Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,

  is your testing framework connected to HDMI? Only notable discrepancy
from generic config is enabled 'efidebug' command.


Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
USB Keyboard failure rates:
connected console            02/10
connected hdmi               06/10
connected console + hdmi     07/10
** USB Keyboard always detected by 'usb info', just does not respond.

USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
connected console + hdmi     00/10



  Just to note subsequent Grub2 menu redraw to my 180x50 console terminal
is about 5x faster when hdmi disconnected.



Thanks for testing!
Regards,
Filip



FW: rpi-firmware-20220830-1.0.1.el8.noarch
Revision: 9bd3d354a1a0712ac27c717df9ad60566b0406ee







On 12/17/22 23:24, Simon Glass wrote:
> Hi Filip,
> 
> On Sat, 17 Dec 2022 at 05:50, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>
>>
>>
>> Hello,
>>    change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
>> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
>> When reverted, usb keyboard works as expected.
>>
>> Anybody sitting front of RPi3B+ care to confirm?
>>
>>
>> Regards,
>> Filip
>>
>>
>>
>>
>>
>> Patch:
>> https://urldefense.com/v3/__https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjjc1sSD1$
>>
>>
>> Debug:
>> USB KBD: found interrupt EP: 0x81
>> USB KBD: set boot protocol
>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>> udev->dev='usb_kbd', portnr=3
>> USB KBD: set idle interval...
>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>> udev->dev='usb_kbd', portnr=3
>> USB KBD: enable interrupt pipe...
>> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
>>
>>
>> Tested:
>> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
>> HW: USB 1.1 and 2.0 keyboards, RPi3B+
>> JeOS:
>> https://urldefense.com/v3/__http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjkilUmrv$
>> (u-boot-rpiarm64-2022.10-1.1.aarch64)
>>
>>
> 
> This seems to work on current master:
> 
> do-try-int.sh rpi3
> Revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee, board rpi3
> 
> Checking revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee
> /vid/software/devel/ubtest
> tbot starting ...
> ├─Parameters:
> │     rev        = '9bd3d354a1a0712ac27c717df9ad60566b0406ee'
> │     clean      = False
> ├─Calling uboot_build_and_flash ...
> │   ├─Calling uboot_build ...
> │   │   ├─Calling uboot_checkout ...
> │   │   │   ├─Builder: rpi3
> │   │   │   └─Done. (0.181s)
> │   │   ├─Configuring build ...
> │   │   ├─Calling uboot_make ...
> │   │   │   └─Done. (1.504s)
> │   │   └─Done. (2.178s)
> │   ├─Calling uboot_flash ...
> │   │   ├─Calling copy ...
> │   │   │   └─Done. (0.004s)
> │   │   └─Done. (1.119s)
> │   └─Done. (3.663s)
> ├─────────────────────────────────────────
> └─SUCCESS (3.799s)
> tbot starting ...
> ├─Calling interactive_board ...
> │   ├─POWERON (rpi3)
> │   ├─Entering interactive shell (CTRL+D to exit) ...
> 
> 
> U-Boot 2023.01-rc3-00057-g9bd3d354a1a (Dec 17 2022 - 15:23:14 -0700)
> 
> DRAM:  992 MiB
> RPI 3 Model B (0xa22082)
> Core:  66 devices, 14 uclasses, devicetree: embed
> MMC:   mmc@7e202000: 0, mmc@7e300000: 1
> Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1...
> In:    serial
> Out:   vidconsole
> Err:   vidconsole
> Net:   No ethernet found.
> starting USB...
> Bus usb@7e980000: USB DWC2
> scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll
> on interrupt endpoint
> Failed to get keyboard state from device 0c40:8000
> 6 USB Device(s) found
>         scanning usb for storage devices... 1 Storage Device(s) found
> Hit any key to stop autoboot:  0
> U-Boot> usb info
> 1: Hub,  USB Revision 1.10
>   -  U-Boot Root Hub
>   - Class: Hub
>   - PacketSize: 8  Configurations: 1
>   - Vendor: 0x0000  Product 0x0000 Version 0.0
>     Configuration: 1
>     - Interfaces: 1 Self Powered 0mA
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 1
>       - Class Hub
>       - Endpoint 1 In Interrupt MaxPacket 2 Interval 255ms
> 
> 2: Hub,  USB Revision 2.0
>   - Class: Hub
>   - PacketSize: 64  Configurations: 1
>   - Vendor: 0x0424  Product 0x9514 Version 2.0
>     Configuration: 1
>     - Interfaces: 1 Self Powered Remote Wakeup 2mA
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 1
>       - Class Hub
>       - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
>       - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
> 
> 3: Human Interface,  USB Revision 1.10
>   - Logitech USB Keyboard
>   - Class: (from Interface) Human Interface
>   - PacketSize: 8  Configurations: 1
>   - Vendor: 0x046d  Product 0xc31c Version 100.0
>     Configuration: 1
>     - Interfaces: 2 Bus Powered Remote Wakeup 90mA
>     - String: "U64.00_B0001"
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 1
>       - Class Human Interface, Subclass: Boot Keyboard
>       - String: "USB Keyboard"
>       - Endpoint 1 In Interrupt MaxPacket 8 Interval 10ms
>       Interface: 1
>       - Alternate Setting 0, Endpoints: 1
>       - Class Human Interface, Subclass: None
>       - String: "USB Keyboard"
>       - Endpoint 2 In Interrupt MaxPacket 4 Interval 255ms
> 
> 4: Human Interface,  USB Revision 0.1
>   - QDtech MPI5001 ?
>   - Class: (from Interface) Human Interface
>   - PacketSize: 64  Configurations: 1
>   - Vendor: 0x0483  Product 0x5750 Version 2.0
>     Configuration: 1
>     - Interfaces: 1 Bus Powered 64mA
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 1
>       - Class Human Interface, Subclass: None
>       - Endpoint 2 In Interrupt MaxPacket 64 Interval 1ms
>       - Endpoint 2 Out Interrupt MaxPacket 64 Interval 1ms
> 
> 5: Vendor specific,  USB Revision 2.0
>   - Class: Vendor specific
>   - PacketSize: 64  Configurations: 1
>   - Vendor: 0x0424  Product 0xec00 Version 2.0
>     Configuration: 1
>     - Interfaces: 1 Self Powered Remote Wakeup 2mA
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 3
>       - Class Vendor specific
>       - Endpoint 1 In Bulk MaxPacket 512
>       - Endpoint 2 Out Bulk MaxPacket 512
>       - Endpoint 3 In Interrupt MaxPacket 16 Interval 4ms
> 
> 6: Mass Storage,  USB Revision 2.10
>   - SanDisk USB Extreme Pro 513456791960
>   - Class: (from Interface) Mass Storage
>   - PacketSize: 64  Configurations: 1
>   - Vendor: 0x0781  Product 0x5588 Version 1.0
>     Configuration: 1
>     - Interfaces: 1 Self Powered 0mA
>       Interface: 0
>       - Alternate Setting 0, Endpoints: 2
>       - Class Mass Storage, Transp. SCSI, Bulk only
>       - Endpoint 1 In Bulk MaxPacket 512
>       - Endpoint 2 Out Bulk MaxPacket 512
> 
> U-Boot> hello
> Unknown command 'hello' - try 'help'
> U-Boot>
> │   ├─POWEROFF (rpi3)
> │   ├─Exiting poweroff
> │   └─Done. (34.381s)
> ├─────────────────────────────────────────
> └─SUCCESS (34.481s)
> 
> (the 'hello' is typed on the keyboard)
> 
> Regards,
> Simon

Re: [External] : Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Mon, 19 Dec 2022 at 02:29, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>
>
>
> Hi Simon,
>
>   is your testing framework connected to HDMI? Only notable discrepancy
> from generic config is enabled 'efidebug' command.
>
>
> Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
> USB Keyboard failure rates:
> connected console            02/10
> connected hdmi               06/10
> connected console + hdmi     07/10
> ** USB Keyboard always detected by 'usb info', just does not respond.
>
> USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
> connected console + hdmi     00/10

Yes this one does have HDMI. Are you wanting me to run multiple runs?
With or without the display?

Regards,
Simon


>
>
>
>   Just to note subsequent Grub2 menu redraw to my 180x50 console terminal
> is about 5x faster when hdmi disconnected.
>
>
>
> Thanks for testing!
> Regards,
> Filip
>
>
>
> FW: rpi-firmware-20220830-1.0.1.el8.noarch
> Revision: 9bd3d354a1a0712ac27c717df9ad60566b0406ee
>
>
>
>
>
>
>
> On 12/17/22 23:24, Simon Glass wrote:
> > Hi Filip,
> >
> > On Sat, 17 Dec 2022 at 05:50, Filip Žaludek <filip.zaludek@oracle.com> wrote:
> >>
> >>
> >>
> >> Hello,
> >>    change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
> >> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
> >> When reverted, usb keyboard works as expected.
> >>
> >> Anybody sitting front of RPi3B+ care to confirm?
> >>
> >>
> >> Regards,
> >> Filip
> >>
> >>
> >>
> >>
> >>
> >> Patch:
> >> https://urldefense.com/v3/__https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjjc1sSD1$
> >>
> >>
> >> Debug:
> >> USB KBD: found interrupt EP: 0x81
> >> USB KBD: set boot protocol
> >> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> >> udev->dev='usb_kbd', portnr=3
> >> USB KBD: set idle interval...
> >> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> >> udev->dev='usb_kbd', portnr=3
> >> USB KBD: enable interrupt pipe...
> >> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> >>
> >>
> >> Tested:
> >> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
> >> HW: USB 1.1 and 2.0 keyboards, RPi3B+
> >> JeOS:
> >> https://urldefense.com/v3/__http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjkilUmrv$
> >> (u-boot-rpiarm64-2022.10-1.1.aarch64)
> >>
> >>
> >
> > This seems to work on current master:
> >
> > do-try-int.sh rpi3
> > Revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee, board rpi3
> >
> > Checking revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee
> > /vid/software/devel/ubtest
> > tbot starting ...
> > ├─Parameters:
> > │     rev        = '9bd3d354a1a0712ac27c717df9ad60566b0406ee'
> > │     clean      = False
> > ├─Calling uboot_build_and_flash ...
> > │   ├─Calling uboot_build ...
> > │   │   ├─Calling uboot_checkout ...
> > │   │   │   ├─Builder: rpi3
> > │   │   │   └─Done. (0.181s)
> > │   │   ├─Configuring build ...
> > │   │   ├─Calling uboot_make ...
> > │   │   │   └─Done. (1.504s)
> > │   │   └─Done. (2.178s)
> > │   ├─Calling uboot_flash ...
> > │   │   ├─Calling copy ...
> > │   │   │   └─Done. (0.004s)
> > │   │   └─Done. (1.119s)
> > │   └─Done. (3.663s)
> > ├─────────────────────────────────────────
> > └─SUCCESS (3.799s)
> > tbot starting ...
> > ├─Calling interactive_board ...
> > │   ├─POWERON (rpi3)
> > │   ├─Entering interactive shell (CTRL+D to exit) ...
> >
> >
> > U-Boot 2023.01-rc3-00057-g9bd3d354a1a (Dec 17 2022 - 15:23:14 -0700)
> >
> > DRAM:  992 MiB
> > RPI 3 Model B (0xa22082)
> > Core:  66 devices, 14 uclasses, devicetree: embed
> > MMC:   mmc@7e202000: 0, mmc@7e300000: 1
> > Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1...
> > In:    serial
> > Out:   vidconsole
> > Err:   vidconsole
> > Net:   No ethernet found.
> > starting USB...
> > Bus usb@7e980000: USB DWC2
> > scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll
> > on interrupt endpoint
> > Failed to get keyboard state from device 0c40:8000
> > 6 USB Device(s) found
> >         scanning usb for storage devices... 1 Storage Device(s) found
> > Hit any key to stop autoboot:  0
> > U-Boot> usb info
> > 1: Hub,  USB Revision 1.10
> >   -  U-Boot Root Hub
> >   - Class: Hub
> >   - PacketSize: 8  Configurations: 1
> >   - Vendor: 0x0000  Product 0x0000 Version 0.0
> >     Configuration: 1
> >     - Interfaces: 1 Self Powered 0mA
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 1
> >       - Class Hub
> >       - Endpoint 1 In Interrupt MaxPacket 2 Interval 255ms
> >
> > 2: Hub,  USB Revision 2.0
> >   - Class: Hub
> >   - PacketSize: 64  Configurations: 1
> >   - Vendor: 0x0424  Product 0x9514 Version 2.0
> >     Configuration: 1
> >     - Interfaces: 1 Self Powered Remote Wakeup 2mA
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 1
> >       - Class Hub
> >       - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
> >       - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
> >
> > 3: Human Interface,  USB Revision 1.10
> >   - Logitech USB Keyboard
> >   - Class: (from Interface) Human Interface
> >   - PacketSize: 8  Configurations: 1
> >   - Vendor: 0x046d  Product 0xc31c Version 100.0
> >     Configuration: 1
> >     - Interfaces: 2 Bus Powered Remote Wakeup 90mA
> >     - String: "U64.00_B0001"
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 1
> >       - Class Human Interface, Subclass: Boot Keyboard
> >       - String: "USB Keyboard"
> >       - Endpoint 1 In Interrupt MaxPacket 8 Interval 10ms
> >       Interface: 1
> >       - Alternate Setting 0, Endpoints: 1
> >       - Class Human Interface, Subclass: None
> >       - String: "USB Keyboard"
> >       - Endpoint 2 In Interrupt MaxPacket 4 Interval 255ms
> >
> > 4: Human Interface,  USB Revision 0.1
> >   - QDtech MPI5001 ?
> >   - Class: (from Interface) Human Interface
> >   - PacketSize: 64  Configurations: 1
> >   - Vendor: 0x0483  Product 0x5750 Version 2.0
> >     Configuration: 1
> >     - Interfaces: 1 Bus Powered 64mA
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 1
> >       - Class Human Interface, Subclass: None
> >       - Endpoint 2 In Interrupt MaxPacket 64 Interval 1ms
> >       - Endpoint 2 Out Interrupt MaxPacket 64 Interval 1ms
> >
> > 5: Vendor specific,  USB Revision 2.0
> >   - Class: Vendor specific
> >   - PacketSize: 64  Configurations: 1
> >   - Vendor: 0x0424  Product 0xec00 Version 2.0
> >     Configuration: 1
> >     - Interfaces: 1 Self Powered Remote Wakeup 2mA
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 3
> >       - Class Vendor specific
> >       - Endpoint 1 In Bulk MaxPacket 512
> >       - Endpoint 2 Out Bulk MaxPacket 512
> >       - Endpoint 3 In Interrupt MaxPacket 16 Interval 4ms
> >
> > 6: Mass Storage,  USB Revision 2.10
> >   - SanDisk USB Extreme Pro 513456791960
> >   - Class: (from Interface) Mass Storage
> >   - PacketSize: 64  Configurations: 1
> >   - Vendor: 0x0781  Product 0x5588 Version 1.0
> >     Configuration: 1
> >     - Interfaces: 1 Self Powered 0mA
> >       Interface: 0
> >       - Alternate Setting 0, Endpoints: 2
> >       - Class Mass Storage, Transp. SCSI, Bulk only
> >       - Endpoint 1 In Bulk MaxPacket 512
> >       - Endpoint 2 Out Bulk MaxPacket 512
> >
> > U-Boot> hello
> > Unknown command 'hello' - try 'help'
> > U-Boot>
> > │   ├─POWEROFF (rpi3)
> > │   ├─Exiting poweroff
> > │   └─Done. (34.381s)
> > ├─────────────────────────────────────────
> > └─SUCCESS (34.481s)
> >
> > (the 'hello' is typed on the keyboard)
> >
> > Regards,
> > Simon

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,


On 12/19/22 20:20, Simon Glass wrote:
> Hi Filip,
> 
> On Mon, 19 Dec 2022 at 02:29, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>
>>
>>
>> Hi Simon,
>>
>>    is your testing framework connected to HDMI? Only notable discrepancy
>> from generic config is enabled 'efidebug' command.
>>
>>
>> Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
>> USB Keyboard failure rates:
>> connected console            02/10
>> connected hdmi               06/10
>> connected console + hdmi     07/10
>> ** USB Keyboard always detected by 'usb info', just does not respond.
>>
>> USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
>> connected console + hdmi     00/10
> 
> Yes this one does have HDMI. Are you wanting me to run multiple runs?
> With or without the display?


  Yes please! With HDMI as there is better chance you will hit issue I am experiencing,
to confirm 96991e652f541323a03c5b7e075d54a117091618 is inferior for RPi3.
You should see usb_kbd detected, but no input possible.


Thanks,
Filip





> 
> Regards,
> Simon
> 
> 
>>
>>
>>
>>    Just to note subsequent Grub2 menu redraw to my 180x50 console terminal
>> is about 5x faster when hdmi disconnected.
>>
>>
>>
>> Thanks for testing!
>> Regards,
>> Filip
>>
>>
>>
>> FW: rpi-firmware-20220830-1.0.1.el8.noarch
>> Revision: 9bd3d354a1a0712ac27c717df9ad60566b0406ee
>>
>>
>>
>>
>>
>>
>>
>> On 12/17/22 23:24, Simon Glass wrote:
>>> Hi Filip,
>>>
>>> On Sat, 17 Dec 2022 at 05:50, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>>>
>>>>
>>>>
>>>> Hello,
>>>>     change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
>>>> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
>>>> When reverted, usb keyboard works as expected.
>>>>
>>>> Anybody sitting front of RPi3B+ care to confirm?
>>>>
>>>>
>>>> Regards,
>>>> Filip
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Patch:
>>>> https://urldefense.com/v3/__https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjjc1sSD1$
>>>>
>>>>
>>>> Debug:
>>>> USB KBD: found interrupt EP: 0x81
>>>> USB KBD: set boot protocol
>>>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>>>> udev->dev='usb_kbd', portnr=3
>>>> USB KBD: set idle interval...
>>>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>>>> udev->dev='usb_kbd', portnr=3
>>>> USB KBD: enable interrupt pipe...
>>>> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
>>>>
>>>>
>>>> Tested:
>>>> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
>>>> HW: USB 1.1 and 2.0 keyboards, RPi3B+
>>>> JeOS:
>>>> https://urldefense.com/v3/__http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz__;!!ACWV5N9M2RV99hQ!MunypRlNhelLw1nrbbLFCTeZK2pCAlIH_WPhgvmgFhfy3wJnPA5titNwl3o2fkc-mCqWAgJxjkilUmrv$
>>>> (u-boot-rpiarm64-2022.10-1.1.aarch64)
>>>>
>>>>
>>>
>>> This seems to work on current master:
>>>
>>> do-try-int.sh rpi3
>>> Revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee, board rpi3
>>>
>>> Checking revision 9bd3d354a1a0712ac27c717df9ad60566b0406ee
>>> /vid/software/devel/ubtest
>>> tbot starting ...
>>> ├─Parameters:
>>> │     rev        = '9bd3d354a1a0712ac27c717df9ad60566b0406ee'
>>> │     clean      = False
>>> ├─Calling uboot_build_and_flash ...
>>> │   ├─Calling uboot_build ...
>>> │   │   ├─Calling uboot_checkout ...
>>> │   │   │   ├─Builder: rpi3
>>> │   │   │   └─Done. (0.181s)
>>> │   │   ├─Configuring build ...
>>> │   │   ├─Calling uboot_make ...
>>> │   │   │   └─Done. (1.504s)
>>> │   │   └─Done. (2.178s)
>>> │   ├─Calling uboot_flash ...
>>> │   │   ├─Calling copy ...
>>> │   │   │   └─Done. (0.004s)
>>> │   │   └─Done. (1.119s)
>>> │   └─Done. (3.663s)
>>> ├─────────────────────────────────────────
>>> └─SUCCESS (3.799s)
>>> tbot starting ...
>>> ├─Calling interactive_board ...
>>> │   ├─POWERON (rpi3)
>>> │   ├─Entering interactive shell (CTRL+D to exit) ...
>>>
>>>
>>> U-Boot 2023.01-rc3-00057-g9bd3d354a1a (Dec 17 2022 - 15:23:14 -0700)
>>>
>>> DRAM:  992 MiB
>>> RPI 3 Model B (0xa22082)
>>> Core:  66 devices, 14 uclasses, devicetree: embed
>>> MMC:   mmc@7e202000: 0, mmc@7e300000: 1
>>> Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1...
>>> In:    serial
>>> Out:   vidconsole
>>> Err:   vidconsole
>>> Net:   No ethernet found.
>>> starting USB...
>>> Bus usb@7e980000: USB DWC2
>>> scanning bus usb@7e980000 for devices... usb_kbd usb_kbd: Timeout poll
>>> on interrupt endpoint
>>> Failed to get keyboard state from device 0c40:8000
>>> 6 USB Device(s) found
>>>          scanning usb for storage devices... 1 Storage Device(s) found
>>> Hit any key to stop autoboot:  0
>>> U-Boot> usb info
>>> 1: Hub,  USB Revision 1.10
>>>    -  U-Boot Root Hub
>>>    - Class: Hub
>>>    - PacketSize: 8  Configurations: 1
>>>    - Vendor: 0x0000  Product 0x0000 Version 0.0
>>>      Configuration: 1
>>>      - Interfaces: 1 Self Powered 0mA
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 1
>>>        - Class Hub
>>>        - Endpoint 1 In Interrupt MaxPacket 2 Interval 255ms
>>>
>>> 2: Hub,  USB Revision 2.0
>>>    - Class: Hub
>>>    - PacketSize: 64  Configurations: 1
>>>    - Vendor: 0x0424  Product 0x9514 Version 2.0
>>>      Configuration: 1
>>>      - Interfaces: 1 Self Powered Remote Wakeup 2mA
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 1
>>>        - Class Hub
>>>        - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
>>>        - Endpoint 1 In Interrupt MaxPacket 1 Interval 12ms
>>>
>>> 3: Human Interface,  USB Revision 1.10
>>>    - Logitech USB Keyboard
>>>    - Class: (from Interface) Human Interface
>>>    - PacketSize: 8  Configurations: 1
>>>    - Vendor: 0x046d  Product 0xc31c Version 100.0
>>>      Configuration: 1
>>>      - Interfaces: 2 Bus Powered Remote Wakeup 90mA
>>>      - String: "U64.00_B0001"
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 1
>>>        - Class Human Interface, Subclass: Boot Keyboard
>>>        - String: "USB Keyboard"
>>>        - Endpoint 1 In Interrupt MaxPacket 8 Interval 10ms
>>>        Interface: 1
>>>        - Alternate Setting 0, Endpoints: 1
>>>        - Class Human Interface, Subclass: None
>>>        - String: "USB Keyboard"
>>>        - Endpoint 2 In Interrupt MaxPacket 4 Interval 255ms
>>>
>>> 4: Human Interface,  USB Revision 0.1
>>>    - QDtech MPI5001 ?
>>>    - Class: (from Interface) Human Interface
>>>    - PacketSize: 64  Configurations: 1
>>>    - Vendor: 0x0483  Product 0x5750 Version 2.0
>>>      Configuration: 1
>>>      - Interfaces: 1 Bus Powered 64mA
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 1
>>>        - Class Human Interface, Subclass: None
>>>        - Endpoint 2 In Interrupt MaxPacket 64 Interval 1ms
>>>        - Endpoint 2 Out Interrupt MaxPacket 64 Interval 1ms
>>>
>>> 5: Vendor specific,  USB Revision 2.0
>>>    - Class: Vendor specific
>>>    - PacketSize: 64  Configurations: 1
>>>    - Vendor: 0x0424  Product 0xec00 Version 2.0
>>>      Configuration: 1
>>>      - Interfaces: 1 Self Powered Remote Wakeup 2mA
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 3
>>>        - Class Vendor specific
>>>        - Endpoint 1 In Bulk MaxPacket 512
>>>        - Endpoint 2 Out Bulk MaxPacket 512
>>>        - Endpoint 3 In Interrupt MaxPacket 16 Interval 4ms
>>>
>>> 6: Mass Storage,  USB Revision 2.10
>>>    - SanDisk USB Extreme Pro 513456791960
>>>    - Class: (from Interface) Mass Storage
>>>    - PacketSize: 64  Configurations: 1
>>>    - Vendor: 0x0781  Product 0x5588 Version 1.0
>>>      Configuration: 1
>>>      - Interfaces: 1 Self Powered 0mA
>>>        Interface: 0
>>>        - Alternate Setting 0, Endpoints: 2
>>>        - Class Mass Storage, Transp. SCSI, Bulk only
>>>        - Endpoint 1 In Bulk MaxPacket 512
>>>        - Endpoint 2 Out Bulk MaxPacket 512
>>>
>>> U-Boot> hello
>>> Unknown command 'hello' - try 'help'
>>> U-Boot>
>>> │   ├─POWEROFF (rpi3)
>>> │   ├─Exiting poweroff
>>> │   └─Done. (34.381s)
>>> ├─────────────────────────────────────────
>>> └─SUCCESS (34.481s)
>>>
>>> (the 'hello' is typed on the keyboard)
>>>
>>> Regards,
>>> Simon

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Mon, 19 Dec 2022 at 14:25, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>
>
>
> Hi Simon,
>
>
> On 12/19/22 20:20, Simon Glass wrote:
> > Hi Filip,
> >
> > On Mon, 19 Dec 2022 at 02:29, Filip Žaludek <filip.zaludek@oracle.com> wrote:
> >>
> >>
> >>
> >> Hi Simon,
> >>
> >>    is your testing framework connected to HDMI? Only notable discrepancy
> >> from generic config is enabled 'efidebug' command.
> >>
> >>
> >> Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
> >> USB Keyboard failure rates:
> >> connected console            02/10
> >> connected hdmi               06/10
> >> connected console + hdmi     07/10
> >> ** USB Keyboard always detected by 'usb info', just does not respond.
> >>
> >> USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
> >> connected console + hdmi     00/10
> >
> > Yes this one does have HDMI. Are you wanting me to run multiple runs?
> > With or without the display?
>
>
>   Yes please! With HDMI as there is better chance you will hit issue I am experiencing,
> to confirm 96991e652f541323a03c5b7e075d54a117091618 is inferior for RPi3.
> You should see usb_kbd detected, but no input possible.

I am not seeing this problem, but the HDMI is not actually working. It
could be because of a setting in the silly config.txt file, e.g.
gpu_freq=250

The display comes to life and then says no signal.

But I tried it 10 times on us/next and the USB keyboard worked each time.

Regards,
Simon

[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,
  hmm this is strange. I am hitting this usually before 10 repetitions,
for sure within 30 repetitions. 'gpu_freq' is missing, thus default.
I can see this also from stock JeOS/RPi3 from OpenSUSE.
Do you have enabled debug as it works as workaround? [usb_hub.c]


  Dissected more, thoughts into record:
** not reproduced on underclocked RPi4
** not reproduced with hardcoded poll_delay=0 [common/usb_kbd.c]
** reproduced with hardcoded poll_delay=1 [common/usb_kbd.c]
** not reproduced when '#define LOG_DEBUG' enabled in common/usb_hub.c
** workaround is to output character in non 'UCLASS_USB_HUB' branch,
  even mdelay(500) instead of printing character would not help.


  Seems to be synchronization problem, please advise if you have any
ideas/suggestions how to troubleshoot.



%< -------------------------------------------------------------
diff --git a/common/usb_hub.c b/common/usb_hub.c
index 95f1449..817e15e 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -73,8 +73,10 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev)
  #if CONFIG_IS_ENABLED(DM_USB)
  bool usb_hub_is_root_hub(struct udevice *hub)
  {
-       if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB)
+       if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB) {
+               puts(".");
                 return true;
+       }

         return false;
  }

%< -------------------------------------------------------------





Happy new year!
Regards,
Filip





On 1/3/23 18:02, Simon Glass wrote:
> Hi Filip,
> 
> On Mon, 19 Dec 2022 at 14:25, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>
>>
>>
>> Hi Simon,
>>
>>
>> On 12/19/22 20:20, Simon Glass wrote:
>>> Hi Filip,
>>>
>>> On Mon, 19 Dec 2022 at 02:29, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>>>
>>>>
>>>>
>>>> Hi Simon,
>>>>
>>>>     is your testing framework connected to HDMI? Only notable discrepancy
>>>> from generic config is enabled 'efidebug' command.
>>>>
>>>>
>>>> Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
>>>> USB Keyboard failure rates:
>>>> connected console            02/10
>>>> connected hdmi               06/10
>>>> connected console + hdmi     07/10
>>>> ** USB Keyboard always detected by 'usb info', just does not respond.
>>>>
>>>> USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
>>>> connected console + hdmi     00/10
>>>
>>> Yes this one does have HDMI. Are you wanting me to run multiple runs?
>>> With or without the display?
>>
>>
>>    Yes please! With HDMI as there is better chance you will hit issue I am experiencing,
>> to confirm 96991e652f541323a03c5b7e075d54a117091618 is inferior for RPi3.
>> You should see usb_kbd detected, but no input possible.
> 
> I am not seeing this problem, but the HDMI is not actually working. It
> could be because of a setting in the silly config.txt file, e.g.
> gpu_freq=250
> 
> The display comes to life and then says no signal.
> 
> But I tried it 10 times on us/next and the USB keyboard worked each time.
> 
> Regards,
> Simon

[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,
   purchased 'Logitech K120 keyboard Vend: 0x046d  Prod: 0xc31c' and 'SanDisk USB Extreme Pro 53A45678B3C1'
to mimic your setup, I can still reproduce the issue. Major discrepancy is u-boot does not recognize
SanDisk USB Extreme Pro.

Could you please share your u-boot config, config.txt, firmware version and dtbs? (Or better os image you are using for 
testing)

Could you please retest once again, but unplugged all usb devices except keyboard? Please see refined reproducer..



Prerequisities:
* RPi3B or RPi3B+
* unplugged all usb devices except keyboard, (both usb 1.1 and 2.0 tested)
* RPi3 connected to console and HDMI monitor, (monitor is not requirement)
* u-boot from master compiled without debugging as it works as workaround (reproducible with current JeOS-20230110)

Refined reproducer:
* Enter 'U-Boot>' shell using usb keyboard, (always works)
* repeat 'usb reset' until usb keyboard responds, (press ENTER, or ARROW UP followed by ENTER [to see responsiveness])
* keyboard can be resurrected by subsequent 'usb reset' from console

Workarounds:
* revert '96991e652f541323a03c5b7e075d54a117091618'
* hardcode poll_delay=0 [common/usb_kbd.c]
* enable '#define LOG_DEBUG' in [common/usb_hub.c]
* output character from semi random places [usb.c, usb_hub.c, device.c, usb-uclass.c, dwc2.c]



Cordial regards,
Filip







On 1/3/23 18:36, Filip Žaludek wrote:
> 
> 
> Hi Simon,
>   hmm this is strange. I am hitting this usually before 10 repetitions,
> for sure within 30 repetitions. 'gpu_freq' is missing, thus default.
> I can see this also from stock JeOS/RPi3 from OpenSUSE.
> Do you have enabled debug as it works as workaround? [usb_hub.c]
> 
> 
>   Dissected more, thoughts into record:
> ** not reproduced on underclocked RPi4
> ** not reproduced with hardcoded poll_delay=0 [common/usb_kbd.c]
> ** reproduced with hardcoded poll_delay=1 [common/usb_kbd.c]
> ** not reproduced when '#define LOG_DEBUG' enabled in common/usb_hub.c
> ** workaround is to output character in non 'UCLASS_USB_HUB' branch,
>   even mdelay(500) instead of printing character would not help.
> 
> 
>   Seems to be synchronization problem, please advise if you have any
> ideas/suggestions how to troubleshoot.
> 
> 
> 
> %< -------------------------------------------------------------
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index 95f1449..817e15e 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -73,8 +73,10 @@ static inline bool usb_hub_is_superspeed(struct usb_device *hdev)
>   #if CONFIG_IS_ENABLED(DM_USB)
>   bool usb_hub_is_root_hub(struct udevice *hub)
>   {
> -       if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB)
> +       if (device_get_uclass_id(hub->parent) != UCLASS_USB_HUB) {
> +               puts(".");
>                  return true;
> +       }
> 
>          return false;
>   }
> 
> %< -------------------------------------------------------------
> 
> 
> 
> 
> 
> Happy new year!
> Regards,
> Filip
> 
> 
> 
> 
> 
> On 1/3/23 18:02, Simon Glass wrote:
>> Hi Filip,
>>
>> On Mon, 19 Dec 2022 at 14:25, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>>
>>>
>>>
>>> Hi Simon,
>>>
>>>
>>> On 12/19/22 20:20, Simon Glass wrote:
>>>> Hi Filip,
>>>>
>>>> On Mon, 19 Dec 2022 at 02:29, Filip Žaludek <filip.zaludek@oracle.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>>     is your testing framework connected to HDMI? Only notable discrepancy
>>>>> from generic config is enabled 'efidebug' command.
>>>>>
>>>>>
>>>>> Tested more (cycled 'U-Boot>' and 'reset'), both RPi3B and RPi3B+..
>>>>> USB Keyboard failure rates:
>>>>> connected console            02/10
>>>>> connected hdmi               06/10
>>>>> connected console + hdmi     07/10
>>>>> ** USB Keyboard always detected by 'usb info', just does not respond.
>>>>>
>>>>> USB Keyboard failure rates, reverted 96991e652f541323a03c5b7e075d54a117091618:
>>>>> connected console + hdmi     00/10
>>>>
>>>> Yes this one does have HDMI. Are you wanting me to run multiple runs?
>>>> With or without the display?
>>>
>>>
>>>    Yes please! With HDMI as there is better chance you will hit issue I am experiencing,
>>> to confirm 96991e652f541323a03c5b7e075d54a117091618 is inferior for RPi3.
>>> You should see usb_kbd detected, but no input possible.
>>
>> I am not seeing this problem, but the HDMI is not actually working. It
>> could be because of a setting in the silly config.txt file, e.g.
>> gpu_freq=250
>>
>> The display comes to life and then says no signal.
>>
>> But I tried it 10 times on us/next and the USB keyboard worked each time.
>>
>> Regards,
>> Simon

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Michal Suchánek]

Hello,

On Sat, Dec 17, 2022 at 01:49:47PM +0100, Filip Žaludek wrote:
> 
> 
> Hello,
>  change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
> When reverted, usb keyboard works as expected.
> 
> Anybody sitting front of RPi3B+ care to confirm?

For me an USB keyboard connected to a Raspberry Pi 3B+ is only detected
when something is typed during the probe.

The message

usb_kbd usb_kbd: Timeout poll on interrupt endpoint

is a sign of nothing typed on the keyboard and then it does not work.

It is limitation of the dwc2 hardware with hardwired USB hub.

Same on both v2022.10 & v2023.01 rpiarm64 and rpi_3 configs.

Thanks

Michal

> 
> 
> Regards,
> Filip
> 
> 
> 
> 
> 
> Patch:
> https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618
> 
> 
> Debug:
> USB KBD: found interrupt EP: 0x81
> USB KBD: set boot protocol
> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> udev->dev='usb_kbd', portnr=3
> USB KBD: set idle interval...
> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> udev->dev='usb_kbd', portnr=3
> USB KBD: enable interrupt pipe...
> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> 
> 
> Tested:
> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
> HW: USB 1.1 and 2.0 keyboards, RPi3B+
> JeOS:
> http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz
> (u-boot-rpiarm64-2022.10-1.1.aarch64)
> 
> 

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

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.
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

'console: usb: kbd: Limit poll frequency to improve performance'
commit 96991e652f541323a03c5b7e075d54a117091618


What will be attitude on this, should we try to bring workaround preserving both patches?



Regards,
Filip






On 1/17/23 19:46, Michal Suchánek wrote:
> Hello,
> 
> On Sat, Dec 17, 2022 at 01:49:47PM +0100, Filip Žaludek wrote:
>>
>>
>> Hello,
>>   change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
>> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
>> When reverted, usb keyboard works as expected.
>>
>> Anybody sitting front of RPi3B+ care to confirm?
> 
> For me an USB keyboard connected to a Raspberry Pi 3B+ is only detected
> when something is typed during the probe.
> 
> The message
> 
> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> 
> is a sign of nothing typed on the keyboard and then it does not work.
> 
> It is limitation of the dwc2 hardware with hardwired USB hub.
> 
> Same on both v2022.10 & v2023.01 rpiarm64 and rpi_3 configs.
> 
> Thanks
> 
> Michal
> 
>>
>>
>> Regards,
>> Filip
>>
>>
>>
>>
>>
>> Patch:
>> https://urldefense.com/v3/__https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618__;!!ACWV5N9M2RV99hQ!J3bR4ePIrYCcbqK8Zd5qG9y4yP6W_sNqMV0BhlIJqrwImk8KRkbMK8K5tzXHZU3BZ3ai6k7v25WDUCtgOBMVk8o$
>>
>>
>> Debug:
>> USB KBD: found interrupt EP: 0x81
>> USB KBD: set boot protocol
>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>> udev->dev='usb_kbd', portnr=3
>> USB KBD: set idle interval...
>> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
>> udev->dev='usb_kbd', portnr=3
>> USB KBD: enable interrupt pipe...
>> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
>>
>>
>> Tested:
>> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
>> HW: USB 1.1 and 2.0 keyboards, RPi3B+
>> JeOS:
>> https://urldefense.com/v3/__http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz__;!!ACWV5N9M2RV99hQ!J3bR4ePIrYCcbqK8Zd5qG9y4yP6W_sNqMV0BhlIJqrwImk8KRkbMK8K5tzXHZU3BZ3ai6k7v25WDUCtgKPUw0V8$
>> (u-boot-rpiarm64-2022.10-1.1.aarch64)
>>
>>

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Wed, 18 Jan 2023 at 09:03, Filip Žaludek <filip.zaludek@oracle.com> 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.
> 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
>
> 'console: usb: kbd: Limit poll frequency to improve performance'
> commit 96991e652f541323a03c5b7e075d54a117091618
>
>
> What will be attitude on this, should we try to bring workaround preserving both patches?

(please try to avoid top-posting)

You can use a USB quirk to handle this.

Marek do you have any thoughts here?

Regards,
Simon



>
>
>
> Regards,
> Filip
>
>
>
>
>
>
> On 1/17/23 19:46, Michal Suchánek wrote:
> > Hello,
> >
> > On Sat, Dec 17, 2022 at 01:49:47PM +0100, Filip Žaludek wrote:
> >>
> >>
> >> Hello,
> >>   change seems to be unfriendly to RPi3B+, it allows to enter 'U-Boot>' shell but usb keyboard
> >> does not respond. Keyboard is detected by 'usb info' in v2023.01-rc3, not in v2022.10.
> >> When reverted, usb keyboard works as expected.
> >>
> >> Anybody sitting front of RPi3B+ care to confirm?
> >
> > For me an USB keyboard connected to a Raspberry Pi 3B+ is only detected
> > when something is typed during the probe.
> >
> > The message
> >
> > usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> >
> > is a sign of nothing typed on the keyboard and then it does not work.
> >
> > It is limitation of the dwc2 hardware with hardwired USB hub.
> >
> > Same on both v2022.10 & v2023.01 rpiarm64 and rpi_3 configs.
> >
> > Thanks
> >
> > Michal
> >
> >>
> >>
> >> Regards,
> >> Filip
> >>
> >>
> >>
> >>
> >>
> >> Patch:
> >> https://urldefense.com/v3/__https://github.com/u-boot/u-boot/commit/96991e652f541323a03c5b7e075d54a117091618__;!!ACWV5N9M2RV99hQ!J3bR4ePIrYCcbqK8Zd5qG9y4yP6W_sNqMV0BhlIJqrwImk8KRkbMK8K5tzXHZU3BZ3ai6k7v25WDUCtgOBMVk8o$
> >>
> >>
> >> Debug:
> >> USB KBD: found interrupt EP: 0x81
> >> USB KBD: set boot protocol
> >> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> >> udev->dev='usb_kbd', portnr=3
> >> USB KBD: set idle interval...
> >> dwc2_submit_control_msg: dev='usb@7e980000', udev=000000003af4be00,
> >> udev->dev='usb_kbd', portnr=3
> >> USB KBD: enable interrupt pipe...
> >> usb_kbd usb_kbd: Timeout poll on interrupt endpoint
> >>
> >>
> >> Tested:
> >> SW: v2022.10 & v2023.01-rc3 compiled from sources as 'rpiarm64'
> >> HW: USB 1.1 and 2.0 keyboards, RPi3B+
> >> JeOS:
> >> https://urldefense.com/v3/__http://download.opensuse.org/ports/aarch64/tumbleweed/appliances/openSUSE-Tumbleweed-ARM-JeOS-raspberrypi.aarch64-2022.10.11-Snapshot20221112.raw.xz__;!!ACWV5N9M2RV99hQ!J3bR4ePIrYCcbqK8Zd5qG9y4yP6W_sNqMV0BhlIJqrwImk8KRkbMK8K5tzXHZU3BZ3ai6k7v25WDUCtgKPUw0V8$
> >> (u-boot-rpiarm64-2022.10-1.1.aarch64)
> >>
> >>

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Michal Suchánek]

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.

Thanks

Michal

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Mark Kettenis]

> Date: Wed, 8 Feb 2023 19:45:36 +0100
> From: Michal Suchánek <msuchanek@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.

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi,

On 2/8/23 19:45, Michal Suchánek wrote:
> 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.

Common denominator here is most likely dwc2 controller on RPi3.



> 
> 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.

There are 3 states:
Keyboard not detected, keyboard detected but does not work, keyboard detected and works.

If keyboard is not detected/does not work then subsequent grub timeout is misleading/pointless..



> 
> The timeout is limitation of the dwc2 controller handling of usb hubs.

  Michal, can you please elaborate more [pointers to docs, discussions] why/how is the timeout
limitation of the dwc2 controller handling of usb hubs?
  I am trying to dissect it more by printing single character from myriad of places what actually
works as a workaround, but when substituting with mdelay(x) exceeding printing does not.
[common/usb.c, common/usb_hub.c, drivers/core/device.c, drivers/usb/host/dwc2.c, drivers/usb/host/usb-uclass.c]



Thanks,
Filip






> 
> 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.
> 
> Thanks
> 
> Michal

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

On 2/8/23 20:01, Mark Kettenis wrote:
>> Date: Wed, 8 Feb 2023 19:45:36 +0100
>> From: Michal Suchánek <msuchanek@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.


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().

  Is it possible/feasible to implement calls in CONFIG_SYS_ARCH_TIMER
and/or CONFIG_SP804_TIMER?

  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.


Kind regards,
Filip

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Tue, 11 Apr 2023 at 14:24, Filip Žaludek <filip.zaludek@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 <msuchanek@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.
>
>
> 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 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

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,


On 4/19/23 03:49, Simon Glass wrote:
> Hi Filip,
> 
> On Tue, 11 Apr 2023 at 14:24, Filip Žaludek <filip.zaludek@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 <msuchanek@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.
>>
>>
>> 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.




>>
>>    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.
* 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.

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.



Thanks,
Filip

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Tue, 25 Apr 2023 at 06:36, Filip Žaludek <filip.zaludek@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.zaludek@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 <msuchanek@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.
> >>
> >>
> >> 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.

>
>
>
>
> >>
> >>    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.

>
> 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

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

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.zaludek@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.zaludek@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 <msuchanek@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);

         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.




> 
>>
>>
>>
>>
>>>>
>>>>     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..




>>
>> 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.


Kind regards,
Filip

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Filip,

On Tue, 2 May 2023 at 12:43, Filip Žaludek <filip.zaludek@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.zaludek@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.zaludek@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 <msuchanek@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

[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Filip Žaludek]

Hi Simon,


On 5/3/23 03:28, Simon Glass wrote:
> Hi Filip,
> 
> On Tue, 2 May 2023 at 12:43, Filip Žaludek <filip.zaludek@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.zaludek@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.zaludek@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 <msuchanek@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.
> 


   After more experiments I finally concluded issue is not due to cache coherency at all.
Workaround actually profited from side effect, time spent cleaning&invalidating dcache.
Flushing dcache works for 512M but not for 256M regardless of target address.

flush_dcache_range(random_addr, random_addr + 256M); //  ~45us
flush_dcache_range(random_addr, random_addr + 512M); //  ~75us
flush_dcache_all();                                  // ~115us

Please see..
'[PATCH] usb: kbd: dwc2: Increase wait for dwc2 controller reset by 125us'




>>
>>           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.
> 

  Simon, could you please outline proper approach how to implement early timer for RPi,
as all current timer_early_get_*() functions are implemented in 'drivers/timer/',
and early timer functionality still rely on dm_timer_init?

1/ Implement timer_early_get_*() in drivers/timer/sp804_timer.c once underestood superior sp804?
2a/ Implement dm version of System Timer, extending arch/arm/cpu/armv8/generic_timer.c?
2b/ Re-implement dm version of System Timer in drivers/timer/?




>>>>>>
>>>>>>      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,
Filip

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

()Hi Thomas,

On Thu, 10 Feb 2022 at 18:43, Thomas Watson <twatson52@icloud.com> wrote:
>
>
> On Feb 10, 2022, at 5:04 PM, Marek Vasut <marex@denx.de> wrote:
>
> On 2/11/22 00:02, Simon Glass wrote:
>
> Hi Marek,
> On Thu, 10 Feb 2022 at 14:42, Marek Vasut <marex@denx.de> wrote:
>
>
> On 2/10/22 22:13, Thomas Watson wrote:
>
> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
> 30-40ms to run. The exact time is dependent on the polling interval the
> keyboard requests in its descriptor, and likely cannot be significantly
> reduced without major rework to the XHCI driver.
>
> The U-Boot EFI console service sets a timer to poll the keyboard every 5
> microseconds, and this timer is checked every time a block is read off
> disk. The net effect is that, on my system, loading a ~40MiB kernel and
> initrd takes about 62 seconds with a slower keyboard and 53 seconds
> with a faster one, with the vast majority of the time spent polling the
> keyboard.
>
> To solve this problem, this patch adds a 20ms delay between consecutive
> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
> total loading time to under half a second for both keyboards, and does
> not impact the perceived keystroke latency.
>
> Signed-off-by: Thomas Watson <twatson52@icloud.com>
> ---
> This revision fixes a sandbox test failure by honoring the test's
> request to skip delays.
>
>   common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
>
> We don't want to delay the CI tests which take enough time as it is.
> So time needs to be faked.
> It could be an if() though, rather than #if, perhaps?
>
>
> Since there are multiple instances of the ifdef already , subsequent patch to clean them up would be fine.
>
>
> That function `state_get_skip_delays()` and that state.h header file in general
> is only defined in the sandbox architecture, so I don't think it's possible to
> remove the #ifdef unless that is changed.

We can add it to a generic header, e.g. timer_skip_delays(), with a
null implementation for other boards and a call to
state_get_skip_delays() for sandbox. We have done that in a few other
cases. I agree we should avoid arch-specific code.

Regards,
Simon

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Thomas Watson]

> On Feb 10, 2022, at 5:04 PM, Marek Vasut <marex@denx.de> wrote:
> 
> On 2/11/22 00:02, Simon Glass wrote:
>> Hi Marek,
>> On Thu, 10 Feb 2022 at 14:42, Marek Vasut <marex@denx.de> wrote:
>>> 
>>> On 2/10/22 22:13, Thomas Watson wrote:
>>>> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
>>>> 30-40ms to run. The exact time is dependent on the polling interval the
>>>> keyboard requests in its descriptor, and likely cannot be significantly
>>>> reduced without major rework to the XHCI driver.
>>>> 
>>>> The U-Boot EFI console service sets a timer to poll the keyboard every 5
>>>> microseconds, and this timer is checked every time a block is read off
>>>> disk. The net effect is that, on my system, loading a ~40MiB kernel and
>>>> initrd takes about 62 seconds with a slower keyboard and 53 seconds
>>>> with a faster one, with the vast majority of the time spent polling the
>>>> keyboard.
>>>> 
>>>> To solve this problem, this patch adds a 20ms delay between consecutive
>>>> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
>>>> total loading time to under half a second for both keyboards, and does
>>>> not impact the perceived keystroke latency.
>>>> 
>>>> Signed-off-by: Thomas Watson <twatson52@icloud.com>
>>>> ---
>>>> This revision fixes a sandbox test failure by honoring the test's
>>>> request to skip delays.
>>>> 
>>>>   common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>>>>   1 file changed, 26 insertions(+), 5 deletions(-)
>> We don't want to delay the CI tests which take enough time as it is.
>> So time needs to be faked.
>> It could be an if() though, rather than #if, perhaps?
> 
> Since there are multiple instances of the ifdef already , subsequent patch to clean them up would be fine.

That function `state_get_skip_delays()` and that state.h header file in general
is only defined in the sandbox architecture, so I don't think it's possible to
remove the #ifdef unless that is changed.

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Marek Vasut]

On 2/11/22 00:02, Simon Glass wrote:
> Hi Marek,
> 
> On Thu, 10 Feb 2022 at 14:42, Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/10/22 22:13, Thomas Watson wrote:
>>> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
>>> 30-40ms to run. The exact time is dependent on the polling interval the
>>> keyboard requests in its descriptor, and likely cannot be significantly
>>> reduced without major rework to the XHCI driver.
>>>
>>> The U-Boot EFI console service sets a timer to poll the keyboard every 5
>>> microseconds, and this timer is checked every time a block is read off
>>> disk. The net effect is that, on my system, loading a ~40MiB kernel and
>>> initrd takes about 62 seconds with a slower keyboard and 53 seconds
>>> with a faster one, with the vast majority of the time spent polling the
>>> keyboard.
>>>
>>> To solve this problem, this patch adds a 20ms delay between consecutive
>>> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
>>> total loading time to under half a second for both keyboards, and does
>>> not impact the perceived keystroke latency.
>>>
>>> Signed-off-by: Thomas Watson <twatson52@icloud.com>
>>> ---
>>> This revision fixes a sandbox test failure by honoring the test's
>>> request to skip delays.
>>>
>>>    common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>>>    1 file changed, 26 insertions(+), 5 deletions(-)
> 
> We don't want to delay the CI tests which take enough time as it is.
> So time needs to be faked.
> 
> It could be an if() though, rather than #if, perhaps?

Since there are multiple instances of the ifdef already , subsequent 
patch to clean them up would be fine.

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Simon Glass]

Hi Marek,

On Thu, 10 Feb 2022 at 14:42, Marek Vasut <marex@denx.de> wrote:
>
> On 2/10/22 22:13, Thomas Watson wrote:
> > Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
> > 30-40ms to run. The exact time is dependent on the polling interval the
> > keyboard requests in its descriptor, and likely cannot be significantly
> > reduced without major rework to the XHCI driver.
> >
> > The U-Boot EFI console service sets a timer to poll the keyboard every 5
> > microseconds, and this timer is checked every time a block is read off
> > disk. The net effect is that, on my system, loading a ~40MiB kernel and
> > initrd takes about 62 seconds with a slower keyboard and 53 seconds
> > with a faster one, with the vast majority of the time spent polling the
> > keyboard.
> >
> > To solve this problem, this patch adds a 20ms delay between consecutive
> > calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
> > total loading time to under half a second for both keyboards, and does
> > not impact the perceived keystroke latency.
> >
> > Signed-off-by: Thomas Watson <twatson52@icloud.com>
> > ---
> > This revision fixes a sandbox test failure by honoring the test's
> > request to skip delays.
> >
> >   common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
> >   1 file changed, 26 insertions(+), 5 deletions(-)

We don't want to delay the CI tests which take enough time as it is.
So time needs to be faked.

It could be an if() though, rather than #if, perhaps?

Regards,
Simon

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Marek Vasut]

On 2/10/22 23:00, Thomas Watson wrote:
> 
>> On Feb 10, 2022, at 3:42 PM, Marek Vasut <marex@denx.de> wrote:
>>
>> On 2/10/22 22:13, Thomas Watson wrote:
>>> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
>>> 30-40ms to run. The exact time is dependent on the polling interval the
>>> keyboard requests in its descriptor, and likely cannot be significantly
>>> reduced without major rework to the XHCI driver.
>>> The U-Boot EFI console service sets a timer to poll the keyboard every 5
>>> microseconds, and this timer is checked every time a block is read off
>>> disk. The net effect is that, on my system, loading a ~40MiB kernel and
>>> initrd takes about 62 seconds with a slower keyboard and 53 seconds
>>> with a faster one, with the vast majority of the time spent polling the
>>> keyboard.
>>> To solve this problem, this patch adds a 20ms delay between consecutive
>>> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
>>> total loading time to under half a second for both keyboards, and does
>>> not impact the perceived keystroke latency.
>>> Signed-off-by: Thomas Watson <twatson52@icloud.com>
>>> ---
>>> This revision fixes a sandbox test failure by honoring the test's
>>> request to skip delays.
>>>   common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>>>   1 file changed, 26 insertions(+), 5 deletions(-)
>>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>>> index afad260d3d..352d86fb2e 100644
>>> --- a/common/usb_kbd.c
>>> +++ b/common/usb_kbd.c
>>> @@ -17,6 +17,9 @@
>>>   #include <stdio_dev.h>
>>>   #include <watchdog.h>
>>>   #include <asm/byteorder.h>
>>> +#ifdef CONFIG_SANDBOX
>>> +#include <asm/state.h>
>>> +#endif
>>>     #include <usb.h>
>>>   @@ -118,7 +121,7 @@ struct usb_kbd_pdata {
>>>   extern int __maybe_unused net_busy_flag;
>>>     /* The period of time between two calls of usb_kbd_testc(). */
>>> -static unsigned long __maybe_unused kbd_testc_tms;
>>> +static unsigned long kbd_testc_tms;
>>>     /* Puts character in the queue and sets up the in and out pointer. */
>>>   static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c)
>>> @@ -394,21 +397,39 @@ static int usb_kbd_testc(struct stdio_dev *sdev)
>>>   	struct usb_device *usb_kbd_dev;
>>>   	struct usb_kbd_pdata *data;
>>>   +	/*
>>> +	 * Polling the keyboard for an event can take dozens of milliseconds.
>>> +	 * Add a delay between polls to avoid blocking activity which polls
>>> +	 * rapidly, like the UEFI console timer.
>>> +	 */
>>> +	unsigned long poll_delay = CONFIG_SYS_HZ / 50;
>>> +
>>>   #ifdef CONFIG_CMD_NET
>>>   	/*
>>>   	 * If net_busy_flag is 1, NET transfer is running,
>>>   	 * then we check key-pressed every second (first check may be
>>>   	 * less than 1 second) to improve TFTP booting performance.
>>>   	 */
>>> -	if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
>>> -		return 0;
>>> -	kbd_testc_tms = get_timer(0);
>>> +	if (net_busy_flag)
>>> +		poll_delay = CONFIG_SYS_HZ;
>>> +#endif
>>> +
>>> +#ifdef CONFIG_SANDBOX
>>> +	/*
>>> +	 * Skip delaying polls if a test requests it.
>>> +	 */
>>> +	if (state_get_skip_delays())
>>> +		poll_delay = 0;
>>>   #endif
>>
>> Is this architecture-specific ifdef really needed in common code?
> 
> I modeled this check off a similar one around line 184 of common/usb_hub.c which
> effectively sets some get_timer() based delay values to 0 like I propose here.
> Checking this pre-existing flag here seems like a less invasive option to me
> than modifying the core timer code to also check the flag.
> 
> Modifying the test to add a delay would also theoretically be possible, but I am
> not sure how to accomplish that and it would make the test take longer.

I CCed Simon, maybe he can help out.

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Thomas Watson]

> On Feb 10, 2022, at 3:42 PM, Marek Vasut <marex@denx.de> wrote:
> 
> On 2/10/22 22:13, Thomas Watson wrote:
>> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
>> 30-40ms to run. The exact time is dependent on the polling interval the
>> keyboard requests in its descriptor, and likely cannot be significantly
>> reduced without major rework to the XHCI driver.
>> The U-Boot EFI console service sets a timer to poll the keyboard every 5
>> microseconds, and this timer is checked every time a block is read off
>> disk. The net effect is that, on my system, loading a ~40MiB kernel and
>> initrd takes about 62 seconds with a slower keyboard and 53 seconds
>> with a faster one, with the vast majority of the time spent polling the
>> keyboard.
>> To solve this problem, this patch adds a 20ms delay between consecutive
>> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
>> total loading time to under half a second for both keyboards, and does
>> not impact the perceived keystroke latency.
>> Signed-off-by: Thomas Watson <twatson52@icloud.com>
>> ---
>> This revision fixes a sandbox test failure by honoring the test's
>> request to skip delays.
>>  common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>>  1 file changed, 26 insertions(+), 5 deletions(-)
>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
>> index afad260d3d..352d86fb2e 100644
>> --- a/common/usb_kbd.c
>> +++ b/common/usb_kbd.c
>> @@ -17,6 +17,9 @@
>>  #include <stdio_dev.h>
>>  #include <watchdog.h>
>>  #include <asm/byteorder.h>
>> +#ifdef CONFIG_SANDBOX
>> +#include <asm/state.h>
>> +#endif
>>    #include <usb.h>
>>  @@ -118,7 +121,7 @@ struct usb_kbd_pdata {
>>  extern int __maybe_unused net_busy_flag;
>>    /* The period of time between two calls of usb_kbd_testc(). */
>> -static unsigned long __maybe_unused kbd_testc_tms;
>> +static unsigned long kbd_testc_tms;
>>    /* Puts character in the queue and sets up the in and out pointer. */
>>  static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c)
>> @@ -394,21 +397,39 @@ static int usb_kbd_testc(struct stdio_dev *sdev)
>>  	struct usb_device *usb_kbd_dev;
>>  	struct usb_kbd_pdata *data;
>>  +	/*
>> +	 * Polling the keyboard for an event can take dozens of milliseconds.
>> +	 * Add a delay between polls to avoid blocking activity which polls
>> +	 * rapidly, like the UEFI console timer.
>> +	 */
>> +	unsigned long poll_delay = CONFIG_SYS_HZ / 50;
>> +
>>  #ifdef CONFIG_CMD_NET
>>  	/*
>>  	 * If net_busy_flag is 1, NET transfer is running,
>>  	 * then we check key-pressed every second (first check may be
>>  	 * less than 1 second) to improve TFTP booting performance.
>>  	 */
>> -	if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
>> -		return 0;
>> -	kbd_testc_tms = get_timer(0);
>> +	if (net_busy_flag)
>> +		poll_delay = CONFIG_SYS_HZ;
>> +#endif
>> +
>> +#ifdef CONFIG_SANDBOX
>> +	/*
>> +	 * Skip delaying polls if a test requests it.
>> +	 */
>> +	if (state_get_skip_delays())
>> +		poll_delay = 0;
>>  #endif
> 
> Is this architecture-specific ifdef really needed in common code?

I modeled this check off a similar one around line 184 of common/usb_hub.c which
effectively sets some get_timer() based delay values to 0 like I propose here.
Checking this pre-existing flag here seems like a less invasive option to me
than modifying the core timer code to also check the flag.

Modifying the test to add a delay would also theoretically be possible, but I am
not sure how to accomplish that and it would make the test take longer.

Re: [PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Marek Vasut]

On 2/10/22 22:13, Thomas Watson wrote:
> Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
> 30-40ms to run. The exact time is dependent on the polling interval the
> keyboard requests in its descriptor, and likely cannot be significantly
> reduced without major rework to the XHCI driver.
> 
> The U-Boot EFI console service sets a timer to poll the keyboard every 5
> microseconds, and this timer is checked every time a block is read off
> disk. The net effect is that, on my system, loading a ~40MiB kernel and
> initrd takes about 62 seconds with a slower keyboard and 53 seconds
> with a faster one, with the vast majority of the time spent polling the
> keyboard.
> 
> To solve this problem, this patch adds a 20ms delay between consecutive
> calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
> total loading time to under half a second for both keyboards, and does
> not impact the perceived keystroke latency.
> 
> Signed-off-by: Thomas Watson <twatson52@icloud.com>
> ---
> This revision fixes a sandbox test failure by honoring the test's
> request to skip delays.
> 
>   common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
>   1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/common/usb_kbd.c b/common/usb_kbd.c
> index afad260d3d..352d86fb2e 100644
> --- a/common/usb_kbd.c
> +++ b/common/usb_kbd.c
> @@ -17,6 +17,9 @@
>   #include <stdio_dev.h>
>   #include <watchdog.h>
>   #include <asm/byteorder.h>
> +#ifdef CONFIG_SANDBOX
> +#include <asm/state.h>
> +#endif
>   
>   #include <usb.h>
>   
> @@ -118,7 +121,7 @@ struct usb_kbd_pdata {
>   extern int __maybe_unused net_busy_flag;
>   
>   /* The period of time between two calls of usb_kbd_testc(). */
> -static unsigned long __maybe_unused kbd_testc_tms;
> +static unsigned long kbd_testc_tms;
>   
>   /* Puts character in the queue and sets up the in and out pointer. */
>   static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c)
> @@ -394,21 +397,39 @@ static int usb_kbd_testc(struct stdio_dev *sdev)
>   	struct usb_device *usb_kbd_dev;
>   	struct usb_kbd_pdata *data;
>   
> +	/*
> +	 * Polling the keyboard for an event can take dozens of milliseconds.
> +	 * Add a delay between polls to avoid blocking activity which polls
> +	 * rapidly, like the UEFI console timer.
> +	 */
> +	unsigned long poll_delay = CONFIG_SYS_HZ / 50;
> +
>   #ifdef CONFIG_CMD_NET
>   	/*
>   	 * If net_busy_flag is 1, NET transfer is running,
>   	 * then we check key-pressed every second (first check may be
>   	 * less than 1 second) to improve TFTP booting performance.
>   	 */
> -	if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
> -		return 0;
> -	kbd_testc_tms = get_timer(0);
> +	if (net_busy_flag)
> +		poll_delay = CONFIG_SYS_HZ;
> +#endif
> +
> +#ifdef CONFIG_SANDBOX
> +	/*
> +	 * Skip delaying polls if a test requests it.
> +	 */
> +	if (state_get_skip_delays())
> +		poll_delay = 0;
>   #endif

Is this architecture-specific ifdef really needed in common code?

[PATCH v3] console: usb: kbd: Limit poll frequency to improve performance

[Thomas Watson]

Using the XHCI driver, the function `usb_kbd_poll_for_event` takes
30-40ms to run. The exact time is dependent on the polling interval the
keyboard requests in its descriptor, and likely cannot be significantly
reduced without major rework to the XHCI driver.

The U-Boot EFI console service sets a timer to poll the keyboard every 5
microseconds, and this timer is checked every time a block is read off
disk. The net effect is that, on my system, loading a ~40MiB kernel and
initrd takes about 62 seconds with a slower keyboard and 53 seconds
with a faster one, with the vast majority of the time spent polling the
keyboard.

To solve this problem, this patch adds a 20ms delay between consecutive
calls to `usb_kbd_poll_for_event`. This is sufficient to reduce the
total loading time to under half a second for both keyboards, and does
not impact the perceived keystroke latency.

Signed-off-by: Thomas Watson <twatson52@icloud.com>
---
This revision fixes a sandbox test failure by honoring the test's
request to skip delays.

 common/usb_kbd.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index afad260d3d..352d86fb2e 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -17,6 +17,9 @@
 #include <stdio_dev.h>
 #include <watchdog.h>
 #include <asm/byteorder.h>
+#ifdef CONFIG_SANDBOX
+#include <asm/state.h>
+#endif
 
 #include <usb.h>
 
@@ -118,7 +121,7 @@ struct usb_kbd_pdata {
 extern int __maybe_unused net_busy_flag;
 
 /* The period of time between two calls of usb_kbd_testc(). */
-static unsigned long __maybe_unused kbd_testc_tms;
+static unsigned long kbd_testc_tms;
 
 /* Puts character in the queue and sets up the in and out pointer. */
 static void usb_kbd_put_queue(struct usb_kbd_pdata *data, u8 c)
@@ -394,21 +397,39 @@ static int usb_kbd_testc(struct stdio_dev *sdev)
 	struct usb_device *usb_kbd_dev;
 	struct usb_kbd_pdata *data;
 
+	/*
+	 * Polling the keyboard for an event can take dozens of milliseconds.
+	 * Add a delay between polls to avoid blocking activity which polls
+	 * rapidly, like the UEFI console timer.
+	 */
+	unsigned long poll_delay = CONFIG_SYS_HZ / 50;
+
 #ifdef CONFIG_CMD_NET
 	/*
 	 * If net_busy_flag is 1, NET transfer is running,
 	 * then we check key-pressed every second (first check may be
 	 * less than 1 second) to improve TFTP booting performance.
 	 */
-	if (net_busy_flag && (get_timer(kbd_testc_tms) < CONFIG_SYS_HZ))
-		return 0;
-	kbd_testc_tms = get_timer(0);
+	if (net_busy_flag)
+		poll_delay = CONFIG_SYS_HZ;
+#endif
+
+#ifdef CONFIG_SANDBOX
+	/*
+	 * Skip delaying polls if a test requests it.
+	 */
+	if (state_get_skip_delays())
+		poll_delay = 0;
 #endif
+
 	dev = stdio_get_by_name(sdev->name);
 	usb_kbd_dev = (struct usb_device *)dev->priv;
 	data = usb_kbd_dev->privptr;
 
-	usb_kbd_poll_for_event(usb_kbd_dev);
+	if (get_timer(kbd_testc_tms) >= poll_delay) {
+		usb_kbd_poll_for_event(usb_kbd_dev);
+		kbd_testc_tms = get_timer(0);
+	}
 
 	return !(data->usb_in_pointer == data->usb_out_pointer);
 }
-- 
2.31.1