From 1a5d54a3213041262caf1605bb19c66ddded41f7 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Wed, 22 Apr 2026 10:09:44 +0200 Subject: [PATCH] bes2600: use request_firmware() for factory.txt read The BES2600 factory calibration file (bes2600_factory.txt) was being read via filp_open() + kernel_read() from a hard-coded absolute path baked in at compile time via the FACTORY_PATH Makefile macro (default: /lib/firmware/bes2600_factory.txt). This had several problems: 1. Path mismatch - linux-firmware-style packaging (and danctnix 0.2-5 device-pine64-pinetab2) ships the file at /lib/firmware/bes2600/bes2600_factory.txt, not /lib/firmware/. The driver logged '(NULL device *): read and check /lib/firmware/bes2600_factory.txt error' on every boot on PineTab2 running linux-pinetab2 6.19.10-danctnix1-1. 2. Direct filesystem access via filp_open() / kernel_read() from a driver is an anti-pattern that upstream rejects: drivers should use request_firmware() to get binary data from userspace-managed firmware directories. request_firmware() natively searches the firmware_class path list (typically /lib/firmware + derivatives), associates the load with a uevent, and respects the firmware-loading infrastructure. 3. The (NULL device *) prefix in error messages indicated the absence of proper device-context logging. While this patch does not yet thread struct device through, the upstream path uses request_firmware() which works with dev=NULL and is the building block for a follow-up patch that adds per-chip device context. Repoint the FACTORY_PATH default to the firmware-class name (bes2600/bes2600_factory.txt) - request_firmware() prepends /lib/firmware/ from the configured search paths. The macro remains overridable at build time for non-standard deployments. Rewrite factory_section_read_file() to: * Call request_firmware(&fw, path, NULL). * Size-check fw->size against FACTORY_MAX_SIZE. * memcpy the data into the caller's buffer. * Always call release_firmware() on exit. The file write path (factory_section_write_file + kernel_write) is left unchanged in this patch; it is the subject of a follow-up patch that removes kernel_write and moves any remaining userspace-visible factory configuration to a standard kernel-userspace boundary (debugfs or nl80211 testmode). No caller signature changes. No Makefile flag drops. Bisectable. Tested-on: PineTab2 (BES2600WM + RK3566) running linux-pinetab2 6.19.10-danctnix1-1, deployed via /lib/modules//extra/. Verified post-reboot: original 'read and check /lib/firmware/bes2600_factory.txt error' is gone; request_firmware reads the file successfully (a separate factory_parse() bug, previously masked by the read failure, is now exposed and tracked separately). Signed-off-by: Markus Fritsche --- bes2600/Makefile | 2 +- bes2600/bes2600_factory.c | 33 ++++++++++++++------------------- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/bes2600/Makefile b/bes2600/Makefile index 300912b..788aee2 100644 --- a/bes2600/Makefile +++ b/bes2600/Makefile @@ -66,7 +66,7 @@ BES2600_DRV_VERSION := bes2600_0.3.5_2024.0116 ifeq ($(CONFIG_BES2600_CALIB_FROM_LINUX),y) FACTORY_CRC_CHECK ?= n STANDARD_FACTORY_EFUSE_FLAG ?= y -FACTORY_PATH ?= /lib/firmware/bes2600_factory.txt +FACTORY_PATH ?= bes2600/bes2600_factory.txt endif # basic function diff --git a/bes2600/bes2600_factory.c b/bes2600/bes2600_factory.c index dc5d3da..8d60b7c 100644 --- a/bes2600/bes2600_factory.c +++ b/bes2600/bes2600_factory.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -137,38 +138,32 @@ static int bes2600_factory_crc_check(struct factory_t *factory_data) */ static int factory_section_read_file(char *path, void *buffer) { - int ret = 0; - struct file *fp; + const struct firmware *fw; + int ret; if (!path || !buffer) { bes_err("%s NULL pointer err\n", __func__); return -1; } - bes_devel("reading %s \n", path); + bes_devel("requesting firmware-class %s\n", path); - fp = filp_open(path, O_RDONLY, 0); //S_IRUSR - if (IS_ERR(fp)) { - bes_devel("BES2600 : can't open %s\n",path); + ret = request_firmware(&fw, path, NULL); + if (ret) { + bes_devel("BES2600: request_firmware(%s) failed: %d\n", path, ret); return -1; } - if (fp->f_inode->i_size <= 0 || fp->f_inode->i_size > FACTORY_MAX_SIZE) { - bes_err( "bes2600_factory.txt size check failed, read_size: %lld max_size: %d\n", - fp->f_inode->i_size, FACTORY_MAX_SIZE); - filp_close(fp, NULL); + if (fw->size == 0 || fw->size > FACTORY_MAX_SIZE) { + bes_err("bes2600_factory.txt size check failed, read_size: %zu max_size: %d\n", + fw->size, FACTORY_MAX_SIZE); + release_firmware(fw); return -1; } - ret = kernel_read(fp, buffer, fp->f_inode->i_size, &fp->f_pos); - - filp_close(fp, NULL); - - if (ret != fp->f_inode->i_size) { - bes_err("bes2600_factory.txt read fail\n"); - ret = -1; - } - + memcpy(buffer, fw->data, fw->size); + ret = (int)fw->size; + release_firmware(fw); return ret; } -- 2.53.0