[PATCH v2 5/5] arm: apple: Do not list bootflows on boot

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The bootflow list is only seen briefly and is probably more confusing
than helpful.

Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index c30aec7c55..4eac1a9e2d 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -8,6 +8,7 @@ CONFIG_SYS_CBSIZE=256
 CONFIG_SYS_PBSIZE=276
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_BOOTCOMMAND="bootflow scan -b"
 CONFIG_BOARD_LATE_INIT=y
 CONFIG_CMD_SELECT_FONT=y
 # CONFIG_NET is not set

-- 
2.44.0

[PATCH v2 1/5] apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA

[Janne Grunau via B4 Relay]

From: Hector Martin 

This makes USB HDDs >2TiB work. The only reason this hasn't bitten us
for the internal NVMe yet is the 4K sector size, because the largest SSD
Apple sells is 8TB and we can handle up to 16TiB with that sector size.
Close call.

Signed-off-by: Hector Martin 
Reviewed-by: Mark Kettenis 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index e00d72e8be..31d966f0ab 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -10,6 +10,7 @@ CONFIG_SYS_PBSIZE=276
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_LATE_INIT=y
 # CONFIG_NET is not set
+CONFIG_SYS_64BIT_LBA=y
 CONFIG_APPLE_SPI_KEYB=y
 # CONFIG_MMC is not set
 CONFIG_NVME_APPLE=y

-- 
2.44.0

[PATCH v2 3/5] configs: apple: Enable CMD_SELECT_FONT and FONT_16X32

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple devices have high DPI displays so the larger fonts are preferable
for improved readability. This does not yet change the used font based
on the display's pixel density so the standard 8x16 font is still used
by default.

Reviewed-by: Mark Kettenis 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index 31d966f0ab..c30aec7c55 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -9,6 +9,7 @@ CONFIG_SYS_PBSIZE=276
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_LATE_INIT=y
+CONFIG_CMD_SELECT_FONT=y
 # CONFIG_NET is not set
 CONFIG_SYS_64BIT_LBA=y
 CONFIG_APPLE_SPI_KEYB=y
@@ -19,6 +20,7 @@ CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_XHCI_PCI=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_VIDEO_FONT_16X32=y
 CONFIG_SYS_WHITE_ON_BLACK=y
 CONFIG_NO_FB_CLEAR=y
 CONFIG_VIDEO_SIMPLE=y

-- 
2.44.0

[PATCH v2 0/5] configs: apple: Switch to standard boot + small adjustments

[Janne Grunau via B4 Relay]

This series contains a few misc config changes for Apple silicon
systems:
- switch from the deprecated distro boot scripts to standard boot
- allows EFI console resizing based on the video console size
- enables 16x32 bitmap fonts as Apple devices come with high DPI
  displays
- enables 64-bit LBA addressing

Signed-off-by: Janne Grunau 
---
Changes in v2:
- added Reviewed-by: tags
- switched to BOOTSTD_FULL to enable efi_bootmgr and make interactive
  use easier
- override BOOTCOMMAND to not list the bootflows
- rebased onto v2024.04
- Link to v1: 
https://lore.kernel.org/r/20240317-apple_config-v1-0-1b862bc14...@jannau.net

---
Hector Martin (1):
  apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA

Janne Grunau (4):
  configs: apple: Use "vidconsole,serial" as stdout/stderr
  configs: apple: Enable CMD_SELECT_FONT and FONT_16X32
  arm: apple: Switch to standard boot
  arm: apple: Do not list bootflows on boot

 arch/arm/Kconfig   |  2 +-
 configs/apple_m1_defconfig |  4 
 include/configs/apple.h| 24 
 3 files changed, 9 insertions(+), 21 deletions(-)
---
base-commit: 25049ad560826f7dc1c4740883b0016014a59789
change-id: 20240317-apple_config-981fcd9028b9

Best regards,
-- 
Janne Grunau 

[PATCH v2 2/5] configs: apple: Use "vidconsole,serial" as stdout/stderr

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The display size querying in efi_console relies on this order. The
display should be the primary output device and should be used to
display more than 80x25 chars.

Reviewed-by: Mark Kettenis 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 include/configs/apple.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/apple.h b/include/configs/apple.h
index 0576bc04c9..a70440b3ad 100644
--- a/include/configs/apple.h
+++ b/include/configs/apple.h
@@ -6,8 +6,8 @@
 /* Environment */
 #define ENV_DEVICE_SETTINGS \
"stdin=serial,usbkbd,spikbd\0" \
-   "stdout=serial,vidconsole\0" \
-   "stderr=serial,vidconsole\0"
+   "stdout=vidconsole,serial\0" \
+   "stderr=vidconsole,serial\0"
 
 #if IS_ENABLED(CONFIG_CMD_NVME)
#define BOOT_TARGET_NVME(func) func(NVME, nvme, 0)

-- 
2.44.0

[PATCH v2 4/5] arm: apple: Switch to standard boot

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Use standard boot instead of the distro boot scripts. Use
BOOTSTD_FULL instead of BOOTSTD_DEFAULTS for easier interactive use.

Signed-off-by: Janne Grunau 
---
 arch/arm/Kconfig|  2 +-
 include/configs/apple.h | 20 ++--
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01d6556c42..9b83b2e6f8 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1034,7 +1034,7 @@ config ARCH_APPLE
select USB
imply CMD_DM
imply CMD_GPT
-   imply DISTRO_DEFAULTS
+   imply BOOTSTD_FULL
imply OF_HAS_PRIOR_STAGE
 
 config ARCH_OWL
diff --git a/include/configs/apple.h b/include/configs/apple.h
index a70440b3ad..1e08b11448 100644
--- a/include/configs/apple.h
+++ b/include/configs/apple.h
@@ -9,26 +9,10 @@
"stdout=vidconsole,serial\0" \
"stderr=vidconsole,serial\0"
 
-#if IS_ENABLED(CONFIG_CMD_NVME)
-   #define BOOT_TARGET_NVME(func) func(NVME, nvme, 0)
-#else
-   #define BOOT_TARGET_NVME(func)
-#endif
-
-#if IS_ENABLED(CONFIG_CMD_USB)
-   #define BOOT_TARGET_USB(func) func(USB, usb, 0)
-#else
-   #define BOOT_TARGET_USB(func)
-#endif
-
-#define BOOT_TARGET_DEVICES(func) \
-   BOOT_TARGET_NVME(func) \
-   BOOT_TARGET_USB(func)
-
-#include 
+#define BOOT_TARGETS   "nvme usb"
 
 #define CFG_EXTRA_ENV_SETTINGS \
ENV_DEVICE_SETTINGS \
-   BOOTENV
+   "boot_targets=" BOOT_TARGETS "\0"
 
 #endif

-- 
2.44.0

Re: [PATCH 4/4] arm: apple: Switch to standard boot

[Janne Grunau]

On Mon, Apr 15, 2024 at 04:24:51PM +0200, Mark Kettenis wrote:
> > From: Janne Grunau via B4 Relay 
> > Date: Sun, 17 Mar 2024 15:54:50 +0100
> > 
> > From: Janne Grunau 
> > 
> > Use standard boot instead of the distro boot scripts.
> > 
> > Signed-off-by: Janne Grunau 
> 
> As per a somewhat recent discussion about this for the rockchip SoCs,
> I think we want BOOTSTD_FULL instead of BOOTSTD_DEFAULT.  Even though
> I think that BOOTSTD_FULL is a bit too chatty at the moment.

I noticed and added BOOTSTD_FULL to the defconfig downstream. To solve
the chattyness we could override BOOTCMD to "bootflow scan -b" in the
defconfig.

> That also solves the issue that BOOTSTD_DEFAULTS doesn't run the EFI
> bootmgr (which does happen with distroboot).  Although Heinrich has a
> diff to fix that.

I've seen but I think for interactive use BOOTSTD_FULL is still the
better choice. The full bootstd commands are helpful for booting from a
non-default partition.

I'll resend with BOOTSTD_FULL and will think whether to change BOOTCMD
in apple_m1_defconfig or in Kconfig. I don't think it makes much sense
to list the boot methods by default before boot.

Janne

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

[Janne Grunau]

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 
> >>>>
> >>>> 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 
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') {
-- 
2.43.2

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

[Janne Grunau]

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 
> >>
> >> 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
>From 214ec2aad1ed42d6e1256bbc7eaeff84e17443e0 Mon Sep 17 00:00:00 2001
From: Janne Grunau 
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(>descriptor.idProduct);
le16_to_cpus(>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

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

[Janne Grunau]

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

Thanks

Janne
>From 57d54303eb2b60e92bd478e4250a9cc63cfc277e Mon Sep 17 00:00:00 2001
From: Janne Grunau 
Date: Fri, 5 Apr 2024 21:00:44 +0200
Subject: [PATCH 1/1] fixup! usb: Add environment based device ignorelist

---
 common/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/usb.c b/common/usb.c
index 44db07742e..8bc85c58b2 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1146,7 +1146,7 @@ int usb_select_config(struct usb_device *dev)
err = usb_device_is_ignored(dev->descriptor.idVendor,
dev->descriptor.idProduct);
if (err == -ENODEV) {
-   dev_dbg(dev->dev, "Ignoring USB device 0x%x:0x%x\n",
+   debug("Ignoring USB device 0x%x:0x%x\n",
dev->descriptor.idVendor, dev->descriptor.idProduct);
return err;
} else if (err == -EINVAL) {
-- 
2.44.0

[PATCH v4 1/6] usb: xhci: refactor xhci_set_configuration

[Janne Grunau via B4 Relay]

From: Janne Grunau 

In the next step endpoints for multiple interfaces are set up. Move most
of the per endpoint initialization to separate function to avoid another
identation level.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 119 +---
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d13cbff9b3..534c4b973f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -475,67 +475,34 @@ static int xhci_configure_endpoints(struct usb_device 
*udev, bool ctx_change)
 }
 
 /**
- * Configure the endpoint, programming the device contexts.
+ * Fill endpoint contexts for interface descriptor ifdesc.
  *
- * @param udev pointer to the USB device structure
- * Return: returns the status of the xhci_configure_endpoints
+ * @param udev pointer to the USB device structure
+ * @param ctrl pointer to the xhci pravte device structure
+ * @param virt_dev pointer to the xhci virtual device structure
+ * @param ifdesc   pointer to the USB interface config descriptor
+ * Return: returns the status of xhci_init_ep_contexts_if
  */
-static int xhci_set_configuration(struct usb_device *udev)
+static int xhci_init_ep_contexts_if(struct usb_device *udev,
+   struct xhci_ctrl *ctrl,
+   struct xhci_virt_device *virt_dev,
+   struct usb_interface *ifdesc
+   )
 {
-   struct xhci_container_ctx *in_ctx;
-   struct xhci_container_ctx *out_ctx;
-   struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_ep_ctx *ep_ctx[MAX_EP_CTX_NUM];
int cur_ep;
-   int max_ep_flag = 0;
int ep_index;
unsigned int dir;
unsigned int ep_type;
-   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
-   int num_of_ep;
-   int ep_flag = 0;
u64 trb_64 = 0;
-   int slot_id = udev->slot_id;
-   struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
-   struct usb_interface *ifdesc;
u32 max_esit_payload;
unsigned int interval;
unsigned int mult;
unsigned int max_burst;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+   int num_of_ep = ifdesc->no_of_ep;
 
-   out_ctx = virt_dev->out_ctx;
-   in_ctx = virt_dev->in_ctx;
-
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
-   ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
-   /* Initialize the input context control */
-   ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
-   ctrl_ctx->drop_flags = 0;
-
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
-   }
-
-   xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
-
-   /* slot context */
-   xhci_slot_copy(ctrl, in_ctx, out_ctx);
-   slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);
-   slot_ctx->dev_info &= ~(cpu_to_le32(LAST_CTX_MASK));
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(max_ep_flag + 1) | 0);
-
-   xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
-
-   /* filling up ep contexts */
for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
struct usb_endpoint_descriptor *endpt_desc = NULL;
struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
@@ -561,7 +528,8 @@ static int xhci_set_configuration(struct usb_device *udev)
avg_trb_len = max_esit_payload;
 
ep_index = xhci_get_ep_index(endpt_desc);
-   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
+   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx,
+  ep_index);
 
/* Allocate the ep rings */
virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
@@ -614,6 +582,65 @@ static int xhci_set_configuration(struct usb_device *udev)
}
}
 
+   return 0;
+}
+
+/**
+ * Configure the endpoint, programming the device contexts.
+ *
+ * @param udev pointer to the USB device structure
+ * Return: returns the status of the xhci_configure_endpoints
+ */
+static int xhci_set_configuration(struct usb_device *udev)
+{
+   struct xhci_container_ctx *out_ctx;
+   struct xhci_container_ctx *in_ctx;
+   struct xhci_input_control_ctx *ctrl_ctx;
+   struct xhci_slot_ctx *slot_ctx;
+   int err;
+   int cur_ep;
+   int max_ep_f

[PATCH v4 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The xhci driver currently only does the necessary initialization for
endpoints found in the first interface descriptor. Apple USB keyboards
(released 2021) use the second interface descriptor for the HID keyboard
boot protocol. To allow USB drivers to use endpoints from other
interface descriptors the xhci driver needs to ensure these endpoints
are initialized as well.
Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
are considered during endpoint initialisation.
For now define it to 2 as that is sufficient for supporting the Apple
keyboards.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 31 +++
 include/usb.h   |  6 ++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 534c4b973f..741e186ee0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device *udev)
int slot_id = udev->slot_id;
struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
struct usb_interface *ifdesc;
+   unsigned int ifnum;
+   unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
+(unsigned int)udev->config.no_of_if);
 
out_ctx = virt_dev->out_ctx;
in_ctx = virt_dev->in_ctx;
 
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
/* Initialize the input context control */
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
ctrl_ctx->drop_flags = 0;
 
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   num_of_ep = ifdesc->no_of_ep;
+   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
+   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
+   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
+   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
+   if (max_ep_flag < ep_flag)
+   max_ep_flag = ep_flag;
+   }
}
 
xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
@@ -637,9 +641,12 @@ static int xhci_set_configuration(struct usb_device *udev)
xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
 
/* filling up ep contexts */
-   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
-   if (err < 0)
-   return err;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
+   if (err < 0)
+   return err;
+   }
 
return xhci_configure_endpoints(udev, false);
 }
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb30..3aafdc8bfd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -49,6 +49,12 @@ extern bool usb_started; /* flag for the started/stopped USB 
status */
  */
 #define USB_TIMEOUT_MS(pipe) (usb_pipebulk(pipe) ? 5000 : 1000)
 
+/*
+ * The xhcd hcd driver prepares only a limited number interfaces / endpoints.
+ * Define this limit so that drivers do not exceed it.
+ */
+#define USB_MAX_ACTIVE_INTERFACES  2
+
 /* device request (setup) */
 struct devrequest {
__u8requesttype;

-- 
2.44.0

[PATCH v4 4/6] usb: Add environment based device ignorelist

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Add the environment variable "usb_ignorelist" to prevent USB devices
listed in it from being bound to drivers. This allows to ignore devices
which are undesirable or trigger bugs in u-boot's USB stack.
Devices emulating keyboards are one example of undesirable devices as
u-boot currently supports only a single USB keyboard device. Most
commonly, people run into this with Yubikeys, so let's ignore those in
the default environment.

Based on previous USB keyboard specific patches for the same purpose.

Link: 
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Reviewed-by: Neal Gompa 
Reviewed-by: Marek Vasut 
Signed-off-by: Janne Grunau 
---
 common/usb.c  | 64 +++
 doc/usage/environment.rst | 13 ++
 include/env_default.h | 11 
 3 files changed, 88 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..44db07742e 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1084,6 +1085,48 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
return 0;
 }
 
+static int usb_device_is_ignored(u16 id_vendor, u16 id_product)
+{
+   ulong vid, pid;
+   char *end;
+   const char *cur = env_get("usb_ignorelist");
+
+   /* parse "usb_ignorelist" strictly */
+   while (cur && cur[0] != '\0') {
+   vid = simple_strtoul(cur, , 0);
+   /*
+* If strtoul did not parse a single digit or the next char is
+* not ':' the ignore list is malformed.
+*/
+   if (cur == end || end[0] != ':')
+   return -EINVAL;
+
+   cur = end + 1;
+   pid = simple_strtoul(cur, , 0);
+   /* Consider '*' as wildcard for the product ID */
+   if (cur == end && end[0] == '*') {
+   pid = U16_MAX + 1;
+   end++;
+   }
+   /*
+* The ignore list is malformed if no product ID / wildcard was
+* parsed or entries are not separated by ',' or terminated with
+* '\0'.
+*/
+   if (cur == end || (end[0] != ',' && end[0] != '\0'))
+   return -EINVAL;
+
+   if (id_vendor == vid && (pid > U16_MAX || id_product == pid))
+   return -ENODEV;
+
+   if (end[0] == '\0')
+   break;
+   cur = end + 1;
+   }
+
+   return 0;
+}
+
 int usb_select_config(struct usb_device *dev)
 {
unsigned char *tmpbuf = NULL;
@@ -1099,6 +1142,27 @@ int usb_select_config(struct usb_device *dev)
le16_to_cpus(>descriptor.idProduct);
le16_to_cpus(>descriptor.bcdDevice);
 
+   /* ignore devices from usb_ignorelist */
+   err = usb_device_is_ignored(dev->descriptor.idVendor,
+   dev->descriptor.idProduct);
+   if (err == -ENODEV) {
+   dev_dbg(dev->dev, "Ignoring USB device 0x%x:0x%x\n",
+   dev->descriptor.idVendor, dev->descriptor.idProduct);
+   return err;
+   } else if (err == -EINVAL) {
+   /*
+* Continue on "usb_ignorelist" parsing errors. The list is
+* parsed for each device returning the error would result in
+* ignoring all USB devices.
+* Since the parsing error is independent of the probed device
+* report errors with printf instead of dev_err.
+*/
+   printf("usb_ignorelist parse error in \"%s\"\n",
+  env_get("usb_ignorelist"));
+   } else if (err < 0) {
+   return err;
+   }
+
/*
 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
 * about this first Get Descriptor request. If there are any other
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index ebf75fa948..7d4b448cb3 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -366,6 +366,19 @@ tftpwindowsize
 This means the count of blocks we can receive before
 sending ack to server.
 
+usb_ignorelist
+Ignore USB devices to prevent binding them to an USB device driver. This 
can
+be used to ignore devices are for some reason undesirable or causes crashes
+u-boot's USB stack.
+An example for undesired behavior is the keyboard emulation of security 
keys
+like Yubikeys. U-boot currently supports only a single USB keyboard device
+so try to probe an useful keyboard device. The default environment blocks
+Yubico devices as common devices emulating ke

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

[Janne Grunau via B4 Relay]

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 
---
Changes in v4:
- collects "Reviewed-by:" tagDITME: describe what is new in this series 
revision.
- adds comment about usb_ignorelist parse errors
- Link to v3: 
https://lore.kernel.org/r/20240322-asahi-keyboards-v3-0-3106dd4c4...@jannau.net

Changes in v3:
- collected "Reviewed-by:" tags
- rename usb_blocklist to usb_ignorelist
- use BIT macro for USB KBD quirk bit
- refactor usb_device_is_blocked() to use 0 / negated errors as return
  value, sed -e 's/block/ignore/', simplify it and add comments
- rewritten usb_ignorelist documentation
- Link to v2: 
https://lore.kernel.org/r/20240317-asahi-keyboards-v2-0-d3f4b8384...@jannau.net

Changes in v2:
- rewritten commit message for "[PATCH 2/6] usb: xhci: Set up endpoints for the 
first 2 interfaces"
- Replaced the usb keyboard Yubikey block with an env based USB device blocklist
- Use "-EINVAL" as return value in "[PATCH 3/6] usb: xhci: Abort transfers with 
unallocated rings"
- added "Reviewed-by:" tags
- Link to v1: 
https://lore.kernel.org/r/20240221-asahi-keyboards-v1-0-814b2e741...@jannau.net

---
Janne Grunau (6):
  usb: xhci: refactor xhci_set_configuration
  usb: xhci: Set up endpoints for the first 2 interfaces
  usb: xhci: Abort transfers with unallocated rings
  usb: Add environment based device ignorelist
  usb: kbd: support Apple Magic Keyboards (2021)
  usb: kbd: Add probe quirk for Apple and Keychron keyboards

 common/usb.c |  64 ++
 common/usb_kbd.c |  59 ++--
 doc/usage/environment.rst|  13 +
 drivers/usb/host/xhci-ring.c |   5 ++
 drivers/usb/host/xhci.c  | 126 +++
 include/env_default.h|  11 
 include/usb.h|   6 +++
 7 files changed, 235 insertions(+), 49 deletions(-)
---
base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585
change-id: 20240218-asahi-keyboards-f2ddaf0022b2

Best regards,
-- 
Janne Grunau 

[PATCH v4 5/6] usb: kbd: support Apple Magic Keyboards (2021)

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry
the HID keyboard boot protocol on the second interface descriptor.
Probe via vendor and product IDs since the class/subclass/protocol match
uses the first interface descriptor.
Probe the two first interface descriptors for the HID keyboard boot
protocol.

USB configuration descriptor for reference:

| Bus 003 Device 002: ID 05ac:029c Apple, Inc. Magic Keyboard
| Device Descriptor:
|   bLength18
|   bDescriptorType 1
|   bcdUSB   2.00
|   bDeviceClass0 [unknown]
|   bDeviceSubClass 0 [unknown]
|   bDeviceProtocol 0
|   bMaxPacketSize064
|   idVendor   0x05ac Apple, Inc.
|   idProduct  0x029c Magic Keyboard
|   bcdDevice3.90
|   iManufacturer   1 Apple Inc.
|   iProduct2 Magic Keyboard
|   iSerial 3 ...
|   bNumConfigurations  1
|   Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength   0x003b
| bNumInterfaces  2
| bConfigurationValue 1
| iConfiguration  4 Keyboard
| bmAttributes 0xa0
|   (Bus Powered)
|   Remote Wakeup
| MaxPower  500mA
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber0
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  0 [unknown]
|   bInterfaceProtocol  0
|   iInterface  5 Device Management
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode0 Not supported
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength  83
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81  EP 1 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber1
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  1 Boot Interface Subclass
|   bInterfaceProtocol  1 Keyboard
|   iInterface  6 Keyboard / Boot
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode   13 International (ISO)
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength 207
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x82  EP 2 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 4cbc9acb73..b2361bbf18 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -23,6 +23,14 @@
 
 #include 
 
+/*
+ * USB vendor and product IDs used for quirks.
+ */
+#define USB_VENDOR_ID_APPLE0x05ac
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_20210x029c
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -106,6 +114,8 @@ struct usb_kbd_pdata {
unsigned long   last_report;
struct int_queue *intq;
 
+   uint32_tifnum;
+
uint32_trepeat_delay;
 
uint32_tusb_in_pointer;
@@ -150,8 +160,8 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, 
u8 c)
  */
 static void usb_kbd_setled(struct usb_device *dev)
 {
-   struct usb_interface *iface = >config.if_desc[0];
struct usb_kbd_pdata *data = dev->privptr;
+   struct usb_int

[PATCH v4 3/6] usb: xhci: Abort transfers with unallocated rings

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Discovered while trying to use the second interface in the USB keyboard
driver necessary on Apple USB keyboards.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci-ring.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b60661fe05..910c5f3352 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
reset_ep(udev, ep_index);
 
ring = virt_dev->eps[ep_index].ring;
+   if (!ring)
+   return -EINVAL;
+
/*
 * How much data is (potentially) left before the 64KB boundary?
 * XHCI Spec puts restriction( TABLE 49 and 6.4.1 section of XHCI Spec)
@@ -871,6 +874,8 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
ep_index = usb_pipe_ep_index(pipe);
 
ep_ring = virt_dev->eps[ep_index].ring;
+   if (!ep_ring)
+   return -EINVAL;
 
/*
 * Check to see if the max packet size for the default control

-- 
2.44.0

[PATCH v4 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Those keyboards do not return the current device state. Polling will
timeout unless there are key presses. This is not a problem during
operation but the initial device state query during probing will fail.
Skip this step in usb_kbd_probe_dev() to make these devices useable.
Not all Apple keyboards behave like this. A keyboard with USB
vendor/product ID 05ac:0221 is reported to work with the current code.
Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and
show the same behavior (Keychron C2, 05ac:024f for example).

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b2361bbf18..820f591fc5 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -31,6 +31,10 @@
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
 
+#define USB_VENDOR_ID_KEYCHRON 0x3434
+
+#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE  BIT(0)
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -474,6 +478,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
struct usb_interface *iface;
struct usb_endpoint_descriptor *ep;
struct usb_kbd_pdata *data;
+   unsigned int quirks = 0;
int epNum;
 
if (dev->descriptor.bNumConfigurations != 1)
@@ -506,6 +511,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
 
debug("USB KBD: found interrupt EP: 0x%x\n", ep->bEndpointAddress);
 
+   switch (dev->descriptor.idVendor) {
+   case USB_VENDOR_ID_APPLE:
+   case USB_VENDOR_ID_KEYCHRON:
+   quirks |= USB_HID_QUIRK_POLL_NO_REPORT_IDLE;
+   break;
+   default:
+   break;
+   }
+
data = malloc(sizeof(struct usb_kbd_pdata));
if (!data) {
printf("USB KBD: Error allocating private data\n");
@@ -546,6 +560,14 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
 #endif
 
+   /*
+* Apple and Keychron keyboards do not report the device state. Reports
+* are only returned during key presses.
+*/
+   if (quirks & USB_HID_QUIRK_POLL_NO_REPORT_IDLE) {
+   debug("USB KBD: quirk: skip testing device state\n");
+   return 1;
+   }
debug("USB KBD: enable interrupt pipe...\n");
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
data->intq = create_int_queue(dev, data->intpipe, 1,

-- 
2.44.0

Re: [PATCH v3 4/6] usb: Add environment based device ignorelist

[Janne Grunau]

On Fri, Mar 22, 2024 at 12:56:37PM +0100, Marek Vasut wrote:
> On 3/22/24 8:47 AM, Janne Grunau via B4 Relay wrote:
> 
> [...]
> 
> > @@ -1099,6 +1142,20 @@ int usb_select_config(struct usb_device *dev)
> > le16_to_cpus(>descriptor.idProduct);
> > le16_to_cpus(>descriptor.bcdDevice);
> >   
> > +   /* ignore devices from usb_ignorelist */
> > +   err = usb_device_is_ignored(dev->descriptor.idVendor,
> > +   dev->descriptor.idProduct);
> > +   if (err == -ENODEV) {
> > +   dev_dbg(dev->dev, "Ignoring USB device 0x%x:0x%x\n",
> > +   dev->descriptor.idVendor, dev->descriptor.idProduct);
> > +   return err;
> > +   } else if (err == -EINVAL) {
> > +   printf("usb_ignorelist parse error in \"%s\"\n",
> > +  env_get("usb_ignorelist"));
> 
> Please use dev_err() here consistently with dev_dbg() above.

I didn't use dev_err() since the parsing error is not specific to the
device. It doesn't matter much. I'll change it and resend after we've
settled the new discussion about the interface limit.

> With that fixed:
> 
> Reviewed-by: Marek Vasut 

thanks,

Janne

Re: [PATCH v3 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau]

On Fri, Mar 22, 2024 at 09:17:08AM +0100, Heinrich Schuchardt wrote:
> On 3/22/24 08:47, Janne Grunau via B4 Relay wrote:
> > From: Janne Grunau 
> > 
> > The xhci driver currently only does the necessary initialization for
> > endpoints found in the first interface descriptor. Apple USB keyboards
> > (released 2021) use the second interface descriptor for the HID keyboard
> > boot protocol. To allow USB drivers to use endpoints from other
> > interface descriptors the xhci driver needs to ensure these endpoints
> > are initialized as well.
> > Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
> > are considered during endpoint initialisation.
> > For now define it to 2 as that is sufficient for supporting the Apple
> > keyboards.
> > 
> > Reviewed-by: Marek Vasut 
> > Reviewed-by: Neal Gompa 
> > Signed-off-by: Janne Grunau 
> > ---
> >   drivers/usb/host/xhci.c | 31 +++
> >   include/usb.h   |  6 ++
> >   2 files changed, 25 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> > index 534c4b973f..741e186ee0 100644
> > --- a/drivers/usb/host/xhci.c
> > +++ b/drivers/usb/host/xhci.c
> > @@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device 
> > *udev)
> > int slot_id = udev->slot_id;
> > struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
> > struct usb_interface *ifdesc;
> > +   unsigned int ifnum;
> > +   unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
> 
> no_of_if being of type u8 limits the number of interfaces to 255. 
> Introducing USB_MAX_ACTIVE_INTERFACES limit us to the first two 
> interfaces. Is this really needed? Handling all interface would avoid 
> the introduction of artificial limitations which may hit us on the next 
> device.

It's not strictly necessary but assume that the code was only tested
with a single interface. Given the general state of u-boot's USB stack
I wouldn't be surprised if it would blew up with USB descriptors maxing
out the number of interfaces / endpoints. So limiting it to the minimum
of what's needed to support the device in front of me looks like the
safer option.

Janne

[PATCH v3 0/6] USB keyboard improvements for asahi / desktop systems

[Janne Grunau via B4 Relay]

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 
---
Changes in v3:
- collected "Reviewed-by:" tags
- rename usb_blocklist to usb_ignorelist
- use BIT macro for USB KBD quirk bit
- refactor usb_device_is_blocked() to use 0 / negated errors as return
  value, sed -e 's/block/ignore/', simplify it and add comments
- rewritten usb_ignorelist documentation
- Link to v2: 
https://lore.kernel.org/r/20240317-asahi-keyboards-v2-0-d3f4b8384...@jannau.net

Changes in v2:
- rewritten commit message for "[PATCH 2/6] usb: xhci: Set up endpoints for the 
first 2 interfaces"
- Replaced the usb keyboard Yubikey block with an env based USB device blocklist
- Use "-EINVAL" as return value in "[PATCH 3/6] usb: xhci: Abort transfers with 
unallocated rings"
- added "Reviewed-by:" tags
- Link to v1: 
https://lore.kernel.org/r/20240221-asahi-keyboards-v1-0-814b2e741...@jannau.net

---
Janne Grunau (6):
  usb: xhci: refactor xhci_set_configuration
  usb: xhci: Set up endpoints for the first 2 interfaces
  usb: xhci: Abort transfers with unallocated rings
  usb: Add environment based device ignorelist
  usb: kbd: support Apple Magic Keyboards (2021)
  usb: kbd: Add probe quirk for Apple and Keychron keyboards

 common/usb.c |  57 
 common/usb_kbd.c |  59 ++--
 doc/usage/environment.rst|  13 +
 drivers/usb/host/xhci-ring.c |   5 ++
 drivers/usb/host/xhci.c  | 126 +++
 include/env_default.h|  11 
 include/usb.h|   6 +++
 7 files changed, 228 insertions(+), 49 deletions(-)
---
base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585
change-id: 20240218-asahi-keyboards-f2ddaf0022b2

Best regards,
-- 
Janne Grunau 

[PATCH v3 1/6] usb: xhci: refactor xhci_set_configuration

[Janne Grunau via B4 Relay]

From: Janne Grunau 

In the next step endpoints for multiple interfaces are set up. Move most
of the per endpoint initialization to separate function to avoid another
identation level.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 119 +---
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d13cbff9b3..534c4b973f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -475,67 +475,34 @@ static int xhci_configure_endpoints(struct usb_device 
*udev, bool ctx_change)
 }
 
 /**
- * Configure the endpoint, programming the device contexts.
+ * Fill endpoint contexts for interface descriptor ifdesc.
  *
- * @param udev pointer to the USB device structure
- * Return: returns the status of the xhci_configure_endpoints
+ * @param udev pointer to the USB device structure
+ * @param ctrl pointer to the xhci pravte device structure
+ * @param virt_dev pointer to the xhci virtual device structure
+ * @param ifdesc   pointer to the USB interface config descriptor
+ * Return: returns the status of xhci_init_ep_contexts_if
  */
-static int xhci_set_configuration(struct usb_device *udev)
+static int xhci_init_ep_contexts_if(struct usb_device *udev,
+   struct xhci_ctrl *ctrl,
+   struct xhci_virt_device *virt_dev,
+   struct usb_interface *ifdesc
+   )
 {
-   struct xhci_container_ctx *in_ctx;
-   struct xhci_container_ctx *out_ctx;
-   struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_ep_ctx *ep_ctx[MAX_EP_CTX_NUM];
int cur_ep;
-   int max_ep_flag = 0;
int ep_index;
unsigned int dir;
unsigned int ep_type;
-   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
-   int num_of_ep;
-   int ep_flag = 0;
u64 trb_64 = 0;
-   int slot_id = udev->slot_id;
-   struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
-   struct usb_interface *ifdesc;
u32 max_esit_payload;
unsigned int interval;
unsigned int mult;
unsigned int max_burst;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+   int num_of_ep = ifdesc->no_of_ep;
 
-   out_ctx = virt_dev->out_ctx;
-   in_ctx = virt_dev->in_ctx;
-
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
-   ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
-   /* Initialize the input context control */
-   ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
-   ctrl_ctx->drop_flags = 0;
-
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
-   }
-
-   xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
-
-   /* slot context */
-   xhci_slot_copy(ctrl, in_ctx, out_ctx);
-   slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);
-   slot_ctx->dev_info &= ~(cpu_to_le32(LAST_CTX_MASK));
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(max_ep_flag + 1) | 0);
-
-   xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
-
-   /* filling up ep contexts */
for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
struct usb_endpoint_descriptor *endpt_desc = NULL;
struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
@@ -561,7 +528,8 @@ static int xhci_set_configuration(struct usb_device *udev)
avg_trb_len = max_esit_payload;
 
ep_index = xhci_get_ep_index(endpt_desc);
-   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
+   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx,
+  ep_index);
 
/* Allocate the ep rings */
virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
@@ -614,6 +582,65 @@ static int xhci_set_configuration(struct usb_device *udev)
}
}
 
+   return 0;
+}
+
+/**
+ * Configure the endpoint, programming the device contexts.
+ *
+ * @param udev pointer to the USB device structure
+ * Return: returns the status of the xhci_configure_endpoints
+ */
+static int xhci_set_configuration(struct usb_device *udev)
+{
+   struct xhci_container_ctx *out_ctx;
+   struct xhci_container_ctx *in_ctx;
+   struct xhci_input_control_ctx *ctrl_ctx;
+   struct xhci_slot_ctx *slot_ctx;
+   int err;
+   int cur_ep;
+   int max_ep_f

[PATCH v3 5/6] usb: kbd: support Apple Magic Keyboards (2021)

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry
the HID keyboard boot protocol on the second interface descriptor.
Probe via vendor and product IDs since the class/subclass/protocol match
uses the first interface descriptor.
Probe the two first interface descriptors for the HID keyboard boot
protocol.

USB configuration descriptor for reference:

| Bus 003 Device 002: ID 05ac:029c Apple, Inc. Magic Keyboard
| Device Descriptor:
|   bLength18
|   bDescriptorType 1
|   bcdUSB   2.00
|   bDeviceClass0 [unknown]
|   bDeviceSubClass 0 [unknown]
|   bDeviceProtocol 0
|   bMaxPacketSize064
|   idVendor   0x05ac Apple, Inc.
|   idProduct  0x029c Magic Keyboard
|   bcdDevice3.90
|   iManufacturer   1 Apple Inc.
|   iProduct2 Magic Keyboard
|   iSerial 3 ...
|   bNumConfigurations  1
|   Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength   0x003b
| bNumInterfaces  2
| bConfigurationValue 1
| iConfiguration  4 Keyboard
| bmAttributes 0xa0
|   (Bus Powered)
|   Remote Wakeup
| MaxPower  500mA
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber0
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  0 [unknown]
|   bInterfaceProtocol  0
|   iInterface  5 Device Management
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode0 Not supported
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength  83
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81  EP 1 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber1
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  1 Boot Interface Subclass
|   bInterfaceProtocol  1 Keyboard
|   iInterface  6 Keyboard / Boot
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode   13 International (ISO)
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength 207
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x82  EP 2 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 37 ++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 4cbc9acb73..b2361bbf18 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -23,6 +23,14 @@
 
 #include 
 
+/*
+ * USB vendor and product IDs used for quirks.
+ */
+#define USB_VENDOR_ID_APPLE0x05ac
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_20210x029c
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -106,6 +114,8 @@ struct usb_kbd_pdata {
unsigned long   last_report;
struct int_queue *intq;
 
+   uint32_tifnum;
+
uint32_trepeat_delay;
 
uint32_tusb_in_pointer;
@@ -150,8 +160,8 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, 
u8 c)
  */
 static void usb_kbd_setled(struct usb_device *dev)
 {
-   struct usb_interface *iface = >config.if_desc[0];
struct usb_kbd_pdata *data = dev->privptr;
+   struct usb_int

[PATCH v3 4/6] usb: Add environment based device ignorelist

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Add the environment variable "usb_ignorelist" to prevent USB devices
listed in it from being bound to drivers. This allows to ignore devices
which are undesirable or trigger bugs in u-boot's USB stack.
Devices emulating keyboards are one example of undesirable devices as
u-boot currently supports only a single USB keyboard device. Most
commonly, people run into this with Yubikeys, so let's ignore those in
the default environment.

Based on previous USB keyboard specific patches for the same purpose.

Link: 
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 common/usb.c  | 57 +++
 doc/usage/environment.rst | 13 +++
 include/env_default.h | 11 +
 3 files changed, 81 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..1421b1bb13 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -28,6 +28,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1084,6 +1085,48 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
return 0;
 }
 
+static int usb_device_is_ignored(u16 id_vendor, u16 id_product)
+{
+   ulong vid, pid;
+   char *end;
+   const char *cur = env_get("usb_ignorelist");
+
+   /* parse "usb_ignorelist" strictly */
+   while (cur && cur[0] != '\0') {
+   vid = simple_strtoul(cur, , 0);
+   /*
+* If strtoul did not parse a single digit or the next char is
+* not ':' the ignore list is malformed.
+*/
+   if (cur == end || end[0] != ':')
+   return -EINVAL;
+
+   cur = end + 1;
+   pid = simple_strtoul(cur, , 0);
+   /* Consider '*' as wildcard for the product ID */
+   if (cur == end && end[0] == '*') {
+   pid = U16_MAX + 1;
+   end++;
+   }
+   /*
+* The ignore list is malformed if no product ID / wildcard was
+* parsed or entries are not separated by ',' or terminated with
+* '\0'.
+*/
+   if (cur == end || (end[0] != ',' && end[0] != '\0'))
+   return -EINVAL;
+
+   if (id_vendor == vid && (pid > U16_MAX || id_product == pid))
+   return -ENODEV;
+
+   if (end[0] == '\0')
+   break;
+   cur = end + 1;
+   }
+
+   return 0;
+}
+
 int usb_select_config(struct usb_device *dev)
 {
unsigned char *tmpbuf = NULL;
@@ -1099,6 +1142,20 @@ int usb_select_config(struct usb_device *dev)
le16_to_cpus(>descriptor.idProduct);
le16_to_cpus(>descriptor.bcdDevice);
 
+   /* ignore devices from usb_ignorelist */
+   err = usb_device_is_ignored(dev->descriptor.idVendor,
+   dev->descriptor.idProduct);
+   if (err == -ENODEV) {
+   dev_dbg(dev->dev, "Ignoring USB device 0x%x:0x%x\n",
+   dev->descriptor.idVendor, dev->descriptor.idProduct);
+   return err;
+   } else if (err == -EINVAL) {
+   printf("usb_ignorelist parse error in \"%s\"\n",
+  env_get("usb_ignorelist"));
+   } else if (err < 0) {
+   return err;
+   }
+
/*
 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
 * about this first Get Descriptor request. If there are any other
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index ebf75fa948..7d4b448cb3 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -366,6 +366,19 @@ tftpwindowsize
 This means the count of blocks we can receive before
 sending ack to server.
 
+usb_ignorelist
+Ignore USB devices to prevent binding them to an USB device driver. This 
can
+be used to ignore devices are for some reason undesirable or causes crashes
+u-boot's USB stack.
+An example for undesired behavior is the keyboard emulation of security 
keys
+like Yubikeys. U-boot currently supports only a single USB keyboard device
+so try to probe an useful keyboard device. The default environment blocks
+Yubico devices as common devices emulating keyboards.
+Devices are matched by idVendor and idProduct. The variable contains a 
comma
+separated list of idVendor:idProduct pairs as hexadecimal numbers joined
+by a colon. '*' functions as a wildcard for idProduct to block all devices
+with the specified idVendor.
+
 vlan
 When set to a value < 4095 the traffic over
 Ethernet is encapsulated/received over 802.1q
diff --git a/inclu

[PATCH v3 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The xhci driver currently only does the necessary initialization for
endpoints found in the first interface descriptor. Apple USB keyboards
(released 2021) use the second interface descriptor for the HID keyboard
boot protocol. To allow USB drivers to use endpoints from other
interface descriptors the xhci driver needs to ensure these endpoints
are initialized as well.
Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
are considered during endpoint initialisation.
For now define it to 2 as that is sufficient for supporting the Apple
keyboards.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 31 +++
 include/usb.h   |  6 ++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 534c4b973f..741e186ee0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device *udev)
int slot_id = udev->slot_id;
struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
struct usb_interface *ifdesc;
+   unsigned int ifnum;
+   unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
+(unsigned int)udev->config.no_of_if);
 
out_ctx = virt_dev->out_ctx;
in_ctx = virt_dev->in_ctx;
 
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
/* Initialize the input context control */
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
ctrl_ctx->drop_flags = 0;
 
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   num_of_ep = ifdesc->no_of_ep;
+   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
+   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
+   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
+   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
+   if (max_ep_flag < ep_flag)
+   max_ep_flag = ep_flag;
+   }
}
 
xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
@@ -637,9 +641,12 @@ static int xhci_set_configuration(struct usb_device *udev)
xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
 
/* filling up ep contexts */
-   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
-   if (err < 0)
-   return err;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
+   if (err < 0)
+   return err;
+   }
 
return xhci_configure_endpoints(udev, false);
 }
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb30..3aafdc8bfd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -49,6 +49,12 @@ extern bool usb_started; /* flag for the started/stopped USB 
status */
  */
 #define USB_TIMEOUT_MS(pipe) (usb_pipebulk(pipe) ? 5000 : 1000)
 
+/*
+ * The xhcd hcd driver prepares only a limited number interfaces / endpoints.
+ * Define this limit so that drivers do not exceed it.
+ */
+#define USB_MAX_ACTIVE_INTERFACES  2
+
 /* device request (setup) */
 struct devrequest {
__u8requesttype;

-- 
2.44.0

[PATCH v3 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Those keyboards do not return the current device state. Polling will
timeout unless there are key presses. This is not a problem during
operation but the initial device state query during probing will fail.
Skip this step in usb_kbd_probe_dev() to make these devices useable.
Not all Apple keyboards behave like this. A keyboard with USB
vendor/product ID 05ac:0221 is reported to work with the current code.
Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and
show the same behavior (Keychron C2, 05ac:024f for example).

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index b2361bbf18..820f591fc5 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -31,6 +31,10 @@
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
 
+#define USB_VENDOR_ID_KEYCHRON 0x3434
+
+#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE  BIT(0)
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -474,6 +478,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
struct usb_interface *iface;
struct usb_endpoint_descriptor *ep;
struct usb_kbd_pdata *data;
+   unsigned int quirks = 0;
int epNum;
 
if (dev->descriptor.bNumConfigurations != 1)
@@ -506,6 +511,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
 
debug("USB KBD: found interrupt EP: 0x%x\n", ep->bEndpointAddress);
 
+   switch (dev->descriptor.idVendor) {
+   case USB_VENDOR_ID_APPLE:
+   case USB_VENDOR_ID_KEYCHRON:
+   quirks |= USB_HID_QUIRK_POLL_NO_REPORT_IDLE;
+   break;
+   default:
+   break;
+   }
+
data = malloc(sizeof(struct usb_kbd_pdata));
if (!data) {
printf("USB KBD: Error allocating private data\n");
@@ -546,6 +560,14 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
 #endif
 
+   /*
+* Apple and Keychron keyboards do not report the device state. Reports
+* are only returned during key presses.
+*/
+   if (quirks & USB_HID_QUIRK_POLL_NO_REPORT_IDLE) {
+   debug("USB KBD: quirk: skip testing device state\n");
+   return 1;
+   }
debug("USB KBD: enable interrupt pipe...\n");
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
data->intq = create_int_queue(dev, data->intpipe, 1,

-- 
2.44.0

[PATCH v3 3/6] usb: xhci: Abort transfers with unallocated rings

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Discovered while trying to use the second interface in the USB keyboard
driver necessary on Apple USB keyboards.

Reviewed-by: Marek Vasut 
Reviewed-by: Neal Gompa 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci-ring.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b60661fe05..910c5f3352 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
reset_ep(udev, ep_index);
 
ring = virt_dev->eps[ep_index].ring;
+   if (!ring)
+   return -EINVAL;
+
/*
 * How much data is (potentially) left before the 64KB boundary?
 * XHCI Spec puts restriction( TABLE 49 and 6.4.1 section of XHCI Spec)
@@ -871,6 +874,8 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
ep_index = usb_pipe_ep_index(pipe);
 
ep_ring = virt_dev->eps[ep_index].ring;
+   if (!ep_ring)
+   return -EINVAL;
 
/*
 * Check to see if the max packet size for the default control

-- 
2.44.0

Re: [PATCH v3 4/7] efi_selftest: Add international characters test

[Janne Grunau]

On Sun, Mar 17, 2024 at 10:24:13AM +0100, Heinrich Schuchardt wrote:
> On 3/16/24 22:50, Janne Grunau via B4 Relay wrote:
> > From: Andre Przywara 
> >
> > UEFI relies entirely on unicode output, which actual fonts displayed on
> > the screen might not be ready for.
> >
> > Add a test displaying some international characters, to reveal missing
> > glyphs, especially in our builtin fonts.
> > This would be needed to be manually checked on the screen for
> > correctness.
> >
> > Signed-off-by: Andre Przywara 
> > Suggested-by: Heinrich Schuchardt 
> > Signed-off-by: Janne Grunau 
> > ---
> >   lib/efi_selftest/efi_selftest_textoutput.c | 21 +
> >   1 file changed, 21 insertions(+)
> >
> > diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
> > b/lib/efi_selftest/efi_selftest_textoutput.c
> > index cc44b38bc2..917903473d 100644
> > --- a/lib/efi_selftest/efi_selftest_textoutput.c
> > +++ b/lib/efi_selftest/efi_selftest_textoutput.c
> > @@ -31,6 +31,21 @@ static int execute(void)
> > 0xD804, 0xDC22,
> > 0};
> >
> > +   const u16 text[] =
> > +u"This should render international characters as described\n"
> > +u"U+00D6 \u00D6 - Latin capital letter O with diaresis\n"
> > +u"U+00DF \u00DF - Latin small letter sharp s\n"
> > +u"U+00E5 \u00E5 - Latin small letter a with ring above\n"
> > +u"U+00E9 \u00E9 - Latin small letter e with acute\n"
> > +u"U+00F1 \u00F1 - Latin small letter n with tilde\n"
> > +u"U+00F6 \u00F6 - Latin small letter o with diaresis\n"
> > +u"The following characters will render as '?' with bitmap fonts\n"
> > +u"U+00F8 \u00F8 - Latin small letter o with stroke\n"
> > +u"U+03AC \u03AC - Greek small letter alpha with tonus\n"
> > +u"U+03BB \u03BB - Greek small letter lambda\n"
> > +u"U+03C2 \u03C2 - Greek small letter final sigma\n"
> > +u"U+1F19 \u1F19 - Greek capital letter epsilon with dasia\n";
> 
> The strings should be indented by two tabs.

done locally for all selftests, I'm waiting on fedback for Patch 2 and 3
before resending.

thanks

Janne

Re: [PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

On Mon, Mar 18, 2024 at 03:17:33PM +0100, Marek Vasut wrote:
> On 3/18/24 8:33 AM, Janne Grunau wrote:
> 
> >>>>> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> >>>>> +{
> >>>>> +   printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> >>>>> +  blocklist);
> >>>>> +   return 0;
> >>>>
> >>>> This could be static void without return 0 at the end.
> >>>
> >>> the return is there to break out of the while loop on parsing errors in a 
> >>> single statement. This probably won't be necessary after using strsep and 
> >>> sscanf in the parsing function but see below.
> >>
> >> Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?
> > 
> > It returns 0 so that parsing errors in the blocklist do not result
> > in blocking every USB device. That looked to me like the
> > less bad error behavior to me.
> 
> In unix , 0 is considered success and non-zero failure .
> 
> How about this:
> 
> - Return -EINVAL here (parsing failed)

If we return 0 / negated errors we do not need this function and we can
simply report the parsing error when usb_device_is_blocked() return
-EINVAL.

> - Instead of 'return 1' in usb_device_is_blocked() return -ENODEV
> - In usb_select_config(), check
>if usb_device_is_blocked returned 0, no device blocked OK
>if usb_device_is_blocked returned -ENODEV, device blocked,
>  return -ENODEV
>if usb_device_is_blocked returned any other error, parsing error
>  return that error

I think the preferable option is to ignore parsing errors. If we
would propagate the error the result would be that every USB device is
ignored. 
 
> What do you think ?

Fine by me, -EINVAL makes the parsing error reporting less awkward. Only
open question is what should happen on parsing errors.

locally modified and ready to resend once we agree on the behavior on
parsing errors

Janne

Re: [PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

Hej,

On Sun, Mar 17, 2024, at 22:39, E Shattow wrote:
> Should be usb_denylist, since "block devices" is ambiguous meaning if
> it is a list of data chunks (blocks) or if it blocks (deny). There is
> no question what a denylist is.

I briefly thought of that concluded that the spelling "blocklist" removed
the ambiguity. I not sure about "denylist". It sounds weird to me in the
context of probing USB devices. I'd rather use "ignorelist" as alternative.

Janne

Re: [PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

On Mon, Mar 18, 2024, at 06:06, Marek Vasut wrote:
> On 3/17/24 7:15 PM, Janne Grunau wrote:
>> Hej,
>
> Hi,
>
>> On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
>>> On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
>>>> From: Janne Grunau 
>>>>
>>>> Add the environment variable "usb_blocklist" to prevent USB devices
>>>> listed in it from being used. This allows to ignore devices which
>>>> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
>>>> Devices emulating keyboards are one example. U-boot currently supports
>>>> only one USB keyboard device. Most commonly, people run into this with
>>>> Yubikeys, so let's ignore those in the default environment.
>>>>
>>>> Based on previous USB keyboard specific patches for the same purpose.
>>>>
>>>> Link: 
>>>> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
>>>> Signed-off-by: Janne Grunau 
>>>> ---
>>>>common/usb.c  | 56 
>>>> +++
>>>>doc/usage/environment.rst | 12 ++
>>>>include/env_default.h | 11 ++
>>>>3 files changed, 79 insertions(+)
>>>>
>>>> diff --git a/common/usb.c b/common/usb.c
>>>> index 836506dcd9..73af5be066 100644
>>>> --- a/common/usb.c
>>>> +++ b/common/usb.c
>>>> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device 
>>>> *dev, int addr, bool do_read,
>>>>return 0;
>>>>}
>>>>
>>>> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
>>>> +{
>>>> +  printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
>>>> + blocklist);
>>>> +  return 0;
>>>
>>> This could be static void without return 0 at the end.
>> 
>> the return is there to break out of the while loop on parsing errors in a 
>> single statement. This probably won't be necessary after using strsep and 
>> sscanf in the parsing function but see below.
>
> Ahh, now I see it. But then, shouldn't this return -ENODEV here already ?

It returns 0 so that parsing errors in the blocklist do not result
in blocking every USB device. That looked to me like the
less bad error behavior to me.

>>>> +}
>>>> +
>>>> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
>>>> +{
>>>> +  ulong vid, pid;
>>>> +  char *end;
>>>> +  const char *block_str = env_get("usb_blocklist");
>>>> +  const char *cur = block_str;
>>>> +
>>>> +  /* parse "usb_blocklist" strictly */
>>>> +  while (cur && cur[0] != '\0') {
>>>
>>> Have a look at strsep() , namely strsep(block_str, ","); This will split
>>> the string up for you at "," delimiters.
>>>
>>> Example is in drivers/dfu/dfu.c dfu_config_interfaces() .
>> 
>> strsep() is probably a good idea even if it alone won't make the code that 
>> much simpler for strict parsing.
>> 
>>> And then, on each token, you can try and run sscanf(token, "%04x:%04x",
>>> vid, pid);, that will parse the token format for you. See e.g.
>>> test/lib/sscanf.c for further examples.
>>>
>>> That should simplify the parsing a lot.
>> 
>> It would but sscanf() is optional and is only selected by CONFIG_XEN so I 
>> assumed there would be concerns over binary size increase if USB_HOST would 
>> require sscanf.
>
> Good point, lets postpone sscanf() . Also, looking at it, sscanf would 
> work for numbers, but not for the special characters. So ... do you want 
> to try tweaking this further with strsep() or shall we go with this 
> implementation ?

I started converting it to strsep. It makes the intent clearer but it doesn't
simplify the implementation much. strsep() has the disadvantage that
it can't work in place and needs to copy the string. If we go with strsep
I would look into parsing the list once at USB init time and use a list of
numeric vendor, product ID pairs at device probe time.
If have a slight preference for the current implementation (with ignore
or deny instead of blocklist) as long as the list remains short.

Janne

Re: [PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

Hej,

On Sun, Mar 17, 2024, at 17:18, Marek Vasut wrote:
> On 3/17/24 12:07 PM, Janne Grunau via B4 Relay wrote:
>> From: Janne Grunau 
>> 
>> Add the environment variable "usb_blocklist" to prevent USB devices
>> listed in it from being used. This allows to ignore devices which
>> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
>> Devices emulating keyboards are one example. U-boot currently supports
>> only one USB keyboard device. Most commonly, people run into this with
>> Yubikeys, so let's ignore those in the default environment.
>> 
>> Based on previous USB keyboard specific patches for the same purpose.
>> 
>> Link: 
>> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
>> Signed-off-by: Janne Grunau 
>> ---
>>   common/usb.c  | 56 
>> +++
>>   doc/usage/environment.rst | 12 ++
>>   include/env_default.h | 11 ++
>>   3 files changed, 79 insertions(+)
>> 
>> diff --git a/common/usb.c b/common/usb.c
>> index 836506dcd9..73af5be066 100644
>> --- a/common/usb.c
>> +++ b/common/usb.c
>> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev, 
>> int addr, bool do_read,
>>  return 0;
>>   }
>>   
>> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
>> +{
>> +printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
>> +   blocklist);
>> +return 0;
>
> This could be static void without return 0 at the end.

the return is there to break out of the while loop on parsing errors in a 
single statement. This probably won't be necessary after using strsep and 
sscanf in the parsing function but see below.

>> +}
>> +
>> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
>> +{
>> +ulong vid, pid;
>> +char *end;
>> +const char *block_str = env_get("usb_blocklist");
>> +const char *cur = block_str;
>> +
>> +/* parse "usb_blocklist" strictly */
>> +while (cur && cur[0] != '\0') {
>
> Have a look at strsep() , namely strsep(block_str, ","); This will split 
> the string up for you at "," delimiters.
>
> Example is in drivers/dfu/dfu.c dfu_config_interfaces() .

strsep() is probably a good idea even if it alone won't make the code that much 
simpler for strict parsing.

> And then, on each token, you can try and run sscanf(token, "%04x:%04x", 
> vid, pid);, that will parse the token format for you. See e.g. 
> test/lib/sscanf.c for further examples.
>
> That should simplify the parsing a lot.

It would but sscanf() is optional and is only selected by CONFIG_XEN so I 
assumed there would be concerns over binary size increase if USB_HOST would 
require sscanf.

Janne

[PATCH 1/4] apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA

[Janne Grunau via B4 Relay]

From: Hector Martin 

This makes USB HDDs >2TiB work. The only reason this hasn't bitten us
for the internal NVMe yet is the 4K sector size, because the largest SSD
Apple sells is 8TB and we can handle up to 16TiB with that sector size.
Close call.

Signed-off-by: Hector Martin 
Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index e00d72e8be..31d966f0ab 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -10,6 +10,7 @@ CONFIG_SYS_PBSIZE=276
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_LATE_INIT=y
 # CONFIG_NET is not set
+CONFIG_SYS_64BIT_LBA=y
 CONFIG_APPLE_SPI_KEYB=y
 # CONFIG_MMC is not set
 CONFIG_NVME_APPLE=y

-- 
2.44.0

[PATCH 3/4] configs: apple: Enable CMD_SELECT_FONT and FONT_16X32

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple devices have high DPI displays so the larger fonts are preferable
for improved readability. This does not yet change the used font based
on the display's pixel density so the standard 8x16 font is still used
by default.

Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index 31d966f0ab..c30aec7c55 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -9,6 +9,7 @@ CONFIG_SYS_PBSIZE=276
 # CONFIG_DISPLAY_CPUINFO is not set
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_BOARD_LATE_INIT=y
+CONFIG_CMD_SELECT_FONT=y
 # CONFIG_NET is not set
 CONFIG_SYS_64BIT_LBA=y
 CONFIG_APPLE_SPI_KEYB=y
@@ -19,6 +20,7 @@ CONFIG_USB_XHCI_DWC3=y
 CONFIG_USB_XHCI_PCI=y
 CONFIG_USB_DWC3=y
 CONFIG_USB_KEYBOARD=y
+CONFIG_VIDEO_FONT_16X32=y
 CONFIG_SYS_WHITE_ON_BLACK=y
 CONFIG_NO_FB_CLEAR=y
 CONFIG_VIDEO_SIMPLE=y

-- 
2.44.0

[PATCH 2/4] configs: apple: Use "vidconsole,serial" as stdout/stderr

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The display size querying in efi_console relies on this order. The
display should be the primary output device and should be used to
display more than 80x25 chars.

Signed-off-by: Janne Grunau 
---
 include/configs/apple.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/configs/apple.h b/include/configs/apple.h
index 0576bc04c9..a70440b3ad 100644
--- a/include/configs/apple.h
+++ b/include/configs/apple.h
@@ -6,8 +6,8 @@
 /* Environment */
 #define ENV_DEVICE_SETTINGS \
"stdin=serial,usbkbd,spikbd\0" \
-   "stdout=serial,vidconsole\0" \
-   "stderr=serial,vidconsole\0"
+   "stdout=vidconsole,serial\0" \
+   "stderr=vidconsole,serial\0"
 
 #if IS_ENABLED(CONFIG_CMD_NVME)
#define BOOT_TARGET_NVME(func) func(NVME, nvme, 0)

-- 
2.44.0

[PATCH 4/4] arm: apple: Switch to standard boot

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Use standard boot instead of the distro boot scripts.

Signed-off-by: Janne Grunau 
---
 arch/arm/Kconfig|  2 +-
 include/configs/apple.h | 20 ++--
 2 files changed, 3 insertions(+), 19 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 01d6556c42..ad89abde41 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1034,7 +1034,7 @@ config ARCH_APPLE
select USB
imply CMD_DM
imply CMD_GPT
-   imply DISTRO_DEFAULTS
+   imply BOOTSTD_DEFAULTS
imply OF_HAS_PRIOR_STAGE
 
 config ARCH_OWL
diff --git a/include/configs/apple.h b/include/configs/apple.h
index a70440b3ad..1e08b11448 100644
--- a/include/configs/apple.h
+++ b/include/configs/apple.h
@@ -9,26 +9,10 @@
"stdout=vidconsole,serial\0" \
"stderr=vidconsole,serial\0"
 
-#if IS_ENABLED(CONFIG_CMD_NVME)
-   #define BOOT_TARGET_NVME(func) func(NVME, nvme, 0)
-#else
-   #define BOOT_TARGET_NVME(func)
-#endif
-
-#if IS_ENABLED(CONFIG_CMD_USB)
-   #define BOOT_TARGET_USB(func) func(USB, usb, 0)
-#else
-   #define BOOT_TARGET_USB(func)
-#endif
-
-#define BOOT_TARGET_DEVICES(func) \
-   BOOT_TARGET_NVME(func) \
-   BOOT_TARGET_USB(func)
-
-#include 
+#define BOOT_TARGETS   "nvme usb"
 
 #define CFG_EXTRA_ENV_SETTINGS \
ENV_DEVICE_SETTINGS \
-   BOOTENV
+   "boot_targets=" BOOT_TARGETS "\0"
 
 #endif

-- 
2.44.0

[PATCH 0/4] configs: apple: Switch to standard boot + small adjustments

[Janne Grunau via B4 Relay]

This series contains a few misc config changes for Apple silicon
systems:
- switch from the deprecated distro boot scripts to standard boot
- allows EFI console resizing based on the video console size
- enables 16x32 bitmap fonts as Apple devices come with high DPI
  displays
- enables 64-bit LBA addressing for USB storage devices larger than 2TB

Signed-off-by: Janne Grunau 
---
Hector Martin (1):
  apple_m1_defconfig: Turn on CONFIG_SYS_64BIT_LBA

Janne Grunau (3):
  configs: apple: Use "vidconsole,serial" as stdout/stderr
  configs: apple: Enable CMD_SELECT_FONT and FONT_16X32
  arm: apple: Switch to standard boot

 arch/arm/Kconfig   |  2 +-
 configs/apple_m1_defconfig |  3 +++
 include/configs/apple.h| 24 
 3 files changed, 8 insertions(+), 21 deletions(-)
---
base-commit: f3c979dd0053c082d2df170446923e7ce5edbc2d
change-id: 20240317-apple_config-981fcd9028b9

Best regards,
-- 
Janne Grunau 

Re: [PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau]

Hej,

On Sun, Mar 17, 2024, at 12:07, Janne Grunau via B4 Relay wrote:
> From: Janne Grunau 
>
> Add the environment variable "usb_blocklist" to prevent USB devices
> listed in it from being used. This allows to ignore devices which
> trigger bugs in u-boot's USB stack or are undesirable for other reasons.
> Devices emulating keyboards are one example. U-boot currently supports
> only one USB keyboard device. Most commonly, people run into this with
> Yubikeys, so let's ignore those in the default environment.
>
> Based on previous USB keyboard specific patches for the same purpose.
>
> Link: 
> https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
> Signed-off-by: Janne Grunau 
> ---
>  common/usb.c  | 56 
> +++
>  doc/usage/environment.rst | 12 ++
>  include/env_default.h | 11 ++
>  3 files changed, 79 insertions(+)
>
> diff --git a/common/usb.c b/common/usb.c
> index 836506dcd9..73af5be066 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device 
> *dev, int addr, bool do_read,
>   return 0;
>  }
> 
> +static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
> +{
> + printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
> +blocklist);
> + return 0;
> +}
> +
> +static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
> +{
> + ulong vid, pid;
> + char *end;
> + const char *block_str = env_get("usb_blocklist");
> + const char *cur = block_str;
> +
> + /* parse "usb_blocklist" strictly */
> + while (cur && cur[0] != '\0') {
> + vid = simple_strtoul(cur, , 0);
> + if (cur == end || end[0] != ':')
> + return usb_blocklist_parse_error(block_str,
> +  cur - block_str);
> +
> + cur = end + 1;
> + pid = simple_strtoul(cur, , 0);
> + if (cur == end && end[0] != '*')
> + return usb_blocklist_parse_error(block_str,
> +  cur - block_str);
> +
> + if (end[0] == '*') {
> + /* use out of range idProduct as wildcard indication */
> + pid = U16_MAX + 1;
> + end++;
> + }
> + if (end[0] != ',' && end[0] != '\0')
> + return usb_blocklist_parse_error(block_str,
> +  cur - block_str);
> +
> + if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
> + printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
> +   id_product);

this is a leftover from testing, already removed locally

Janne

[PATCH v2 0/6] USB keyboard improvements for asahi / desktop systems

[Janne Grunau via B4 Relay]

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 
---
Changes in v2:
- rewritten commit message for "[PATCH 2/6] usb: xhci: Set up endpoints for the 
first 2 interfaces"
- Replaced the usb keyboard Yubikey block with an env based USB device blocklist
- Use "-EINVAL" as return value in "[PATCH 3/6] usb: xhci: Abort transfers with 
unallocated rings"
- added "Reviewed-by:" tags
- Link to v1: 
https://lore.kernel.org/r/20240221-asahi-keyboards-v1-0-814b2e741...@jannau.net

---
Janne Grunau (6):
  usb: xhci: refactor xhci_set_configuration
  usb: xhci: Set up endpoints for the first 2 interfaces
  usb: xhci: Abort transfers with unallocated rings
  usb: Add environment based device blocklist
  usb: kbd: support Apple Magic Keyboards (2021)
  usb: kbd: Add probe quirk for Apple and Keychron keyboards

 common/usb.c |  56 +++
 common/usb_kbd.c |  61 +++--
 doc/usage/environment.rst|  12 +
 drivers/usb/host/xhci-ring.c |   5 ++
 drivers/usb/host/xhci.c  | 126 +++
 include/env_default.h|  11 
 include/usb.h|   6 +++
 7 files changed, 227 insertions(+), 50 deletions(-)
---
base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585
change-id: 20240218-asahi-keyboards-f2ddaf0022b2

Best regards,
-- 
Janne Grunau 

[PATCH v2 5/6] usb: kbd: support Apple Magic Keyboards (2021)

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry
the HID keyboard boot protocol on the second interface descriptor.
Probe via vendor and product IDs since the class/subclass/protocol match
uses the first interface descriptor.
Probe the two first interface descriptors for the HID keyboard boot
protocol.

USB configuration descriptor for reference:

| Bus 003 Device 002: ID 05ac:029c Apple, Inc. Magic Keyboard
| Device Descriptor:
|   bLength18
|   bDescriptorType 1
|   bcdUSB   2.00
|   bDeviceClass0 [unknown]
|   bDeviceSubClass 0 [unknown]
|   bDeviceProtocol 0
|   bMaxPacketSize064
|   idVendor   0x05ac Apple, Inc.
|   idProduct  0x029c Magic Keyboard
|   bcdDevice3.90
|   iManufacturer   1 Apple Inc.
|   iProduct2 Magic Keyboard
|   iSerial 3 ...
|   bNumConfigurations  1
|   Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength   0x003b
| bNumInterfaces  2
| bConfigurationValue 1
| iConfiguration  4 Keyboard
| bmAttributes 0xa0
|   (Bus Powered)
|   Remote Wakeup
| MaxPower  500mA
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber0
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  0 [unknown]
|   bInterfaceProtocol  0
|   iInterface  5 Device Management
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode0 Not supported
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength  83
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81  EP 1 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber1
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  1 Boot Interface Subclass
|   bInterfaceProtocol  1 Keyboard
|   iInterface  6 Keyboard / Boot
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode   13 International (ISO)
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength 207
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x82  EP 2 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8

Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 39 +++
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 4cbc9acb73..8cc3345075 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -23,6 +23,14 @@
 
 #include 
 
+/*
+ * USB vendor and product IDs used for quirks.
+ */
+#define USB_VENDOR_ID_APPLE0x05ac
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_20210x029c
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -106,6 +114,8 @@ struct usb_kbd_pdata {
unsigned long   last_report;
struct int_queue *intq;
 
+   uint32_tifnum;
+
uint32_trepeat_delay;
 
uint32_tusb_in_pointer;
@@ -150,8 +160,8 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, 
u8 c)
  */
 static void usb_kbd_setled(struct usb_device *dev)
 {
-   struct usb_interface *iface = >config.if_desc[0];
struct usb_kbd_pdata *data = dev->privptr;
+   struct usb_interface *iface = >config.if_desc[da

[PATCH v2 1/6] usb: xhci: refactor xhci_set_configuration

[Janne Grunau via B4 Relay]

From: Janne Grunau 

In the next step endpoints for multiple interfaces are set up. Move most
of the per endpoint initialization to separate function to avoid another
identation level.

Reviewed-by: Marek Vasut 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 119 +---
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d13cbff9b3..534c4b973f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -475,67 +475,34 @@ static int xhci_configure_endpoints(struct usb_device 
*udev, bool ctx_change)
 }
 
 /**
- * Configure the endpoint, programming the device contexts.
+ * Fill endpoint contexts for interface descriptor ifdesc.
  *
- * @param udev pointer to the USB device structure
- * Return: returns the status of the xhci_configure_endpoints
+ * @param udev pointer to the USB device structure
+ * @param ctrl pointer to the xhci pravte device structure
+ * @param virt_dev pointer to the xhci virtual device structure
+ * @param ifdesc   pointer to the USB interface config descriptor
+ * Return: returns the status of xhci_init_ep_contexts_if
  */
-static int xhci_set_configuration(struct usb_device *udev)
+static int xhci_init_ep_contexts_if(struct usb_device *udev,
+   struct xhci_ctrl *ctrl,
+   struct xhci_virt_device *virt_dev,
+   struct usb_interface *ifdesc
+   )
 {
-   struct xhci_container_ctx *in_ctx;
-   struct xhci_container_ctx *out_ctx;
-   struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_ep_ctx *ep_ctx[MAX_EP_CTX_NUM];
int cur_ep;
-   int max_ep_flag = 0;
int ep_index;
unsigned int dir;
unsigned int ep_type;
-   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
-   int num_of_ep;
-   int ep_flag = 0;
u64 trb_64 = 0;
-   int slot_id = udev->slot_id;
-   struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
-   struct usb_interface *ifdesc;
u32 max_esit_payload;
unsigned int interval;
unsigned int mult;
unsigned int max_burst;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+   int num_of_ep = ifdesc->no_of_ep;
 
-   out_ctx = virt_dev->out_ctx;
-   in_ctx = virt_dev->in_ctx;
-
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
-   ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
-   /* Initialize the input context control */
-   ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
-   ctrl_ctx->drop_flags = 0;
-
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
-   }
-
-   xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
-
-   /* slot context */
-   xhci_slot_copy(ctrl, in_ctx, out_ctx);
-   slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);
-   slot_ctx->dev_info &= ~(cpu_to_le32(LAST_CTX_MASK));
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(max_ep_flag + 1) | 0);
-
-   xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
-
-   /* filling up ep contexts */
for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
struct usb_endpoint_descriptor *endpt_desc = NULL;
struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
@@ -561,7 +528,8 @@ static int xhci_set_configuration(struct usb_device *udev)
avg_trb_len = max_esit_payload;
 
ep_index = xhci_get_ep_index(endpt_desc);
-   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
+   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx,
+  ep_index);
 
/* Allocate the ep rings */
virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
@@ -614,6 +582,65 @@ static int xhci_set_configuration(struct usb_device *udev)
}
}
 
+   return 0;
+}
+
+/**
+ * Configure the endpoint, programming the device contexts.
+ *
+ * @param udev pointer to the USB device structure
+ * Return: returns the status of the xhci_configure_endpoints
+ */
+static int xhci_set_configuration(struct usb_device *udev)
+{
+   struct xhci_container_ctx *out_ctx;
+   struct xhci_container_ctx *in_ctx;
+   struct xhci_input_control_ctx *ctrl_ctx;
+   struct xhci_slot_ctx *slot_ctx;
+   int err;
+   int cur_ep;
+   int max_ep_flag = 0;
+   struct xhci_ct

[PATCH v2 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Those keyboards do not return the current device state. Polling will
timeout unless there are key presses. This is not a problem during
operation but the inital device state query during probing will fail.
Skip this step in usb_kbd_probe_dev() to make these devices useable.
Not all Apple keyboards behave like this. A keyboard with USB
vendor/product ID 05ac:0221 is reported to work with the current code.
Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and
show the same behavior (Keychron C2, 05ac:024f for example).

Reviewed-by: Marek Vasut 
Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 8cc3345075..43c7668671 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -31,6 +31,10 @@
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
 
+#define USB_VENDOR_ID_KEYCHRON 0x3434
+
+#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE  (1 << 0)
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -474,6 +478,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
struct usb_interface *iface;
struct usb_endpoint_descriptor *ep;
struct usb_kbd_pdata *data;
+   unsigned int quirks = 0;
int epNum;
 
if (dev->descriptor.bNumConfigurations != 1)
@@ -506,6 +511,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
 
debug("USB KBD: found interrupt EP: 0x%x\n", ep->bEndpointAddress);
 
+   switch (dev->descriptor.idVendor) {
+   case USB_VENDOR_ID_APPLE:
+   case USB_VENDOR_ID_KEYCHRON:
+   quirks |= USB_HID_QUIRK_POLL_NO_REPORT_IDLE;
+   break;
+   default:
+   break;
+   }
+
data = malloc(sizeof(struct usb_kbd_pdata));
if (!data) {
printf("USB KBD: Error allocating private data\n");
@@ -546,6 +560,14 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
 #endif
 
+   /*
+* Apple and Keychron keyboards do not report the device state. Reports
+* are only returned during key presses.
+*/
+   if (quirks & USB_HID_QUIRK_POLL_NO_REPORT_IDLE) {
+   debug("USB KBD: quirk: skip testing device state\n");
+   return 1;
+   }
debug("USB KBD: enable interrupt pipe...\n");
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
data->intq = create_int_queue(dev, data->intpipe, 1,

-- 
2.44.0

[PATCH v2 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The xhci driver currently only does the necessary initialization for
endpoints found in the first interface descriptor. Apple USB keyboards
(released 2021) use the second interface descriptor for the HID keyboard
boot protocol. To allow USB drivers to use endpoints from other
interface descriptors the xhci driver needs to ensure these endpoints
are initialized as well.
Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
are considered during endpoint initialisation.
For now define it to 2 as that is sufficient for supporting the Apple
keyboards.

Reviewed-by: Marek Vasut 
Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 31 +++
 include/usb.h   |  6 ++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 534c4b973f..741e186ee0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device *udev)
int slot_id = udev->slot_id;
struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
struct usb_interface *ifdesc;
+   unsigned int ifnum;
+   unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
+(unsigned int)udev->config.no_of_if);
 
out_ctx = virt_dev->out_ctx;
in_ctx = virt_dev->in_ctx;
 
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
/* Initialize the input context control */
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
ctrl_ctx->drop_flags = 0;
 
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   num_of_ep = ifdesc->no_of_ep;
+   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
+   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
+   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
+   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
+   if (max_ep_flag < ep_flag)
+   max_ep_flag = ep_flag;
+   }
}
 
xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
@@ -637,9 +641,12 @@ static int xhci_set_configuration(struct usb_device *udev)
xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
 
/* filling up ep contexts */
-   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
-   if (err < 0)
-   return err;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
+   if (err < 0)
+   return err;
+   }
 
return xhci_configure_endpoints(udev, false);
 }
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb30..3aafdc8bfd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -49,6 +49,12 @@ extern bool usb_started; /* flag for the started/stopped USB 
status */
  */
 #define USB_TIMEOUT_MS(pipe) (usb_pipebulk(pipe) ? 5000 : 1000)
 
+/*
+ * The xhcd hcd driver prepares only a limited number interfaces / endpoints.
+ * Define this limit so that drivers do not exceed it.
+ */
+#define USB_MAX_ACTIVE_INTERFACES  2
+
 /* device request (setup) */
 struct devrequest {
__u8requesttype;

-- 
2.44.0

[PATCH v2 4/6] usb: Add environment based device blocklist

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Add the environment variable "usb_blocklist" to prevent USB devices
listed in it from being used. This allows to ignore devices which
trigger bugs in u-boot's USB stack or are undesirable for other reasons.
Devices emulating keyboards are one example. U-boot currently supports
only one USB keyboard device. Most commonly, people run into this with
Yubikeys, so let's ignore those in the default environment.

Based on previous USB keyboard specific patches for the same purpose.

Link: 
https://lore.kernel.org/u-boot/7ab604fb-0fec-4f5e-8708-7a3a7e2cb...@denx.de/
Signed-off-by: Janne Grunau 
---
 common/usb.c  | 56 +++
 doc/usage/environment.rst | 12 ++
 include/env_default.h | 11 ++
 3 files changed, 79 insertions(+)

diff --git a/common/usb.c b/common/usb.c
index 836506dcd9..73af5be066 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -1084,6 +1084,57 @@ static int usb_prepare_device(struct usb_device *dev, 
int addr, bool do_read,
return 0;
 }
 
+static int usb_blocklist_parse_error(const char *blocklist, size_t pos)
+{
+   printf("usb_blocklist parse error at char %zu in \"%s\"\n", pos,
+  blocklist);
+   return 0;
+}
+
+static int usb_device_is_blocked(u16 id_vendor, u16 id_product)
+{
+   ulong vid, pid;
+   char *end;
+   const char *block_str = env_get("usb_blocklist");
+   const char *cur = block_str;
+
+   /* parse "usb_blocklist" strictly */
+   while (cur && cur[0] != '\0') {
+   vid = simple_strtoul(cur, , 0);
+   if (cur == end || end[0] != ':')
+   return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+   cur = end + 1;
+   pid = simple_strtoul(cur, , 0);
+   if (cur == end && end[0] != '*')
+   return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+   if (end[0] == '*') {
+   /* use out of range idProduct as wildcard indication */
+   pid = U16_MAX + 1;
+   end++;
+   }
+   if (end[0] != ',' && end[0] != '\0')
+   return usb_blocklist_parse_error(block_str,
+cur - block_str);
+
+   if (id_vendor == vid && (pid > U16_MAX || id_product == pid)) {
+   printf("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
+   debug("Ignoring USB device 0x%x:0x%x\n",id_vendor,
+ id_product);
+   return 1;
+   }
+   if (end[0] == '\0')
+   break;
+   cur = end + 1;
+   }
+
+   return 0;
+}
+
 int usb_select_config(struct usb_device *dev)
 {
unsigned char *tmpbuf = NULL;
@@ -1099,6 +1150,11 @@ int usb_select_config(struct usb_device *dev)
le16_to_cpus(>descriptor.idProduct);
le16_to_cpus(>descriptor.bcdDevice);
 
+   /* ignore devices from usb_blocklist */
+   if (usb_device_is_blocked(dev->descriptor.idVendor,
+ dev->descriptor.idProduct))
+   return -ENODEV;
+
/*
 * Kingston DT Ultimate 32GB USB 3.0 seems to be extremely sensitive
 * about this first Get Descriptor request. If there are any other
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index ebf75fa948..e42702adb2 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -366,6 +366,18 @@ tftpwindowsize
 This means the count of blocks we can receive before
 sending ack to server.
 
+usb_blocklist
+Block USB devices from being bound to an USB device driver. This can be 
used
+to ignore devices which causes crashes in u-boot's USB stack or are
+undesirable for other reasons.
+The default environment blocks Yubico devices since they emulate USB
+keyboards. U-boot currently supports only a single USB keyboard device and
+it is undesirable that a Yubikey is used as keyboard.
+Devices are matched by idVendor and idProduct. The variable contains a 
comma
+separated list of idVendor:idProduct pairs as hexadecimal numbers joined
+by a colon. '*' functions as a wildcard for idProduct to block all devices
+with the specified idVendor.
+
 vlan
 When set to a value < 4095 the traffic over
 Ethernet is encapsulated/received over 802.1q
diff --git a/include/env_default.h b/include/env_default.h
index 2ca4a087d3..ba4c7516b4 100644
--- a/include/env_default.h
+++ b/include/env_default.h
@@ -99,6 +99,17 @@ const char defau

[PATCH v2 3/6] usb: xhci: Abort transfers with unallocated rings

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Discovered while trying to use the second interface in the USB keyboard
driver necessary on Apple USB keyboards.

Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci-ring.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b60661fe05..910c5f3352 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
reset_ep(udev, ep_index);
 
ring = virt_dev->eps[ep_index].ring;
+   if (!ring)
+   return -EINVAL;
+
/*
 * How much data is (potentially) left before the 64KB boundary?
 * XHCI Spec puts restriction( TABLE 49 and 6.4.1 section of XHCI Spec)
@@ -871,6 +874,8 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
ep_index = usb_pipe_ep_index(pipe);
 
ep_ring = virt_dev->eps[ep_index].ring;
+   if (!ep_ring)
+   return -EINVAL;
 
/*
 * Check to see if the max packet size for the default control

-- 
2.44.0

[PATCH v3 7/7] efi_selftest: Update StrToFat() unit test after CP473 map extension

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Test that Unicode code points which map to CP437 code points 1-31 are
converted to '_'. This ensures no FAT file names do not contain chars
which are control characters in other code pages (CP 1250 for example).

Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_unicode_collation.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_unicode_collation.c 
b/lib/efi_selftest/efi_selftest_unicode_collation.c
index 32c99caf35..ad7dfa9fb9 100644
--- a/lib/efi_selftest/efi_selftest_unicode_collation.c
+++ b/lib/efi_selftest/efi_selftest_unicode_collation.c
@@ -220,6 +220,18 @@ static int test_str_to_fat(void)
return EFI_ST_FAILURE;
}
 
+   /*
+* Test unicode code points which map to CP 437 0x01 - 0x1f are
+* converted to '_'.
+*/
+   boottime->set_mem(fat, 16, 0);
+   ret = unicode_collation_protocol->str_to_fat(unicode_collation_protocol,
+   u"\u263a\u2666\u2022\u25d8\u2642\u2194\u00b6\u203c", 8, fat);
+   if (!ret || efi_st_strcmp_16_8(u"", fat)) {
+   efi_st_error("str_to_fat returned %u, \"%s\"\n", ret, fat);
+   return EFI_ST_FAILURE;
+   }
+
return EFI_ST_SUCCESS;
 }
 

-- 
2.44.0

[PATCH v3 2/7] video: console: Parse UTF-8 character sequences

[Janne Grunau via B4 Relay]

From: Janne Grunau 

efi_console / UEFI applications (grub2, sd-boot, ...) pass UTF-8
character sequences to vidconsole which results in wrong glyphs for code
points outside of ASCII. The truetype console expects Unicode code
points and bitmap font based consoles expect code page 437 code points.
To support both convert UTF-8 to UTF-32 and pass Unicode code points in
vidconsole_ops.putc_xy(). These can be used directly in console_truetype
and after conversion to code page 437 in console_{normal,rotate}.

This fixes rendering of international, symbol and box drawing characters
used by UEFI applications.

Signed-off-by: Janne Grunau 
---
 drivers/video/console_normal.c  |  6 --
 drivers/video/console_rotate.c  | 16 ++--
 drivers/video/console_truetype.c|  8 
 drivers/video/vidconsole-uclass.c   | 18 +-
 drivers/video/vidconsole_internal.h | 19 +++
 include/video_console.h | 10 ++
 6 files changed, 56 insertions(+), 21 deletions(-)

diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index a0231293f3..34ef5a5229 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,7 +64,7 @@ static int console_move_rows(struct udevice *dev, uint rowdst,
return 0;
 }
 
-static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -73,8 +74,9 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, 
uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c
index 65358a1c6e..e4303dfb36 100644
--- a/drivers/video/console_rotate.c
+++ b/drivers/video/console_rotate.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,7 +68,7 @@ static int console_move_rows_1(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -77,8 +78,9 @@ static int console_putc_xy_1(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -145,7 +147,7 @@ static int console_move_rows_2(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -155,8 +157,9 @@ static int console_putc_xy_2(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -227,7 +230,7 @@ static int console_move_rows_3(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -237,8 +240,9 @@ static int console_putc_xy_3(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 

[PATCH v3 1/7] lib: charset: Fix and extend utf8_to_utf32_stream() documentation

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Suggested-by: Heinrich Schuchardt 
Reviewed-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 include/charset.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/charset.h b/include/charset.h
index 44034c71d3..f1050c903d 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -324,11 +324,21 @@ int utf_to_cp(s32 *c, const u16 *codepage);
 int utf8_to_cp437_stream(u8 c, char *buffer);
 
 /**
- * utf8_to_utf32_stream() - convert UTF-8 stream to UTF-32
+ * utf8_to_utf32_stream() - convert UTF-8 byte stream to Unicode code points
+ *
+ * The function is called for each byte @c in a UTF-8 stream. The byte is
+ * appended to the temporary storage @buffer until the UTF-8 stream in
+ * @buffer describes a Unicode code point.
+ *
+ * When a new code point has been decoded it is returned and buffer[0] is
+ * set to '\0', otherwise the return value is 0.
+ *
+ * The buffer must be at least 5 characters long. Before the first function
+ * invocation buffer[0] must be set to '\0'."
  *
  * @c: next UTF-8 character to convert
  * @buffer:buffer, at least 5 characters
- * Return: next codepage 437 character or 0
+ * Return: Unicode code point or 0
  */
 int utf8_to_utf32_stream(u8 c, char *buffer);
 

-- 
2.44.0

[PATCH v3 6/7] efi_selftest: Add geometric shapes character selftest

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Draw symbols from Unicode's "Geometric shapes" page which translate to
code page 437 code points 1-31. These are used by UEFI applications to
draw user interfaces using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.
The output has to be checked manually on the screen for correctness.

Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index b56fd2ab76..2208a9877a 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -60,6 +60,13 @@ 
u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
 u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
 u"\u2500\u2500\u2500\u2500\u2518\n";
 
+   const u16 shapes[] =
+u"Geometric shapes as described\n"
+u"U+25B2 \u25B2 - Black up-pointing triangle\n"
+u"U+25BA \u25BA - Black right-pointing pointer\n"
+u"U+25BC \u25BC - Black down-pointing triangle\n"
+u"U+25C4 \u25C4 - Black left-pointing pointer\n";
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -160,6 +167,12 @@ u"\u2500\u2500\u2500\u2500\u2518\n";
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+   ret = con_out->output_string(con_out, shapes);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for geometric shapes\n");
+   return EFI_ST_FAILURE;
+   }
+   efi_st_printf("\n");
 
return EFI_ST_SUCCESS;
 }

-- 
2.44.0

[PATCH v3 5/7] efi_selftest: Add box drawing character selftest

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI applications rely on Unicode output capability, and might use that
for drawing pseudo-graphical interfaces using Unicode defined box
drawing characters.

Add a simple test to display the most basic box characters, which would
need to be checked manually on the screen for correctness.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index 917903473d..b56fd2ab76 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -46,6 +46,20 @@ u"U+03BB \u03BB - Greek small letter lambda\n"
 u"U+03C2 \u03C2 - Greek small letter final sigma\n"
 u"U+1F19 \u1F19 - Greek capital letter epsilon with dasia\n";
 
+   const u16 boxes[] =
+u"This should render as four boxes with text\n"
+u"\u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n\u2502"
+u" left top\u2502 right top \u2502\n\u251c\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n\u2502 "
+u"left bottom \u2502 right bottom  \u2502\n\u2514\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2518\n";
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -140,6 +154,12 @@ u"U+1F19 \u1F19 - Greek capital letter epsilon with 
dasia\n";
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+   ret = con_out->output_string(con_out, boxes);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for box drawing chars\n");
+   return EFI_ST_FAILURE;
+   }
+   efi_st_printf("\n");
 
return EFI_ST_SUCCESS;
 }

-- 
2.44.0

[PATCH v3 4/7] efi_selftest: Add international characters test

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI relies entirely on unicode output, which actual fonts displayed on
the screen might not be ready for.

Add a test displaying some international characters, to reveal missing
glyphs, especially in our builtin fonts.
This would be needed to be manually checked on the screen for
correctness.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc44b38bc2..917903473d 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -31,6 +31,21 @@ static int execute(void)
0xD804, 0xDC22,
0};
 
+   const u16 text[] =
+u"This should render international characters as described\n"
+u"U+00D6 \u00D6 - Latin capital letter O with diaresis\n"
+u"U+00DF \u00DF - Latin small letter sharp s\n"
+u"U+00E5 \u00E5 - Latin small letter a with ring above\n"
+u"U+00E9 \u00E9 - Latin small letter e with acute\n"
+u"U+00F1 \u00F1 - Latin small letter n with tilde\n"
+u"U+00F6 \u00F6 - Latin small letter o with diaresis\n"
+u"The following characters will render as '?' with bitmap fonts\n"
+u"U+00F8 \u00F8 - Latin small letter o with stroke\n"
+u"U+03AC \u03AC - Greek small letter alpha with tonus\n"
+u"U+03BB \u03BB - Greek small letter lambda\n"
+u"U+03C2 \u03C2 - Greek small letter final sigma\n"
+u"U+1F19 \u1F19 - Greek capital letter epsilon with dasia\n";
+
/* SetAttribute */
efi_st_printf("\nColor palette\n");
for (foreground = 0; foreground < 0x10; ++foreground) {
@@ -119,6 +134,12 @@ static int execute(void)
return EFI_ST_FAILURE;
}
efi_st_printf("\n");
+   ret = con_out->output_string(con_out, text);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for international chars\n");
+   return EFI_ST_FAILURE;
+   }
+   efi_st_printf("\n");
 
return EFI_ST_SUCCESS;
 }

-- 
2.44.0

[PATCH v3 0/7] video: Add UTF-8 support for UEFI applications

[Janne Grunau via B4 Relay]

Andre submitted 2 years ago DM_VIDEO improvements for UEFI applications
using UEFI's EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL but did not follow with
suggested changes. This series takes care of the UTF-8 support which
required to draw symbol and box drawing characters used by UEFI
applications like grub2 and sd-boot correctly.

Compared to Andre's version this version has the following changes:
- use and extend existing conversion functions from lib/charset.c
- convert to Unicode code points for truetype console support
- conversion is conditional on CONFIG_CHARSET
- use escape sequences in tests as proposed by Heinrich

Link: 
https://lore.kernel.org/u-boot/20220110005638.21599-1-andre.przyw...@arm.com/
Signed-off-by: Janne Grunau 
---
Changes in v3:
- added Reviewed-by tag
- removed unnecessary u8 casts
- limited utf-8 conversion buffer to 5 bytes as documented
- added missing console_utf_to_cp437() documentation
- adapted EFI text output self-tests according to review comments
- dropped wait after EFI text output self tests
- added StrToFat EFI self test to ensure Unicode code points which map
  to code page 437 code points 1-31 are converted to '_'
- Link to v2: 
https://lore.kernel.org/r/20240210-vidconsole-utf8-uefi-v2-0-88c03db60...@jannau.net

Changes in v2:
- use "CONFIG_IS_ENABLED(CHARSET)" instead of EFI_LOADER
- rewritten commit message for mapping CP437 cp 1-31
- extended utf8_to_utf32_stream() documentation as suggested by
  Heinrich
- Link to RFC: 
https://lore.kernel.org/r/20240117-vidconsole-utf8-uefi-v1-0-539f7ce74...@jannau.net

---
Andre Przywara (2):
  efi_selftest: Add international characters test
  efi_selftest: Add box drawing character selftest

Janne Grunau (5):
  lib: charset: Fix and extend utf8_to_utf32_stream() documentation
  video: console: Parse UTF-8 character sequences
  lib/charset: Map Unicode code points to CP437 code points 1-31
  efi_selftest: Add geometric shapes character selftest
  efi_selftest: Update StrToFat() unit test after CP473 map extension

 drivers/video/console_normal.c|  6 ++-
 drivers/video/console_rotate.c| 16 ---
 drivers/video/console_truetype.c  |  8 ++--
 drivers/video/vidconsole-uclass.c | 18 +---
 drivers/video/vidconsole_internal.h   | 19 
 include/charset.h | 16 +--
 include/cp1250.h  | 12 -
 include/cp437.h   | 12 -
 include/video_console.h   | 10 +++--
 lib/charset.c |  9 ++--
 lib/efi_loader/efi_unicode_collation.c|  2 +-
 lib/efi_selftest/efi_selftest_textoutput.c| 54 +++
 lib/efi_selftest/efi_selftest_unicode_collation.c | 12 +
 13 files changed, 162 insertions(+), 32 deletions(-)
---
base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
change-id: 20240117-vidconsole-utf8-uefi-fa23b4ac65d6

Best regards,
-- 
Janne Grunau 

[PATCH v3 3/7] lib/charset: Map Unicode code points to CP437 code points 1-31

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Code page 437 uses code points 1-31 for glyphs instead of control
characters. Map the appropriate Unicode code points to this code points.
Fixes rendering of grub2's menu as EFI application using the
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL on a console with bitmap fonts.

Signed-off-by: Janne Grunau 
---
 include/charset.h  |  2 +-
 include/cp1250.h   | 12 ++--
 include/cp437.h| 12 ++--
 lib/charset.c  |  9 ++---
 lib/efi_loader/efi_unicode_collation.c |  2 +-
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/include/charset.h b/include/charset.h
index f1050c903d..348bad5883 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -16,7 +16,7 @@
 /*
  * codepage_437 - Unicode to codepage 437 translation table
  */
-extern const u16 codepage_437[128];
+extern const u16 codepage_437[160];
 
 /**
  * console_read_unicode() - read Unicode code point from console
diff --git a/include/cp1250.h b/include/cp1250.h
index adacf8a958..b762c78d9f 100644
--- a/include/cp1250.h
+++ b/include/cp1250.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP1250 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 1250.
+ * Constant CP1250 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 1250.
  */
 #define CP1250 { \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
0x20ac, 0x, 0x201a, 0x, \
0x201e, 0x2026, 0x2020, 0x2021, \
0x, 0x2030, 0x0160, 0x2039, \
diff --git a/include/cp437.h b/include/cp437.h
index 0b2b97132e..5093130f5e 100644
--- a/include/cp437.h
+++ b/include/cp437.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP437 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 437.
+ * Constant CP437 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 437.
  */
 #define CP437 { \
+   0x, 0x263a, 0x263b, 0x2665, \
+   0x2666, 0x2663, 0x2660, 0x2022, \
+   0x25d8, 0x25cb, 0x25d9, 0x2642, \
+   0x2640, 0x266a, 0x266b, 0x263c, \
+   0x25ba, 0x25c4, 0x2195, 0x203c, \
+   0x00b6, 0x00a7, 0x25ac, 0x21a8, \
+   0x2191, 0x2193, 0x2192, 0x2190, \
+   0x221f, 0x2194, 0x25b2, 0x25bc, \
0x00c7, 0x00fc, 0x00e9, 0x00e2, \
0x00e4, 0x00e0, 0x00e5, 0x00e7, \
0x00ea, 0x00eb, 0x00e8, 0x00ef, \
diff --git a/lib/charset.c b/lib/charset.c
index 5e4c4f948a..1f8480150a 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -16,7 +16,7 @@
 /**
  * codepage_437 - Unicode to codepage 437 translation table
  */
-const u16 codepage_437[128] = CP437;
+const u16 codepage_437[160] = CP437;
 
 static struct capitalization_table capitalization_table[] =
 #ifdef CONFIG_EFI_UNICODE_CAPITALIZATION
@@ -517,9 +517,12 @@ int utf_to_cp(s32 *c, const u16 *codepage)
int j;
 
/* Look up codepage translation */
-   for (j = 0; j < 0x80; ++j) {
+   for (j = 0; j < 0xA0; ++j) {
if (*c == codepage[j]) {
-   *c = j + 0x80;
+   if (j < 0x20)
+   *c = j;
+   else
+   *c = j + 0x60;
return 0;
}
}
diff --git a/lib/efi_loader/efi_unicode_collation.c 
b/lib/efi_loader/efi_unicode_collation.c
index c4c7572063..4b2c52918a 100644
--- a/lib/efi_loader/efi_unicode_collation.c
+++ b/lib/efi_loader/efi_unicode_collation.c
@@ -257,7 +257,7 @@ static void EFIAPI efi_fat_to_str(struct 
efi_unicode_collation_protocol *this,
for (i = 0; i < fat_size; ++i) {
c = (unsigned char)fat[i];
if (c > 0x80)
-   c = codepage[c - 0x80];
+   c = codepage[c - 0x60];
string[i] = c;
if (!c)
break;

-- 
2.44.0

Re: [PATCH 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau]

Hej,

On Sun, Feb 25, 2024, at 22:47, Marek Vasut wrote:
> On 2/25/24 4:28 PM, Janne Grunau wrote:
>> 
>> 
>> On Wed, Feb 21, 2024, at 13:39, Marek Vasut wrote:
>>> On 2/21/24 08:25, Janne Grunau via B4 Relay wrote:
>>>> From: Janne Grunau 
>>>>
>>>> Apple USB keyboards carry the HID keyboard boot protocol on the second
>>>> interface. Using the second interface in the USB keyboard driver does
>>>> not work since the xhci has not allocated a transfer ring.
>>>
>>> So, what does this patch do ? That is not clear from the commit message.
>> 
>> rewritten for v2:
>> | usb: xhci: Set up endpoints for the first 2 interfaces
>> |
>> | The xhci driver currently only does the necessary initialization for
>> | endpoints found in the first interface descriptor. Apple USB keyboards
>> | (released 2021) use the second interface descriptor for the HID keyboard
>> | boot protocol. To allow USB drivers to use endpoints from other
>> | interface descriptors the xhci driver needs to ensure these endpoints
>> | are initialized as well.
>> | Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
>> | are initialized and useable. Currently defined to 2 as that is enough to
>> | make the Apple keyboard usable.
>
> Would it make sense to make this a tunable Kconfig option ?

I thought about that but I don't think it's worth it. We would have to default 
it to 2 (at least when the USB keyboard driver is enabled) since we can't 
predict which devices will be connected.
Why would anyone chose a different value than the fixed value? I can't think of 
convincing reasons.

Janne

Re: [PATCH 4/6] usb: kbd: Ignore Yubikeys

[Janne Grunau]

Hej,

On Mon, Feb 26, 2024, at 21:47, Mark Kettenis wrote:
>> Date: Sun, 25 Feb 2024 22:57:23 +0100
>> From: Marek Vasut 
>> 
>> 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 
>> >>>
>> >>> 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 
>> >>> ---
>> >>>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.

That sounds like we should ignore USB devices in usb_scan_device() and not in 
the keyboard driver.

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

We could move the list to a CONFIG symbol which has Yubikey's vendor ID as 
default value now that we do string parsing anyway.

Janne

Re: [PATCH 4/6] usb: kbd: Ignore Yubikeys

[Janne Grunau]

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

Janne

Re: [PATCH 3/6] usb: xhci: Abort transfers with unallocated rings

[Janne Grunau]

Hej,

On Wed, Feb 21, 2024, at 13:40, Marek Vasut wrote:
> On 2/21/24 08:25, Janne Grunau via B4 Relay wrote:
>> From: Janne Grunau 
>> 
>> Discovered while trying to use the second interface in the USB keyboard
>> driver necessary on Apple USB keyboards.
>> 
>> Signed-off-by: Janne Grunau 
>> ---
>>   drivers/usb/host/xhci-ring.c | 5 +
>>   1 file changed, 5 insertions(+)
>> 
>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>> index b60661fe05..4446f5f098 100644
>> --- a/drivers/usb/host/xhci-ring.c
>> +++ b/drivers/usb/host/xhci-ring.c
>> @@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
>> pipe,
>>  reset_ep(udev, ep_index);
>>   
>>  ring = virt_dev->eps[ep_index].ring;
>> +if (!ring)
>> +return -1;
>
> Would it make sense to return e.g. -EINVAL or some other errno ?

Agreed, -EINVAL is good match as ep_index is directly derived from pipe 
argument.

Janne

Re: [PATCH 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau]

On Wed, Feb 21, 2024, at 13:39, Marek Vasut wrote:
> On 2/21/24 08:25, Janne Grunau via B4 Relay wrote:
>> From: Janne Grunau 
>> 
>> Apple USB keyboards carry the HID keyboard boot protocol on the second
>> interface. Using the second interface in the USB keyboard driver does
>> not work since the xhci has not allocated a transfer ring.
>
> So, what does this patch do ? That is not clear from the commit message.

rewritten for v2:
| usb: xhci: Set up endpoints for the first 2 interfaces
|
| The xhci driver currently only does the necessary initialization for
| endpoints found in the first interface descriptor. Apple USB keyboards
| (released 2021) use the second interface descriptor for the HID keyboard
| boot protocol. To allow USB drivers to use endpoints from other
| interface descriptors the xhci driver needs to ensure these endpoints
| are initialized as well.
| Use USB_MAX_ACTIVE_INTERFACES to control how many interface descriptors
| are initialized and useable. Currently defined to 2 as that is enough to
| make the Apple keyboard usable.

>
> btw. missing SoB line.

added

Thanks

Janne

Re: [PATCH v2 5/6] efi_selftest: Add box drawing character selftest

[Janne Grunau]

Hej,

On Mon, Feb 12, 2024, at 17:07, Heinrich Schuchardt wrote:
> On 10.02.24 13:46, Janne Grunau via B4 Relay wrote:
>> From: Andre Przywara 
>>
>> UEFI applications rely on Unicode output capability, and might use that
>> for drawing pseudo-graphical interfaces using Unicode defined box
>> drawing characters.
>>
>> Add a simple test to display the most basic box characters, which would
>> need to be checked manually on the screen for correctness.
>> To facilitate this, add a three second delay after the output at this
>> point.
>>
>> Signed-off-by: Andre Przywara 
>> Suggested-by: Heinrich Schuchardt 
>> Signed-off-by: Janne Grunau 
>> ---
>>   lib/efi_selftest/efi_selftest_textoutput.c | 20 
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
>> b/lib/efi_selftest/efi_selftest_textoutput.c
>> index 2aa81b0a80..cc11a22eee 100644
>> --- a/lib/efi_selftest/efi_selftest_textoutput.c
>> +++ b/lib/efi_selftest/efi_selftest_textoutput.c
>> @@ -33,6 +33,19 @@ static int execute(void)
>>  const u16 text[] =
>>   u"\u00d6sterreich Edelwei\u00df Sm\u00f8rrebr\u00f8d Sm\u00f6rg"
>>   u"\u00e5s Ni\u00f1o Ren\u00e9 >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
>> +const u16 boxes[] =
>> +u"This should render as four boxes with text\n"
>> +u"\u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
>> +u"\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
>> +u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n\u2502"
>> +u" left top\u2502 right top \u2502\n\u251c\u2500"
>> +u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
>> +u"\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
>> +u"\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n\u2502 "
>> +u"left bottom \u2502 right bottom  \u2502\n\u2514\u2500\u2500\u2500"
>> +u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
>> +u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
>> +u"\u2500\u2500\u2500\u2500\u2518\n";
>>
>>  /* SetAttribute */
>>  efi_st_printf("\nColor palette\n");
>> @@ -126,6 +139,13 @@ u"\u00e5s Ni\u00f1o Ren\u00e9 
>> >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
>>  efi_st_error("OutputString failed for international chars\n");
>>  return EFI_ST_FAILURE;
>>  }
>> +ret = con_out->output_string(con_out, boxes);
>> +if (ret != EFI_ST_SUCCESS) {
>> +efi_st_error("OutputString failed for box drawing chars\n");
>> +return EFI_ST_FAILURE;
>> +}
>> +con_out->output_string(con_out, u"waiting for admiration...\n");
>> +EFI_CALL(systab.boottime->stall(300));
>
> We don't want to add any unnecessary waiting times in the unit tests. If
> somebody wants to see the output, he can scroll up.

I'll remove this but this series is adding UTF-8 support to the video consoles 
which do not have scrollback. This is also the reason for the terse test lines.

best regards
Janne

[PATCH 2/6] usb: xhci: Set up endpoints for the first 2 interfaces

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple USB keyboards carry the HID keyboard boot protocol on the second
interface. Using the second interface in the USB keyboard driver does
not work since the xhci has not allocated a transfer ring.
---
 drivers/usb/host/xhci.c | 31 +++
 include/usb.h   |  6 ++
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 534c4b973f..741e186ee0 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -606,24 +606,28 @@ static int xhci_set_configuration(struct usb_device *udev)
int slot_id = udev->slot_id;
struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
struct usb_interface *ifdesc;
+   unsigned int ifnum;
+   unsigned int max_ifnum = min((unsigned int)USB_MAX_ACTIVE_INTERFACES,
+(unsigned int)udev->config.no_of_if);
 
out_ctx = virt_dev->out_ctx;
in_ctx = virt_dev->in_ctx;
 
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
/* Initialize the input context control */
ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
ctrl_ctx->drop_flags = 0;
 
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   num_of_ep = ifdesc->no_of_ep;
+   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
+   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
+   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
+   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
+   if (max_ep_flag < ep_flag)
+   max_ep_flag = ep_flag;
+   }
}
 
xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
@@ -637,9 +641,12 @@ static int xhci_set_configuration(struct usb_device *udev)
xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
 
/* filling up ep contexts */
-   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
-   if (err < 0)
-   return err;
+   for (ifnum = 0; ifnum < max_ifnum; ifnum++) {
+   ifdesc = >config.if_desc[ifnum];
+   err = xhci_init_ep_contexts_if(udev, ctrl, virt_dev, ifdesc);
+   if (err < 0)
+   return err;
+   }
 
return xhci_configure_endpoints(udev, false);
 }
diff --git a/include/usb.h b/include/usb.h
index 09e3f0cb30..3aafdc8bfd 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -49,6 +49,12 @@ extern bool usb_started; /* flag for the started/stopped USB 
status */
  */
 #define USB_TIMEOUT_MS(pipe) (usb_pipebulk(pipe) ? 5000 : 1000)
 
+/*
+ * The xhcd hcd driver prepares only a limited number interfaces / endpoints.
+ * Define this limit so that drivers do not exceed it.
+ */
+#define USB_MAX_ACTIVE_INTERFACES  2
+
 /* device request (setup) */
 struct devrequest {
__u8requesttype;

-- 
2.43.2

[PATCH 6/6] usb: kbd: Add probe quirk for Apple and Keychron keyboards

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Those keyboards do not return the current device state. Polling will
timeout unless there are key presses. This is not a problem during
operation but the inital device state query during probing will fail.
Skip this step in usb_kbd_probe_dev() to make these devices useable.
Not all Apple keyboards behave like this. A keyboard with USB
vendor/product ID 05ac:0221 is reported to work with the current code.
Unfortunately some Keychron keyboards "re-use" Apple's vendor ID and
show the same behavior (Keychron C2, 05ac:024f for example).

Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 7aa803eb4e..b0012ce7ad 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -31,6 +31,10 @@
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
 #define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
 
+#define USB_VENDOR_ID_KEYCHRON 0x3434
+
+#define USB_HID_QUIRK_POLL_NO_REPORT_IDLE  (1 << 0)
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -483,6 +487,7 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
struct usb_interface *iface;
struct usb_endpoint_descriptor *ep;
struct usb_kbd_pdata *data;
+   unsigned int quirks = 0;
int epNum;
int i;
 
@@ -525,6 +530,15 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
 
debug("USB KBD: found interrupt EP: 0x%x\n", ep->bEndpointAddress);
 
+   switch (dev->descriptor.idVendor) {
+   case USB_VENDOR_ID_APPLE:
+   case USB_VENDOR_ID_KEYCHRON:
+   quirks |= USB_HID_QUIRK_POLL_NO_REPORT_IDLE;
+   break;
+   default:
+   break;
+   }
+
data = malloc(sizeof(struct usb_kbd_pdata));
if (!data) {
printf("USB KBD: Error allocating private data\n");
@@ -565,6 +579,14 @@ static int usb_kbd_probe_dev(struct usb_device *dev, 
unsigned int ifnum)
usb_set_idle(dev, iface->desc.bInterfaceNumber, 0, 0);
 #endif
 
+   /*
+* Apple and Keychron keyboards do not report the device state. Reports
+* are only returned during key presses.
+*/
+   if (quirks & USB_HID_QUIRK_POLL_NO_REPORT_IDLE) {
+   debug("USB KBD: quirk: skip testing device state\n");
+   return 1;
+   }
debug("USB KBD: enable interrupt pipe...\n");
 #ifdef CONFIG_SYS_USB_EVENT_POLL_VIA_INT_QUEUE
data->intq = create_int_queue(dev, data->intpipe, 1,

-- 
2.43.2

[PATCH 3/6] usb: xhci: Abort transfers with unallocated rings

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Discovered while trying to use the second interface in the USB keyboard
driver necessary on Apple USB keyboards.

Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci-ring.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b60661fe05..4446f5f098 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -685,6 +685,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
reset_ep(udev, ep_index);
 
ring = virt_dev->eps[ep_index].ring;
+   if (!ring)
+   return -1;
+
/*
 * How much data is (potentially) left before the 64KB boundary?
 * XHCI Spec puts restriction( TABLE 49 and 6.4.1 section of XHCI Spec)
@@ -871,6 +874,8 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
ep_index = usb_pipe_ep_index(pipe);
 
ep_ring = virt_dev->eps[ep_index].ring;
+   if (!ep_ring)
+   return -1;
 
/*
 * Check to see if the max packet size for the default control

-- 
2.43.2

[PATCH 4/6] usb: kbd: Ignore Yubikeys

[Janne Grunau via B4 Relay]

From: Hector Martin 

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 
---
 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;
+   }
+   }
+
for (epNum = 0; epNum < iface->desc.bNumEndpoints; epNum++) {
ep = >ep_desc[epNum];
 

-- 
2.43.2

[PATCH 1/6] usb: xhci: refactor xhci_set_configuration

[Janne Grunau via B4 Relay]

From: Janne Grunau 

In the next step endpoints for multiple interfaces are set up. Move most
of the per endpoint initialization to separate function to avoid another
identation level.

Signed-off-by: Janne Grunau 
---
 drivers/usb/host/xhci.c | 119 +---
 1 file changed, 73 insertions(+), 46 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index d13cbff9b3..534c4b973f 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -475,67 +475,34 @@ static int xhci_configure_endpoints(struct usb_device 
*udev, bool ctx_change)
 }
 
 /**
- * Configure the endpoint, programming the device contexts.
+ * Fill endpoint contexts for interface descriptor ifdesc.
  *
- * @param udev pointer to the USB device structure
- * Return: returns the status of the xhci_configure_endpoints
+ * @param udev pointer to the USB device structure
+ * @param ctrl pointer to the xhci pravte device structure
+ * @param virt_dev pointer to the xhci virtual device structure
+ * @param ifdesc   pointer to the USB interface config descriptor
+ * Return: returns the status of xhci_init_ep_contexts_if
  */
-static int xhci_set_configuration(struct usb_device *udev)
+static int xhci_init_ep_contexts_if(struct usb_device *udev,
+   struct xhci_ctrl *ctrl,
+   struct xhci_virt_device *virt_dev,
+   struct usb_interface *ifdesc
+   )
 {
-   struct xhci_container_ctx *in_ctx;
-   struct xhci_container_ctx *out_ctx;
-   struct xhci_input_control_ctx *ctrl_ctx;
-   struct xhci_slot_ctx *slot_ctx;
struct xhci_ep_ctx *ep_ctx[MAX_EP_CTX_NUM];
int cur_ep;
-   int max_ep_flag = 0;
int ep_index;
unsigned int dir;
unsigned int ep_type;
-   struct xhci_ctrl *ctrl = xhci_get_ctrl(udev);
-   int num_of_ep;
-   int ep_flag = 0;
u64 trb_64 = 0;
-   int slot_id = udev->slot_id;
-   struct xhci_virt_device *virt_dev = ctrl->devs[slot_id];
-   struct usb_interface *ifdesc;
u32 max_esit_payload;
unsigned int interval;
unsigned int mult;
unsigned int max_burst;
unsigned int avg_trb_len;
unsigned int err_count = 0;
+   int num_of_ep = ifdesc->no_of_ep;
 
-   out_ctx = virt_dev->out_ctx;
-   in_ctx = virt_dev->in_ctx;
-
-   num_of_ep = udev->config.if_desc[0].no_of_ep;
-   ifdesc = >config.if_desc[0];
-
-   ctrl_ctx = xhci_get_input_control_ctx(in_ctx);
-   /* Initialize the input context control */
-   ctrl_ctx->add_flags = cpu_to_le32(SLOT_FLAG);
-   ctrl_ctx->drop_flags = 0;
-
-   /* EP_FLAG gives values 1 & 4 for EP1OUT and EP2IN */
-   for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
-   ep_flag = xhci_get_ep_index(>ep_desc[cur_ep]);
-   ctrl_ctx->add_flags |= cpu_to_le32(1 << (ep_flag + 1));
-   if (max_ep_flag < ep_flag)
-   max_ep_flag = ep_flag;
-   }
-
-   xhci_inval_cache((uintptr_t)out_ctx->bytes, out_ctx->size);
-
-   /* slot context */
-   xhci_slot_copy(ctrl, in_ctx, out_ctx);
-   slot_ctx = xhci_get_slot_ctx(ctrl, in_ctx);
-   slot_ctx->dev_info &= ~(cpu_to_le32(LAST_CTX_MASK));
-   slot_ctx->dev_info |= cpu_to_le32(LAST_CTX(max_ep_flag + 1) | 0);
-
-   xhci_endpoint_copy(ctrl, in_ctx, out_ctx, 0);
-
-   /* filling up ep contexts */
for (cur_ep = 0; cur_ep < num_of_ep; cur_ep++) {
struct usb_endpoint_descriptor *endpt_desc = NULL;
struct usb_ss_ep_comp_descriptor *ss_ep_comp_desc = NULL;
@@ -561,7 +528,8 @@ static int xhci_set_configuration(struct usb_device *udev)
avg_trb_len = max_esit_payload;
 
ep_index = xhci_get_ep_index(endpt_desc);
-   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
+   ep_ctx[ep_index] = xhci_get_ep_ctx(ctrl, virt_dev->in_ctx,
+  ep_index);
 
/* Allocate the ep rings */
virt_dev->eps[ep_index].ring = xhci_ring_alloc(ctrl, 1, true);
@@ -614,6 +582,65 @@ static int xhci_set_configuration(struct usb_device *udev)
}
}
 
+   return 0;
+}
+
+/**
+ * Configure the endpoint, programming the device contexts.
+ *
+ * @param udev pointer to the USB device structure
+ * Return: returns the status of the xhci_configure_endpoints
+ */
+static int xhci_set_configuration(struct usb_device *udev)
+{
+   struct xhci_container_ctx *out_ctx;
+   struct xhci_container_ctx *in_ctx;
+   struct xhci_input_control_ctx *ctrl_ctx;
+   struct xhci_slot_ctx *slot_ctx;
+   int err;
+   int cur_ep;
+   int max_ep_flag = 0;
+   struct xhci_ctrl *ctrl = xhci_get_ctrl(

[PATCH 5/6] usb: kbd: support Apple Magic Keyboards (2021)

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Apple USB keyboards (Magic Keyboard from 2021 (product id 0x029c)) carry
the HID keyboard boot protocol on the second interface descriptor.
Probe via vendor and product IDs since the class/subclass/protocol match
uses the first interface descriptor.
Probe the two first interface descriptors for the HID keyboard boot
protocol.

USB configuration descriptor for reference:

| Bus 003 Device 002: ID 05ac:029c Apple, Inc. Magic Keyboard
| Device Descriptor:
|   bLength18
|   bDescriptorType 1
|   bcdUSB   2.00
|   bDeviceClass0 [unknown]
|   bDeviceSubClass 0 [unknown]
|   bDeviceProtocol 0
|   bMaxPacketSize064
|   idVendor   0x05ac Apple, Inc.
|   idProduct  0x029c Magic Keyboard
|   bcdDevice3.90
|   iManufacturer   1 Apple Inc.
|   iProduct2 Magic Keyboard
|   iSerial 3 ...
|   bNumConfigurations  1
|   Configuration Descriptor:
| bLength 9
| bDescriptorType 2
| wTotalLength   0x003b
| bNumInterfaces  2
| bConfigurationValue 1
| iConfiguration  4 Keyboard
| bmAttributes 0xa0
|   (Bus Powered)
|   Remote Wakeup
| MaxPower  500mA
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber0
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  0 [unknown]
|   bInterfaceProtocol  0
|   iInterface  5 Device Management
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode0 Not supported
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength  83
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x81  EP 1 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8
| Interface Descriptor:
|   bLength 9
|   bDescriptorType 4
|   bInterfaceNumber1
|   bAlternateSetting   0
|   bNumEndpoints   1
|   bInterfaceClass 3 Human Interface Device
|   bInterfaceSubClass  1 Boot Interface Subclass
|   bInterfaceProtocol  1 Keyboard
|   iInterface  6 Keyboard / Boot
| HID Device Descriptor:
|   bLength 9
|   bDescriptorType33
|   bcdHID   1.10
|   bCountryCode   13 International (ISO)
|   bNumDescriptors 1
|   bDescriptorType34 Report
|   wDescriptorLength 207
|   Report Descriptors:
| ** UNAVAILABLE **
|   Endpoint Descriptor:
| bLength 7
| bDescriptorType 5
| bEndpointAddress 0x82  EP 2 IN
| bmAttributes3
|   Transfer TypeInterrupt
|   Synch Type   None
|   Usage Type   Data
| wMaxPacketSize 0x0010  1x 16 bytes
| bInterval   8

Signed-off-by: Janne Grunau 
---
 common/usb_kbd.c | 39 +++
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/common/usb_kbd.c b/common/usb_kbd.c
index 774d3555d9..7aa803eb4e 100644
--- a/common/usb_kbd.c
+++ b/common/usb_kbd.c
@@ -23,6 +23,14 @@
 
 #include 
 
+/*
+ * USB vendor and product IDs used for quirks.
+ */
+#define USB_VENDOR_ID_APPLE0x05ac
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_20210x029c
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_FINGERPRINT_20210x029a
+#define USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_NUMPAD_2021 0x029f
+
 /*
  * If overwrite_console returns 1, the stdin, stderr and stdout
  * are switched to the serial port, else the settings in the
@@ -106,6 +114,8 @@ struct usb_kbd_pdata {
unsigned long   last_report;
struct int_queue *intq;
 
+   uint32_tifnum;
+
uint32_trepeat_delay;
 
uint32_tusb_in_pointer;
@@ -159,8 +169,8 @@ static void usb_kbd_put_queue(struct usb_kbd_pdata *data, 
u8 c)
  */
 static void usb_kbd_setled(struct usb_device *dev)
 {
-   struct usb_interface *iface = >config.if_desc[0];
struct usb_kbd_pdata *data = dev->privptr;
+   struct usb_interface *iface = >config.if_desc[da

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

[Janne Grunau via B4 Relay]

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 
---
Hector Martin (1):
  usb: kbd: Ignore Yubikeys

Janne Grunau (5):
  usb: xhci: refactor xhci_set_configuration
  usb: xhci: Set up endpoints for the first 2 interfaces
  usb: xhci: Abort transfers with unallocated rings
  usb: kbd: support Apple Magic Keyboards (2021)
  usb: kbd: Add probe quirk for Apple and Keychron keyboards

 common/usb_kbd.c |  80 +--
 drivers/usb/host/xhci-ring.c |   5 ++
 drivers/usb/host/xhci.c  | 126 +++
 include/usb.h|   6 +++
 4 files changed, 167 insertions(+), 50 deletions(-)
---
base-commit: 37345abb97ef0dd9c50a03b2a72617612dcae585
change-id: 20240218-asahi-keyboards-f2ddaf0022b2

Best regards,
-- 
Janne Grunau 

[PATCH v2 6/6] efi_selftest: Add symbol character selftest

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Draw symbols from code page 437 code points 0x01 - 01f used by UEFI
applications to draw user interfaces using
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.

Add a simple test to displaying the Konami code using symbols used by
grub2. The output has to be checked manually on the screen for
correctness.

Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc11a22eee..9e5d944fa0 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -46,6 +46,8 @@ u"left bottom \u2502 right bottom  
\u2502\n\u2514\u2500\u2500\u2500"
 u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
 u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
 u"\u2500\u2500\u2500\u2500\u2518\n";
+   const u16 konami[] =
+u"Konami code: \u25b2 \u25b2 \u25bc \u25bc \u25c4 \u25ba \u25c4 \u25ba B A\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -144,6 +146,11 @@ u"\u2500\u2500\u2500\u2500\u2518\n";
efi_st_error("OutputString failed for box drawing chars\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, konami);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for symbol chars\n");
+   return EFI_ST_FAILURE;
+   }
con_out->output_string(con_out, u"waiting for admiration...\n");
EFI_CALL(systab.boottime->stall(300));
efi_st_printf("\n");

-- 
2.43.0

[PATCH v2 5/6] efi_selftest: Add box drawing character selftest

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI applications rely on Unicode output capability, and might use that
for drawing pseudo-graphical interfaces using Unicode defined box
drawing characters.

Add a simple test to display the most basic box characters, which would
need to be checked manually on the screen for correctness.
To facilitate this, add a three second delay after the output at this
point.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index 2aa81b0a80..cc11a22eee 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -33,6 +33,19 @@ static int execute(void)
const u16 text[] =
 u"\u00d6sterreich Edelwei\u00df Sm\u00f8rrebr\u00f8d Sm\u00f6rg"
 u"\u00e5s Ni\u00f1o Ren\u00e9 >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
+   const u16 boxes[] =
+u"This should render as four boxes with text\n"
+u"\u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n\u2502"
+u" left top\u2502 right top \u2502\n\u251c\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n\u2502 "
+u"left bottom \u2502 right bottom  \u2502\n\u2514\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2518\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -126,6 +139,13 @@ u"\u00e5s Ni\u00f1o Ren\u00e9 
>\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
efi_st_error("OutputString failed for international chars\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, boxes);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for box drawing chars\n");
+   return EFI_ST_FAILURE;
+   }
+   con_out->output_string(con_out, u"waiting for admiration...\n");
+   EFI_CALL(systab.boottime->stall(300));
efi_st_printf("\n");
 
return EFI_ST_SUCCESS;

-- 
2.43.0

[PATCH v2 1/6] lib: charset: Fix and extend utf8_to_utf32_stream() documentation

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 include/charset.h | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/include/charset.h b/include/charset.h
index 44034c71d3..f1050c903d 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -324,11 +324,21 @@ int utf_to_cp(s32 *c, const u16 *codepage);
 int utf8_to_cp437_stream(u8 c, char *buffer);
 
 /**
- * utf8_to_utf32_stream() - convert UTF-8 stream to UTF-32
+ * utf8_to_utf32_stream() - convert UTF-8 byte stream to Unicode code points
+ *
+ * The function is called for each byte @c in a UTF-8 stream. The byte is
+ * appended to the temporary storage @buffer until the UTF-8 stream in
+ * @buffer describes a Unicode code point.
+ *
+ * When a new code point has been decoded it is returned and buffer[0] is
+ * set to '\0', otherwise the return value is 0.
+ *
+ * The buffer must be at least 5 characters long. Before the first function
+ * invocation buffer[0] must be set to '\0'."
  *
  * @c: next UTF-8 character to convert
  * @buffer:buffer, at least 5 characters
- * Return: next codepage 437 character or 0
+ * Return: Unicode code point or 0
  */
 int utf8_to_utf32_stream(u8 c, char *buffer);
 

-- 
2.43.0

[PATCH v2 4/6] efi_selftest: Add international characters test

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI relies entirely on unicode output, which actual fonts displayed on
the screen might not be ready for.

Add a test displaying some international characters, to reveal missing
glyphs, especially in our builtin fonts.
This would be needed to be manually checked on the screen for
correctness.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc44b38bc2..2aa81b0a80 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -30,6 +30,9 @@ static int execute(void)
0xD804, 0xDC05,
0xD804, 0xDC22,
0};
+   const u16 text[] =
+u"\u00d6sterreich Edelwei\u00df Sm\u00f8rrebr\u00f8d Sm\u00f6rg"
+u"\u00e5s Ni\u00f1o Ren\u00e9 >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -118,6 +121,11 @@ static int execute(void)
efi_st_printf("Unicode not handled properly\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, text);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for international chars\n");
+   return EFI_ST_FAILURE;
+   }
efi_st_printf("\n");
 
return EFI_ST_SUCCESS;

-- 
2.43.0

[PATCH v2 2/6] video: console: Parse UTF-8 character sequences

[Janne Grunau via B4 Relay]

From: Janne Grunau 

efi_console / UEFI applications (grub2, sd-boot, ...) pass UTF-8
character sequences to vidconsole which results in wrong glyphs for code
points outside of ASCII. The truetype console expects Unicode code
points and bitmap font based consoles expect code page 437 code points.
To support both convert UTF-8 to UTF-32 and pass Unicode code points in
vidconsole_ops.putc_xy(). These can be used directly in console_truetype
and after conversion to code page 437 in console_{normal,rotate}.

This fixes rendering of international, symbol and box drawing characters
used by UEFI applications.

Signed-off-by: Janne Grunau 
---
 drivers/video/console_normal.c  |  6 --
 drivers/video/console_rotate.c  | 16 ++--
 drivers/video/console_truetype.c|  8 
 drivers/video/vidconsole-uclass.c   | 18 +-
 drivers/video/vidconsole_internal.h | 15 +++
 include/video_console.h | 10 ++
 6 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index a0231293f3..34ef5a5229 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,7 +64,7 @@ static int console_move_rows(struct udevice *dev, uint rowdst,
return 0;
 }
 
-static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -73,8 +74,9 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, 
uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c
index 65358a1c6e..e4303dfb36 100644
--- a/drivers/video/console_rotate.c
+++ b/drivers/video/console_rotate.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,7 +68,7 @@ static int console_move_rows_1(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -77,8 +78,9 @@ static int console_putc_xy_1(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -145,7 +147,7 @@ static int console_move_rows_2(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -155,8 +157,9 @@ static int console_putc_xy_2(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -227,7 +230,7 @@ static int console_move_rows_3(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -237,8 +240,9 @@ static int console_putc_xy_3(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 

[PATCH v2 0/6] video: Add UTF-8 support for UEFI applications

[Janne Grunau via B4 Relay]

Andre submitted 2 years ago DM_VIDEO improvements for UEFI applications
using UEFI's EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL but did not follow with
suggested changes. This series takes care of the UTF-8 support which
required to draw symbol and box drawing characters used by UEFI
applications like grub2 and sd-boot correctly.

Compared to Andre's version this version has the following changes:
- use and extend existing conversion functions from lib/charset.c
- convert to Unicode code points for truetype console support
- conversion is conditional on CONFIG_CHARSET
- use escape sequences in tests as proposed by Heinrich

Link: 
https://lore.kernel.org/u-boot/20220110005638.21599-1-andre.przyw...@arm.com/
Signed-off-by: Janne Grunau 
---
Changes in v2:
- use "CONFIG_IS_ENABLED(CHARSET)" instead of EFI_LOADER
- rewritten commit message for mapping CP437 cp 1-31
- extended utf8_to_utf32_stream() documentation as suggested by
  Heinrich
- Link to RFC: 
https://lore.kernel.org/r/20240117-vidconsole-utf8-uefi-v1-0-539f7ce74...@jannau.net

---
Andre Przywara (2):
  efi_selftest: Add international characters test
  efi_selftest: Add box drawing character selftest

Janne Grunau (4):
  lib: charset: Fix and extend utf8_to_utf32_stream() documentation
  video: console: Parse UTF-8 character sequences
  lib/charset: Map Unicode code points to CP437 code points 0-31
  efi_selftest: Add symbol character selftest

 drivers/video/console_normal.c |  6 +++--
 drivers/video/console_rotate.c | 16 +-
 drivers/video/console_truetype.c   |  8 +++
 drivers/video/vidconsole-uclass.c  | 18 ++-
 drivers/video/vidconsole_internal.h| 15 +
 include/charset.h  | 16 +++---
 include/cp1250.h   | 12 --
 include/cp437.h| 12 --
 include/video_console.h| 10 +
 lib/charset.c  |  9 +---
 lib/efi_loader/efi_unicode_collation.c |  2 +-
 lib/efi_selftest/efi_selftest_textoutput.c | 35 ++
 12 files changed, 127 insertions(+), 32 deletions(-)
---
base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
change-id: 20240117-vidconsole-utf8-uefi-fa23b4ac65d6

Best regards,
-- 
Janne Grunau 

[PATCH v2 3/6] lib/charset: Map Unicode code points to CP437 code points 0-31

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Code page 437 uses code points 1-31 for glyphs instead of control
characters. Map the appropriate Unicode code points to this code points.
Fixes rendering of grub2's menu as EFI application using the
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL on a console with bitmap fonts.

Signed-off-by: Janne Grunau 
---
 include/charset.h  |  2 +-
 include/cp1250.h   | 12 ++--
 include/cp437.h| 12 ++--
 lib/charset.c  |  9 ++---
 lib/efi_loader/efi_unicode_collation.c |  2 +-
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/include/charset.h b/include/charset.h
index f1050c903d..348bad5883 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -16,7 +16,7 @@
 /*
  * codepage_437 - Unicode to codepage 437 translation table
  */
-extern const u16 codepage_437[128];
+extern const u16 codepage_437[160];
 
 /**
  * console_read_unicode() - read Unicode code point from console
diff --git a/include/cp1250.h b/include/cp1250.h
index adacf8a958..b762c78d9f 100644
--- a/include/cp1250.h
+++ b/include/cp1250.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP1250 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 1250.
+ * Constant CP1250 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 1250.
  */
 #define CP1250 { \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
0x20ac, 0x, 0x201a, 0x, \
0x201e, 0x2026, 0x2020, 0x2021, \
0x, 0x2030, 0x0160, 0x2039, \
diff --git a/include/cp437.h b/include/cp437.h
index 0b2b97132e..5093130f5e 100644
--- a/include/cp437.h
+++ b/include/cp437.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP437 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 437.
+ * Constant CP437 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 437.
  */
 #define CP437 { \
+   0x, 0x263a, 0x263b, 0x2665, \
+   0x2666, 0x2663, 0x2660, 0x2022, \
+   0x25d8, 0x25cb, 0x25d9, 0x2642, \
+   0x2640, 0x266a, 0x266b, 0x263c, \
+   0x25ba, 0x25c4, 0x2195, 0x203c, \
+   0x00b6, 0x00a7, 0x25ac, 0x21a8, \
+   0x2191, 0x2193, 0x2192, 0x2190, \
+   0x221f, 0x2194, 0x25b2, 0x25bc, \
0x00c7, 0x00fc, 0x00e9, 0x00e2, \
0x00e4, 0x00e0, 0x00e5, 0x00e7, \
0x00ea, 0x00eb, 0x00e8, 0x00ef, \
diff --git a/lib/charset.c b/lib/charset.c
index 5e4c4f948a..1f8480150a 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -16,7 +16,7 @@
 /**
  * codepage_437 - Unicode to codepage 437 translation table
  */
-const u16 codepage_437[128] = CP437;
+const u16 codepage_437[160] = CP437;
 
 static struct capitalization_table capitalization_table[] =
 #ifdef CONFIG_EFI_UNICODE_CAPITALIZATION
@@ -517,9 +517,12 @@ int utf_to_cp(s32 *c, const u16 *codepage)
int j;
 
/* Look up codepage translation */
-   for (j = 0; j < 0x80; ++j) {
+   for (j = 0; j < 0xA0; ++j) {
if (*c == codepage[j]) {
-   *c = j + 0x80;
+   if (j < 0x20)
+   *c = j;
+   else
+   *c = j + 0x60;
return 0;
}
}
diff --git a/lib/efi_loader/efi_unicode_collation.c 
b/lib/efi_loader/efi_unicode_collation.c
index c4c7572063..4b2c52918a 100644
--- a/lib/efi_loader/efi_unicode_collation.c
+++ b/lib/efi_loader/efi_unicode_collation.c
@@ -257,7 +257,7 @@ static void EFIAPI efi_fat_to_str(struct 
efi_unicode_collation_protocol *this,
for (i = 0; i < fat_size; ++i) {
c = (unsigned char)fat[i];
if (c > 0x80)
-   c = codepage[c - 0x80];
+   c = codepage[c - 0x60];
string[i] = c;
if (!c)
break;

-- 
2.43.0

Re: [PATCH RFC 4/6] efi_selftest: Add international characters test

[Janne Grunau]

Hej,

On Thu, Jan 18, 2024, at 19:48, Heinrich Schuchardt wrote:
> On 1/17/24 23:24, Janne Grunau via B4 Relay wrote:
>> From: Andre Przywara 
>>
>> UEFI relies entirely on unicode output, which actual fonts displayed on
>> the screen might not be ready for.
>>
>> Add a test displaying some international characters, to reveal missing
>> glyphs, especially in our builtin fonts.
>
> How would I detect missing Bengali and Gujarati glyphs with this test?

Only by knowing what to expect or to check the test code.

> What might help would be an output of the Unicode entries in the font in
> parallel with the glyph.

I'm not sure how useful it would be to extend the amount of text to register 
during the test. Even with the 3 second wait added in the the box drawing test 
time is short to check the results. Running the tests optionally with 
pagination might be an option.

>  U+0040, @
>  U+0041, A
>  U+0A85. અ
>  U+0A86. આ
>  U+0A87. ઇ
>  U+0A88. ઈ
>  U+0A89. ઉ
>
> This would require some new command but not an EFI test.

Wouldn't we need both? Console tests to check if the intended glyphs are shown 
would be useful but the EFI selftest should still tests that it can render 
UTF-8 correctly (partial support on cp437 bitmap font consoles).

Best regards
Janne

Re: [PATCH RFC 0/6] video: Add UTF-8 support for UEFI applications

[Janne Grunau]

Hej Heinrich,

On Thu, Jan 18, 2024, at 17:47, Heinrich Schuchardt wrote:
> On 1/17/24 23:24, Janne Grunau via B4 Relay wrote:
>> Andre submitted 2 years ago DM_VIDEO improvements for UEFI applications
>> using UEFI's EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL but did not follow with
>> suggested changes. This series takes care of the UTF-8 support which
>> required to draw symbol and box drawing characters used by UEFI
>> applications like grub2 and sd-boot correctly.
>>
>> Compared to Andre's version this version has the following changes:
>> - use and extend existing conversion functions from lib/charset.c
>> - convert first to UTF-32 to support the truetype console as well
>> - conversion is conditional on CONFIG_EFI_LOADER
>> - use escape sequences in tests as proposed by Heinrich
>
> Hello Janne,
>
> to correctly render GRUB I think you need the following:
>
> * Use truetype

a truetype font is not necessary, the arrow glyphs used by grub are in codepage 
437 (code points 0x10 - 0x1f, see  
https://en.wikipedia.org/wiki/Code_page_437#Character_set ). grub2 and sd-boot 
render correctly with this patchset and bitmap fonts.

> * Add a mono-spaced Truetype font which has all the needed characters.
> * A bunch of code fixed.
>
> This is the font I once suggested.
>
> [PATCH v2 1/1] video: add DejaVu Mono font
> https://lore.kernel.org/u-boot/20210301181534.7618-1-xypron.g...@gmx.de/
>
> Which font have you been using for testing?

The 8x16 and 16x32 bitmap fonts and the included Anker/Coder Narrow truetype 
font. All render the grub menu correctly but the bitmap fonts are missing some 
of the glyphs in the international character efi_selftest by Andre and you.

> At the time these additional patches were needed.
>
> [PATCH 0/6] efi_loader: Unicode output in UEFI applications
> https://lore.kernel.org/u-boot/20210227130840.166193-1-xypron.g...@gmx.de/#r

That is functionally identical to this patchset except for the Unicode mapping 
for code page 437 code points 0x01 to 0x1f (Patch 3/6 "lib/charset: Map cp437 
low chars (0x01 - 0x1f) from unicode"). I wasn't aware of this and stopped 
looking after Andre's patches. It looks like patches 5 and 6 were never merged. 
Was there a reason for that?
best regards
Janne

>>
>> Link: 
>> https://lore.kernel.org/u-boot/20220110005638.21599-1-andre.przyw...@arm.com/
>> Signed-off-by: Janne Grunau 
>> ---
>> Andre Przywara (2):
>>efi_selftest: Add international characters test
>>efi_selftest: Add box drawing character selftest
>>
>> Janne Grunau (4):
>>lib: charset: Fix utf8_to_utf32_stream() return value doc string
>>video: console: Parse UTF-8 character sequences
>>lib/charset: Map cp437 low chars (0x01 - 0x1f) from unicode
>>efi_selftest: Add symbol character selftest
>>
>>   drivers/video/console_normal.c |  6 +++--
>>   drivers/video/console_rotate.c | 16 +-
>>   drivers/video/console_truetype.c   |  8 +++
>>   drivers/video/vidconsole-uclass.c  | 18 ++-
>>   drivers/video/vidconsole_internal.h| 15 +
>>   include/charset.h  |  4 ++--
>>   include/cp1250.h   | 12 --
>>   include/cp437.h| 12 --
>>   include/video_console.h| 10 +
>>   lib/charset.c  |  9 +---
>>   lib/efi_loader/efi_unicode_collation.c |  2 +-
>>   lib/efi_selftest/efi_selftest_textoutput.c | 35 
>> ++
>>   12 files changed, 116 insertions(+), 31 deletions(-)
>> ---
>> base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
>> change-id: 20240117-vidconsole-utf8-uefi-fa23b4ac65d6
>>
>> Best regards,

[PATCH] video: console: Fix buffer overflow in cmd 'font list'

[Janne Grunau via B4 Relay]

From: Janne Grunau 

vidconsole_ops.get_font is documented to return -ENOENT after the last
video_fontdata entry.

Signed-off-by: Janne Grunau 
---
 drivers/video/console_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/console_core.c b/drivers/video/console_core.c
index d17764d0b0..939363653f 100644
--- a/drivers/video/console_core.c
+++ b/drivers/video/console_core.c
@@ -225,7 +225,7 @@ int console_simple_get_font(struct udevice *dev, int seq, 
struct vidfont_info *i
 {
info->name = fonts[seq].name;
 
-   return 0;
+   return info->name ? 0 : -ENOENT;
 }
 
 int console_simple_select_font(struct udevice *dev, const char *name, uint 
size)

---
base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
change-id: 20240117-cmd_font_bitmap_segfault-cf291e2621c1

Best regards,
-- 
Janne Grunau 

[PATCH] video: Support VIDEO_X2R10G10B10 in truetype console

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Without explicit support for VIDEO_X2R10G10B10 VIDEO_X8R8G8B8 white
will be rendered as cyan-ish. The conversion leaves to lowest 2 bits
unset for more compact code.

Signed-off-by: Janne Grunau 
---
 drivers/video/console_truetype.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c
index 14fb81e956..547e5a8d9c 100644
--- a/drivers/video/console_truetype.c
+++ b/drivers/video/console_truetype.c
@@ -397,7 +397,10 @@ static int console_truetype_putc_xy(struct udevice *dev, 
uint x, uint y,
 
if (vid_priv->colour_bg)
val = 255 - val;
-   out = val | val << 8 | val << 16;
+   if (vid_priv->format == 
VIDEO_X2R10G10B10)
+   out = val << 2 | val << 12 | 
val << 22;
+   else
+   out = val | val << 8 | val << 
16;
if (vid_priv->colour_fg)
*dst++ |= out;
else
@@ -911,7 +914,10 @@ static int truetype_set_cursor_visible(struct udevice 
*dev, bool visible,
for (i = 0; i < width; i++) {
int out;
 
-   out = val | val << 8 | val << 16;
+   if (vid_priv->format == 
VIDEO_X2R10G10B10)
+   out = val << 2 | val << 12 | 
val << 22;
+   else
+   out = val | val << 8 | val << 
16;
if (vid_priv->colour_fg)
*dst++ |= out;
else

---
base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
change-id: 20240117-console_truetype_x2r10g10b10-fe88e5864640

Best regards,
-- 
Janne Grunau 

[PATCH RFC 2/6] video: console: Parse UTF-8 character sequences

[Janne Grunau via B4 Relay]

From: Janne Grunau 

efi_console / UEFI applications (grub2, sd-boot, ...) pass UTF-8
character sequences to vidconsole which results in wrong glyphs for code
points outside of ASCII. The truetype console expects Unicode code
points and bitmap font based consoles expect code page 437 code points.
To support both convert UTF-8 to UTF-32 and pass Unicode code points in
vidconsole_ops.putc_xy(). These can be used directly in console_truetype
and after conversion to code page 437 in console_{normal,rotate}.

This fixes rendering of international, symbol and box drawing characters
used by UEFI applications.

Signed-off-by: Janne Grunau 
---
 drivers/video/console_normal.c  |  6 --
 drivers/video/console_rotate.c  | 16 ++--
 drivers/video/console_truetype.c|  8 
 drivers/video/vidconsole-uclass.c   | 18 +-
 drivers/video/vidconsole_internal.h | 15 +++
 include/video_console.h | 10 ++
 6 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/drivers/video/console_normal.c b/drivers/video/console_normal.c
index a0231293f3..34ef5a5229 100644
--- a/drivers/video/console_normal.c
+++ b/drivers/video/console_normal.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -63,7 +64,7 @@ static int console_move_rows(struct udevice *dev, uint rowdst,
return 0;
 }
 
-static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -73,8 +74,9 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, 
uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
diff --git a/drivers/video/console_rotate.c b/drivers/video/console_rotate.c
index 65358a1c6e..e4303dfb36 100644
--- a/drivers/video/console_rotate.c
+++ b/drivers/video/console_rotate.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -67,7 +68,7 @@ static int console_move_rows_1(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -77,8 +78,9 @@ static int console_putc_xy_1(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int x, linenum, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -145,7 +147,7 @@ static int console_move_rows_2(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -155,8 +157,9 @@ static int console_putc_xy_2(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 ch = console_utf_to_cp437(cp);
uchar *pfont = fontdata->video_fontdata +
-   (u8)ch * fontdata->char_pixel_bytes;
+   ch * fontdata->char_pixel_bytes;
 
if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac)
return -EAGAIN;
@@ -227,7 +230,7 @@ static int console_move_rows_3(struct udevice *dev, uint 
rowdst, uint rowsrc,
return 0;
 }
 
-static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch)
+static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, int cp)
 {
struct vidconsole_priv *vc_priv = dev_get_uclass_priv(dev);
struct udevice *vid = dev->parent;
@@ -237,8 +240,9 @@ static int console_putc_xy_3(struct udevice *dev, uint 
x_frac, uint y, char ch)
int pbytes = VNBYTES(vid_priv->bpix);
int linenum, x, ret;
void *start, *line;
+   u8 

[PATCH RFC 3/6] lib/charset: Map cp437 low chars (0x01 - 0x1f) from unicode

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Add mappings for code points 1 - 31 as those code points in code page 437
are graphics.
Thios fixes rendering issues of various EFI boot loaders (grub2,
sd-boot, ...) using EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.

Signed-off-by: Janne Grunau 
---
 include/charset.h  |  2 +-
 include/cp1250.h   | 12 ++--
 include/cp437.h| 12 ++--
 lib/charset.c  |  9 ++---
 lib/efi_loader/efi_unicode_collation.c |  2 +-
 5 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/include/charset.h b/include/charset.h
index 714382e1c1..c51c29235f 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -16,7 +16,7 @@
 /*
  * codepage_437 - Unicode to codepage 437 translation table
  */
-extern const u16 codepage_437[128];
+extern const u16 codepage_437[160];
 
 /**
  * console_read_unicode() - read Unicode code point from console
diff --git a/include/cp1250.h b/include/cp1250.h
index adacf8a958..b762c78d9f 100644
--- a/include/cp1250.h
+++ b/include/cp1250.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP1250 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 1250.
+ * Constant CP1250 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 1250.
  */
 #define CP1250 { \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
+   0x, 0x, 0x, 0x, \
0x20ac, 0x, 0x201a, 0x, \
0x201e, 0x2026, 0x2020, 0x2021, \
0x, 0x2030, 0x0160, 0x2039, \
diff --git a/include/cp437.h b/include/cp437.h
index 0b2b97132e..5093130f5e 100644
--- a/include/cp437.h
+++ b/include/cp437.h
@@ -1,10 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 /*
- * Constant CP437 contains the Unicode code points for characters 0x80 - 0xff
- * of the code page 437.
+ * Constant CP437 contains the Unicode code points for characters 0x00 - 0x1f
+ * and 0x80 - 0xff of the code page 437.
  */
 #define CP437 { \
+   0x, 0x263a, 0x263b, 0x2665, \
+   0x2666, 0x2663, 0x2660, 0x2022, \
+   0x25d8, 0x25cb, 0x25d9, 0x2642, \
+   0x2640, 0x266a, 0x266b, 0x263c, \
+   0x25ba, 0x25c4, 0x2195, 0x203c, \
+   0x00b6, 0x00a7, 0x25ac, 0x21a8, \
+   0x2191, 0x2193, 0x2192, 0x2190, \
+   0x221f, 0x2194, 0x25b2, 0x25bc, \
0x00c7, 0x00fc, 0x00e9, 0x00e2, \
0x00e4, 0x00e0, 0x00e5, 0x00e7, \
0x00ea, 0x00eb, 0x00e8, 0x00ef, \
diff --git a/lib/charset.c b/lib/charset.c
index 5e4c4f948a..1f8480150a 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -16,7 +16,7 @@
 /**
  * codepage_437 - Unicode to codepage 437 translation table
  */
-const u16 codepage_437[128] = CP437;
+const u16 codepage_437[160] = CP437;
 
 static struct capitalization_table capitalization_table[] =
 #ifdef CONFIG_EFI_UNICODE_CAPITALIZATION
@@ -517,9 +517,12 @@ int utf_to_cp(s32 *c, const u16 *codepage)
int j;
 
/* Look up codepage translation */
-   for (j = 0; j < 0x80; ++j) {
+   for (j = 0; j < 0xA0; ++j) {
if (*c == codepage[j]) {
-   *c = j + 0x80;
+   if (j < 0x20)
+   *c = j;
+   else
+   *c = j + 0x60;
return 0;
}
}
diff --git a/lib/efi_loader/efi_unicode_collation.c 
b/lib/efi_loader/efi_unicode_collation.c
index c4c7572063..4b2c52918a 100644
--- a/lib/efi_loader/efi_unicode_collation.c
+++ b/lib/efi_loader/efi_unicode_collation.c
@@ -257,7 +257,7 @@ static void EFIAPI efi_fat_to_str(struct 
efi_unicode_collation_protocol *this,
for (i = 0; i < fat_size; ++i) {
c = (unsigned char)fat[i];
if (c > 0x80)
-   c = codepage[c - 0x80];
+   c = codepage[c - 0x60];
string[i] = c;
if (!c)
break;

-- 
2.43.0

[PATCH RFC 0/6] video: Add UTF-8 support for UEFI applications

[Janne Grunau via B4 Relay]

Andre submitted 2 years ago DM_VIDEO improvements for UEFI applications
using UEFI's EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL but did not follow with
suggested changes. This series takes care of the UTF-8 support which
required to draw symbol and box drawing characters used by UEFI
applications like grub2 and sd-boot correctly.

Compared to Andre's version this version has the following changes:
- use and extend existing conversion functions from lib/charset.c
- convert first to UTF-32 to support the truetype console as well
- conversion is conditional on CONFIG_EFI_LOADER
- use escape sequences in tests as proposed by Heinrich

Link: 
https://lore.kernel.org/u-boot/20220110005638.21599-1-andre.przyw...@arm.com/
Signed-off-by: Janne Grunau 
---
Andre Przywara (2):
  efi_selftest: Add international characters test
  efi_selftest: Add box drawing character selftest

Janne Grunau (4):
  lib: charset: Fix utf8_to_utf32_stream() return value doc string
  video: console: Parse UTF-8 character sequences
  lib/charset: Map cp437 low chars (0x01 - 0x1f) from unicode
  efi_selftest: Add symbol character selftest

 drivers/video/console_normal.c |  6 +++--
 drivers/video/console_rotate.c | 16 +-
 drivers/video/console_truetype.c   |  8 +++
 drivers/video/vidconsole-uclass.c  | 18 ++-
 drivers/video/vidconsole_internal.h| 15 +
 include/charset.h  |  4 ++--
 include/cp1250.h   | 12 --
 include/cp437.h| 12 --
 include/video_console.h| 10 +
 lib/charset.c  |  9 +---
 lib/efi_loader/efi_unicode_collation.c |  2 +-
 lib/efi_selftest/efi_selftest_textoutput.c | 35 ++
 12 files changed, 116 insertions(+), 31 deletions(-)
---
base-commit: 866ca972d6c3cabeaf6dbac431e8e08bb30b3c8e
change-id: 20240117-vidconsole-utf8-uefi-fa23b4ac65d6

Best regards,
-- 
Janne Grunau 

[PATCH RFC 6/6] efi_selftest: Add symbol character selftest

[Janne Grunau via B4 Relay]

From: Janne Grunau 

Draw symbols from code page 437 code points 0x01 - 01f used by UEFI
applications to draw user interfaces using
EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL.

Add a simple test to displaying the Konami code using symbols used by
grub2. The output has to be checked manually on the screen for
correctness.

Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc11a22eee..9e5d944fa0 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -46,6 +46,8 @@ u"left bottom \u2502 right bottom  
\u2502\n\u2514\u2500\u2500\u2500"
 u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
 u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
 u"\u2500\u2500\u2500\u2500\u2518\n";
+   const u16 konami[] =
+u"Konami code: \u25b2 \u25b2 \u25bc \u25bc \u25c4 \u25ba \u25c4 \u25ba B A\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -144,6 +146,11 @@ u"\u2500\u2500\u2500\u2500\u2518\n";
efi_st_error("OutputString failed for box drawing chars\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, konami);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for symbol chars\n");
+   return EFI_ST_FAILURE;
+   }
con_out->output_string(con_out, u"waiting for admiration...\n");
EFI_CALL(systab.boottime->stall(300));
efi_st_printf("\n");

-- 
2.43.0

[PATCH RFC 5/6] efi_selftest: Add box drawing character selftest

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI applications rely on Unicode output capability, and might use that
for drawing pseudo-graphical interfaces using Unicode defined box
drawing characters.

Add a simple test to display the most basic box characters, which would
need to be checked manually on the screen for correctness.
To facilitate this, add a three second delay after the output at this
point.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index 2aa81b0a80..cc11a22eee 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -33,6 +33,19 @@ static int execute(void)
const u16 text[] =
 u"\u00d6sterreich Edelwei\u00df Sm\u00f8rrebr\u00f8d Sm\u00f6rg"
 u"\u00e5s Ni\u00f1o Ren\u00e9 >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
+   const u16 boxes[] =
+u"This should render as four boxes with text\n"
+u"\u250c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u252c\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2510\n\u2502"
+u" left top\u2502 right top \u2502\n\u251c\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u253c\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2524\n\u2502 "
+u"left bottom \u2502 right bottom  \u2502\n\u2514\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2534"
+u"\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500"
+u"\u2500\u2500\u2500\u2500\u2518\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -126,6 +139,13 @@ u"\u00e5s Ni\u00f1o Ren\u00e9 
>\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
efi_st_error("OutputString failed for international chars\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, boxes);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for box drawing chars\n");
+   return EFI_ST_FAILURE;
+   }
+   con_out->output_string(con_out, u"waiting for admiration...\n");
+   EFI_CALL(systab.boottime->stall(300));
efi_st_printf("\n");
 
return EFI_ST_SUCCESS;

-- 
2.43.0

[PATCH RFC 1/6] lib: charset: Fix utf8_to_utf32_stream() return value doc string

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The comment appears to be copied from utf8_to_cp437_stream() but was not
updated.

Signed-off-by: Janne Grunau 
---
 include/charset.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/charset.h b/include/charset.h
index 44034c71d3..714382e1c1 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -328,7 +328,7 @@ int utf8_to_cp437_stream(u8 c, char *buffer);
  *
  * @c: next UTF-8 character to convert
  * @buffer:buffer, at least 5 characters
- * Return: next codepage 437 character or 0
+ * Return: next Unicode code point or 0
  */
 int utf8_to_utf32_stream(u8 c, char *buffer);
 

-- 
2.43.0

[PATCH RFC 4/6] efi_selftest: Add international characters test

[Janne Grunau via B4 Relay]

From: Andre Przywara 

UEFI relies entirely on unicode output, which actual fonts displayed on
the screen might not be ready for.

Add a test displaying some international characters, to reveal missing
glyphs, especially in our builtin fonts.
This would be needed to be manually checked on the screen for
correctness.

Signed-off-by: Andre Przywara 
Suggested-by: Heinrich Schuchardt 
Signed-off-by: Janne Grunau 
---
 lib/efi_selftest/efi_selftest_textoutput.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_textoutput.c 
b/lib/efi_selftest/efi_selftest_textoutput.c
index cc44b38bc2..2aa81b0a80 100644
--- a/lib/efi_selftest/efi_selftest_textoutput.c
+++ b/lib/efi_selftest/efi_selftest_textoutput.c
@@ -30,6 +30,9 @@ static int execute(void)
0xD804, 0xDC05,
0xD804, 0xDC22,
0};
+   const u16 text[] =
+u"\u00d6sterreich Edelwei\u00df Sm\u00f8rrebr\u00f8d Sm\u00f6rg"
+u"\u00e5s Ni\u00f1o Ren\u00e9 >\u1f19\u03bb\u03bb\u03ac\u03c2<\n";
 
/* SetAttribute */
efi_st_printf("\nColor palette\n");
@@ -118,6 +121,11 @@ static int execute(void)
efi_st_printf("Unicode not handled properly\n");
return EFI_ST_FAILURE;
}
+   ret = con_out->output_string(con_out, text);
+   if (ret != EFI_ST_SUCCESS) {
+   efi_st_error("OutputString failed for international chars\n");
+   return EFI_ST_FAILURE;
+   }
efi_st_printf("\n");
 
return EFI_ST_SUCCESS;

-- 
2.43.0

[PATCH v2] arm: apple: t602x: Add missing MMIO regions to memmap

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The memory maps for Apple's M2 Pro/Max/Ultra left MMIO space out which
was not used by any driver at the time. The display out exposed as
simple-framebuffer use a power-domain controlled by a device in an
unmapped region.
Add a map covering this region as well as another MMIO region in the
range 0x4'' - 0x5''. The added regions cover all MMIO
annotated in Apple's device tree in this range.

Signed-off-by: Janne Grunau 
---
Changes in v2:
- use SZ_1G as block size
- Link to v1: 
https://lore.kernel.org/r/20231130-apple_t602x_extend_memmap-v1-1-cd96b251d...@jannau.net
---
 arch/arm/mach-apple/board.c | 48 +
 1 file changed, 48 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
index 47393babbc..7a6151a972 100644
--- a/arch/arm/mach-apple/board.c
+++ b/arch/arm/mach-apple/board.c
@@ -370,6 +370,22 @@ static struct mm_region t6020_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x4,
+   .phys = 0x4,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x48000,
+   .phys = 0x48000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x58000,
@@ -471,6 +487,22 @@ static struct mm_region t6022_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x4,
+   .phys = 0x4,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x48000,
+   .phys = 0x48000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x58000,
@@ -551,6 +583,22 @@ static struct mm_region t6022_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x24,
+   .phys = 0x24,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x248000,
+   .phys = 0x248000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x258000,

---
base-commit: 43f2873fa98b1da6eb56d756315c7bd7db63db27
change-id: 20231130-apple_t602x_extend_memmap-c82c522ca8c0

Best regards,
-- 
Janne Grunau 

Re: [PATCH] arm: apple: t602x: Add missing MMIO regions to memmap

[Janne Grunau]

Hej Mark,

On Thu, Nov 30, 2023, at 21:45, Mark Kettenis wrote:
>> From: Janne Grunau via B4 Relay 
>> Date: Thu, 30 Nov 2023 13:42:22 +0100
>> 
>> From: Janne Grunau 
>> 
>> The memory maps for Apple's M2 Pro/Max/Ultra left MMIO space out which
>> was not used by any driver at the time. The display out exposed as
>> simple-framebuffer use a power-domain controlled by a device in an
>> unmapped region.
>> Add a map covering this region as well as another MMIO region in the
>> range 0x4'' - 0x5''. The added regions cover all MMIO
>> annotated in Apple's device tree in this range.
>> 
>> Signed-off-by: Janne Grunau 
>> ---
>>  arch/arm/mach-apple/board.c | 48 
>> +
>>  1 file changed, 48 insertions(+)
>
> Hi Janne,
>
> Is there a reason why you can't use a SZ_1G mapping for the blocks
> where you're using a SZ_512M mapping?  With SZ_1G the mapping code
> will use a 1G block descriptor which avoids another level of page
> tables.

no reason except that SZ_512M was sufficient to cover all devices in Apple DT.
Shall I resend the patch?

Janne

[PATCH] arm: apple: t602x: Add missing MMIO regions to memmap

[Janne Grunau via B4 Relay]

From: Janne Grunau 

The memory maps for Apple's M2 Pro/Max/Ultra left MMIO space out which
was not used by any driver at the time. The display out exposed as
simple-framebuffer use a power-domain controlled by a device in an
unmapped region.
Add a map covering this region as well as another MMIO region in the
range 0x4'' - 0x5''. The added regions cover all MMIO
annotated in Apple's device tree in this range.

Signed-off-by: Janne Grunau 
---
 arch/arm/mach-apple/board.c | 48 +
 1 file changed, 48 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
index 47393babbc..e05ec431bc 100644
--- a/arch/arm/mach-apple/board.c
+++ b/arch/arm/mach-apple/board.c
@@ -370,6 +370,22 @@ static struct mm_region t6020_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x4,
+   .phys = 0x4,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x48000,
+   .phys = 0x48000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x58000,
@@ -471,6 +487,22 @@ static struct mm_region t6022_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x4,
+   .phys = 0x4,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x48000,
+   .phys = 0x48000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x58000,
@@ -551,6 +583,22 @@ static struct mm_region t6022_mem_map[] = {
.attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
 PTE_BLOCK_NON_SHARE |
 PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x24,
+   .phys = 0x24,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x248000,
+   .phys = 0x248000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+   PTE_BLOCK_NON_SHARE |
+   PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* I/O */
.virt = 0x258000,

---
base-commit: 43f2873fa98b1da6eb56d756315c7bd7db63db27
change-id: 20231130-apple_t602x_extend_memmap-c82c522ca8c0

Best regards,
-- 
Janne Grunau 

[PATCH] arm: apple: Add initial Apple M2 Ultra support

[Janne Grunau]

Apple's M2 Ultra SoC are somewhat similar to the M1 Ultra but needs
a tweaked memory map as the M2 Pro/Max SoCs.  USB, NVMe, UART, WDT
and PCIe are working with the existing drivers.

Signed-off-by: Janne Grunau 
---
 arch/arm/mach-apple/board.c | 183 
 1 file changed, 183 insertions(+)

diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c
index d50194811843..47393babbc62 100644
--- a/arch/arm/mach-apple/board.c
+++ b/arch/arm/mach-apple/board.c
@@ -444,6 +444,187 @@ static struct mm_region t6020_mem_map[] = {
}
 };
 
+/* Apple M2 Ultra */
+
+static struct mm_region t6022_mem_map[] = {
+   {
+   /* I/O */
+   .virt = 0x28000,
+   .phys = 0x28000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x34000,
+   .phys = 0x34000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x38000,
+   .phys = 0x38000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x58000,
+   .phys = 0x58000,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* PCIE */
+   .virt = 0x5a000,
+   .phys = 0x5a000,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
+PTE_BLOCK_INNER_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* PCIE */
+   .virt = 0x5c000,
+   .phys = 0x5c000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRE) |
+PTE_BLOCK_INNER_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x7,
+   .phys = 0x7,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0xb,
+   .phys = 0xb,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0xf,
+   .phys = 0xf,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x13,
+   .phys = 0x13,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x228000,
+   .phys = 0x228000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x234000,
+   .phys = 0x234000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x238000,
+   .phys = 0x238000,
+   .size = SZ_1G,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* I/O */
+   .virt = 0x258000,
+   .phys = 0x258000,
+   .size = SZ_512M,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN

[PATCH] apple_m1_defconfig: Bump CONFIG_LMB_MAX_REGIONS to 64

[Janne Grunau]

Apple silicon SoCs have numerous embedded co-processors with pre-loaded
firmware. The co-processors text and data sections need to be mapped via
DART iommus controlled by the main processor. Those sections are
exported as reserved-memory. Bump CONFIG_LMB_MAX_REGIONS from 8 to 64 to
deal with the large amount of reserved-memory regions.

Signed-off-by: Janne Grunau 
---
 configs/apple_m1_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/apple_m1_defconfig b/configs/apple_m1_defconfig
index b4ecf73cbc78..755560971e57 100644
--- a/configs/apple_m1_defconfig
+++ b/configs/apple_m1_defconfig
@@ -21,3 +21,4 @@ CONFIG_SYS_WHITE_ON_BLACK=y
 CONFIG_NO_FB_CLEAR=y
 CONFIG_VIDEO_SIMPLE=y
 # CONFIG_GENERATE_SMBIOS_TABLE is not set
+CONFIG_LMB_MAX_REGIONS=64

---
base-commit: 6c1cdf158c4f3ccc8c5f9552f049a3386899dcd2
change-id: 20230313-apple_m1_defconfig_lmb_max_regions-a18f5612d3c3

Best regards,
-- 
Janne Grunau 

[PATCH] pci: apple: Initialize only enabled ports

[Janne Grunau]

The Linux devicetrees for Apple silicon devices are after review
feedback switching from deleting unused PCIe ports to disabling them.

Link: 
https://lore.kernel.org/asahi/1ea2107a-bb86-8c22-0bbc-82c453ab0...@linaro.org/
Signed-off-by: Janne Grunau 
---
 drivers/pci/pcie_apple.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/pcie_apple.c b/drivers/pci/pcie_apple.c
index 9b08e1e5da87..b934fdbc35c2 100644
--- a/drivers/pci/pcie_apple.c
+++ b/drivers/pci/pcie_apple.c
@@ -315,6 +315,8 @@ static int apple_pcie_probe(struct udevice *dev)
for (of_port = ofnode_first_subnode(dev_ofnode(dev));
 ofnode_valid(of_port);
 of_port = ofnode_next_subnode(of_port)) {
+   if (!ofnode_is_enabled(of_port))
+   continue;
ret = apple_pcie_setup_port(pcie, of_port);
if (ret) {
dev_err(pcie->dev, "Port %d setup fail: %d\n", i, ret);

---
base-commit: 6c1cdf158c4f3ccc8c5f9552f049a3386899dcd2
change-id: 20230313-apple_disabled_pcie_ports-469b7feebe32

Best regards,
-- 
Janne Grunau 

Re: u-boot crashes if mass-storage devices are connected via USB-C

[Janne Grunau]

On 2023-03-02 22:05:43 +0100, Marek Vasut wrote:
> On 3/2/23 19:51, Janne Grunau wrote:
> > On 2023-03-02 18:33:15 +0100, Marek Vasut wrote:
> > > On 3/2/23 10:14, Janne Grunau wrote:
> > > > On 2023-03-01 23:51:14 +0100, Marek Vasut wrote:
> > > > > On 3/1/23 21:34, Simon Glass wrote:
> > > > > > +Marek Vasut +Bin Meng +Mark Kettenis +Tom Rini
> > > > > > 
> > > > > > On Wed, 1 Mar 2023 at 08:12, bluetail 
> > > > > >  wrote:
> > > > > > > 
> > > > > > > Essentially, I connect a mass-storage device to the USB-C port of 
> > > > > > > a Mac
> > > > > > > Mini 2020 (M1), and it leads to the issue in the attachment.
> > > > > > > I was able to reproduce it with Icy Box IB-3810 and ICY BOX 
> > > > > > > IB-3805.
> > > > > > > Initially I thought this issue was only for some devices (also 
> > > > > > > attached
> > > > > > > here) https://github.com/AsahiLinux/u-boot/issues/4 but it 
> > > > > > > appears this
> > > > > > > might be a issue that is with many devices.
> > > > > > > 
> > > > > > > If you need any more information, please feel free to ask. I am 
> > > > > > > very
> > > > > > > eager to have this issue fixed because it seems to be a very 
> > > > > > > broad issue
> > > > > > > with mass media storage in general.
> > > > > > > uname-r returns 6.1.0-asahi-2-2-edge-ARCH
> > > > > Would it be possible to check whether current u-boot/master works any 
> > > > > better
> > > > 
> > > > Reproduced with b0eda49bc9b0 ("Merge tag 'u-boot-at91-fixes-2023.04-a'
> > > > of https://source.denx.de/u-boot/custodians/u-boot-at91;) and Icy Box
> > > > IB-3804-C31 (same design as IB-3805/IB-3810 but just a single 4 port
> > > > USB3 hub + 4 independent asmedia usb3 to sata converters).
> > > > 
> > > > | scanning bus usb@b0228 for devices... Device NOT ready
> > > > |Request Sense returned 02 3A 00
> > > > | Device NOT ready
> > > > |Request Sense returned 02 3A 00
> > > > | Resetting EP 0...
> > > > | WARN halted endpoint, queueing URB anyway.
> > > > | Unexpected XHCI event TRB, skipping... (c9208350 010f 1300 
> > > > 04008401)
> > > > | "Synchronous Abort" handler, esr 0x9605
> > > > | elr: 0003934c lr : 0003934c (reloc)
> > > > | elr: 010fcd24034c lr : 010fcd24034c
> > > 
> > > Could you decode the trace and check where exactly this exception 
> > > occurred ?
> > > 
> > > It should be enough to run "aarch64-...-objdump -lSD u-boot | less" on the
> > > U-Boot which matches the one which generated the trace, and then look up 
> > > the
> > > lr/elr address in that trace. That should point you to the exact place in
> > > code where the exception occurred .
> > 
> > Slightly different binary due to unrelated chnages:
> > 
> > | scanning bus usb@70228 for devices... 1 USB Device(s) found
> > | scanning bus usb@b0228 for devices... 1 USB Device(s) found
> > | scanning bus usb@f0228 for devices... 1 USB Device(s) found
> > | scanning bus usb@130228 for devices... 2 USB Device(s) found
> > | scanning bus usb@70228 for devices... 1 USB Device(s) found
> > | scanning bus usb@b0228 for devices... Resetting EP 0...
> > | WARN halted endpoint, queueing URB anyway.
> > | Unexpected XHCI event TRB, skipping... (c92247b0 010f 1300 
> > 02008400)
> > | "Synchronous Abort" handler, esr 0x9605
> > | elr: 00039d9c lr : 00039d9c (reloc)
> > | elr: 010fcd240d9c lr : 010fcd240d9c
> > | x0 :  x1 : 03e8
> > | x2 : 0040 x3 : 003f
> > | x4 : 010fc9223040 x5 : 010fc921ef90
> > | x6 : 1800 x7 : 300c0300
> > | x8 : 0424 x9 : 0008
> > | x10: ffe8 x11: 0010
> > | x12: 0001 x13: 0001
> > | x14: 010fc91ba320 x15: 0021
> > | x16: 010fcd23addc x17: 0040
> > | x18: 010fc91e7d70 x19: 010fc9223040
> > | x20: 010fc9234b20 x21: 0002
> > | x22: 0

Re: u-boot crashes if mass-storage devices are connected via USB-C

[Janne Grunau]

On 2023-03-02 18:33:15 +0100, Marek Vasut wrote:
> On 3/2/23 10:14, Janne Grunau wrote:
> > On 2023-03-01 23:51:14 +0100, Marek Vasut wrote:
> > > On 3/1/23 21:34, Simon Glass wrote:
> > > > +Marek Vasut +Bin Meng +Mark Kettenis +Tom Rini
> > > > 
> > > > On Wed, 1 Mar 2023 at 08:12, bluetail  
> > > > wrote:
> > > > > 
> > > > > Essentially, I connect a mass-storage device to the USB-C port of a 
> > > > > Mac
> > > > > Mini 2020 (M1), and it leads to the issue in the attachment.
> > > > > I was able to reproduce it with Icy Box IB-3810 and ICY BOX IB-3805.
> > > > > Initially I thought this issue was only for some devices (also 
> > > > > attached
> > > > > here) https://github.com/AsahiLinux/u-boot/issues/4 but it appears 
> > > > > this
> > > > > might be a issue that is with many devices.
> > > > > 
> > > > > If you need any more information, please feel free to ask. I am very
> > > > > eager to have this issue fixed because it seems to be a very broad 
> > > > > issue
> > > > > with mass media storage in general.
> > > > > uname-r returns 6.1.0-asahi-2-2-edge-ARCH
> > > Would it be possible to check whether current u-boot/master works any 
> > > better
> > 
> > Reproduced with b0eda49bc9b0 ("Merge tag 'u-boot-at91-fixes-2023.04-a'
> > of https://source.denx.de/u-boot/custodians/u-boot-at91;) and Icy Box
> > IB-3804-C31 (same design as IB-3805/IB-3810 but just a single 4 port
> > USB3 hub + 4 independent asmedia usb3 to sata converters).
> > 
> > | scanning bus usb@b0228 for devices... Device NOT ready
> > |Request Sense returned 02 3A 00
> > | Device NOT ready
> > |Request Sense returned 02 3A 00
> > | Resetting EP 0...
> > | WARN halted endpoint, queueing URB anyway.
> > | Unexpected XHCI event TRB, skipping... (c9208350 010f 1300 
> > 04008401)
> > | "Synchronous Abort" handler, esr 0x9605
> > | elr: 0003934c lr : 0003934c (reloc)
> > | elr: 010fcd24034c lr : 010fcd24034c
> 
> Could you decode the trace and check where exactly this exception occurred ?
> 
> It should be enough to run "aarch64-...-objdump -lSD u-boot | less" on the
> U-Boot which matches the one which generated the trace, and then look up the
> lr/elr address in that trace. That should point you to the exact place in
> code where the exception occurred .

Slightly different binary due to unrelated chnages:

| scanning bus usb@70228 for devices... 1 USB Device(s) found
| scanning bus usb@b0228 for devices... 1 USB Device(s) found
| scanning bus usb@f0228 for devices... 1 USB Device(s) found
| scanning bus usb@130228 for devices... 2 USB Device(s) found
| scanning bus usb@70228 for devices... 1 USB Device(s) found
| scanning bus usb@b0228 for devices... Resetting EP 0...
| WARN halted endpoint, queueing URB anyway.
| Unexpected XHCI event TRB, skipping... (c92247b0 010f 1300 02008400)
| "Synchronous Abort" handler, esr 0x9605
| elr: 00039d9c lr : 00039d9c (reloc)
| elr: 010fcd240d9c lr : 010fcd240d9c
| x0 :  x1 : 03e8
| x2 : 0040 x3 : 003f
| x4 : 010fc9223040 x5 : 010fc921ef90
| x6 : 1800 x7 : 300c0300
| x8 : 0424 x9 : 0008
| x10: ffe8 x11: 0010
| x12: 0001 x13: 0001
| x14: 010fc91ba320 x15: 0021
| x16: 010fcd23addc x17: 0040
| x18: 010fc91e7d70 x19: 010fc9223040
| x20: 010fc9234b20 x21: 0002
| x22: 010fc9233790 x23: 0080
| x24:  x25: 010fc91b9d00
| x26: 010fc91b9d00 x27: 010fc9233790
| x28:  x29: 010fc91b99b0
| 
| Code: 973c 52800401 aa1303e0 97a2 (b9400c00)

objdump -lSD u-boot | grep -C 12 ' 39d9c'

| Disassembly of section .text_rest:
| ...
|39d84:   f9400c36ldr x22, [x1, #24]
| /home/janne/src/asahi/u-boot/drivers/usb/host/xhci-ring.c:544
|   xhci_queue_command(ctrl, 0, udev->slot_id, ep_index, TRB_STOP_RING);
|39d88:   d281mov x1, #0x0// #0
|39d8c:   973cbl  39a7c 
| /home/janne/src/asahi/u-boot/drivers/usb/host/xhci-ring.c:546
|   event = xhci_wait_for_event(ctrl, TRB_TRANSFER);
|39d90:   52800401mov w1, #0x20   // #32
|39d94:   aa1303e0mov x0, x19
|39d98:   97a2bl  39c20 
| /home/janne/src

Re: u-boot crashes if mass-storage devices are connected via USB-C

[Janne Grunau]

On 2023-03-01 23:51:14 +0100, Marek Vasut wrote:
> On 3/1/23 21:34, Simon Glass wrote:
> > +Marek Vasut +Bin Meng +Mark Kettenis +Tom Rini
> > 
> > On Wed, 1 Mar 2023 at 08:12, bluetail  wrote:
> > > 
> > > Hello. user kettenis aka "Mark Kettenis" guided me write my bug report
> > > to this email. "bluetail: please report these usb bugs upstream; they're
> > > almost certainly not hardware-specific"
> > > 
> > > But before, jannau aka Janne Grunau asked me to give the version output
> > > of  `pacman -Qi uboot-asahi` to which I replied 2022.10.asahi1-1.
> > > Because I had the feeling that sometimes the reboot with a USB-C
> > > connected device succeeds, depending how many bays are populated. But I
> > > have no evidence for that.
> > > I did try other USB Type C Cables, but without success of fixing the
> > > underlying issue. The device works fine via USB Type A or C fine if
> > > plugged in AFTER u-boot.
> > > But, u-boot does not support USB Type A yet, which is why it wont break
> > > my boot sequence with USB Type A.
> > > 
> > > Essentially, I connect a mass-storage device to the USB-C port of a Mac
> > > Mini 2020 (M1), and it leads to the issue in the attachment.
> > > I was able to reproduce it with Icy Box IB-3810 and ICY BOX IB-3805.
> > > Initially I thought this issue was only for some devices (also attached
> > > here) https://github.com/AsahiLinux/u-boot/issues/4 but it appears this
> > > might be a issue that is with many devices.
> > > 
> > > If you need any more information, please feel free to ask. I am very
> > > eager to have this issue fixed because it seems to be a very broad issue
> > > with mass media storage in general.
> > > uname-r returns 6.1.0-asahi-2-2-edge-ARCH
> Would it be possible to check whether current u-boot/master works any better

Reproduced with b0eda49bc9b0 ("Merge tag 'u-boot-at91-fixes-2023.04-a' 
of https://source.denx.de/u-boot/custodians/u-boot-at91;) and Icy Box 
IB-3804-C31 (same design as IB-3805/IB-3810 but just a single 4 port 
USB3 hub + 4 independent asmedia usb3 to sata converters).

| scanning bus usb@b0228 for devices... Device NOT ready
|Request Sense returned 02 3A 00
| Device NOT ready
|Request Sense returned 02 3A 00
| Resetting EP 0...
| WARN halted endpoint, queueing URB anyway.
| Unexpected XHCI event TRB, skipping... (c9208350 010f 1300 04008401)
| "Synchronous Abort" handler, esr 0x9605
| elr: 0003934c lr : 0003934c (reloc)
| elr: 010fcd24034c lr : 010fcd24034c
| x0 :  x1 : 03e8
| x2 : 0040 x3 : 003f
| x4 : 010fc92065c0 x5 : 010fc9208210
| x6 : 1800 x7 : 300c0300
| x8 : 0424 x9 : 0004
| x10: ffe8 x11: 0010
| x12: 0001 x13: 0001
| x14: 010fc91c6720 x15: 0021
| x16: 010fcd23a414 x17: 0040
| x18: 010fc91e7d70 x19: 010fc92065c0
| x20: 010fc9443630 x21: 0002
| x22: 010fc94406f0 x23: 0080
| x24:  x25: 010fc91c6100
| x26: 010fc91c6100 x27: 010fc94406f0
| x28:  x29: 010fc91c5db0
| 
| Code: 973c 52800401 aa1303e0 97a2 (b9400c00) Resetting CPU ...

The usb2sata bridges can powered off individually. u-boot crashes with 
just a single bridge with HDD powered on. Works if all bridges are off 
or don't have a HDD connected.

The bridge reports under linux as

Bus 005 Device 003: ID 174c:55aa ASMedia Technology Inc. ASM1051E SATA 6Gb/s 
bridge, ASM1053E SATA 6Gb/s bridge, ASM1153 SATA 3Gb/s bridge, ASM1153E SATA 
6Gb/s bridge

best regards

Janne

Re: RFC: Handling of multiple EFI System Partitions

[Janne Grunau]

On 2022-12-18 20:40:20 +0100, Heinrich Schuchardt wrote:
> 
> 
> Am 18. Dezember 2022 15:21:06 MEZ schrieb Mark Kettenis 
> :
> >The Asahi installer, which is what most people use to get their Apple
> >Silicon Mac into a state where it is possible to install another OS on
> >the machine, offers the posibility to create multiple OS installs.
> >For each of these it creates a separate APFS partition (which holds
> >among other things, some essential system firmware) and separate EFI
> >System Partitions.  This has a few benefits:
> >
> >* It allows control over which version of the system firmware is used
> >  by the OS.  This is especially important for things like the GPU and
> >  DCP (display controller) firmware, where the firmware interface
> >  isn't exactly what we'd call "stable".  This way system firmware is
> >  paired with the OS install (similar to what macOS does for itself)
> >  which prevents breaking other OS installs on the same disk.
> >
> >* It allows us to store a 2nd stage bootloader (m1n1+u-boot+dtb) on
> >  the EFI System Partition (ESP) such that it can be easily upgraded
> >  from within Linux without affecting other OS installs on the same
> >  disk.
> >
> >* It allows the use of the "native" boot picker to switch between
> >  OSes.
> >
> >The approach the Asahi team has taken is to pair the APFS partition
> >with the ESP by adding a proprty that contains the partition UUID to
> >the device tree.  The installer ships u-boot with a patched distro
> >boot script that looks at this device tree property, figures out what
> >the right ESP is and loads the EFI bootloader
> >(efi/boot/bootaarch64.efi) from that partition.
> >
> >This approach has some drawbacks:
> >
> >1. U-Boot will still consider the first ESP as *the* ESP, which means
> >   that the ubootefi.var file is still read from and written from the
> >   first ESP.
> >
> >2. The distro boot script modifications don't work if U-Boot's
> >   built-in efibootmgr is used.  This probably also affects Simon's
> >   bootstd stuff.
> >
> >So my idea is to have U-Boot recognize the device tree property and
> >use it when it determines what partition is *the* ESP.  A proof of
> >concept diff is attached below.  This probably needs a bit of
> >massaging as reading the device tree property in generic EFI code 
> >like
> >this is probably not acceptable.  A better approach might be to have 
> >a
> >function that can be called from board-specific code that sets the 
> >UUID.
> >
> >Thoughts?  Would such a feature be useful on other hardware 
> >platforms?
> 
> efi/boot/bootaarch64.efi is only a fallback if load options are not 
> set up. Once the operating system has generated a load option it is 
> not used anymore.

Setting load options from operation systems is currently not 
implemented. The only readily available method to store variables is a 
file in the ESP. This obviously can not be supported as UEFI runtime 
service while the operation system is using the same disk.
It might be possible to use NOR flash as UEFI variable store. This could 
cause issues with the primary boot loader iboot which we can not avoid 
or with macOS in dual boot configurations.

> The MacBooks only have one drive. Why would you want two ESPs on one 
> drive?

The lack of support for setting boot variable form the operating system 
makes it hard to support multiple OS with a single ESP. The systems 
comes with an accessible graphical boot picker which should be preferred 
over an u-boot based boot manager. Accessibility and the ability to boot 
macOS are the most important advantages.

At the current stage an u-boot based boot manager is not a viable option 
since the devicetree can not be guaranteed to be backwards compatible.  
The most recent breaking change was the addition of a phy phandle 
reference for USB controller nodes for USB3 support. Older kernel will 
obviously miss a driver for the phy and at least on Linux the USB 
controller will not be initialized. This breaks USB completely.

The devicetree is transformed by the loader before u-boot in a 
non-trivial way. If u-boot were to use kernel version specific DTB 
templates from the ESP it would require a non-trivial amount of code to 
update the template by the information added by the loader.

> Why can't the Asahi team use the current UEFI bootflow? We should 
> avoid unneeded deviations. Can the current deviations be removed in 
> Asahi Linux?

See above, the main reason is the lack of support for setting UEFI 
variables at runtime from operating systems. Even if that those were 
supported it's impossible to upgrade all installed operating systems 
when the devicetree changes in a non backward compatible way due to 
additional hardware support.

At the current stage it is not possible to support UEFI bootflow.

Best regards

Janne

[PATCH v3 1/1] usb: storage: continue probe on "Invalid device"

[Janne Grunau]

Fixes a crash during probing of sd card readers without medium present.
Seen with the device below but reported for many other devices.

  idVendor   0x0bda Realtek Semiconductor Corp.
  idProduct  0x0326 Card reader
  bcdDevice   11.24
  iManufacturer   1 Realtek
  iProduct2 USB3.0 Card Reader
  iSerial 3 201404081410

Link: https://github.com/AsahiLinux/linux/issues/44
Link: https://lists.denx.de/pipermail/u-boot/2022-July/489717.html

Signed-off-by: Janne Grunau 
Reviewed-by: Simon Glass 
Reviewed-by: Marek Vasut 
---
 common/usb_storage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/usb_storage.c b/common/usb_storage.c
index eaa31374ef73..f9204552a683 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -239,6 +239,7 @@ static int usb_stor_probe_device(struct usb_device *udev)
ret = device_unbind(dev);
if (ret)
return ret;
+   continue;
}
 
ret = blk_probe_or_unbind(dev);
-- 
2.37.4

Re: [PATCH v2 1/1] usb: storage: continue probe on "Invalid device"

[Janne Grunau]

On 2022-11-03 23:23:52 +0100, Marek Vasut wrote:
> On 11/3/22 22:36, Janne Grunau wrote:
> > On 2022-09-28 04:20:52 -0600, Simon Glass wrote:
> > > +Marek Vasut
> > > +Tom Rini
> > > 
> > > On Sun, 25 Sept 2022 at 23:07, Janne Grunau  wrote:
> > > > 
> > > > On 2022-08-10 21:54:22 +0200, Janne Grunau wrote:
> > > > > Fixes a crash during probing of sd card readers without medium 
> > > > > present.
> > > > > 
> > > > > Link: https://github.com/AsahiLinux/linux/issues/44
> > > > > Link: https://lists.denx.de/pipermail/u-boot/2022-July/489717.html
> > > > > Signed-off-by: Janne Grunau 
> > > > > ---
> > > > > Changes since v1:
> > > > >   - changed unconditiona return to "continue" as proposed by AKASHI 
> > > > > Takahiro
> > > > > 
> > > > >   common/usb_storage.c | 1 +
> > > > >   1 file changed, 1 insertion(+)
> > > > > 
> > > > > diff --git a/common/usb_storage.c b/common/usb_storage.c
> > > > > index eaa31374ef73..f9204552a683 100644
> > > > > --- a/common/usb_storage.c
> > > > > +++ b/common/usb_storage.c
> > > > > @@ -239,6 +239,7 @@ static int usb_stor_probe_device(struct 
> > > > > usb_device *udev)
> > > > >ret = device_unbind(dev);
> > > > >if (ret)
> > > > >return ret;
> > > > > + continue;
> > > > >}
> > > > > 
> > > > >ret = blk_probe_or_unbind(dev);
> > > > 
> > > > ping. Is there anything holding up merging this fix?
> > 
> > ping2
> > 
> > This fixes a 100% reproducible crash when an USB storage device with
> > "medium not ready" is connected.
> 
> Can you please CC me next time when submitting these kinds of USB fixes ?

sure, the cc list was the get_maintainers.pl output.

> Also, can you tell which device this is ?

I don't think the device matters but I'm seeing this problem with

Bus 002 Device 005: ID 0bda:0326 Realtek Semiconductor Corp. Card reader
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   3.00
  bDeviceClass0 bDeviceSubClass 0 bDeviceProtocol 
0 bMaxPacketSize0 9
  idVendor   0x0bda Realtek Semiconductor Corp.
  idProduct  0x0326 Card reader
  bcdDevice   11.24
  iManufacturer   1 Realtek
  iProduct2 USB3.0 Card Reader
  iSerial 3 201404081410
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   0x002c
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  4 CARD READER
bmAttributes 0xa0
  (Bus Powered)
  Remote Wakeup
MaxPower  800mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass 8 Mass Storage
  bInterfaceSubClass  6 SCSI
  bInterfaceProtocol 80 Bulk-Only
  iInterface  5 Bulk-In, Bulk-Out, Interface
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst   7
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82  EP 2 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst   7
Binary Object Store Descriptor:
  bLength 5
  bDescriptorType15
  wTotalLength   0x0016
  bNumDeviceCaps  2
  USB 2.0 Extension Device Capability:
bLength 7
bDescriptorType16
bDevCapabilityType  2
bmAttributes   0xf41e
  BESL Link Power Management (LPM) Supported
BESL value 1024 us Deep BESL value61440 us SuperSpeed USB 
Device Capability:
bLength10
bDescriptorType16
   

Re: [PATCH v2 1/1] usb: storage: continue probe on "Invalid device"

[Janne Grunau]

On 2022-09-28 04:20:52 -0600, Simon Glass wrote:
> +Marek Vasut
> +Tom Rini
> 
> On Sun, 25 Sept 2022 at 23:07, Janne Grunau  wrote:
> >
> > On 2022-08-10 21:54:22 +0200, Janne Grunau wrote:
> > > Fixes a crash during probing of sd card readers without medium present.
> > >
> > > Link: https://github.com/AsahiLinux/linux/issues/44
> > > Link: https://lists.denx.de/pipermail/u-boot/2022-July/489717.html
> > > Signed-off-by: Janne Grunau 
> > > ---
> > > Changes since v1:
> > >  - changed unconditiona return to "continue" as proposed by AKASHI 
> > > Takahiro
> > >
> > >  common/usb_storage.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/common/usb_storage.c b/common/usb_storage.c
> > > index eaa31374ef73..f9204552a683 100644
> > > --- a/common/usb_storage.c
> > > +++ b/common/usb_storage.c
> > > @@ -239,6 +239,7 @@ static int usb_stor_probe_device(struct usb_device 
> > > *udev)
> > >   ret = device_unbind(dev);
> > >   if (ret)
> > >   return ret;
> > > + continue;
> > >   }
> > >
> > >   ret = blk_probe_or_unbind(dev);
> >
> > ping. Is there anything holding up merging this fix?

ping2

This fixes a 100% reproducible crash when an USB storage device with 
"medium not ready" is connected.

Janne 

Re: [PATCH] usb: Add 1ms delay after first Get Descriptor request

[Janne Grunau]

On 2022-10-31 21:57:39 +0100, Marek Vasut wrote:
> On 10/31/22 20:27, Simon Glass wrote:
> > Hi Marek,
> 
> Hi,
> 
> > On Sun, 30 Oct 2022 at 16:38, Marek Vasut  wrote:
> > > 
> > > Logitech Unifying Receiver 046d:c52b bcdDevice 12.10 seems
> > > sensitive about the first Get Descriptor request. If there
> > > are any other requests in the same microframe, the device
> > > reports bogus data, first of the descriptor parts is not
> > > sent to the host. Wait over one microframe duration before
> > > issuing subsequent requests to avoid probe failure with
> > > this device, since it can be used to connect USB keyboards.
> > > 
> > > Signed-off-by: Marek Vasut 
> > > ---
> > > Cc: Janne Grunau 
> > > Cc: Mark Kettenis 
> > > ---
> > >   common/usb.c | 11 +++
> > >   1 file changed, 11 insertions(+)
> > 
> > Is this device complying with the spec or is it broken?

This is not just this device. We have seen this reported with a few 
keyboards on apple silicon devices. Keychron devices are one group I can 
remember.

> > 
> > In any case we need a way to enable/disable this as it will slow down
> > unaffected platforms.
> 
> This makes little difference, since anyone can plug such device into a port
> and suddenly the platform is affected. We cannot really predict what users
> have on their desks.

Would it make sense to limit it to dwc3 host controllers or do you fear 
the same problem could happen with other controllers on fast boards?

In any case patch fixes the problem here as well.

Tested-by: Janne Grunau 

Thanks,

Janne

Re: [PATCH 1/1] usb: request only 8 bytes for USB_SPEED_FULL bMaxPacketSize0 discovery

[Janne Grunau]

On 2022-09-26 01:52:37 +0200, Marek Vasut wrote:
> On 8/29/22 08:31, Janne Grunau wrote:
> > Fixes probing of various keyboards with DWC3 as integrated into Apple
> > silicon SoCs. The problem appears to be requesting more data than the
> > devices's bMaxPacketSize0 of 8. Older Logitech unifying receivers
> > (bcdDevice 12.03 or 12.10) are for eaxample affected.
> > 
> > Signed-off-by: Janne Grunau 
> > Tested-by: Thomas Glanzmann 
> > ---
> >   common/usb.c | 8 +---
> >   1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/common/usb.c b/common/usb.c
> > index 6fcf1e8428e9..48a310e8599d 100644
> > --- a/common/usb.c
> > +++ b/common/usb.c
> > @@ -993,10 +993,12 @@ static int usb_setup_descriptor(struct usb_device 
> > *dev, bool do_read)
> >  *
> >  * At least the DWC2 controller needs to be programmed with
> >  * the number of packets in addition to the number of bytes.
> > -* A request for 64 bytes of data with the maxpacket guessed
> > -* as 64 (above) yields a request for 1 packet.
> > +* Requesting more than 8 bytes causes probing errors on the
> > +* DWC3 controller integrated into Apple silicon SoCs with
> > +* devices with bMaxPacketSize0 of 8. So limit the read request
> > +* to the used size of 8 bytes.
> >  */
> > -   err = get_descriptor_len(dev, 64, 8);
> > +   err = get_descriptor_len(dev, 8, 8);
> > if (err)
> > return err;
> > }
> 
> Sorry for the delay.
> 
> Can you be more specific about those logitech receivers ?

In my case it's a device a little bit larger than a USB-A plug. ~7mm 
heigh black plastic case with "logitech" written on the end and and 
their unifying logo in orange on a side (idVendor: 0x046d, idProduct: 
0xc52b, bcdDevice: 12.03). Russell has one with bcdDevice: 12.10

The problem is not limited to the logitech receivers. It reproduces for 
other keyboards with bMaxPacketSize0 == 8 as well. I suspect it affects 
all devices with bMaxPacketSize0 == 8. Keyboards are simply the type of 
device those breakage is noticed inside u-boot / efi bootloaders on 
desktop class devices and don't transfer that much data so that 
bMaxPacketSize0 == 8 doesn't hurt.

> I might have one
> of those devices, and I have DWC3 in i.MX8MP and i.MX8MQ, as well as ZynqMP,
> so I should be able to try and trigger the problem. Can you share the
> reproducer test case for this problem ?

The device is not detected. It is not listed as detected usb device 
during u-boot's USB probing. If the device is a keyboard or you have a 
matching logitech wireless keyboard it will not work to interrupt the 
auto boot or on the u-boot prompt.

> If the problem is specific to the Apple instance of the controller, 
> maybe we need some sort of quirk instead ?

The code looks evidently broken to me. The comment says that we can only 
expect to receive a single packet. We request 64 bytes but devices might 
have a bMaxPacketSize0 of 8. Requesting more than 8 bytes looks also 
unnecessary as we are only interested in bMaxPacketSize0 which is within 
the first 8 bytes of the device descriptor.

Thanks

Janne

Re: [PATCH v2 1/1] usb: storage: continue probe on "Invalid device"

[Janne Grunau]

On 2022-08-10 21:54:22 +0200, Janne Grunau wrote:
> Fixes a crash during probing of sd card readers without medium present.
> 
> Link: https://github.com/AsahiLinux/linux/issues/44
> Link: https://lists.denx.de/pipermail/u-boot/2022-July/489717.html
> Signed-off-by: Janne Grunau 
> ---
> Changes since v1:
>  - changed unconditiona return to "continue" as proposed by AKASHI Takahiro
> 
>  common/usb_storage.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index eaa31374ef73..f9204552a683 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -239,6 +239,7 @@ static int usb_stor_probe_device(struct usb_device *udev)
>   ret = device_unbind(dev);
>   if (ret)
>   return ret;
> + continue;
>   }
>  
>   ret = blk_probe_or_unbind(dev);

ping. Is there anything holding up merging this fix?

Thanks,
Janne

[PATCH 1/1] usb: request only 8 bytes for USB_SPEED_FULL bMaxPacketSize0 discovery

[Janne Grunau]

Fixes probing of various keyboards with DWC3 as integrated into Apple
silicon SoCs. The problem appears to be requesting more data than the
devices's bMaxPacketSize0 of 8. Older Logitech unifying receivers
(bcdDevice 12.03 or 12.10) are for eaxample affected.

Signed-off-by: Janne Grunau 
Tested-by: Thomas Glanzmann 
---
 common/usb.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 6fcf1e8428e9..48a310e8599d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -993,10 +993,12 @@ static int usb_setup_descriptor(struct usb_device *dev, 
bool do_read)
 *
 * At least the DWC2 controller needs to be programmed with
 * the number of packets in addition to the number of bytes.
-* A request for 64 bytes of data with the maxpacket guessed
-* as 64 (above) yields a request for 1 packet.
+* Requesting more than 8 bytes causes probing errors on the
+* DWC3 controller integrated into Apple silicon SoCs with
+* devices with bMaxPacketSize0 of 8. So limit the read request
+* to the used size of 8 bytes.
 */
-   err = get_descriptor_len(dev, 64, 8);
+   err = get_descriptor_len(dev, 8, 8);
if (err)
return err;
}
-- 
2.35.1