Skip to content

Commit f007cad

Browse files
committed
Revert "firmware: add sanity check on shutdown/suspend"
This reverts commit 81f9507. It causes random failures of firmware loading at resume time (well, random for me, it seems to be more reliable for others) because the firmware disabling is not actually synchronous with any particular resume event, and at least the btusb driver that uses a workqueue to load the firmware at resume seems to occasionally hit the "firmware loading is disabled" logic because the firmware loader hasn't gotten the resume event yet. Some kind of sanity check for not trying to load firmware when it's not possible might be a good thing, but this commit was not it. Greg seems to have silently suffered the same issue, and pointed to the likely culprit, and Gabriel C verified the revert fixed it for him too. Reported-by: Linus Torvalds <[email protected]> Pointed-at-by: Greg Kroah-Hartman <[email protected]> Tested-by: Gabriel C <[email protected]> Cc: Luis R. Rodriguez <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 64414e5 commit f007cad

File tree

2 files changed

+0
-110
lines changed

2 files changed

+0
-110
lines changed

Documentation/driver-api/firmware/request_firmware.rst

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,17 +44,6 @@ request_firmware_nowait
4444
.. kernel-doc:: drivers/base/firmware_class.c
4545
:functions: request_firmware_nowait
4646

47-
Considerations for suspend and resume
48-
=====================================
49-
50-
During suspend and resume only the built-in firmware and the firmware cache
51-
elements of the firmware API can be used. This is managed by fw_pm_notify().
52-
53-
fw_pm_notify
54-
------------
55-
.. kernel-doc:: drivers/base/firmware_class.c
56-
:functions: fw_pm_notify
57-
5847
request firmware API expected driver use
5948
========================================
6049

drivers/base/firmware_class.c

Lines changed: 0 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -258,38 +258,6 @@ static int fw_cache_piggyback_on_request(const char *name);
258258
* guarding for corner cases a global lock should be OK */
259259
static DEFINE_MUTEX(fw_lock);
260260

261-
static bool __enable_firmware = false;
262-
263-
static void enable_firmware(void)
264-
{
265-
mutex_lock(&fw_lock);
266-
__enable_firmware = true;
267-
mutex_unlock(&fw_lock);
268-
}
269-
270-
static void disable_firmware(void)
271-
{
272-
mutex_lock(&fw_lock);
273-
__enable_firmware = false;
274-
mutex_unlock(&fw_lock);
275-
}
276-
277-
/*
278-
* When disabled only the built-in firmware and the firmware cache will be
279-
* used to look for firmware.
280-
*/
281-
static bool firmware_enabled(void)
282-
{
283-
bool enabled = false;
284-
285-
mutex_lock(&fw_lock);
286-
if (__enable_firmware)
287-
enabled = true;
288-
mutex_unlock(&fw_lock);
289-
290-
return enabled;
291-
}
292-
293261
static struct firmware_cache fw_cache;
294262

295263
static struct firmware_buf *__allocate_fw_buf(const char *fw_name,
@@ -1246,12 +1214,6 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
12461214
if (ret <= 0) /* error or already assigned */
12471215
goto out;
12481216

1249-
if (!firmware_enabled()) {
1250-
WARN(1, "firmware request while host is not available\n");
1251-
ret = -EHOSTDOWN;
1252-
goto out;
1253-
}
1254-
12551217
ret = fw_get_filesystem_firmware(device, fw->priv);
12561218
if (ret) {
12571219
if (!(opt_flags & FW_OPT_NO_WARN))
@@ -1762,62 +1724,6 @@ static void device_uncache_fw_images_delay(unsigned long delay)
17621724
msecs_to_jiffies(delay));
17631725
}
17641726

1765-
/**
1766-
* fw_pm_notify - notifier for suspend/resume
1767-
* @notify_block: unused
1768-
* @mode: mode we are switching to
1769-
* @unused: unused
1770-
*
1771-
* Used to modify the firmware_class state as we move in between states.
1772-
* The firmware_class implements a firmware cache to enable device driver
1773-
* to fetch firmware upon resume before the root filesystem is ready. We
1774-
* disable API calls which do not use the built-in firmware or the firmware
1775-
* cache when we know these calls will not work.
1776-
*
1777-
* The inner logic behind all this is a bit complex so it is worth summarizing
1778-
* the kernel's own suspend/resume process with context and focus on how this
1779-
* can impact the firmware API.
1780-
*
1781-
* First a review on how we go to suspend::
1782-
*
1783-
* pm_suspend() --> enter_state() -->
1784-
* sys_sync()
1785-
* suspend_prepare() -->
1786-
* __pm_notifier_call_chain(PM_SUSPEND_PREPARE, ...);
1787-
* suspend_freeze_processes() -->
1788-
* freeze_processes() -->
1789-
* __usermodehelper_set_disable_depth(UMH_DISABLED);
1790-
* freeze all tasks ...
1791-
* freeze_kernel_threads()
1792-
* suspend_devices_and_enter() -->
1793-
* dpm_suspend_start() -->
1794-
* dpm_prepare()
1795-
* dpm_suspend()
1796-
* suspend_enter() -->
1797-
* platform_suspend_prepare()
1798-
* dpm_suspend_late()
1799-
* freeze_enter()
1800-
* syscore_suspend()
1801-
*
1802-
* When we resume we bail out of a loop from suspend_devices_and_enter() and
1803-
* unwind back out to the caller enter_state() where we were before as follows::
1804-
*
1805-
* enter_state() -->
1806-
* suspend_devices_and_enter() --> (bail from loop)
1807-
* dpm_resume_end() -->
1808-
* dpm_resume()
1809-
* dpm_complete()
1810-
* suspend_finish() -->
1811-
* suspend_thaw_processes() -->
1812-
* thaw_processes() -->
1813-
* __usermodehelper_set_disable_depth(UMH_FREEZING);
1814-
* thaw_workqueues();
1815-
* thaw all processes ...
1816-
* usermodehelper_enable();
1817-
* pm_notifier_call_chain(PM_POST_SUSPEND);
1818-
*
1819-
* fw_pm_notify() works through pm_notifier_call_chain().
1820-
*/
18211727
static int fw_pm_notify(struct notifier_block *notify_block,
18221728
unsigned long mode, void *unused)
18231729
{
@@ -1831,7 +1737,6 @@ static int fw_pm_notify(struct notifier_block *notify_block,
18311737
*/
18321738
kill_pending_fw_fallback_reqs(true);
18331739
device_cache_fw_images();
1834-
disable_firmware();
18351740
break;
18361741

18371742
case PM_POST_SUSPEND:
@@ -1844,7 +1749,6 @@ static int fw_pm_notify(struct notifier_block *notify_block,
18441749
mutex_lock(&fw_lock);
18451750
fw_cache.state = FW_LOADER_NO_CACHE;
18461751
mutex_unlock(&fw_lock);
1847-
enable_firmware();
18481752

18491753
device_uncache_fw_images_delay(10 * MSEC_PER_SEC);
18501754
break;
@@ -1893,7 +1797,6 @@ static void __init fw_cache_init(void)
18931797
static int fw_shutdown_notify(struct notifier_block *unused1,
18941798
unsigned long unused2, void *unused3)
18951799
{
1896-
disable_firmware();
18971800
/*
18981801
* Kill all pending fallback requests to avoid both stalling shutdown,
18991802
* and avoid a deadlock with the usermode_lock.
@@ -1909,7 +1812,6 @@ static struct notifier_block fw_shutdown_nb = {
19091812

19101813
static int __init firmware_class_init(void)
19111814
{
1912-
enable_firmware();
19131815
fw_cache_init();
19141816
register_reboot_notifier(&fw_shutdown_nb);
19151817
#ifdef CONFIG_FW_LOADER_USER_HELPER
@@ -1921,7 +1823,6 @@ static int __init firmware_class_init(void)
19211823

19221824
static void __exit firmware_class_exit(void)
19231825
{
1924-
disable_firmware();
19251826
#ifdef CONFIG_PM_SLEEP
19261827
unregister_syscore_ops(&fw_syscore_ops);
19271828
unregister_pm_notifier(&fw_cache.pm_notify);

0 commit comments

Comments
 (0)