i2c-hid: the touchpad crash was a printf format, not the driver

The PSOGM on the ASUS TUF A15 was a page fault in cprintf, from probe(),
in the touchpad driver's own success message. cprintf had no field widths:
"%04x" printed literally and consumed no argument, so the "%s" after it
was handed the product ID as a string pointer. "%lx" misprinted the same
way -- "0x4275838976x" for 0xFEDC2000 -- on the line before.

The driver's formats are fixed, and cprintf now steps over flags and
widths and takes l/ll/z as 64-bit, so a directive it does not render can
no longer shift every argument after it. Until now any %ld or %lu printed
the number followed by a stray letter.

The touchpad on/off entry in the boot menu has served its purpose and is
gone; the verbose step trace stays.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S1giNvrHTrPMPw7V6x1kQ4

boot/uefi/main.c

@@ -118,9 +118,6 @@ static int VERBOSE_BOOT;
 // this particular moment.
 static int DEBUG_BOOT;
 
-// SCHBOOT_OFF_FLAGS: drivers turned off at the boot menu.
-static int OFF_FLAGS;
-
 static void
 serial_putc(char c)
 {
@@ -1542,22 +1539,20 @@ boot_menu(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop)
         // how to make it not refuse. An ISO built for anybody else has no way
         // to turn a network debugger on, because the code that would set the
         // flag is not in it.
-        const char *items[7];
+        const char *items[6];
-        int nitems = 6;
+        int nitems = 5;
         items[0] = "Boot SchOS";
         items[1] = "Boot SchOS (verbose)";
         items[2] = resbuf;
-        items[3] = (OFF_FLAGS & SCHBOOT_OFF_I2C_HID) ? "Touchpad driver: off"
+        items[3] = "System Information";
-                                                     : "Touchpad driver: on";
+        items[4] = "Reboot to firmware";
-        items[4] = "System Information";
-        items[5] = "Reboot to firmware";
 #ifdef SCHOS_DEBUG
-        items[6] = DEBUG_BOOT ? "Remote debugging: on  (schdbgd, port 4444)"
+        items[5] = DEBUG_BOOT ? "Remote debugging: on  (schdbgd, port 4444)"
                               : "Remote debugging: off";
-        nitems = 7;
+        nitems = 6;
 #endif
-        const char *hint = nitems == 7 ? "W/S or 1-7 to choose, Enter to select"
+        const char *hint = nitems == 6 ? "W/S or 1-6 to choose, Enter to select"
-                                       : "W/S or 1-6 to choose, Enter to select";
+                                       : "W/S or 1-5 to choose, Enter to select";
 
         ui_menu("Boot Manager", items, nitems, selected, hint);
 
@@ -1594,15 +1589,14 @@ boot_menu(EFI_GRAPHICS_OUTPUT_PROTOCOL *gop)
             if (selected == 0) return 0;
             if (selected == 1) return 1;
             if (selected == 2) { res_choose(gop); continue; }
-            if (selected == 3) { OFF_FLAGS ^= SCHBOOT_OFF_I2C_HID; echoed = -1; continue; }
+            if (selected == 3) { sys_info(gop); continue; }
-            if (selected == 4) { sys_info(gop); continue; }
 #ifdef SCHOS_DEBUG
             // A toggle rather than an action: picking it changes what the
             // entry says and leaves you in the menu, so the state you booted
             // with is the state you can see before you boot.
-            if (selected == 6) { DEBUG_BOOT = !DEBUG_BOOT; echoed = -1; continue; }
+            if (selected == 5) { DEBUG_BOOT = !DEBUG_BOOT; echoed = -1; continue; }
 #endif
-            if (selected == 5) {
+            if (selected == 4) {
                 ui_clear();
                 ui_text_centered((int)ui_height() / 2, "Halted -- reset to reach firmware setup",
                                  0xB9BEC7u);
@@ -2342,7 +2336,6 @@ EfiMain(EFI_HANDLE image, EFI_SYSTEM_TABLE *systab)
     *(UINT32 *)SCHBOOT_EFI_FLAG = SCHBOOT_EFI_MAGIC;
     *(UINT8  *)SCHBOOT_VERBOSE_FLAG = (UINT8)VERBOSE_BOOT;
     *(UINT8  *)SCHBOOT_DEBUG_FLAG   = (UINT8)DEBUG_BOOT;
-    *(UINT8  *)SCHBOOT_OFF_FLAGS    = (UINT8)OFF_FLAGS;
 
     // The page tables mboot_entry would have built. entry64low's first act is
     // a movabs to 0xffff8000...., so the high half must be mapped before the

include/arch/schboot.h

@@ -78,13 +78,6 @@
 // Zero on every ordinary boot, including every boot of a build made without
 // DEBUG=1 -- where the menu entry does not exist and nothing can set it.
 #define SCHBOOT_DEBUG_FLAG    (SCHBOOT_BASE + 0x102)
-
-// Drivers switched off at the boot menu, one bit each. For bisecting a machine
-// that hangs during boot without a rebuild: the newest driver that touches
-// hardware nobody tested it on is the first suspect, and turning it off from
-// the menu answers the question on the machine itself.
-#define SCHBOOT_OFF_FLAGS     (SCHBOOT_BASE + 0x103)
-#define SCHBOOT_OFF_I2C_HID   0x01      // kernel/acpi/i2c_hid.c: touchpads
 #define SCHBOOT_MEDIA_CD      1
 
 // Real-mode spelling of the same byte, for cdboot.S which has no 32-bit

kernel/acpi/i2c_hid.c

@@ -43,8 +43,6 @@
 #include "core/param.h"
 #include "core/spinlock.h"
 #include "arch/mmu.h"
-#include "arch/memlayout.h"
-#include "arch/schboot.h"
 #include "dev/hid.h"
 #include "dev/acpi.h"
 
@@ -177,10 +175,10 @@ dw_init(struct dwi2c *c, uacpi_namespace_node *node, uint32 speed)
   c->regs = ioremap(c->phys, 0x1000);
   if(c->regs == 0)
     return -1;
-  STEP("i2c: controller registers at 0x%lx\n", c->phys);
+  STEP("i2c: controller registers at 0x%p\n", c->phys);
   type = rd(c, IC_COMP_TYPE);
   if(type != DW_COMP_TYPE){
-    cprintf("i2c: controller at 0x%lx is not DesignWare (0x%x)\n",
+    cprintf("i2c: controller at 0x%p is not DesignWare (0x%x)\n",
             c->phys, type);
     return -1;
   }
@@ -422,7 +420,7 @@ controller(const char *path, uint32 speed)
   if(dw_init(c, node, speed) < 0)
     return 0;
   nctl++;
-  cprintf("i2c: %s at 0x%lx, %s mode, FIFOs %d/%d\n", path, c->phys,
+  cprintf("i2c: %s at 0x%p, %s mode, FIFOs %d/%d\n", path, c->phys,
           c->fast ? "fast" : "standard", c->txdepth, c->rxdepth);
   return c;
 }
@@ -605,7 +603,7 @@ probe(void *user, uacpi_namespace_node *node, uacpi_u32 depth)
     cprintf("i2c-hid: %s is neither a keyboard nor a pointer\n", d->name);
     return UACPI_ITERATION_DECISION_CONTINUE;
   }
-  cprintf("i2c-hid: %s: %04x:%04x at 0x%x, %s\n", d->name,
+  cprintf("i2c-hid: %s: %x:%x at 0x%x, %s\n", d->name,
           desc[20] | desc[21] << 8, desc[22] | desc[23] << 8, d->addr,
           d->pin ? "interrupt pin watched" : "polled");
   ndev++;
@@ -666,10 +664,6 @@ i2c_hid_start(void)
 {
   if(acpi_rsdp_phys() == 0)
     return;
-  if(*(volatile uchar *)P2V(SCHBOOT_OFF_FLAGS) & SCHBOOT_OFF_I2C_HID){
-    cprintf("i2c-hid: off, as chosen at the boot menu\n");
-    return;
-  }
   if(kthread_create(i2c_hid_thread, "i2chid") < 0)
     cprintf("i2c-hid: no thread; touchpads will not work\n");
 }

kernel/core/console.c

@@ -119,20 +119,39 @@ vcprintf(char *fmt, va_list ap)
       continue;
     }
     c = fmt[++i] & 0xff;
+    // Flags and a width are stepped over, and l/ll/z make the conversion
+    // 64-bit. Not because the output honours them -- it does not pad -- but
+    // because a conversion this function did not understand used to consume
+    // no argument, and every one after it then read the wrong one. "%04x"
+    // before a "%s" handed the %s an integer as a string, and the touchpad
+    // driver's first message on real hardware was a page fault in here.
+    while (c == '0' || c == '-' || c == '+' || c == ' ' || c == '#')
+      c = fmt[++i] & 0xff;
+    while (c >= '1' && c <= '9')
+      c = fmt[++i] & 0xff;
+    while (c >= '0' && c <= '9')
+      c = fmt[++i] & 0xff;
+    int wide = 0;
+    while (c == 'l' || c == 'z') {
+      wide = 1;
+      c = fmt[++i] & 0xff;
+    }
     if (c == 0)
       break;
     switch(c) {
     case 'd':
-      print_int(va_arg(ap, int), 10, 1);
+    case 'i':
+      print_int(wide ? va_arg(ap, int64) : va_arg(ap, int), 10, 1);
       break;
     case 'u':
-      print_int(va_arg(ap, uint32), 10, 0);
+      print_int(wide ? va_arg(ap, uint64) : va_arg(ap, uint32), 10, 0);
-      break;
-    case 'l':
-      print_int(va_arg(ap, int64), 10, 1);
       break;
     case 'x':
-      print_int(va_arg(ap, uint32), 16, 0);
+    case 'X':
+      print_int(wide ? va_arg(ap, uint64) : va_arg(ap, uint32), 16, 0);
+      break;
+    case 'c':
+      consputc(va_arg(ap, int));
       break;
     case 'p':
       print_int(va_arg(ap, uint64), 16, 0);

kernel/drivers/bus/xhci/xhci.c

@@ -1059,10 +1059,9 @@ attach_device(struct xhci *x, const struct usb_dev *proto, int parent,
     goto fail;
   }
 
-  // cprintf understands %d %u %l %x %p %b %s and nothing else -- no field
+  // cprintf does not pad: "%04x" prints like "%x". It used to be worse -- an
-  // widths. "%04x" is not a directive to it: it prints '%' and '0' literally,
+  // unknown directive consumed no argument and every one after it landed in
-  // consumes no argument, and every argument after it lands in the wrong
+  // the wrong place, which is what once turned this line into rubbish.
-  // place. That is what turned this line into rubbish, not the enumeration.
   cprintf("usb: port %d%s slot %d %x:%x %s, %s speed, %d configuration%s\n",
           proto->port, proto->route ? " (via hub)" : "", slot,
           desc->idVendor, desc->idProduct,