Merge branch 'main' into mmio-usage

Author: piotrfila

.github/workflows/unused-imports.yml

@@ -0,0 +1,134 @@
+name: CI / Unused Imports Check
+
+on:
+  pull_request:
+    branches: [main]
+  workflow_dispatch:
+
+jobs:
+  check-unused-imports:
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: write
+
+    steps:
+      - name: Checkout PR code
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+
+      - name: Setup Zig
+        uses: mlugg/setup-zig@v2
+        with:
+          version: 0.15.1
+
+      - name: Build zigimports
+        run: |
+          git clone --depth 1 https://github.com/tusharsadhwani/zigimports.git /tmp/zigimports
+          cd /tmp/zigimports
+          zig build --release=safe
+          # Binary is named zigimports-<arch>-<os>, find and symlink it
+          ln -s /tmp/zigimports/zig-out/bin/zigimports-* /tmp/zigimports/zig-out/bin/zigimports
+
+      - name: Get changed Zig files
+        id: changed-files
+        run: |
+          FILES=$(git diff --name-only --diff-filter=d ${{ github.event.pull_request.base.sha }} ${{ github.event.pull_request.head.sha }} | grep '\.zig$' || true)
+          echo "files<<EOF" >> $GITHUB_OUTPUT
+          echo "$FILES" >> $GITHUB_OUTPUT
+          echo "EOF" >> $GITHUB_OUTPUT
+
+      - name: Check for unused imports
+        id: check
+        run: |
+          RESULTS=""
+          echo "${{ steps.changed-files.outputs.files }}" | while read file; do
+            if [ -n "$file" ] && [ -f "$file" ]; then
+              /tmp/zigimports/zig-out/bin/zigimports "$file" 2>&1 || true
+            fi
+          done | tee unused_imports.txt
+
+          # Set output for later steps
+          if grep -q "is unused" unused_imports.txt 2>/dev/null; then
+            echo "has_issues=true" >> $GITHUB_OUTPUT
+          else
+            echo "has_issues=false" >> $GITHUB_OUTPUT
+          fi
+
+      - name: Post PR comment
+        if: steps.check.outputs.has_issues == 'true'
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const fs = require('fs');
+
+            const content = fs.readFileSync('unused_imports.txt', 'utf8').trim();
+            const lines = content.split('\n').filter(l => l.includes('is unused'));
+
+            if (lines.length === 0) return;
+
+            // Check for existing comment and update it
+            const marker = '<!-- unused-imports-check -->';
+            const comments = await github.paginate(
+              github.rest.issues.listComments,
+              {
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: context.issue.number
+              }
+            );
+
+            const existing = comments.find(c =>
+              c.user.login === 'github-actions[bot]' &&
+              c.body.includes(marker)
+            );
+
+            let body = '## ⚠️ Unused Imports Found\n\n';
+            body += '```\n' + lines.join('\n') + '\n```\n\n';
+            body += 'Run `zigimports --fix <file>` locally to automatically remove unused imports.\n\n';
+            body += marker;
+
+            if (existing) {
+              await github.rest.issues.updateComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id,
+                body: body
+              });
+            } else {
+              await github.rest.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: context.issue.number,
+                body: body
+              });
+            }
+
+      - name: Remove comment if no issues
+        if: steps.check.outputs.has_issues == 'false'
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const marker = '<!-- unused-imports-check -->';
+            const comments = await github.paginate(
+              github.rest.issues.listComments,
+              {
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: context.issue.number
+              }
+            );
+
+            const existing = comments.find(c =>
+              c.user.login === 'github-actions[bot]' &&
+              c.body.includes(marker)
+            );
+
+            if (existing) {
+              await github.rest.issues.deleteComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id
+              });
+            }

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 must be a DescriptorAllocator
                 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(.{}));
@@ -251,11 +262,14 @@ 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];
         const driver_fields = @typeInfo(config0.Drivers).@"struct".fields;
         const DriverEnum = std.meta.FieldEnum(config0.Drivers);
@@ -298,24 +312,33 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
             var driver_alloc_attrs: []const StructFieldAttributes = &.{};
 
             for (driver_fields, 0..) |drv, drv_id| {
+                // 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));
+                // Get the descriptor instance from the result.
                 const descriptors = result.descriptor;
 
+                // 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);
                 }
 
                 assert(@alignOf(Descriptors) == 1);
                 size += @sizeOf(Descriptors);
 
+                // 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);
 
+                    // 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 },
@@ -329,10 +352,11 @@ 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;
                     }
 
                     switch (fld.type) {
+                        // Register handler for endpoints
                         descriptor.Endpoint => {
                             const ep_dir = @intFromEnum(desc.endpoint.dir);
                             const ep_num = @intFromEnum(desc.endpoint.num);
@@ -347,6 +371,7 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) 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 => {},
                     }
@@ -356,9 +381,12 @@ pub fn DeviceController(config: Config, driver_args: config.DriverArgs()) type {
                 field_attrs[drv_id] = .{ .default_value_ptr = &descriptors };
             }
 
+            // 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;
+            // 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 (&htypes, &hnames, &hdrivers, 0..) |T, name, drv_id, ep| {
                     if (T != void)
@@ -558,6 +586,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);

core/src/core/usb/drivers/CDC.zig

@@ -3,6 +3,7 @@ const usb = @import("../../usb.zig");
 const assert = std.debug.assert;
 const log = std.log.scoped(.usb_cdc);
 
+/// CDC PSTN Subclass Management Element Requests
 pub const ManagementRequestType = enum(u8) {
     SetCommFeature = 0x02,
     GetCommFeature = 0x03,
@@ -27,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) {
@@ -44,13 +46,14 @@ 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,
     };
 };
 
+// This struct bundles all the descriptors CDC needs into one Configuration.
 pub const Descriptor = extern struct {
     const desc = usb.descriptor;
 
@@ -70,6 +73,11 @@ pub const Descriptor = extern struct {
         itf_data: []const u8 = "",
     };
 
+    /// 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 preferred to making the whole driver generic.
     pub fn create(
         alloc: *usb.DescriptorAllocator,
         max_supported_packet_size: usb.types.Len,
@@ -130,6 +138,8 @@ pub const Descriptor = extern struct {
     }
 };
 
+// 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,
@@ -202,21 +212,18 @@ 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());
     assert(len_half == desc.ep_out.max_packet_size.into());
     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),
 
+        // `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 +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));
 }
 
+/// 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() });