Merge pull request #125 from shenmintao/fix/issue117-request-firmware-legacy

fix(legacy-mcu1): resolve USB chip firmware API paths

drivers/aic8800/aic8800_fdrv/rwnx_platform.c

@@ -609,6 +609,54 @@ static int rwnx_plat_bin_fw_upload(struct rwnx_plat *rwnx_plat, u8* fw_addr,
 #define MD5(x) x[0],x[1],x[2],x[3],x[4],x[5],x[6],x[7],x[8],x[9],x[10],x[11],x[12],x[13],x[14],x[15]
 #define MD5PINRT "file md5:%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\r\n"
 
+#ifdef CONFIG_USE_FW_REQUEST
+static int aic_request_firmware(const struct firmware **fw, const char *name,
+                               struct device *device)
+{
+#if defined(CONFIG_PLATFORM_UBUNTU) && defined(AICWF_USB_SUPPORT)
+    struct aicwf_bus *bus = device ? dev_get_drvdata(device) : NULL;
+    struct aic_usb_dev *usbdev = bus ? bus->bus_priv.usb : NULL;
+    const char *dir = NULL;
+    char path[256];
+    int ret;
+
+    if (usbdev) {
+        switch (usbdev->chipid) {
+        case PRODUCT_ID_AIC8801:
+            dir = "aic8800";
+            break;
+        case PRODUCT_ID_AIC8800DC:
+        case PRODUCT_ID_AIC8800DW:
+            dir = "aic8800DC";
+            break;
+        case PRODUCT_ID_AIC8800D81:
+            dir = "aic8800D80";
+            break;
+        case PRODUCT_ID_AIC8800D81X2:
+        case PRODUCT_ID_AIC8800D89X2:
+            dir = "aic8800D80X2";
+            break;
+        case PRODUCT_ID_AIC8800D80N:
+            dir = "aic8800D80N";
+            break;
+        case PRODUCT_ID_AIC8800DLN:
+            dir = "aic8800DLN";
+            break;
+        }
+    }
+    if (dir) {
+        if (snprintf(path, sizeof(path), "%s/%s", dir, name) >= sizeof(path))
+            return -ENAMETOOLONG;
+        ret = request_firmware(fw, path, device);
+        /* Keep flat firmware layouts, without hiding other load errors. */
+        if (ret != -ENOENT)
+            return ret;
+    }
+#endif
+    return request_firmware(fw, name, device);
+}
+#endif
+
 static int rwnx_load_firmware(u32 **fw_buf, const char *name, struct device *device)
 {
 
@@ -623,25 +671,27 @@ static int rwnx_load_firmware(u32 **fw_buf, const char *name, struct device *dev
 
 	AICWFDBG(LOGINFO, "%s: request firmware = %s \n", __func__ ,name);
 
-	ret = request_firmware(&fw, name, NULL);
+	*fw_buf = NULL;
+	ret = aic_request_firmware(&fw, name, device);
 
 	if (ret < 0) {
 		AICWFDBG(LOGERROR, "Load %s fail\n", name);
 		release_firmware(fw);
-		return -1;
+		return ret;
 	}
 
-	size = fw->size;
+	if (!fw->size || fw->size > INT_MAX) {
-	dst = (u32 *)fw->data;
-
-	if (size <= 0) {
 		AICWFDBG(LOGERROR, "wrong size of firmware file\n");
 		release_firmware(fw);
-		return -1;
+		return -EINVAL;
 	}
-
+	size = fw->size;
+	dst = (u32 *)fw->data;
 	buffer = vmalloc(size);
-	memset(buffer, 0, size);
+	if (!buffer) {
+		release_firmware(fw);
+		return -ENOMEM;
+	}
 	memcpy(buffer, dst, size);
 
 	*fw_buf = buffer;
@@ -785,7 +835,7 @@ int rwnx_request_firmware_common(struct rwnx_hw *rwnx_hw, u32** buffer, const ch
 
     AICWFDBG(LOGINFO, "### Load file %s\n", filename);
 
-    size = rwnx_load_firmware(buffer, filename, NULL);
+    size = rwnx_load_firmware(buffer, filename, rwnx_hw->dev);
 
     return size;
 }

drivers/aic8800/aic_load_fw/aicbluetooth.c

@@ -249,6 +249,52 @@ void aic_bt_platform_deinit(struct aic_usb_dev *usbdev)
 #define MD5(x) x[0],x[1],x[2],x[3],x[4],x[5],x[6],x[7],x[8],x[9],x[10],x[11],x[12],x[13],x[14],x[15]
 #define MD5PINRT "file md5:%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\r\n"
 
+#ifdef CONFIG_USE_FW_REQUEST
+static int aic_request_firmware(const struct firmware **fw, const char *name,
+                               struct device *device)
+{
+#if defined(CONFIG_PLATFORM_UBUNTU) && defined(AICWF_USB_SUPPORT)
+    struct aicwf_bus *bus = device ? dev_get_drvdata(device) : NULL;
+    struct aic_usb_dev *usbdev = bus ? bus->bus_priv.usb : NULL;
+    const char *dir = NULL;
+    char path[256];
+    int ret;
+
+    if (usbdev) {
+        switch (usbdev->chipid) {
+        case PRODUCT_ID_AIC8800:
+        case PRODUCT_ID_AIC8801:
+            dir = "aic8800";
+            break;
+        case PRODUCT_ID_AIC8800DC:
+        case PRODUCT_ID_AIC8800DW:
+            dir = "aic8800DC";
+            break;
+        case PRODUCT_ID_AIC8800D80:
+        case PRODUCT_ID_AIC8800D81:
+            dir = "aic8800D80";
+            break;
+        case PRODUCT_ID_AIC8800D80X2:
+        case PRODUCT_ID_AIC8800D81X2:
+        case PRODUCT_ID_AIC8800D89X2:
+        case PRODUCT_ID_AIC8800D40X2:
+            dir = "aic8800D80X2";
+            break;
+        }
+    }
+    if (dir) {
+        if (snprintf(path, sizeof(path), "%s/%s", dir, name) >= sizeof(path))
+            return -ENAMETOOLONG;
+        ret = request_firmware(fw, path, device);
+        /* Keep flat firmware layouts, without hiding other load errors. */
+        if (ret != -ENOENT)
+            return ret;
+    }
+#endif
+    return request_firmware(fw, name, device);
+}
+#endif
+
 static int aic_load_firmware(u32 ** fw_buf, const char *name, struct device *device)
 {
 
@@ -264,26 +310,27 @@ static int aic_load_firmware(u32 ** fw_buf, const char *name, struct device *dev
 	printk("%s: request firmware = %s \n", __func__ ,name);
 
 
-	ret = request_firmware(&fw, name, NULL);
+	*fw_buf = NULL;
+	ret = aic_request_firmware(&fw, name, device);
 	
 	if (ret < 0) {
 		printk("Load %s fail\n", name);
 		release_firmware(fw);
-		return -1;
+		return ret;
 	}
 	
-	size = fw->size;
+	if (!fw->size || fw->size > INT_MAX) {
-	dst = (u32 *)fw->data;
-
-	if (size <= 0) {
 		printk("wrong size of firmware file\n");
 		release_firmware(fw);
-		return -1;
+		return -EINVAL;
 	}
-
+	size = fw->size;
-
+	dst = (u32 *)fw->data;
 	buffer = vmalloc(size);
-	memset(buffer, 0, size);
+	if (!buffer) {
+		release_firmware(fw);
+		return -ENOMEM;
+	}
 	memcpy(buffer, dst, size);
 	
 	*fw_buf = buffer;

tests/firmware/README.md

@@ -0,0 +1,31 @@
+# Firmware loading regression checks
+
+Run `python3 tests/firmware/test_request_firmware.py` with a C compiler available.
+The fixture extracts both modules' real request and load functions and the
+Wi-Fi common caller. It checks all USB chip directory mappings, device context,
+flat-layout fallback only on `-ENOENT`, missing device data, non-USB/platform
+behavior, path overflow, invalid file sizes, and allocation failure. Kernel
+services are mocked; this does not exercise firmware startup or hardware.
+
+`python3 tests/firmware/test_firmware.py --modules BUILD_DIR` also checks the
+installed inventory against the built modules' firmware declarations and the
+loader's probe error paths.
+
+For a kernel build with the firmware API enabled:
+
+```sh
+bash tests/kernel-build/build.sh /path/to/kernel/build CC=gcc-14 CONFIG_USE_FW_REQUEST=y
+```
+
+The build default remains `CONFIG_USE_FW_REQUEST=n`. The `y` path searches the
+kernel firmware locations using chip-relative names such as
+`aic8800D80/fmacfw_8800d80_u02.bin`, then tries the bare name if the chip-relative
+file is absent. The two modules use different chip ID enums; their mappings
+must remain separate. Both retain the real USB device as the request context.
+Custom absolute `aic_fw_path` directories belong to the existing `n` path;
+the `y` path uses the kernel firmware search configuration instead.
+
+Related: [issue #117](https://github.com/shenmintao/aic8800d80/issues/117).
+The path correction follows ggorbi99-dev's report, with the operational-driver
+IDs and its previously null device argument corrected as well. It complements
+the initramfs declarations; it does not establish the reporter's cold-boot fix.

tests/firmware/test_request_firmware.py

@@ -0,0 +1,181 @@
+#!/usr/bin/env python3
+"""Exercise the real firmware request/load functions with mocked kernel services."""
+from pathlib import Path
+import os
+import re
+import subprocess
+import tempfile
+import unittest
+
+ROOT = Path(__file__).resolve().parents[2]
+
+
+def function(source, signature):
+    start = source.index(signature)
+    brace = source.index("{", start)
+    end, depth = brace + 1, 1
+    while depth:
+        depth += (source[end] == "{") - (source[end] == "}")
+        end += 1
+    return source[start:end]
+
+
+PRELUDE = r'''
+#include <assert.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#define CONFIG_USE_FW_REQUEST 1
+#define printk(...) ((void)0)
+#define AICWFDBG(...) ((void)0)
+typedef uint32_t u32;
+typedef int MD5_CTX;
+static void MD5Init(MD5_CTX *m) {}
+static void MD5Update(MD5_CTX *m, unsigned char *b, int size) {}
+static void MD5Final(MD5_CTX *m, unsigned char *b) {}
+struct firmware { size_t size; const unsigned char *data; };
+struct aic_usb_dev { int chipid; };
+struct aicwf_bus { struct { struct aic_usb_dev *usb; } bus_priv; };
+struct device { void *data; };
+struct rwnx_hw { struct device *dev; };
+static struct aic_usb_dev usb;
+static struct aicwf_bus bus = { .bus_priv.usb = &usb };
+static struct device dev = { .data = &bus };
+static struct device *expected_device;
+static struct rwnx_hw hw;
+static const unsigned char bytes[] = { 1, 3, 5, 7, 9 };
+static struct firmware fw = { sizeof(bytes), bytes };
+static const char *names[2];
+static int errors[2], requests, releases, allocation_fail;
+static void *dev_get_drvdata(struct device *d) { return d->data; }
+static int request_firmware(const struct firmware **out, const char *name,
+                            struct device *d) {
+    assert(requests < 2 && names[requests]);
+    assert(!strcmp(name, names[requests]));
+    assert(d == expected_device);
+    int err = errors[requests++];
+    *out = err ? NULL : &fw;
+    return err;
+}
+static void release_firmware(const struct firmware *f) {
+    if (f) { assert(f == &fw); releases++; }
+}
+static void *vmalloc(size_t size) { return allocation_fail ? NULL : malloc(size); }
+static void setup(int chip, const char *name) {
+    usb.chipid = chip; bus.bus_priv.usb = &usb; dev.data = &bus;
+    hw.dev = expected_device = &dev;
+    names[0] = name; names[1] = NULL;
+    errors[0] = errors[1] = requests = releases = allocation_fail = 0;
+    fw.size = sizeof(bytes);
+}
+'''
+
+CHECKS = r'''
+static void success(void) {
+    u32 *out = (void *)1;
+    assert(load(&out, "fw.bin") == sizeof(bytes));
+    assert(out && !memcmp(out, bytes, sizeof(bytes)));
+    assert(releases == 1);
+    free(out);
+}
+int main(void) {
+    for (unsigned int i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
+        char path[256];
+        snprintf(path, sizeof(path), "%s/fw.bin", cases[i].dir);
+#if defined(CONFIG_PLATFORM_UBUNTU) && defined(AICWF_USB_SUPPORT)
+        setup(cases[i].chip, path);
+#else
+        setup(cases[i].chip, "fw.bin");
+#endif
+        success(); assert(requests == 1);
+    }
+    setup(-1, "fw.bin"); success();
+    setup(cases[0].chip, "fw.bin"); hw.dev = expected_device = NULL; success();
+    setup(cases[0].chip, "fw.bin"); dev.data = NULL; success();
+    setup(cases[0].chip, "fw.bin"); bus.bus_priv.usb = NULL; success();
+#if defined(CONFIG_PLATFORM_UBUNTU) && defined(AICWF_USB_SUPPORT)
+    char path[256];
+    snprintf(path, sizeof(path), "%s/fw.bin", cases[0].dir);
+    setup(cases[0].chip, path);
+    errors[0] = -ENOENT; names[1] = "fw.bin";
+    success(); assert(requests == 2);
+    setup(cases[0].chip, path);
+    errors[0] = -ENOENT; names[1] = "fw.bin"; errors[1] = -ENOENT;
+    u32 *out = (void *)1;
+    assert(load(&out, "fw.bin") == -ENOENT && !out && requests == 2);
+    setup(cases[0].chip, path); errors[0] = -EACCES; out = (void *)1;
+    assert(load(&out, "fw.bin") == -EACCES && !out && requests == 1);
+    setup(cases[0].chip, path); errors[0] = -ENOMEM; out = (void *)1;
+    assert(load(&out, "fw.bin") == -ENOMEM && !out && requests == 1);
+    char long_name[300]; memset(long_name, 'a', sizeof(long_name));
+    long_name[sizeof(long_name) - 1] = 0;
+    setup(cases[0].chip, NULL); out = (void *)1;
+    assert(load(&out, long_name) == -ENAMETOOLONG && !out && !requests);
+#endif
+    u32 *buffer = (void *)1;
+    setup(-1, "fw.bin"); allocation_fail = 1;
+    assert(load(&buffer, "fw.bin") == -ENOMEM && !buffer && releases == 1);
+    setup(-1, "fw.bin"); fw.size = 0; buffer = (void *)1;
+    assert(load(&buffer, "fw.bin") == -EINVAL && !buffer && releases == 1);
+    setup(-1, "fw.bin"); fw.size = (size_t)INT_MAX + 1; buffer = (void *)1;
+    assert(load(&buffer, "fw.bin") == -EINVAL && !buffer && releases == 1);
+    return 0;
+}
+'''
+
+
+class RequestFirmwareTests(unittest.TestCase):
+    def test_paths_device_context_and_failures(self):
+        variants = {
+            "aic_load_fw": ("aicbluetooth.c", "aic_load_firmware", {
+                "aic8800": ["AIC8800", "AIC8801"],
+                "aic8800DC": ["AIC8800DC", "AIC8800DW"],
+                "aic8800D80": ["AIC8800D80", "AIC8800D81"],
+                "aic8800D80X2": ["AIC8800D80X2", "AIC8800D81X2",
+                                  "AIC8800D89X2", "AIC8800D40X2"],
+            }),
+            "aic8800_fdrv": ("rwnx_platform.c", "rwnx_load_firmware", {
+                "aic8800": ["AIC8801"],
+                "aic8800DC": ["AIC8800DC", "AIC8800DW"],
+                "aic8800D80": ["AIC8800D81"],
+                "aic8800D80X2": ["AIC8800D81X2", "AIC8800D89X2"],
+                "aic8800D80N": ["AIC8800D80N"],
+                "aic8800DLN": ["AIC8800DLN"],
+            }),
+        }
+        for module, (filename, loader, mapping) in variants.items():
+            folder = ROOT / "drivers/aic8800" / module
+            source = (folder / filename).read_text()
+            enums = re.search(r"enum AICWF_IC\s*\{.*?\};",
+                              (folder / "aicwf_usb.h").read_text(), re.S).group()
+            code = PRELUDE + enums + function(source, "static int aic_request_firmware(")
+            code += function(source, "static int " + loader + "(")
+            if module == "aic8800_fdrv":
+                code += function(source, "int rwnx_request_firmware_common(")
+                call = "rwnx_request_firmware_common(&hw, out, name)"
+            else:
+                call = "aic_load_firmware(out, name, hw.dev)"
+            code += "\nstatic int load(u32 **out, const char *name) { return " + call + "; }\n"
+            code += "static const struct { int chip; const char *dir; } cases[] = {\n"
+            for directory, chips in mapping.items():
+                for chip in chips:
+                    code += '{PRODUCT_ID_' + chip + ', "' + directory + '"},\n'
+            code += "};\n" + CHECKS
+            for defines in ([], ["CONFIG_PLATFORM_UBUNTU", "AICWF_USB_SUPPORT"],
+                            ["CONFIG_PLATFORM_UBUNTU"]):
+                with self.subTest(module=module, defines=defines):
+                    with tempfile.TemporaryDirectory(prefix="aic-fw-request-") as temp:
+                        c_file, binary = Path(temp) / "test.c", Path(temp) / "test"
+                        c_file.write_text(code)
+                        subprocess.run([os.environ.get("CC", "cc"), "-std=gnu11",
+                                        *["-D" + d for d in defines], "-o", str(binary),
+                                        str(c_file)], check=True)
+                        subprocess.run([str(binary)], check=True)
+
+
+if __name__ == "__main__":
+    unittest.main()