Skip to content

Commit

Permalink
Revert "firmware: add sanity check on shutdown/suspend"
Browse files Browse the repository at this point in the history 
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]>
  • Loading branch information
@torvalds
torvalds committed Sep 11, 2017
1 parent 64414e5 commit f007cad
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 110 deletions.
11 changes: 0 additions & 11 deletions Documentation/driver-api/firmware/request_firmware.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,6 @@ request_firmware_nowait
.. kernel-doc:: drivers/base/firmware_class.c
:functions: request_firmware_nowait

Considerations for suspend and resume
=====================================

During suspend and resume only the built-in firmware and the firmware cache
elements of the firmware API can be used. This is managed by fw_pm_notify().

fw_pm_notify
------------
.. kernel-doc:: drivers/base/firmware_class.c
:functions: fw_pm_notify

request firmware API expected driver use
========================================

Expand Down
99 changes: 0 additions & 99 deletions drivers/base/firmware_class.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,38 +258,6 @@ static int fw_cache_piggyback_on_request(const char *name);
* guarding for corner cases a global lock should be OK */
static DEFINE_MUTEX(fw_lock);

static bool __enable_firmware = false;

static void enable_firmware(void)
{
mutex_lock(&fw_lock);
__enable_firmware = true;
mutex_unlock(&fw_lock);
}

static void disable_firmware(void)
{
mutex_lock(&fw_lock);
__enable_firmware = false;
mutex_unlock(&fw_lock);
}

/*
* When disabled only the built-in firmware and the firmware cache will be
* used to look for firmware.
*/
static bool firmware_enabled(void)
{
bool enabled = false;

mutex_lock(&fw_lock);
if (__enable_firmware)
enabled = true;
mutex_unlock(&fw_lock);

return enabled;
}

static struct firmware_cache fw_cache;

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

if (!firmware_enabled()) {
WARN(1, "firmware request while host is not available\n");
ret = -EHOSTDOWN;
goto out;
}

ret = fw_get_filesystem_firmware(device, fw->priv);
if (ret) {
if (!(opt_flags & FW_OPT_NO_WARN))
Expand Down Expand Up @@ -1762,62 +1724,6 @@ static void device_uncache_fw_images_delay(unsigned long delay)
msecs_to_jiffies(delay));
}

/**
* fw_pm_notify - notifier for suspend/resume
* @notify_block: unused
* @mode: mode we are switching to
* @unused: unused
*
* Used to modify the firmware_class state as we move in between states.
* The firmware_class implements a firmware cache to enable device driver
* to fetch firmware upon resume before the root filesystem is ready. We
* disable API calls which do not use the built-in firmware or the firmware
* cache when we know these calls will not work.
*
* The inner logic behind all this is a bit complex so it is worth summarizing
* the kernel's own suspend/resume process with context and focus on how this
* can impact the firmware API.
*
* First a review on how we go to suspend::
*
* pm_suspend() --> enter_state() -->
* sys_sync()
* suspend_prepare() -->
* __pm_notifier_call_chain(PM_SUSPEND_PREPARE, ...);
* suspend_freeze_processes() -->
* freeze_processes() -->
* __usermodehelper_set_disable_depth(UMH_DISABLED);
* freeze all tasks ...
* freeze_kernel_threads()
* suspend_devices_and_enter() -->
* dpm_suspend_start() -->
* dpm_prepare()
* dpm_suspend()
* suspend_enter() -->
* platform_suspend_prepare()
* dpm_suspend_late()
* freeze_enter()
* syscore_suspend()
*
* When we resume we bail out of a loop from suspend_devices_and_enter() and
* unwind back out to the caller enter_state() where we were before as follows::
*
* enter_state() -->
* suspend_devices_and_enter() --> (bail from loop)
* dpm_resume_end() -->
* dpm_resume()
* dpm_complete()
* suspend_finish() -->
* suspend_thaw_processes() -->
* thaw_processes() -->
* __usermodehelper_set_disable_depth(UMH_FREEZING);
* thaw_workqueues();
* thaw all processes ...
* usermodehelper_enable();
* pm_notifier_call_chain(PM_POST_SUSPEND);
*
* fw_pm_notify() works through pm_notifier_call_chain().
*/
static int fw_pm_notify(struct notifier_block *notify_block,
unsigned long mode, void *unused)
{
Expand All @@ -1831,7 +1737,6 @@ static int fw_pm_notify(struct notifier_block *notify_block,
*/
kill_pending_fw_fallback_reqs(true);
device_cache_fw_images();
disable_firmware();
break;

case PM_POST_SUSPEND:
Expand All @@ -1844,7 +1749,6 @@ static int fw_pm_notify(struct notifier_block *notify_block,
mutex_lock(&fw_lock);
fw_cache.state = FW_LOADER_NO_CACHE;
mutex_unlock(&fw_lock);
enable_firmware();

device_uncache_fw_images_delay(10 * MSEC_PER_SEC);
break;
Expand Down Expand Up @@ -1893,7 +1797,6 @@ static void __init fw_cache_init(void)
static int fw_shutdown_notify(struct notifier_block *unused1,
unsigned long unused2, void *unused3)
{
disable_firmware();
/*
* Kill all pending fallback requests to avoid both stalling shutdown,
* and avoid a deadlock with the usermode_lock.
Expand All @@ -1909,7 +1812,6 @@ static struct notifier_block fw_shutdown_nb = {

static int __init firmware_class_init(void)
{
enable_firmware();
fw_cache_init();
register_reboot_notifier(&fw_shutdown_nb);
#ifdef CONFIG_FW_LOADER_USER_HELPER
Expand All @@ -1921,7 +1823,6 @@ static int __init firmware_class_init(void)

static void __exit firmware_class_exit(void)
{
disable_firmware();
#ifdef CONFIG_PM_SLEEP
unregister_syscore_ops(&fw_syscore_ops);
unregister_pm_notifier(&fw_cache.pm_notify);
Expand Down

0 comments on commit f007cad

Please sign in to comment.