Re: [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting

[Chao Gao <chao.gao@intel.com>]

On Tue, Apr 07, 2026 at 01:24:03PM -0600, Russ Weight wrote:
>On Tue, Mar 31, 2026 at 02:47:23PM -0700, Dan Williams wrote:
>> Chao Gao raised a module reference circular dependency report resulting
>> from *correct* usage of the firmware_upload_register() API [1]. The
>> module reference count is not necessary nor sufficient for protecting
>> against racing unregister against in-flight requests. After that is
>> fixed, a couple more cleanups fall out.
>> 
>> [1]: https://sashiko.dev/#/patchset/20260326084448.29947-1-chao.gao%40intel.com?patch=10705
>> 
>> 
>> Dan Williams (3):
>>   firmware_loader: Stop pinning modules on registration
>>   firmware_loader: Stop pinning parent device per workqueue invocation
>>   treewide: firmware_loader: Drop the unused @module argument
>> 
>>  .../driver-api/firmware/fw_upload.rst         |  2 +-
>>  include/linux/firmware.h                      | 15 +++---
>>  drivers/base/firmware_loader/sysfs_upload.c   | 48 ++++++++-----------
>>  drivers/cxl/core/memdev.c                     |  4 +-
>>  drivers/firmware/microchip/mpfs-auto-update.c |  2 +-
>>  drivers/fpga/intel-m10-bmc-sec-update.c       |  4 +-
>>  drivers/greybus/gb-beagleplay.c               |  2 +-
>>  drivers/media/i2c/thp7312.c                   |  2 +-
>>  drivers/net/pse-pd/pd692x0.c                  |  4 +-
>>  lib/test_firmware.c                           |  3 +-
>>  10 files changed, 38 insertions(+), 48 deletions(-)
>> 
>> 
>> base-commit: f338e77383789c0cae23ca3d48adcc5e9e137e3c
>
>Hi Dan,
>
>Thanks for making these changes! Overall, the changes look good to
>me. However, when I apply these changes to the specified base-commit
>and attempt to build, I'm getting these errors:

(+Dan's new email)

Yes, I noticed that Sashiko reported them.

>
>drivers/base/firmware_loader/sysfs_upload.c:229:24: warning: unused variable ‘fw_dev’ [-Wunused-variable]
>  229 |         struct device *fw_dev = &fw_sysfs->dev;
>      |                        ^~~~~~

This can be fixed by applying the following diff to patch 2:

diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index e0cf4c55b520..4ce64411f656 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -226,7 +226,6 @@ static void fw_upload_main(struct work_struct *work)
 int fw_upload_start(struct fw_sysfs *fw_sysfs)
 {
	struct fw_priv *fw_priv = fw_sysfs->fw_priv;
-	struct device *fw_dev = &fw_sysfs->dev;
	struct fw_upload_priv *fwlp;
 
	if (!fw_sysfs->fw_upload_priv)

>drivers/base/firmware_loader/sysfs_upload.c: In function ‘firmware_upload_register’: 
>drivers/base/firmware_loader/sysfs_upload.c:323:34: error: ‘module’ undeclared (first use in this function)
>  323 |         fw_upload_priv->module = module;
>      |                                  ^~~~~~

the @module field should be removed in patch 3:

diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index 4ce64411f656..ee4d78443e3b 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -319,7 +319,6 @@ struct fw_upload *firmware_upload_register(struct device *parent,
	fw_upload_priv->fw_upload = fw_upload;
	fw_upload_priv->ops = ops;
	mutex_init(&fw_upload_priv->lock);
-	fw_upload_priv->module = module;
	fw_upload_priv->name = name;
	fw_upload_priv->err_code = 0;
	fw_upload_priv->progress = FW_UPLOAD_PROG_IDLE;
diff --git a/drivers/base/firmware_loader/sysfs_upload.h b/drivers/base/firmware_loader/sysfs_upload.h
index 31931ff7808a..dc7ccdceb96f 100644
--- a/drivers/base/firmware_loader/sysfs_upload.h
+++ b/drivers/base/firmware_loader/sysfs_upload.h
@@ -26,7 +26,6 @@ enum fw_upload_prog {
 
 struct fw_upload_priv {
	struct fw_upload *fw_upload;
-	struct module *module;
	const char *name;
	const struct fw_upload_ops *ops;
	struct mutex lock;		  /* protect data structure contents */