From a482c871d510fbbbb0f21c3c7b098d9203d09853 Mon Sep 17 00:00:00 2001 From: Shen Mintao Date: Fri, 30 Oct 2026 09:36:33 +0800 Subject: [PATCH] fix: resolve chip firmware paths through the firmware API Follow up issue #117: use each module chip enum and the real Wi-Fi device, preserve flat-layout fallback on missing files, and handle allocation and size failures. Based on the report by ggorbi99-dev. --- drivers/aic8800/aic8800_fdrv/rwnx_platform.c | 70 ++++++- drivers/aic8800/aic_load_fw/aicbluetooth.c | 67 ++++++- tests/firmware/README.md | 31 ++++ tests/firmware/test_request_firmware.py | 181 +++++++++++++++++++ 4 files changed, 329 insertions(+), 20 deletions(-) create mode 100644 tests/firmware/README.md create mode 100644 tests/firmware/test_request_firmware.py diff --git a/drivers/aic8800/aic8800_fdrv/rwnx_platform.c b/drivers/aic8800/aic8800_fdrv/rwnx_platform.c index da41d2b..7de244b 100644 --- a/drivers/aic8800/aic8800_fdrv/rwnx_platform.c +++ b/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; - dst = (u32 *)fw->data; - - if (size <= 0) { + if (!fw->size || fw->size > INT_MAX) { 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; } diff --git a/drivers/aic8800/aic_load_fw/aicbluetooth.c b/drivers/aic8800/aic_load_fw/aicbluetooth.c index 80a8f81..4058ccd 100644 --- a/drivers/aic8800/aic_load_fw/aicbluetooth.c +++ b/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; - dst = (u32 *)fw->data; - - if (size <= 0) { + if (!fw->size || fw->size > INT_MAX) { 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; diff --git a/tests/firmware/README.md b/tests/firmware/README.md new file mode 100644 index 0000000..0fd5d23 --- /dev/null +++ b/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. diff --git a/tests/firmware/test_request_firmware.py b/tests/firmware/test_request_firmware.py new file mode 100644 index 0000000..e47f686 --- /dev/null +++ b/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 +#include +#include +#include +#include +#include +#include +#include +#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()