On Sun, Apr 07, 2024 at 03:05:59AM +0200, Marek Vasut wrote:
On 4/6/24 10:04 PM, Janne Grunau wrote:
On Sat, Apr 06, 2024 at 08:52:17PM +0200, Marek Vasut wrote:
On 4/5/24 9:05 PM, Janne Grunau wrote:
On Fri, Apr 05, 2024 at 04:52:32PM +0200, Marek Vasut wrote:
On 4/4/24 8:25 AM, Janne Grunau via B4 Relay wrote:
Apple USB Keyboards from 2021 need quirks to be useable. The boot HID keyboard protocol is unfortunately not described in the first interface descriptor but the second. This needs several changes. The USB keyboard driver has to look at all (2) interface descriptors during probing. Since I didn't want to rebuild the USB driver probe code the Apple keyboards are bound to the keyboard driver via USB vendor and product IDs. To make the keyboards useable on Apple silicon devices the xhci driver needs to initializes rings for the endpoints of the first two interface descriptors. If this is causes concerns regarding regressions or memory use the USB_MAX_ACTIVE_INTERFACES define could be turned into a CONFIG option. Even after this changes the keyboards still do not probe successfully since they apparently do not behave HID standard compliant. They only generate reports on key events. This leads the final check whether the keyboard is operational to fail unless the user presses keys during the probe. Skip this check for known keyboards. Keychron seems to emulate Apple keyboards (some models even "re-use" Apple's USB vendor ID) so apply this quirk as well.
Some devices like Yubikeys emulate a keyboard. since u-boot only binds a single keyboard block this kind of devices from the USB keyboard driver.
Signed-off-by: Janne Grunau j@jannau.net
I picked the series, but CI indicates build errors, can you have a look ?
https://source.denx.de/u-boot/custodians/u-boot-usb/-/pipelines/20215
The issue seems to be that the field dev in struct usb_device exists only for DM_USB. That means we can't use dev_dbg. Either take the following fixup patch or I can resend the series.
I squashed the extra patch in, but I think the CI still complains:
Pipeline #20236 ( https://source.denx.de/u-boot/custodians/u-boot-usb/-/pipelines/20236 ) triggered by Marek Vasut ( https://source.denx.de/marex ) had 1 failed job.
Job #812215 ( https://source.denx.de/u-boot/custodians/u-boot-usb/-/jobs/812215/raw )
env_get() missing without CONFIG_ENV_SUPPORT. I'm too accustomed to the kernel's stub functions. Best option seems to to just #if the functionality out in this case. See attached fixup patch.
Sorry,
No worries.
Does it work if you do this instead ?
static int usb_device_is_ignored(u16 id_vendor, u16 id_product) { ulong vid, pid;
/* No env support, nothing can be ignored */if (CONFIG_IS_ENABLED(ENV_SUPPORT))return 0;That way, the function is always compiled and if it is unreachable, then compiler throws it out. This should improve code compile coverage.
Seems to work here with a broken imx8 config from the CI. Is it ok to rely on dead code elimination? Apparently it is, build with KCFLAGS=-O0 has already several other missing symbols.
See attached fixup
Janne
From ab3b825d7bc0571cfb38eb80a1e449e51d5d2f6d Mon Sep 17 00:00:00 2001
From: Janne Grunau j@jannau.net Date: Mon, 8 Apr 2024 09:44:54 +0200 Subject: [PATCH 1/1] fixup! usb: Add environment based device ignorelist
--- common/usb.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/common/usb.c b/common/usb.c index 8bc85c58b28c..99e6b857c74c 100644 --- a/common/usb.c +++ b/common/usb.c @@ -1089,7 +1089,13 @@ static int usb_device_is_ignored(u16 id_vendor, u16 id_product) { ulong vid, pid; char *end; - const char *cur = env_get("usb_ignorelist"); + const char *cur = NULL; + + /* ignore list depends on env support */ + if (!CONFIG_IS_ENABLED(ENV_SUPPORT)) + return 0; + + cur = env_get("usb_ignorelist");
/* parse "usb_ignorelist" strictly */ while (cur && cur[0] != '\0') {