Re: [PATCH v4 0/6] USB keyboard improvements for asahi / desktop systems

[Janne Grunau <j@jannau.net>]

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

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,

Janne

[-- Attachment #2: 0001-fixup-usb-Add-environment-based-device-ignorelist.patch --]
[-- Type: text/plain, Size: 1333 bytes --]

From 214ec2aad1ed42d6e1256bbc7eaeff84e17443e0 Mon Sep 17 00:00:00 2001
From: Janne Grunau <j@jannau.net>
Date: Sat, 6 Apr 2024 21:46:44 +0200
Subject: [PATCH 1/1] fixup! usb: Add environment based device ignorelist

---
 common/usb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 8bc85c58b2..52f77431b0 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1085,6 +1085,7 @@ static int usb_prepare_device(struct usb_device *dev, int addr, bool do_read,
 	return 0;
 }
 
+#if CONFIG_IS_ENABLED(ENV_SUPPORT)
 static int usb_device_is_ignored(u16 id_vendor, u16 id_product)
 {
 	ulong vid, pid;
@@ -1126,6 +1127,7 @@ static int usb_device_is_ignored(u16 id_vendor, u16 id_product)
 
 	return 0;
 }
+#endif
 
 int usb_select_config(struct usb_device *dev)
 {
@@ -1142,6 +1144,7 @@ int usb_select_config(struct usb_device *dev)
 	le16_to_cpus(&dev->descriptor.idProduct);
 	le16_to_cpus(&dev->descriptor.bcdDevice);
 
+#if CONFIG_IS_ENABLED(ENV_SUPPORT)
 	/* ignore devices from usb_ignorelist */
 	err = usb_device_is_ignored(dev->descriptor.idVendor,
 				    dev->descriptor.idProduct);
@@ -1162,6 +1165,7 @@ int usb_select_config(struct usb_device *dev)
 	} else if (err < 0) {
 		return err;
 	}
+#endif
 
 	/*
 	 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
-- 
2.44.0