> Date: Sun, 25 Feb 2024 22:57:23 +0100 > From: Marek Vasut <ma...@denx.de> > > On 2/25/24 5:07 PM, Janne Grunau wrote: > > Hej, > > > > On Wed, Feb 21, 2024, at 13:41, Marek Vasut wrote: > >> On 2/21/24 08:25, Janne Grunau via B4 Relay wrote: > >>> From: Hector Martin <mar...@marcan.st> > >>> > >>> We currently only support one USB keyboard device, but some devices > >>> emulate keyboards for other purposes. Most commonly, people run into > >>> this with Yubikeys, so let's ignore those. > >>> > >>> Even if we end up supporting multiple keyboards in the future, it's > >>> safer to ignore known non-keyboard devices. > >>> > >>> Signed-off-by: Hector Martin <mar...@marcan.st> > >>> --- > >>> common/usb_kbd.c | 19 +++++++++++++++++++ > >>> 1 file changed, 19 insertions(+) > >>> > >>> diff --git a/common/usb_kbd.c b/common/usb_kbd.c > >>> index 4cbc9acb73..774d3555d9 100644 > >>> --- a/common/usb_kbd.c > >>> +++ b/common/usb_kbd.c > >>> @@ -120,6 +120,15 @@ struct usb_kbd_pdata { > >>> > >>> extern int __maybe_unused net_busy_flag; > >>> > >>> +/* > >>> + * Since we only support one usbkbd device in the iomux, > >>> + * ignore common keyboard-emulating devices that aren't > >>> + * real keyboards. > >>> + */ > >>> +const uint16_t vid_blocklist[] = { > >>> + 0x1050, /* Yubico */ > >>> +}; > >>> + > >>> /* The period of time between two calls of usb_kbd_testc(). */ > >>> static unsigned long kbd_testc_tms; > >>> > >>> @@ -465,6 +474,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, > >>> unsigned int ifnum) > >>> struct usb_endpoint_descriptor *ep; > >>> struct usb_kbd_pdata *data; > >>> int epNum; > >>> + int i; > >>> > >>> if (dev->descriptor.bNumConfigurations != 1) > >>> return 0; > >>> @@ -480,6 +490,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, > >>> unsigned int ifnum) > >>> if (iface->desc.bInterfaceProtocol != USB_PROT_HID_KEYBOARD) > >>> return 0; > >>> > >>> + for (i = 0; i < ARRAY_SIZE(vid_blocklist); i++) { > >>> + if (dev->descriptor.idVendor == vid_blocklist[i]) { > >>> + printf("Ignoring keyboard device 0x%x:0x%x\n", > >>> + dev->descriptor.idVendor, > >>> + dev->descriptor.idProduct); > >>> + return 0; > >>> + } > >>> + } > >> > >> I vaguely recall a discussion about previous version of this, I think > >> the suggestion was to make the list of ignored devices configurable via > >> environment variable, so users can add to that list from U-Boot shell. > >> Would it be possible to make it work this way ? > > > > oh, I completely forgot that this patch was already submitted. I briefly > > looked through asahi tree for related patches and did not check whether > > this was previously submitted. > > I've added environment based blocking as separate patch with blocking > > either complete vendor IDs or vendor, product ID combinations. A separate > > patch to simplify authorship tracking and the implementation doesn't share > > any code. > > It would be better to have only one patch which does not hard-code any > USB IDs, and then add those blocked IDs via U-Boot default environment > for this specific machine. We cannot predict what yubico will do in the > future, whether they might make a device that shouldn't be blocked for > example. If they do, the user should be able to unblock their device by > running e.g. '=> setenv usb_blocklist' instead of updating their bootloader. > > I think a simple list of blocked VID:PID pairs, maybe with wildcards, > would be nice, i.e. something like 'usb_blocklist=1234:5678,1050:*' to > block device 0x1234:0x5678 and all devices with VID 0x1050 . That should > be easy to parse with strtok()/strtol() or some such and the code should > not be too complex.
I do like the idea of having a configurable list of usb devices to ignore. The U-Boot USB stack is still not perfect and there are still USB devices that will prevent us from booting when connected. The list will provide a nice workaround for that issue. But the yubikeys will cause the same problem on other boards as well. So I think it makes sense to put those in a default list.