From 4efb33b46350e2d61f949b6cd6d73a565bf286fa Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Fri, 30 Jan 2026 22:08:45 -0500
Subject: [PATCH 1/9] comments on usb stuff

---
 core/src/core/usb.zig                       | 45 +++++++++++++++++----
 core/src/core/usb/drivers/CDC.zig           | 19 +++++++++
 core/src/core/usb/drivers/EchoExample.zig   | 31 +++++++-------
 examples/raspberrypi/rp2xxx/src/usb_cdc.zig | 24 +++++++----
 4 files changed, 88 insertions(+), 31 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index e1cd56409..fded0cde8 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -20,15 +20,16 @@ pub const StructFieldAttributes = struct {
     default_value_ptr: ?*const anyopaque = null,
 };
 
-/// Meant to make transition to zig 0.16 easier
+/// Helper to create a struct, wrapping around @Type, meant to make transition to zig 0.16 easier
 pub fn Struct(
-    comptime layout: std.builtin.Type.ContainerLayout,
-    comptime BackingInt: ?type,
-    comptime field_names: []const [:0]const u8,
-    comptime field_types: *const [field_names.len]type,
-    comptime field_attrs: *const [field_names.len]StructFieldAttributes,
+    layout: std.builtin.Type.ContainerLayout,
+    BackingInt: ?type,
+    field_names: []const [:0]const u8,
+    field_types: *const [field_names.len]type,
+    field_attrs: *const [field_names.len]StructFieldAttributes,
 ) type {
-    comptime var fields: []const std.builtin.Type.StructField = &.{};
+    var fields: []const std.builtin.Type.StructField = &.{};
+    // Iterate over the names, field types, and attributes, creating a new struct field entry
     for (field_names, field_types, field_attrs) |n, T, a| {
         fields = fields ++ &[1]std.builtin.Type.StructField{.{
             .name = n,
@@ -47,6 +48,7 @@ pub fn Struct(
     } });
 }
 
+// What does this do? It lets you iterate through interfaces and endpoints?
 pub const DescriptorAllocator = struct {
     next_ep_num: [2]u8,
     next_itf_num: u8,
@@ -96,6 +98,7 @@ pub const DescriptorAllocator = struct {
     }
 };
 
+/// Wraps a Descriptor type. Returned by the `create` method of the Descriptor.
 pub fn DescriptorCreateResult(Descriptor: type) type {
     return struct {
         descriptor: Descriptor,
@@ -108,7 +111,7 @@ pub fn DescriptorCreateResult(Descriptor: type) type {
 }
 
 /// USB Device interface
-/// Any device implementation used with DeviceController must implement those functions
+/// Any device implementation used with DeviceController must implement these functions
 pub const DeviceInterface = struct {
     pub const VTable = struct {
         ep_writev: *const fn (*DeviceInterface, types.Endpoint.Num, []const []const u8) types.Len,
@@ -191,16 +194,24 @@ pub const Config = struct {
         max_current_ma: u9,
         Drivers: type,
 
+        /// Generate A struct with a field for each field in Drivers, where the type is the third
+        /// arg of the Drivers' Descriptor's 'create' method.
         pub fn Args(self: @This()) type {
             const fields = @typeInfo(self.Drivers).@"struct".fields;
             var field_names: [fields.len][:0]const u8 = undefined;
             var field_types: [fields.len]type = undefined;
             for (fields, 0..) |fld, i| {
+                // Collect field names
                 field_names[i] = fld.name;
+                // Collect the type info  for the Descriptor.create function parameter
                 const params = @typeInfo(@TypeOf(fld.type.Descriptor.create)).@"fn".params;
+                // Ensure it takes 3 parameters
                 assert(params.len == 3);
+                // The first is a descriptor allocator?
                 assert(params[0].type == *DescriptorAllocator);
+                // The second is usb.types.Len
                 assert(params[1].type == types.Len);
+                // And save the type of the third
                 field_types[i] = params[2].type.?;
             }
             return Struct(.auto, null, &field_names, &field_types, &@splat(.{}));
@@ -256,8 +267,11 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
     std.debug.assert(config.configurations.len == 1);
 
     return struct {
+        // GM: This is likely because in practice only configuration 0 is selected by the host?
         const config0 = config.configurations[0];
+        // GM: Fields of the drivers struct are the separate drivers
         const driver_fields = @typeInfo(config0.Drivers).@"struct".fields;
+        // GM: Make an enum from the field names in the struct?
         const DriverEnum = std.meta.FieldEnum(config0.Drivers);
 
         /// This parses the drivers' descriptors and creates:
@@ -297,11 +311,18 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
             var driver_alloc_types: []const type = &.{};
             var driver_alloc_attrs: []const StructFieldAttributes = &.{};
 
+            // GM: For each driver listed in the config.config0
             for (driver_fields, 0..) |drv, drv_id| {
+                // GM: So drv (the field).type is the type of the Driver, e.g. CDC.zig. The
+                // Descriptor field is grabbed.
+                // GM: Shouldn't this be singular?
                 const Descriptors = drv.type.Descriptor;
+                // Call the create method on the descriptor, which wraps it with the allow stuff.
                 const result = Descriptors.create(&alloc, max_psize, @field(driver_args[0], drv.name));
+                // Get the descriptor instance from the result.
                 const descriptors = result.descriptor;
 
+                // GM: If they have alloc_bytes, then we can get the name and stuff?
                 if (result.alloc_bytes) |len| {
                     driver_alloc_names = driver_alloc_names ++ &[1][:0]const u8{drv.name};
                     driver_alloc_types = driver_alloc_types ++ &[1]type{[len]u8};
@@ -313,9 +334,15 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 assert(@alignOf(Descriptors) == 1);
                 size += @sizeOf(Descriptors);
 
+                // GM: Go over all the fields in the descriptor
+                // That is itf, ep_out, and ep_in for the EchoExample driver. It's a lot more for
+                // the CDC driver. What do these field? Are they events or something? transaction
+                // types?
+                // Wel,, they have types defined in usb, so they are a mix of things
                 for (@typeInfo(Descriptors).@"struct".fields, 0..) |fld, desc_num| {
                     const desc = @field(descriptors, fld.name);
 
+                    // For the first field ONLY
                     if (desc_num == 0) {
                         const itf_start, const itf_count = switch (fld.type) {
                             descriptor.InterfaceAssociation => .{ desc.first_interface, desc.interface_count },
@@ -332,6 +359,8 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                         itf_handlers = itf_handlers ++ &[1]DriverEnum{@field(DriverEnum, drv.name)} ** itf_count;
                     }
 
+                    // GM: Ok, here we have Endpoints and InterfaceAssociations. What about
+                    // Interfaces, Headers, etc?
                     switch (fld.type) {
                         descriptor.Endpoint => {
                             const ep_dir = @intFromEnum(desc.endpoint.dir);
diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index d48a75ec9..0887e557a 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -3,6 +3,8 @@ const usb = @import("../../usb.zig");
 const assert = std.debug.assert;
 const log = std.log.scoped(.usb_cdc);
 
+// GM: If this is 'standard' CDC stuff, then shouldn't it go into (lowercase) cdc.zig? e.g. outside
+// of the driver? So it would be accessed through usb.descriptor.cdc.ManagementRequestType.
 pub const ManagementRequestType = enum(u8) {
     SetCommFeature = 0x02,
     GetCommFeature = 0x03,
@@ -70,6 +72,14 @@ pub const Descriptor = extern struct {
         itf_data: []const u8 = "",
     };
 
+    // GM: Why does this method return a DescriptorCreateResult of This?
+    // Couldn't the verification function instead have just make sure that
+    // those fields were present?
+    /// This function is used during descriptor creation. Endpoint and interface numbers are
+    /// allocated through the `alloc` parameter. Third argument can be of any type, it's passed
+    /// by the user when creating the device controller type. If multiple instances of a driver
+    /// are used, this function is called for each, with different arguments. Passing arguments
+    /// through this function is preffered to making the whole driver generic.
     pub fn create(
         alloc: *usb.DescriptorAllocator,
         max_supported_packet_size: usb.types.Len,
@@ -130,6 +140,10 @@ pub const Descriptor = extern struct {
     }
 };
 
+// GM: Is 'notifi' just a typo? or does it mean something to USB.
+// These are supposed to be a map from endpoint descriptor field names to the handler.
+// IN and OUT make sense in USB parlance (they are transaction types), but I
+// don't know what concept that notify is supposed to map to.
 pub const handlers: usb.DriverHandlers(@This()) = .{
     .ep_notifi = on_notifi_ready,
     .ep_out = on_rx,
@@ -202,6 +216,7 @@ pub fn flush(self: *@This()) bool {
     return true;
 }
 
+// Called when the host selects a configuration.
 pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterface, data: []u8) void {
     const len_half = @divExact(data.len, 2);
     assert(len_half == desc.ep_in.max_packet_size.into());
@@ -217,6 +232,7 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
         },
         .notifi_ready = .init(true),
 
+        // OK so `init` provides a data buffer, which we split in half for rx and tx data.
         .rx_data = data[0..len_half],
         .rx_seek = 0,
         .rx_end = 0,
@@ -229,6 +245,9 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
     device.ep_listen(desc.ep_out.endpoint.num, @intCast(self.rx_data.len));
 }
 
+// Is this basically a class-specific sort of message, e.g. CDC specific? how
+// does the USB 'ingress' know to route to this? Is it just anything on
+// endpoint 0?
 pub fn class_request(self: *@This(), setup: *const usb.types.SetupPacket) ?[]const u8 {
     const mgmt_request: ManagementRequestType = @enumFromInt(setup.request);
     log.debug("cdc setup: {any} {} {}", .{ mgmt_request, setup.length.into(), setup.value.into() });
diff --git a/core/src/core/usb/drivers/EchoExample.zig b/core/src/core/usb/drivers/EchoExample.zig
index 6105bd96f..51c40c9d0 100644
--- a/core/src/core/usb/drivers/EchoExample.zig
+++ b/core/src/core/usb/drivers/EchoExample.zig
@@ -12,13 +12,11 @@ pub const Descriptor = extern struct {
     ep_out: desc.Endpoint,
     ep_in: desc.Endpoint,
 
-    /// This function is used during descriptor creation. Endpoint and
-    /// interface numbers are allocated through the `alloc` parameter.
-    /// Third argument can be of any type, it's passed by the user when
-    /// creating the device controller type. If multiple instances of
-    /// a driver are used, this function is called for each, with different
-    /// arguments. Passing arguments through this function is preffered to
-    /// making the whole driver generic.
+    /// This function is used during descriptor creation. Endpoint and interface numbers are
+    /// allocated through the `alloc` parameter. Third argument can be of any type, it's passed
+    /// by the user when creating the device controller type. If multiple instances of a driver
+    /// are used, this function is called for each, with different arguments. Passing arguments
+    /// through this function is preffered to making the whole driver generic.
     pub fn create(
         alloc: *usb.DescriptorAllocator,
         max_supported_packet_size: usb.types.Len,
@@ -43,9 +41,9 @@ pub const Descriptor = extern struct {
     }
 };
 
-/// This is a mapping from endpoint descriptor field names to handler
-/// function names. Counterintuitively, usb devices send data on 'in'
-/// endpoints and receive on 'out' endpoints.
+/// This is a mapping from endpoint descriptor field names to handler function names.
+/// 'in' and 'out' are from the perspective of the host, so, usb devices send data on 'in' endpoints
+/// and receive on 'out' endpoints.
 pub const handlers: usb.DriverHandlers(@This()) = .{
     .ep_in = on_tx_ready,
     .ep_out = on_rx,
@@ -56,9 +54,8 @@ descriptor: *const Descriptor,
 packet_buffer: []u8,
 tx_ready: std.atomic.Value(bool),
 
-/// This function is called when the host chooses a configuration
-/// that contains this driver. `self` points to undefined memory.
-/// `data` is of the length specified in `Descriptor.create()`.
+/// This function is called when the host chooses a configuration that contains this driver. `self`
+/// points to undefined memory. `data` is of the length specified in `Descriptor.create()`.
 pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterface, data: []u8) void {
     assert(data.len == desc.ep_in.max_packet_size.into());
     self.* = .{
@@ -77,19 +74,21 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
 /// Data returned by this function is sent on endpoint 0.
 pub fn class_request(self: *@This(), setup: *const usb.types.SetupPacket) ?[]const u8 {
     _ = self;
-    _ = setup;
+    log.debug("setup: {x}, {}, {}", .{ setup.request, setup.length.into(), setup.value.into() });
     return usb.ack;
 }
 
+/// Transmit handler callback.
 /// Each endpoint (as defined in the descriptor) has its own handler.
-/// Endpoint number is passed as an argument so that it does not need
-/// to be stored in the driver.
+/// Endpoint number is passed as an argument so that it does not need to be stored in the driver.
 pub fn on_tx_ready(self: *@This(), ep_tx: usb.types.Endpoint.Num) void {
     log.info("tx ready ({t})", .{ep_tx});
     // Mark transmission as available
     self.tx_ready.store(true, .seq_cst);
 }
 
+/// Receive handler callback.
+/// Endpoint number is passed as an argument so that it does not need to be stored in the driver.
 pub fn on_rx(self: *@This(), ep_rx: usb.types.Endpoint.Num) void {
     var buf: [64]u8 = undefined;
     // Read incoming packet into a local buffer
diff --git a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
index a04f60788..d4621578b 100644
--- a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
+++ b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
@@ -8,8 +8,13 @@ const usb = microzig.core.usb;
 const USB_Device = rp2xxx.usb.Polled(.{});
 const USB_Serial = usb.drivers.CDC;
 
+// GM: This is just a handle to the usb peripheral in the USB HAL for this chip.
 var usb_device: USB_Device = undefined;
 
+// GM: This tells how to route packets to the appropriate driver? But how?
+// This is a type constructor, which takes a config and 'driver args'.
+// The driver args have fields that match the fields in the Drivers, so it must be a way for the
+// controller to know how to form calls to some handler for the driver?
 var usb_controller: usb.DeviceController(.{
     .bcd_usb = USB_Device.max_supported_bcd_usb,
     .device_triple = .unspecified,
@@ -18,6 +23,8 @@ var usb_controller: usb.DeviceController(.{
     .bcd_device = .v1_00,
     .serial = "someserial",
     .max_supported_packet_size = USB_Device.max_supported_packet_size,
+    // GM: Technically a tuple, but would they ever support more than one configuration? this is
+    // config0.
     .configurations = &.{.{
         .attributes = .{ .self_powered = false },
         .max_current_ma = 50,
@@ -25,6 +32,7 @@ var usb_controller: usb.DeviceController(.{
     }},
 }, .{.{
     .serial = .{ .itf_notifi = "Board CDC", .itf_data = "Board CDC Data" },
+    // Is the reset driver used at all?
     .reset = "",
 }}) = .init;
 
@@ -71,6 +79,8 @@ pub fn main() !void {
         // You can now poll for USB events
         usb_device.poll(&usb_controller);
 
+        // GM: The if is just to get a pointer to the driver if it's setup and null otherwise
+        // The type here is the anonymous struct above with two fields: serial and reset.
         if (usb_controller.drivers()) |drivers| {
             new = time.get_time_since_boot().to_us();
             if (new - old > 500000) {
@@ -93,8 +103,9 @@ pub fn main() !void {
 
 var usb_tx_buff: [1024]u8 = undefined;
 
-// Transfer data to host
-// NOTE: After each USB chunk transfer, we have to call the USB task so that bus TX events can be handled
+/// Transfer data to host
+/// NOTE: After each USB chunk transfer, we have to call the USB task so that bus TX events can be
+/// handled
 pub fn usb_cdc_write(serial: *USB_Serial, comptime fmt: []const u8, args: anytype) void {
     var tx = std.fmt.bufPrint(&usb_tx_buff, fmt, args) catch &.{};
 
@@ -109,11 +120,10 @@ pub fn usb_cdc_write(serial: *USB_Serial, comptime fmt: []const u8, args: anytyp
 
 var usb_rx_buff: [1024]u8 = undefined;
 
-// Receive data from host
-// NOTE: Read code was not tested extensively. In case of issues, try to call USB task before every read operation
-pub fn usb_cdc_read(
-    serial: *USB_Serial,
-) []const u8 {
+/// Receive data from host
+/// NOTE: Read code was not tested extensively. In case of issues, try to call USB task before every
+/// read operation
+pub fn usb_cdc_read(serial: *USB_Serial) []const u8 {
     var rx_len: usize = 0;
 
     while (true) {

From 30dee46d6a4d8926d8db9a54658ab65274bb978b Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 31 Jan 2026 07:15:38 -0500
Subject: [PATCH 2/9] cleanup

---
 port/raspberrypi/rp2xxx/src/hal/usb.zig | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/port/raspberrypi/rp2xxx/src/hal/usb.zig b/port/raspberrypi/rp2xxx/src/hal/usb.zig
index 24c7afc2c..d728a004a 100644
--- a/port/raspberrypi/rp2xxx/src/hal/usb.zig
+++ b/port/raspberrypi/rp2xxx/src/hal/usb.zig
@@ -66,8 +66,8 @@ fn PerEndpoint(T: type) type {
 const BufferControlMmio = microzig.mmio.Mmio(@TypeOf(peripherals.USB_DPRAM.EP0_IN_BUFFER_CONTROL).underlying_type);
 const buffer_control: *volatile [16]PerEndpoint(BufferControlMmio) = @ptrCast(&peripherals.USB_DPRAM.EP0_IN_BUFFER_CONTROL);
 
-const EndpointControlMimo = microzig.mmio.Mmio(@TypeOf(peripherals.USB_DPRAM.EP1_IN_CONTROL).underlying_type);
-const endpoint_control: *volatile [15]PerEndpoint(EndpointControlMimo) = @ptrCast(&peripherals.USB_DPRAM.EP1_IN_CONTROL);
+const EndpointControlMmio = microzig.mmio.Mmio(@TypeOf(peripherals.USB_DPRAM.EP1_IN_CONTROL).underlying_type);
+const endpoint_control: *volatile [15]PerEndpoint(EndpointControlMmio) = @ptrCast(&peripherals.USB_DPRAM.EP1_IN_CONTROL);
 
 // +++++++++++++++++++++++++++++++++++++++++++++++++
 // Code
@@ -239,8 +239,9 @@ pub fn Polled(config: Config) type {
             };
 
             @memset(std.mem.asBytes(&self.endpoints), 0);
-            ep_open(&self.interface, &.control(.in(.ep0), max_supported_packet_size));
-            ep_open(&self.interface, &.control(.out(.ep0), max_supported_packet_size));
+            // Set up endpoints.
+            self.interface.ep_open(&.control(.in(.ep0), max_supported_packet_size));
+            self.interface.ep_open(&.control(.out(.ep0), max_supported_packet_size));
 
             // Present full-speed device by enabling pullup on DP. This is the point
             // where the host will notice our presence.

From 4f2d9c7f1ef6bb2f91a329845b83346cf2d028ae Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 31 Jan 2026 07:46:38 -0500
Subject: [PATCH 3/9] cleanup more

---
 port/raspberrypi/rp2xxx/src/hal/usb.zig | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/port/raspberrypi/rp2xxx/src/hal/usb.zig b/port/raspberrypi/rp2xxx/src/hal/usb.zig
index d728a004a..9386b6552 100644
--- a/port/raspberrypi/rp2xxx/src/hal/usb.zig
+++ b/port/raspberrypi/rp2xxx/src/hal/usb.zig
@@ -26,8 +26,10 @@ const HardwareEndpointData = struct {
 };
 
 const rp2xxx_buffers = struct {
-    // Address 0x100-0xfff (3840 bytes) can be used for data buffers
-    const USB_DPRAM_DATA_BUFFER_BASE = 0x50100100;
+    // Address 0x100-0xfff (3840 bytes) can be used for data buffers.
+    // The first 0x100 bytes are registers (last one at offset 0xfc), the rest is available for
+    // endpoint data buffers.
+    const USB_DPRAM_DATA_BUFFER_BASE = @intFromPtr(peripherals.USB_DPRAM) + 0x100;
 
     const CTRL_EP_BUFFER_SIZE = 64;
 

From e393150974f4fce92db86bc5c333c80f7274b86b Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 31 Jan 2026 08:38:08 -0500
Subject: [PATCH 4/9] cdc: Fix line coding init use

---
 core/src/core/usb/drivers/CDC.zig | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index 0887e557a..e44871d15 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -3,8 +3,7 @@ const usb = @import("../../usb.zig");
 const assert = std.debug.assert;
 const log = std.log.scoped(.usb_cdc);
 
-// GM: If this is 'standard' CDC stuff, then shouldn't it go into (lowercase) cdc.zig? e.g. outside
-// of the driver? So it would be accessed through usb.descriptor.cdc.ManagementRequestType.
+/// CDC PSTN Subclass Management Element Requests
 pub const ManagementRequestType = enum(u8) {
     SetCommFeature = 0x02,
     GetCommFeature = 0x03,
@@ -29,6 +28,7 @@ pub const ManagementRequestType = enum(u8) {
     _,
 };
 
+/// Line coding structure for SetLineCoding/GetLineCoding requests
 pub const LineCoding = extern struct {
     pub const StopBits = enum(u8) { @"1" = 0, @"1.5" = 1, @"2" = 2, _ };
     pub const Parity = enum(u8) {
@@ -46,9 +46,9 @@ pub const LineCoding = extern struct {
     data_bits: u8,
 
     pub const init: @This() = .{
-        .bit_rate = 115200,
-        .stop_bits = 0,
-        .parity = 0,
+        .bit_rate = .from(115200),
+        .stop_bits = .@"1",
+        .parity = .none,
         .data_bits = 8,
     };
 };
@@ -224,12 +224,7 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
     self.* = .{
         .device = device,
         .descriptor = desc,
-        .line_coding = .{
-            .bit_rate = .from(115200),
-            .stop_bits = .@"1",
-            .parity = .none,
-            .data_bits = 8,
-        },
+        .line_coding = .init,
         .notifi_ready = .init(true),
 
         // OK so `init` provides a data buffer, which we split in half for rx and tx data.

From b7905e489f3d8dc5992ebbba0f482f442f164c3a Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 31 Jan 2026 09:03:04 -0500
Subject: [PATCH 5/9] more cleanup

---
 core/src/core/usb.zig                       | 3 ++-
 core/src/core/usb/drivers/CDC.zig           | 3 ++-
 examples/raspberrypi/rp2xxx/src/usb_cdc.zig | 6 +-----
 port/raspberrypi/rp2xxx/src/hal/usb.zig     | 4 ++--
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index fded0cde8..3ee8d06e2 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -267,7 +267,7 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
     std.debug.assert(config.configurations.len == 1);
 
     return struct {
-        // GM: This is likely because in practice only configuration 0 is selected by the host?
+        // We only support one configuration
         const config0 = config.configurations[0];
         // GM: Fields of the drivers struct are the separate drivers
         const driver_fields = @typeInfo(config0.Drivers).@"struct".fields;
@@ -587,6 +587,7 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
             }
         }
 
+        // Return the appropriate descriptor type as determined by the top 8 bits of the value.
         fn get_descriptor(value: u16) ?[]const u8 {
             const asBytes = std.mem.asBytes;
             const desc_type: descriptor.Type = @enumFromInt(value >> 8);
diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index e44871d15..2ad213fa6 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -53,6 +53,7 @@ pub const LineCoding = extern struct {
     };
 };
 
+// This struct bundles all the descriptors CDC needs into one Configuration.
 pub const Descriptor = extern struct {
     const desc = usb.descriptor;
 
@@ -79,7 +80,7 @@ pub const Descriptor = extern struct {
     /// allocated through the `alloc` parameter. Third argument can be of any type, it's passed
     /// by the user when creating the device controller type. If multiple instances of a driver
     /// are used, this function is called for each, with different arguments. Passing arguments
-    /// through this function is preffered to making the whole driver generic.
+    /// through this function is preferred to making the whole driver generic.
     pub fn create(
         alloc: *usb.DescriptorAllocator,
         max_supported_packet_size: usb.types.Len,
diff --git a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
index d4621578b..ccac205e8 100644
--- a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
+++ b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
@@ -23,8 +23,6 @@ var usb_controller: usb.DeviceController(.{
     .bcd_device = .v1_00,
     .serial = "someserial",
     .max_supported_packet_size = USB_Device.max_supported_packet_size,
-    // GM: Technically a tuple, but would they ever support more than one configuration? this is
-    // config0.
     .configurations = &.{.{
         .attributes = .{ .self_powered = false },
         .max_current_ma = 50,
@@ -32,7 +30,6 @@ var usb_controller: usb.DeviceController(.{
     }},
 }, .{.{
     .serial = .{ .itf_notifi = "Board CDC", .itf_data = "Board CDC Data" },
-    // Is the reset driver used at all?
     .reset = "",
 }}) = .init;
 
@@ -79,8 +76,7 @@ pub fn main() !void {
         // You can now poll for USB events
         usb_device.poll(&usb_controller);
 
-        // GM: The if is just to get a pointer to the driver if it's setup and null otherwise
-        // The type here is the anonymous struct above with two fields: serial and reset.
+        // Ensure that the host as finished enumerating our USB device
         if (usb_controller.drivers()) |drivers| {
             new = time.get_time_since_boot().to_us();
             if (new - old > 500000) {
diff --git a/port/raspberrypi/rp2xxx/src/hal/usb.zig b/port/raspberrypi/rp2xxx/src/hal/usb.zig
index 9386b6552..7bdc24013 100644
--- a/port/raspberrypi/rp2xxx/src/hal/usb.zig
+++ b/port/raspberrypi/rp2xxx/src/hal/usb.zig
@@ -119,13 +119,13 @@ pub fn Polled(config: Config) type {
                 // Clear the status flag (write-one-to-clear)
                 peripherals.USB.SIE_STATUS.modify(.{ .SETUP_REC = 1 });
 
-                // The PAC models this buffer as two 32-bit registers.
+                // The SVD exposes this buffer as two 32-bit registers.
                 const setup: usb.types.SetupPacket = @bitCast([2]u32{
                     peripherals.USB_DPRAM.SETUP_PACKET_LOW.raw,
                     peripherals.USB_DPRAM.SETUP_PACKET_HIGH.raw,
                 });
 
-                log.debug("setup  {any}", .{setup});
+                log.debug("setup {any}", .{setup});
                 controller.on_setup_req(&self.interface, &setup);
             }
 

From 0548930d54d6147ad937e17b0dd57d5e1921ce08 Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 7 Feb 2026 10:16:25 -0500
Subject: [PATCH 6/9] cleanup

---
 core/src/core/usb.zig                   | 97 +++++++++++++++++++++++--
 core/src/core/usb/drivers/CDC.zig       |  3 -
 port/raspberrypi/rp2xxx/src/hal/usb.zig |  2 +
 3 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index 3ee8d06e2..0ad5c6cd4 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -262,16 +262,16 @@ pub fn validate_controller(T: type) void {
 
 /// USB device controller
 ///
-/// This code handles usb enumeration and configuration and routes packets to drivers.
+/// Responds to host requests and dispatches to the appropriate drivers.
+/// When this type is build (at comptime), it builds descriptor and handler tables based on the
+/// provided config.
 pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
     std.debug.assert(config.configurations.len == 1);
 
     return struct {
         // We only support one configuration
         const config0 = config.configurations[0];
-        // GM: Fields of the drivers struct are the separate drivers
         const driver_fields = @typeInfo(config0.Drivers).@"struct".fields;
-        // GM: Make an enum from the field names in the struct?
         const DriverEnum = std.meta.FieldEnum(config0.Drivers);
 
         /// This parses the drivers' descriptors and creates:
@@ -311,11 +311,8 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
             var driver_alloc_types: []const type = &.{};
             var driver_alloc_attrs: []const StructFieldAttributes = &.{};
 
-            // GM: For each driver listed in the config.config0
             for (driver_fields, 0..) |drv, drv_id| {
-                // GM: So drv (the field).type is the type of the Driver, e.g. CDC.zig. The
-                // Descriptor field is grabbed.
-                // GM: Shouldn't this be singular?
+                // Get descriptor type for the current driver
                 const Descriptors = drv.type.Descriptor;
                 // Call the create method on the descriptor, which wraps it with the allow stuff.
                 const result = Descriptors.create(&alloc, max_psize, @field(driver_args[0], drv.name));
@@ -323,6 +320,23 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 const descriptors = result.descriptor;
 
                 // GM: If they have alloc_bytes, then we can get the name and stuff?
+                // CLAUDE: This builds the DriverAlloc struct. If a driver's create() returned
+                // alloc_bytes (like CDC requesting 2*max_packet_size), a field is added to
+                // DriverAlloc with that driver's name and an array of that size. Later in
+                // process_set_config, this memory is passed to the driver's init function.
+                // Drivers that don't need extra memory (like ResetDriver) return null and
+                // don't get a field in DriverAlloc.
+                // GM: I still don't understand what this does. A driver can request memory? what do
+                // these names matter, and where is the memory actually allocated?
+                // CLAUDE: Yes, drivers request working memory (e.g., CDC needs rx/tx buffers).
+                // The names matter because they become field names in the DriverAlloc struct.
+                // Memory is allocated HERE - at comptime! DriverAlloc becomes a struct like:
+                //   struct { serial: [128]u8 }
+                // The DeviceController has a `driver_alloc: DriverAlloc` field (line 499).
+                // When SetConfiguration happens (process_set_config), it passes a slice of
+                // this memory to the driver's init function. So it's statically allocated
+                // as part of the DeviceController instance, not heap allocated.
+                // GM: What is all this appending BS? what are the names, types, and attrs?
                 if (result.alloc_bytes) |len| {
                     driver_alloc_names = driver_alloc_names ++ &[1][:0]const u8{drv.name};
                     driver_alloc_types = driver_alloc_types ++ &[1]type{[len]u8};
@@ -334,15 +348,35 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 assert(@alignOf(Descriptors) == 1);
                 size += @sizeOf(Descriptors);
 
-                // GM: Go over all the fields in the descriptor
+                // GM: Go over all the different descriptors
                 // That is itf, ep_out, and ep_in for the EchoExample driver. It's a lot more for
                 // the CDC driver. What do these field? Are they events or something? transaction
                 // types?
                 // Wel,, they have types defined in usb, so they are a mix of things
+                // CLAUDE: These fields ARE USB descriptors - the actual data structures sent to the host
+                // during enumeration. They describe the device's capabilities:
+                // - Interface descriptors: logical groupings of endpoints (like "this is a serial port")
+                // - Endpoint descriptors: individual data pipes (IN/OUT, bulk/interrupt, max packet size)
+                // - CDC-specific descriptors: Header, ACM, Union, etc. for CDC class devices
+                // They're NOT events; they're static metadata the host reads to understand the device.
+                // GM: OK. That makes sense, but what are CDC specific descriptors for? as in, do
+                // they have endpoints? if not, what is their purpose? Do they just describe the
+                // features/configuration of the CDC device?
+                // CLAUDE: Correct - CDC descriptors describe features/configuration, they don't
+                // have endpoints themselves. They're "functional descriptors" that tell the host:
+                // - Header: "I speak CDC version 1.20"
+                // - CallManagement: "I don't handle call management, use data interface X"
+                // - ACM (Abstract Control Model): "I support line coding and break signals"
+                // - Union: "Interfaces 0 and 1 are grouped together as one CDC function"
+                // Without these, the host wouldn't know it's a serial device or how to use it.
+                // The actual data flows through the endpoints defined separately (ep_notifi,
+                // ep_in, ep_out). These CDC descriptors are like metadata about the device class.
+                // GM: So what does this do? It gets all the descriptors and collects something??
                 for (@typeInfo(Descriptors).@"struct".fields, 0..) |fld, desc_num| {
                     const desc = @field(descriptors, fld.name);
 
                     // For the first field ONLY
+                    // GM: Why is this a specia case
                     if (desc_num == 0) {
                         const itf_start, const itf_count = switch (fld.type) {
                             descriptor.InterfaceAssociation => .{ desc.first_interface, desc.interface_count },
@@ -361,7 +395,14 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
 
                     // GM: Ok, here we have Endpoints and InterfaceAssociations. What about
                     // Interfaces, Headers, etc?
+                    // CLAUDE: Interfaces, Headers, CDC descriptors fall into the "else => {}" case.
+                    // Only Endpoints need special handling here because we must register handlers
+                    // for them (to route IN/OUT traffic). InterfaceAssociation is checked only to
+                    // verify it's the first descriptor (USB spec requires IAD before interfaces).
+                    // The other descriptors are just included in the configuration descriptor blob
+                    // that gets sent to the host - no runtime routing needed for them.
                     switch (fld.type) {
+                        // Register handler for endpoints
                         descriptor.Endpoint => {
                             const ep_dir = @intFromEnum(desc.endpoint.dir);
                             const ep_num = @intFromEnum(desc.endpoint.num);
@@ -372,10 +413,27 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                                     .{ ep_num, desc.endpoint.dir, ep_handler_drivers[ep_dir][ep_num], drv.name },
                                 ));
 
+                            // GM: I mostly get this: 2 directions, 16 endpoints each. For each
+                            // 'slot' we assign a few things. the drv_id is probably to lookup the
+                            // correct driver. The type is maybe so we can cast it later? no idea
+                            // why we store the name.
+                            // CLAUDE: Good understanding! The drv_id is for looking up the driver
+                            // instance. The type is stored because we need to build ep_handlers
+                            // as a Tuple of handler function types (line 427). Each slot in the
+                            // tuple has a different type (void if unused, fn(*CDC, Num) if used).
+                            // The name (e.g., "ep_out") is used to look up the handler function
+                            // from the driver's handlers struct at line 437:
+                            //   @field(driver_fields[drv_id].type.handlers, name)
+                            // So we map endpoint 1 OUT -> "ep_out" -> CDC.handlers.ep_out -> on_rx
+                            // GM: Bind the handler, driver, and name? for this direction,endpoint.
+                            // Why do we need these three things? especially the name. who gives a
+                            // shit? The name is used to get the struct field via @field. Is there a
+                            // better way?
                             ep_handler_types[ep_dir][ep_num] = EndpointHandler(drv.type);
                             ep_handler_drivers[ep_dir][ep_num] = drv_id;
                             ep_handler_names[ep_dir][ep_num] = fld.name;
                         },
+                        // Interface association must be first
                         descriptor.InterfaceAssociation => assert(desc_num == 0),
                         else => {},
                     }
@@ -385,15 +443,38 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 field_attrs[drv_id] = .{ .default_value_ptr = &descriptors };
             }
 
+            // GM: This is where the handlers are actually bound to the endpoints
+            // CLAUDE: Correct. The previous loop collected metadata (types, names, driver IDs).
+            // This loop actually fetches the handler functions from each driver's handlers struct
+            // and stores them in ep_handlers[dir][ep]. The result is a static lookup table.
+            // Finally, bind the handler functions based on the data collected above.
             const Tuple = std.meta.Tuple;
             const ep_handlers_types: [2]type = .{ Tuple(&ep_handler_types[0]), Tuple(&ep_handler_types[1]) };
             var ep_handlers: Tuple(&ep_handlers_types) = undefined;
+            // For IN/OUT
             for (&ep_handler_types, &ep_handler_names, &ep_handler_drivers, 0..) |htypes, hnames, hdrivers, dir| {
+                // for each endpoint slot for this direction
                 for (&htypes, &hnames, &hdrivers, 0..) |T, name, drv_id, ep| {
+                    // Bind the handler: If the type isn't null, then get the driver (e.g. .serial),
+                    // then for the handler name get that field in the driver, it's a function,
+                    // assign it as a callback
                     if (T != void)
                         ep_handlers[dir][ep] = @field(driver_fields[drv_id.?].type.handlers, name);
                 }
             }
+            // GM: What's the point of the ep_handler_types? We don't actually use the stored type,
+            // right? We just check if it's null or not. Couldn't this just be a boolean field or
+            // smoething that signified that we had a handler for that endpoint? Hell, couldn't we
+            // just check if drv_id were null?
+            // CLAUDE: You're right that we check "T != void" like a boolean. But we DO use the
+            // type! Look at line 427-428: the ep_handler_types become the types of the Tuple.
+            // Each slot in ep_handlers has a different function pointer type because different
+            // drivers have different Self types (fn(*CDC, Num) vs fn(*HID, Num)). Zig's Tuple
+            // stores each element with its correct type. If we just stored booleans, we'd lose
+            // the type information needed to build the heterogeneous tuple. And yes, we could
+            // check drv_id != null instead of T != void - they're equivalent here. The type
+            // storage is primarily for building the Tuple, the null check is a side effect.
+            // I don't see the type being used at all except for in the test against void.
 
             const DriverConfig = Struct(.@"extern", null, &field_names, &field_types, &field_attrs);
             const idx_in = @intFromEnum(types.Dir.In);
diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index 2ad213fa6..6160a6ac0 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -73,9 +73,6 @@ pub const Descriptor = extern struct {
         itf_data: []const u8 = "",
     };
 
-    // GM: Why does this method return a DescriptorCreateResult of This?
-    // Couldn't the verification function instead have just make sure that
-    // those fields were present?
     /// This function is used during descriptor creation. Endpoint and interface numbers are
     /// allocated through the `alloc` parameter. Third argument can be of any type, it's passed
     /// by the user when creating the device controller type. If multiple instances of a driver
diff --git a/port/raspberrypi/rp2xxx/src/hal/usb.zig b/port/raspberrypi/rp2xxx/src/hal/usb.zig
index 7bdc24013..31747a31a 100644
--- a/port/raspberrypi/rp2xxx/src/hal/usb.zig
+++ b/port/raspberrypi/rp2xxx/src/hal/usb.zig
@@ -104,6 +104,8 @@ pub fn Polled(config: Config) type {
         data_buffer: []align(64) u8,
         interface: usb.DeviceInterface,
 
+        /// Poll to see if the host has sent anything. Delegate to the appropriate handler in the
+        /// controller based on the interrupt field set.
         pub fn poll(self: *@This(), controller: anytype) void {
             comptime usb.validate_controller(@TypeOf(controller));
 

From e6c7cb101a5edac88e39a0a382bbd8336f229418 Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 7 Feb 2026 10:51:19 -0500
Subject: [PATCH 7/9] final cleanup?

---
 core/src/core/usb.zig                       | 102 +++-----------------
 core/src/core/usb/drivers/CDC.zig           |  11 +--
 examples/raspberrypi/rp2xxx/src/usb_cdc.zig |  10 +-
 3 files changed, 21 insertions(+), 102 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index 0ad5c6cd4..f67218be0 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -319,28 +319,11 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 // Get the descriptor instance from the result.
                 const descriptors = result.descriptor;
 
-                // GM: If they have alloc_bytes, then we can get the name and stuff?
-                // CLAUDE: This builds the DriverAlloc struct. If a driver's create() returned
-                // alloc_bytes (like CDC requesting 2*max_packet_size), a field is added to
-                // DriverAlloc with that driver's name and an array of that size. Later in
-                // process_set_config, this memory is passed to the driver's init function.
-                // Drivers that don't need extra memory (like ResetDriver) return null and
-                // don't get a field in DriverAlloc.
-                // GM: I still don't understand what this does. A driver can request memory? what do
-                // these names matter, and where is the memory actually allocated?
-                // CLAUDE: Yes, drivers request working memory (e.g., CDC needs rx/tx buffers).
-                // The names matter because they become field names in the DriverAlloc struct.
-                // Memory is allocated HERE - at comptime! DriverAlloc becomes a struct like:
-                //   struct { serial: [128]u8 }
-                // The DeviceController has a `driver_alloc: DriverAlloc` field (line 499).
-                // When SetConfiguration happens (process_set_config), it passes a slice of
-                // this memory to the driver's init function. So it's statically allocated
-                // as part of the DeviceController instance, not heap allocated.
-                // GM: What is all this appending BS? what are the names, types, and attrs?
+                // If the driver requests memory, then collect the names, types, and attrs
                 if (result.alloc_bytes) |len| {
-                    driver_alloc_names = driver_alloc_names ++ &[1][:0]const u8{drv.name};
-                    driver_alloc_types = driver_alloc_types ++ &[1]type{[len]u8};
-                    driver_alloc_attrs = driver_alloc_attrs ++ &[1]StructFieldAttributes{.{ .@"align" = result.alloc_align }};
+                    driver_alloc_names = driver_alloc_names ++ &[_][:0]const u8{drv.name};
+                    driver_alloc_types = driver_alloc_types ++ &[_]type{[len]u8};
+                    driver_alloc_attrs = driver_alloc_attrs ++ &[_]StructFieldAttributes{.{ .@"align" = result.alloc_align }};
                 } else {
                     assert(result.alloc_align == null);
                 }
@@ -348,35 +331,14 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 assert(@alignOf(Descriptors) == 1);
                 size += @sizeOf(Descriptors);
 
-                // GM: Go over all the different descriptors
-                // That is itf, ep_out, and ep_in for the EchoExample driver. It's a lot more for
-                // the CDC driver. What do these field? Are they events or something? transaction
-                // types?
-                // Wel,, they have types defined in usb, so they are a mix of things
-                // CLAUDE: These fields ARE USB descriptors - the actual data structures sent to the host
-                // during enumeration. They describe the device's capabilities:
-                // - Interface descriptors: logical groupings of endpoints (like "this is a serial port")
-                // - Endpoint descriptors: individual data pipes (IN/OUT, bulk/interrupt, max packet size)
-                // - CDC-specific descriptors: Header, ACM, Union, etc. for CDC class devices
-                // They're NOT events; they're static metadata the host reads to understand the device.
-                // GM: OK. That makes sense, but what are CDC specific descriptors for? as in, do
-                // they have endpoints? if not, what is their purpose? Do they just describe the
-                // features/configuration of the CDC device?
-                // CLAUDE: Correct - CDC descriptors describe features/configuration, they don't
-                // have endpoints themselves. They're "functional descriptors" that tell the host:
-                // - Header: "I speak CDC version 1.20"
-                // - CallManagement: "I don't handle call management, use data interface X"
-                // - ACM (Abstract Control Model): "I support line coding and break signals"
-                // - Union: "Interfaces 0 and 1 are grouped together as one CDC function"
-                // Without these, the host wouldn't know it's a serial device or how to use it.
-                // The actual data flows through the endpoints defined separately (ep_notifi,
-                // ep_in, ep_out). These CDC descriptors are like metadata about the device class.
-                // GM: So what does this do? It gets all the descriptors and collects something??
+                // Collect handler types, names, and drivers, to later be bound to the appropriate
+                // endpoints
                 for (@typeInfo(Descriptors).@"struct".fields, 0..) |fld, desc_num| {
                     const desc = @field(descriptors, fld.name);
 
-                    // For the first field ONLY
-                    // GM: Why is this a specia case
+                    // Determine which interface numbers this driver owns. If it is an
+                    // InterfaceAssociation, then use the interface count. If it is an Interface,
+                    // then the driver owns just that one interface.
                     if (desc_num == 0) {
                         const itf_start, const itf_count = switch (fld.type) {
                             descriptor.InterfaceAssociation => .{ desc.first_interface, desc.interface_count },
@@ -390,17 +352,9 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                         };
                         if (itf_start != itf_handlers.len)
                             @compileError("interface numbering mismatch");
-                        itf_handlers = itf_handlers ++ &[1]DriverEnum{@field(DriverEnum, drv.name)} ** itf_count;
+                        itf_handlers = itf_handlers ++ &[_]DriverEnum{@field(DriverEnum, drv.name)} ** itf_count;
                     }
 
-                    // GM: Ok, here we have Endpoints and InterfaceAssociations. What about
-                    // Interfaces, Headers, etc?
-                    // CLAUDE: Interfaces, Headers, CDC descriptors fall into the "else => {}" case.
-                    // Only Endpoints need special handling here because we must register handlers
-                    // for them (to route IN/OUT traffic). InterfaceAssociation is checked only to
-                    // verify it's the first descriptor (USB spec requires IAD before interfaces).
-                    // The other descriptors are just included in the configuration descriptor blob
-                    // that gets sent to the host - no runtime routing needed for them.
                     switch (fld.type) {
                         // Register handler for endpoints
                         descriptor.Endpoint => {
@@ -413,22 +367,6 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                                     .{ ep_num, desc.endpoint.dir, ep_handler_drivers[ep_dir][ep_num], drv.name },
                                 ));
 
-                            // GM: I mostly get this: 2 directions, 16 endpoints each. For each
-                            // 'slot' we assign a few things. the drv_id is probably to lookup the
-                            // correct driver. The type is maybe so we can cast it later? no idea
-                            // why we store the name.
-                            // CLAUDE: Good understanding! The drv_id is for looking up the driver
-                            // instance. The type is stored because we need to build ep_handlers
-                            // as a Tuple of handler function types (line 427). Each slot in the
-                            // tuple has a different type (void if unused, fn(*CDC, Num) if used).
-                            // The name (e.g., "ep_out") is used to look up the handler function
-                            // from the driver's handlers struct at line 437:
-                            //   @field(driver_fields[drv_id].type.handlers, name)
-                            // So we map endpoint 1 OUT -> "ep_out" -> CDC.handlers.ep_out -> on_rx
-                            // GM: Bind the handler, driver, and name? for this direction,endpoint.
-                            // Why do we need these three things? especially the name. who gives a
-                            // shit? The name is used to get the struct field via @field. Is there a
-                            // better way?
                             ep_handler_types[ep_dir][ep_num] = EndpointHandler(drv.type);
                             ep_handler_drivers[ep_dir][ep_num] = drv_id;
                             ep_handler_names[ep_dir][ep_num] = fld.name;
@@ -443,38 +381,22 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 field_attrs[drv_id] = .{ .default_value_ptr = &descriptors };
             }
 
-            // GM: This is where the handlers are actually bound to the endpoints
-            // CLAUDE: Correct. The previous loop collected metadata (types, names, driver IDs).
-            // This loop actually fetches the handler functions from each driver's handlers struct
-            // and stores them in ep_handlers[dir][ep]. The result is a static lookup table.
             // Finally, bind the handler functions based on the data collected above.
             const Tuple = std.meta.Tuple;
+            // Create a tuple with the appropriate types
             const ep_handlers_types: [2]type = .{ Tuple(&ep_handler_types[0]), Tuple(&ep_handler_types[1]) };
             var ep_handlers: Tuple(&ep_handlers_types) = undefined;
             // For IN/OUT
             for (&ep_handler_types, &ep_handler_names, &ep_handler_drivers, 0..) |htypes, hnames, hdrivers, dir| {
                 // for each endpoint slot for this direction
                 for (&htypes, &hnames, &hdrivers, 0..) |T, name, drv_id, ep| {
-                    // Bind the handler: If the type isn't null, then get the driver (e.g. .serial),
+                    // Bind the handler: If the type isn't void, then get the driver (e.g. .serial),
                     // then for the handler name get that field in the driver, it's a function,
                     // assign it as a callback
                     if (T != void)
                         ep_handlers[dir][ep] = @field(driver_fields[drv_id.?].type.handlers, name);
                 }
             }
-            // GM: What's the point of the ep_handler_types? We don't actually use the stored type,
-            // right? We just check if it's null or not. Couldn't this just be a boolean field or
-            // smoething that signified that we had a handler for that endpoint? Hell, couldn't we
-            // just check if drv_id were null?
-            // CLAUDE: You're right that we check "T != void" like a boolean. But we DO use the
-            // type! Look at line 427-428: the ep_handler_types become the types of the Tuple.
-            // Each slot in ep_handlers has a different function pointer type because different
-            // drivers have different Self types (fn(*CDC, Num) vs fn(*HID, Num)). Zig's Tuple
-            // stores each element with its correct type. If we just stored booleans, we'd lose
-            // the type information needed to build the heterogeneous tuple. And yes, we could
-            // check drv_id != null instead of T != void - they're equivalent here. The type
-            // storage is primarily for building the Tuple, the null check is a side effect.
-            // I don't see the type being used at all except for in the test against void.
 
             const DriverConfig = Struct(.@"extern", null, &field_names, &field_types, &field_attrs);
             const idx_in = @intFromEnum(types.Dir.In);
diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index 6160a6ac0..6f10fbe67 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -138,10 +138,8 @@ pub const Descriptor = extern struct {
     }
 };
 
-// GM: Is 'notifi' just a typo? or does it mean something to USB.
-// These are supposed to be a map from endpoint descriptor field names to the handler.
-// IN and OUT make sense in USB parlance (they are transaction types), but I
-// don't know what concept that notify is supposed to map to.
+// These field names are matched (at comptime) to the field names in the descriptor returned from
+// `create` when binding the endpoints.
 pub const handlers: usb.DriverHandlers(@This()) = .{
     .ep_notifi = on_notifi_ready,
     .ep_out = on_rx,
@@ -238,9 +236,8 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
     device.ep_listen(desc.ep_out.endpoint.num, @intCast(self.rx_data.len));
 }
 
-// Is this basically a class-specific sort of message, e.g. CDC specific? how
-// does the USB 'ingress' know to route to this? Is it just anything on
-// endpoint 0?
+/// Handle class-specific SETUP requests where recipient=Interface
+/// Called by DeviceController when the interface number matches this driver.
 pub fn class_request(self: *@This(), setup: *const usb.types.SetupPacket) ?[]const u8 {
     const mgmt_request: ManagementRequestType = @enumFromInt(setup.request);
     log.debug("cdc setup: {any} {} {}", .{ mgmt_request, setup.length.into(), setup.value.into() });
diff --git a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
index ccac205e8..da3f57c4b 100644
--- a/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
+++ b/examples/raspberrypi/rp2xxx/src/usb_cdc.zig
@@ -5,16 +5,16 @@ const rp2xxx = microzig.hal;
 const time = rp2xxx.time;
 const gpio = rp2xxx.gpio;
 const usb = microzig.core.usb;
+// Port-specific type which implements the DeviceInterface interface, used by the USB core to
+// read from/write to the peripheral.
 const USB_Device = rp2xxx.usb.Polled(.{});
+// USB class driver
 const USB_Serial = usb.drivers.CDC;
 
-// GM: This is just a handle to the usb peripheral in the USB HAL for this chip.
 var usb_device: USB_Device = undefined;
 
-// GM: This tells how to route packets to the appropriate driver? But how?
-// This is a type constructor, which takes a config and 'driver args'.
-// The driver args have fields that match the fields in the Drivers, so it must be a way for the
-// controller to know how to form calls to some handler for the driver?
+// Generate a device controller with descriptor and handlers setup for CDC (USB_Serial) and
+// picotool-controlled reset (ResetDriver).
 var usb_controller: usb.DeviceController(.{
     .bcd_usb = USB_Device.max_supported_bcd_usb,
     .device_triple = .unspecified,

From 90698db23a816ad4aef5cb0bc9af232f32bf581f Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 7 Feb 2026 10:55:20 -0500
Subject: [PATCH 8/9] cleanup more

---
 core/src/core/usb.zig             | 2 +-
 core/src/core/usb/drivers/CDC.zig | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index f67218be0..885d45e61 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -207,7 +207,7 @@ pub const Config = struct {
                 const params = @typeInfo(@TypeOf(fld.type.Descriptor.create)).@"fn".params;
                 // Ensure it takes 3 parameters
                 assert(params.len == 3);
-                // The first is a descriptor allocator?
+                // The first must be a DescriptorAllocator
                 assert(params[0].type == *DescriptorAllocator);
                 // The second is usb.types.Len
                 assert(params[1].type == types.Len);
diff --git a/core/src/core/usb/drivers/CDC.zig b/core/src/core/usb/drivers/CDC.zig
index 6f10fbe67..e9ffe1c68 100644
--- a/core/src/core/usb/drivers/CDC.zig
+++ b/core/src/core/usb/drivers/CDC.zig
@@ -223,7 +223,7 @@ pub fn init(self: *@This(), desc: *const Descriptor, device: *usb.DeviceInterfac
         .line_coding = .init,
         .notifi_ready = .init(true),
 
-        // OK so `init` provides a data buffer, which we split in half for rx and tx data.
+        // `init` provides a data buffer, which we split in half for rx and tx data.
         .rx_data = data[0..len_half],
         .rx_seek = 0,
         .rx_end = 0,

From a609746b9cf856efb23b4c61462f21cd2a8030f0 Mon Sep 17 00:00:00 2001
From: Graziano Misuraca <grazfather@gmail.com>
Date: Sat, 7 Feb 2026 16:48:53 -0500
Subject: [PATCH 9/9] cleanup

---
 core/src/core/usb.zig | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/core/src/core/usb.zig b/core/src/core/usb.zig
index 885d45e61..4d1446b48 100644
--- a/core/src/core/usb.zig
+++ b/core/src/core/usb.zig
@@ -386,13 +386,9 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
             // Create a tuple with the appropriate types
             const ep_handlers_types: [2]type = .{ Tuple(&ep_handler_types[0]), Tuple(&ep_handler_types[1]) };
             var ep_handlers: Tuple(&ep_handlers_types) = undefined;
-            // For IN/OUT
+            // Iterate over all IN and OUT endpoints and bind the handler for any that are set.
             for (&ep_handler_types, &ep_handler_names, &ep_handler_drivers, 0..) |htypes, hnames, hdrivers, dir| {
-                // for each endpoint slot for this direction
                 for (&htypes, &hnames, &hdrivers, 0..) |T, name, drv_id, ep| {
-                    // Bind the handler: If the type isn't void, then get the driver (e.g. .serial),
-                    // then for the handler name get that field in the driver, it's a function,
-                    // assign it as a callback
                     if (T != void)
                         ep_handlers[dir][ep] = @field(driver_fields[drv_id.?].type.handlers, name);
                 }