* [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting
@ 2026-03-31 21:47 Dan Williams
2026-03-31 21:47 ` [PATCH 1/3] firmware_loader: Stop pinning modules on registration Dan Williams
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Dan Williams @ 2026-03-31 21:47 UTC (permalink / raw)
To: mcgrof, russ.weight, dakr
Cc: linux-kernel, Chao Gao, Greg Kroah-Hartman, Rafael J. Wysocki
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
--
2.53.0
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] firmware_loader: Stop pinning modules on registration 2026-03-31 21:47 [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Dan Williams @ 2026-03-31 21:47 ` Dan Williams 2026-03-31 21:47 ` [PATCH 2/3] firmware_loader: Stop pinning parent device per workqueue invocation Dan Williams ` (2 subsequent siblings) 3 siblings, 0 replies; 7+ messages in thread From: Dan Williams @ 2026-03-31 21:47 UTC (permalink / raw) To: mcgrof, russ.weight, dakr Cc: linux-kernel, Greg Kroah-Hartman, Rafael J. Wysocki, Chao Gao The module reference counting can result in callers pinning themselves in a circular loop. The module reference counting is unnecessary. firmware_upload_unregister() must be able to guarantee that all ops are idle at return. All ops are either called from sysfs or the workqueue, so unregister sysfs to stop submissions, cancel any started transfers, flush cancelled transfers, and then release the device. This also solves a theoretical race of new submissions starting between flush_work() and device_unregister(). The module reference was not protecting against that race. Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: Russ Weight <russ.weight@linux.dev> Cc: Danilo Krummrich <dakr@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Reported-by: Chao Gao <chao.gao@intel.com> Tested-by: Chao Gao <chao.gao@intel.com> Closes: https://sashiko.dev/#/patchset/20260326084448.29947-1-chao.gao%40intel.com?patch=10705 Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support") Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/base/firmware_loader/sysfs_upload.c | 31 ++++++++++----------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c index f59a7856934c..87c4f2a9a21b 100644 --- a/drivers/base/firmware_loader/sysfs_upload.c +++ b/drivers/base/firmware_loader/sysfs_upload.c @@ -312,14 +312,9 @@ firmware_upload_register(struct module *module, struct device *parent, return ERR_PTR(-EINVAL); } - if (!try_module_get(module)) - return ERR_PTR(-EFAULT); - fw_upload = kzalloc_obj(*fw_upload); - if (!fw_upload) { - ret = -ENOMEM; - goto exit_module_put; - } + if (!fw_upload) + return ERR_PTR(-ENOMEM); fw_upload_priv = kzalloc_obj(*fw_upload_priv); if (!fw_upload_priv) { @@ -360,7 +355,7 @@ firmware_upload_register(struct module *module, struct device *parent, if (ret) { dev_err(fw_dev, "%s: device_register failed\n", __func__); put_device(fw_dev); - goto exit_module_put; + return ERR_PTR(ret); } return fw_upload; @@ -374,9 +369,6 @@ firmware_upload_register(struct module *module, struct device *parent, free_fw_upload: kfree(fw_upload); -exit_module_put: - module_put(module); - return ERR_PTR(ret); } EXPORT_SYMBOL_GPL(firmware_upload_register); @@ -388,23 +380,28 @@ EXPORT_SYMBOL_GPL(firmware_upload_register); void firmware_upload_unregister(struct fw_upload *fw_upload) { struct fw_sysfs *fw_sysfs = fw_upload->priv; + struct device *parent = fw_sysfs->dev.parent; struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv; - struct module *module = fw_upload_priv->module; + + /* hold a parent reference while child is unregistered */ + get_device(parent); + + /* shutdown the sysfs interface to block new requests */ + device_del(&fw_sysfs->dev); mutex_lock(&fw_upload_priv->lock); if (fw_upload_priv->progress == FW_UPLOAD_PROG_IDLE) { mutex_unlock(&fw_upload_priv->lock); - goto unregister; + goto release; } fw_upload_priv->ops->cancel(fw_upload); mutex_unlock(&fw_upload_priv->lock); +release: /* Ensure lower-level device-driver is finished */ flush_work(&fw_upload_priv->work); - -unregister: - device_unregister(&fw_sysfs->dev); - module_put(module); + put_device(&fw_sysfs->dev); + put_device(parent); } EXPORT_SYMBOL_GPL(firmware_upload_unregister); -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] firmware_loader: Stop pinning parent device per workqueue invocation 2026-03-31 21:47 [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Dan Williams 2026-03-31 21:47 ` [PATCH 1/3] firmware_loader: Stop pinning modules on registration Dan Williams @ 2026-03-31 21:47 ` Dan Williams 2026-03-31 21:47 ` [PATCH 3/3] treewide: firmware_loader: Drop the unused @module argument Dan Williams 2026-04-07 19:24 ` [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Russ Weight 3 siblings, 0 replies; 7+ messages in thread From: Dan Williams @ 2026-03-31 21:47 UTC (permalink / raw) To: mcgrof, russ.weight, dakr Cc: linux-kernel, Greg Kroah-Hartman, Rafael J. Wysocki, Chao Gao The device core pins parent devices while children are registered. As long as all usage of the parent device by the firmware_loader ends at firmware_upload_unregister(), no per queue_work() reference is needed. Now that firmware_upload_unregister() holds its own parent device reference over the child device_del() and flush_work() events, the per queue_work() reference can be deleted. Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: Russ Weight <russ.weight@linux.dev> Cc: Danilo Krummrich <dakr@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Chao Gao <chao.gao@intel.com> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- drivers/base/firmware_loader/sysfs_upload.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c index 87c4f2a9a21b..23f6cdaf29c5 100644 --- a/drivers/base/firmware_loader/sysfs_upload.c +++ b/drivers/base/firmware_loader/sysfs_upload.c @@ -176,7 +176,7 @@ static void fw_upload_main(struct work_struct *work) ret = fwlp->ops->prepare(fwl, fwlp->data, fwlp->remaining_size); if (ret != FW_UPLOAD_ERR_NONE) { fw_upload_set_error(fwlp, ret); - goto putdev_exit; + goto out; } fw_upload_update_progress(fwlp, FW_UPLOAD_PROG_TRANSFERRING); @@ -204,9 +204,7 @@ static void fw_upload_main(struct work_struct *work) done: if (fwlp->ops->cleanup) fwlp->ops->cleanup(fwl); - -putdev_exit: - put_device(fw_dev->parent); +out: /* * Note: fwlp->remaining_size is left unmodified here to provide @@ -249,8 +247,6 @@ int fw_upload_start(struct fw_sysfs *fw_sysfs) return -EBUSY; } - get_device(fw_dev->parent); /* released in fw_upload_main */ - fwlp->progress = FW_UPLOAD_PROG_RECEIVING; fwlp->err_code = 0; fwlp->remaining_size = fw_priv->size; -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] treewide: firmware_loader: Drop the unused @module argument 2026-03-31 21:47 [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Dan Williams 2026-03-31 21:47 ` [PATCH 1/3] firmware_loader: Stop pinning modules on registration Dan Williams 2026-03-31 21:47 ` [PATCH 2/3] firmware_loader: Stop pinning parent device per workqueue invocation Dan Williams @ 2026-03-31 21:47 ` Dan Williams 2026-04-07 19:24 ` [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Russ Weight 3 siblings, 0 replies; 7+ messages in thread From: Dan Williams @ 2026-03-31 21:47 UTC (permalink / raw) To: mcgrof, russ.weight, dakr Cc: linux-kernel, Chao Gao, Greg Kroah-Hartman, Rafael J. Wysocki Now that the firmware loader properly ceases all operations at firmware_upload_unregister() and no longer takes module references, clean up the unused parameter. Cc: Chao Gao <chao.gao@intel.com> Cc: Luis Chamberlain <mcgrof@kernel.org> Cc: Russ Weight <russ.weight@linux.dev> Cc: Danilo Krummrich <dakr@kernel.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: "Rafael J. Wysocki" <rafael@kernel.org> Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- Documentation/driver-api/firmware/fw_upload.rst | 2 +- include/linux/firmware.h | 15 +++++++-------- drivers/base/firmware_loader/sysfs_upload.c | 9 ++++----- 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, 22 insertions(+), 25 deletions(-) diff --git a/Documentation/driver-api/firmware/fw_upload.rst b/Documentation/driver-api/firmware/fw_upload.rst index edf1d0c5e7c3..702b1ed77551 100644 --- a/Documentation/driver-api/firmware/fw_upload.rst +++ b/Documentation/driver-api/firmware/fw_upload.rst @@ -57,7 +57,7 @@ function calls firmware_upload_unregister() such as:: len = (truncate) ? truncate - fw_name : strlen(fw_name); sec->fw_name = kmemdup_nul(fw_name, len, GFP_KERNEL); - fwl = firmware_upload_register(THIS_MODULE, sec->dev, sec->fw_name, + fwl = firmware_upload_register(sec->dev, sec->fw_name, &m10bmc_ops, sec); if (IS_ERR(fwl)) { dev_err(sec->dev, "Firmware Upload driver failed to start\n"); diff --git a/include/linux/firmware.h b/include/linux/firmware.h index aae1b85ffc10..1cda26ef2d8d 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -187,20 +187,19 @@ static inline int request_partial_firmware_into_buf #ifdef CONFIG_FW_UPLOAD -struct fw_upload * -firmware_upload_register(struct module *module, struct device *parent, - const char *name, const struct fw_upload_ops *ops, - void *dd_handle); +struct fw_upload *firmware_upload_register(struct device *parent, + const char *name, + const struct fw_upload_ops *ops, + void *dd_handle); void firmware_upload_unregister(struct fw_upload *fw_upload); #else static inline struct fw_upload * -firmware_upload_register(struct module *module, struct device *parent, - const char *name, const struct fw_upload_ops *ops, - void *dd_handle) +firmware_upload_register(struct device *parent, const char *name, + const struct fw_upload_ops *ops, void *dd_handle) { - return ERR_PTR(-EINVAL); + return ERR_PTR(-EINVAL); } static inline void firmware_upload_unregister(struct fw_upload *fw_upload) diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c index 23f6cdaf29c5..e0cf4c55b520 100644 --- a/drivers/base/firmware_loader/sysfs_upload.c +++ b/drivers/base/firmware_loader/sysfs_upload.c @@ -274,7 +274,6 @@ void fw_upload_free(struct fw_sysfs *fw_sysfs) /** * firmware_upload_register() - register for the firmware upload sysfs API - * @module: kernel module of this device * @parent: parent device instantiating firmware upload * @name: firmware name to be associated with this device * @ops: pointer to structure of firmware upload ops @@ -286,10 +285,10 @@ void fw_upload_free(struct fw_sysfs *fw_sysfs) * Return: struct fw_upload pointer or ERR_PTR() * **/ -struct fw_upload * -firmware_upload_register(struct module *module, struct device *parent, - const char *name, const struct fw_upload_ops *ops, - void *dd_handle) +struct fw_upload *firmware_upload_register(struct device *parent, + const char *name, + const struct fw_upload_ops *ops, + void *dd_handle) { u32 opt_flags = FW_OPT_NOCACHE; struct fw_upload *fw_upload; diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c index 273c22118d3d..79c2ca393cbc 100644 --- a/drivers/cxl/core/memdev.c +++ b/drivers/cxl/core/memdev.c @@ -1045,8 +1045,8 @@ int devm_cxl_setup_fw_upload(struct device *host, struct cxl_memdev_state *mds) if (!test_bit(CXL_MEM_COMMAND_ID_GET_FW_INFO, cxl_mbox->enabled_cmds)) return 0; - fwl = firmware_upload_register(THIS_MODULE, dev, dev_name(dev), - &cxl_memdev_fw_ops, mds); + fwl = firmware_upload_register(dev, dev_name(dev), &cxl_memdev_fw_ops, + mds); if (IS_ERR(fwl)) return PTR_ERR(fwl); return devm_add_action_or_reset(host, cxl_remove_fw_upload, fwl); diff --git a/drivers/firmware/microchip/mpfs-auto-update.c b/drivers/firmware/microchip/mpfs-auto-update.c index 46b19d803446..9b25787467f0 100644 --- a/drivers/firmware/microchip/mpfs-auto-update.c +++ b/drivers/firmware/microchip/mpfs-auto-update.c @@ -435,7 +435,7 @@ static int mpfs_auto_update_probe(struct platform_device *pdev) return dev_err_probe(dev, ret, "The current bitstream does not support auto-update\n"); - fw_uploader = firmware_upload_register(THIS_MODULE, dev, "mpfs-auto-update", + fw_uploader = firmware_upload_register(dev, "mpfs-auto-update", &mpfs_auto_update_ops, priv); if (IS_ERR(fw_uploader)) return dev_err_probe(dev, PTR_ERR(fw_uploader), diff --git a/drivers/fpga/intel-m10-bmc-sec-update.c b/drivers/fpga/intel-m10-bmc-sec-update.c index 10f678b9ed36..1dfa0b7019b0 100644 --- a/drivers/fpga/intel-m10-bmc-sec-update.c +++ b/drivers/fpga/intel-m10-bmc-sec-update.c @@ -713,8 +713,8 @@ static int m10bmc_sec_probe(struct platform_device *pdev) goto fw_name_fail; } - fwl = firmware_upload_register(THIS_MODULE, sec->dev, sec->fw_name, - &m10bmc_ops, sec); + fwl = firmware_upload_register(sec->dev, sec->fw_name, &m10bmc_ops, + sec); if (IS_ERR(fwl)) { dev_err(sec->dev, "Firmware Upload driver failed to start\n"); ret = PTR_ERR(fwl); diff --git a/drivers/greybus/gb-beagleplay.c b/drivers/greybus/gb-beagleplay.c index 87186f891a6a..eceffffea829 100644 --- a/drivers/greybus/gb-beagleplay.c +++ b/drivers/greybus/gb-beagleplay.c @@ -1067,7 +1067,7 @@ static int gb_fw_init(struct gb_beagleplay *bg) return PTR_ERR(desc); bg->rst_gpio = desc; - fwl = firmware_upload_register(THIS_MODULE, &bg->sd->dev, "cc1352p7", + fwl = firmware_upload_register(&bg->sd->dev, "cc1352p7", &cc1352_bootloader_ops, bg); if (IS_ERR(fwl)) return PTR_ERR(fwl); diff --git a/drivers/media/i2c/thp7312.c b/drivers/media/i2c/thp7312.c index 775cfba188d8..ebbe14393c9f 100644 --- a/drivers/media/i2c/thp7312.c +++ b/drivers/media/i2c/thp7312.c @@ -1909,7 +1909,7 @@ static int thp7312_register_flash_mode(struct thp7312_device *thp7312) goto error; } - fwl = firmware_upload_register(THIS_MODULE, dev, "thp7312-firmware", + fwl = firmware_upload_register(dev, "thp7312-firmware", &thp7312_fw_upload_ops, thp7312); if (IS_ERR(fwl)) { ret = PTR_ERR(fwl); diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c index 4a3c852780f5..7463f7d81201 100644 --- a/drivers/net/pse-pd/pd692x0.c +++ b/drivers/net/pse-pd/pd692x0.c @@ -1834,8 +1834,8 @@ static int pd692x0_i2c_probe(struct i2c_client *client) return dev_err_probe(dev, ret, "failed to register PSE controller\n"); - fwl = firmware_upload_register(THIS_MODULE, dev, dev_name(dev), - &pd692x0_fw_ops, priv); + fwl = firmware_upload_register(dev, dev_name(dev), &pd692x0_fw_ops, + priv); if (IS_ERR(fwl)) return dev_err_probe(dev, PTR_ERR(fwl), "failed to register to the Firmware Upload API\n"); diff --git a/lib/test_firmware.c b/lib/test_firmware.c index b471d720879a..cfe5475d9d18 100644 --- a/lib/test_firmware.c +++ b/lib/test_firmware.c @@ -1322,8 +1322,7 @@ static ssize_t upload_register_store(struct device *dev, goto free_tst; } - fwl = firmware_upload_register(THIS_MODULE, dev, tst->name, - &upload_test_ops, tst); + fwl = firmware_upload_register(dev, tst->name, &upload_test_ops, tst); if (IS_ERR(fwl)) { ret = PTR_ERR(fwl); goto free_buf; -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting 2026-03-31 21:47 [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Dan Williams ` (2 preceding siblings ...) 2026-03-31 21:47 ` [PATCH 3/3] treewide: firmware_loader: Drop the unused @module argument Dan Williams @ 2026-04-07 19:24 ` Russ Weight 2026-04-08 2:26 ` Chao Gao 3 siblings, 1 reply; 7+ messages in thread From: Russ Weight @ 2026-04-07 19:24 UTC (permalink / raw) To: Dan Williams Cc: mcgrof, dakr, linux-kernel, Chao Gao, Greg Kroah-Hartman, Rafael J. Wysocki 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: drivers/base/firmware_loader/sysfs_upload.c:229:24: warning: unused variable ‘fw_dev’ [-Wunused-variable] 229 | struct device *fw_dev = &fw_sysfs->dev; | ^~~~~~ 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; | ^~~~~~ Thanks, - Russ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting 2026-04-07 19:24 ` [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Russ Weight @ 2026-04-08 2:26 ` Chao Gao 2026-04-08 2:34 ` Dan Williams 0 siblings, 1 reply; 7+ messages in thread From: Chao Gao @ 2026-04-08 2:26 UTC (permalink / raw) To: Russ Weight, djbw Cc: Dan Williams, mcgrof, dakr, linux-kernel, Greg Kroah-Hartman, Rafael J. Wysocki 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 */ ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting 2026-04-08 2:26 ` Chao Gao @ 2026-04-08 2:34 ` Dan Williams 0 siblings, 0 replies; 7+ messages in thread From: Dan Williams @ 2026-04-08 2:34 UTC (permalink / raw) To: Chao Gao Cc: Dan Williams, mcgrof, dakr, linux-kernel, Greg Kroah-Hartman, Rafael J. Wysocki Chao Gao wrote: > 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) Thanks! I will note that thanks to the korgalore tool I am subscribed to my old email, at least if someone also copies a public-inbox list. > Yes, I noticed that Sashiko reported them. Let's just say last week was "hectic". Sorry the noise. > >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: [..] > the @module field should be removed in patch 3: Fixups look good. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-08 2:34 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-31 21:47 [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Dan Williams 2026-03-31 21:47 ` [PATCH 1/3] firmware_loader: Stop pinning modules on registration Dan Williams 2026-03-31 21:47 ` [PATCH 2/3] firmware_loader: Stop pinning parent device per workqueue invocation Dan Williams 2026-03-31 21:47 ` [PATCH 3/3] treewide: firmware_loader: Drop the unused @module argument Dan Williams 2026-04-07 19:24 ` [PATCH 0/3] firmware_loader: Fix shutdown ordering and reference counting Russ Weight 2026-04-08 2:26 ` Chao Gao 2026-04-08 2:34 ` Dan Williams
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox