After commit 92cadedd9d5f ("brcmfmac: Avoid keeping power to SDIO card
unless WOWL is used"), the wifi adapter by default is turned off on suspend
and then re-probed on resume.
In at least 2 model x86/acpi tablets with brcmfmac43430a1 wifi adapters,
the newly added re-probe on resume fails like this:
brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout
ieee80211 phy1: brcmf_bus_started: failed: -110
ieee80211 phy1: brcmf_attach: dongle is not responding: err=-110
brcmfmac: brcmf_sdio_firmware_callback: brcmf_attach failed
It seems this specific brcmfmac model does not like being reprobed without
it actually being turned off first.
And the adapter is not being turned off during suspend because of
commit f0992ace680c ("brcmfmac: prohibit ACPI power management for brcmfmac
driver").
Now that the driver is being reprobed on resume, the disabling of ACPI
pm is no longer necessary, except when WOWL is used (in which case there
is no-reprobe).
Move the dis-/en-abling of ACPI pm to brcmf_sdio_wowl_config(), this fixes
the brcmfmac43430a1 suspend/resume regression and should help save some
power when suspended.
This change means that the code now also may re-enable ACPI pm when WOWL
gets disabled. ACPI pm should only be re-enabled if it was enabled by
the ACPI core originally. Add a brcmf_sdiod_acpi_save_power_manageable()
to save the original state for this.
This has been tested on the following devices:
Asus T100TA brcmfmac43241b4-sdio
Acer Iconia One 7 B1-750 brcmfmac43340-sdio
Chuwi Hi8 brcmfmac43430a0-sdio
Chuwi Hi8 brcmfmac43430a1-sdio
(the Asus T100TA is the device for which the prohibiting of ACPI pm
was originally added)
Fixes: 92cadedd9d5f ("brcmfmac: Avoid keeping power to SDIO card unless WOWL is used")
Cc: Ulf Hansson <[email protected]>
Signed-off-by: Hans de Goede <[email protected]>
---
.../broadcom/brcm80211/brcmfmac/bcmsdh.c | 33 +++++++++++++++----
.../broadcom/brcm80211/brcmfmac/sdio.h | 2 ++
2 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index b7c918f241c9..3d8e5d8a52a0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -994,15 +994,34 @@ static const struct sdio_device_id brcmf_sdmmc_ids[] = {
MODULE_DEVICE_TABLE(sdio, brcmf_sdmmc_ids);
-static void brcmf_sdiod_acpi_set_power_manageable(struct device *dev,
- int val)
+static void brcmf_sdiod_acpi_save_power_manageable(struct brcmf_sdio_dev *sdiodev)
{
#if IS_ENABLED(CONFIG_ACPI)
struct acpi_device *adev;
- adev = ACPI_COMPANION(dev);
+ adev = ACPI_COMPANION(&sdiodev->func1->dev);
if (adev)
- adev->flags.power_manageable = 0;
+ sdiodev->func1_power_manageable = adev->flags.power_manageable;
+
+ adev = ACPI_COMPANION(&sdiodev->func2->dev);
+ if (adev)
+ sdiodev->func2_power_manageable = adev->flags.power_manageable;
+#endif
+}
+
+static void brcmf_sdiod_acpi_set_power_manageable(struct brcmf_sdio_dev *sdiodev,
+ int enable)
+{
+#if IS_ENABLED(CONFIG_ACPI)
+ struct acpi_device *adev;
+
+ adev = ACPI_COMPANION(&sdiodev->func1->dev);
+ if (adev)
+ adev->flags.power_manageable = enable ? sdiodev->func1_power_manageable : 0;
+
+ adev = ACPI_COMPANION(&sdiodev->func2->dev);
+ if (adev)
+ adev->flags.power_manageable = enable ? sdiodev->func2_power_manageable : 0;
#endif
}
@@ -1025,9 +1044,6 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
/* Set MMC_QUIRK_LENIENT_FN0 for this card */
func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
- /* prohibit ACPI power management for this device */
- brcmf_sdiod_acpi_set_power_manageable(dev, 0);
-
/* Consume func num 1 but dont do anything with it. */
if (func->num == 1)
return 0;
@@ -1059,6 +1075,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
dev_set_drvdata(&sdiodev->func1->dev, bus_if);
sdiodev->dev = &sdiodev->func1->dev;
+ brcmf_sdiod_acpi_save_power_manageable(sdiodev);
brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_DOWN);
brcmf_dbg(SDIO, "F2 found, calling brcmf_sdiod_probe...\n");
@@ -1124,6 +1141,8 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
if (sdiodev->settings->bus.sdio.oob_irq_supported ||
pm_caps & MMC_PM_WAKE_SDIO_IRQ) {
+ /* Stop ACPI from turning off the device when wowl is enabled */
+ brcmf_sdiod_acpi_set_power_manageable(sdiodev, !enabled);
sdiodev->wowl_enabled = enabled;
brcmf_dbg(SDIO, "Configuring WOWL, enabled=%d\n", enabled);
return;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index b76d34d36bde..0d18ed15b403 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -188,6 +188,8 @@ struct brcmf_sdio_dev {
char nvram_name[BRCMF_FW_NAME_LEN];
char clm_name[BRCMF_FW_NAME_LEN];
bool wowl_enabled;
+ bool func1_power_manageable;
+ bool func2_power_manageable;
enum brcmf_sdiod_state state;
struct brcmf_sdiod_freezer *freezer;
const struct firmware *clm_fw;
--
2.39.1
Hi Hans,
I love your patch! Perhaps something to improve:
[auto build test WARNING on wireless-next/main]
[also build test WARNING on wireless/main linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Hans-de-Goede/wifi-brcmfmac-Fix-SDIO-suspend-resume-regression/20230320-181456
base: https://git.kernel.org/pub/scm/linux/kernel/git/wireless/wireless-next.git main
patch link: https://lore.kernel.org/r/20230320101345.105714-1-hdegoede%40redhat.com
patch subject: [PATCH] wifi: brcmfmac: Fix SDIO suspend/resume regression
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230320/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/cc08330c25aed804a422164413a39e67bfc1c277
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Hans-de-Goede/wifi-brcmfmac-Fix-SDIO-suspend-resume-regression/20230320-181456
git checkout cc08330c25aed804a422164413a39e67bfc1c277
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/wireless/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c: In function 'brcmf_ops_sdio_probe':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c:1034:24: warning: variable 'dev' set but not used [-Wunused-but-set-variable]
1034 | struct device *dev;
| ^~~
vim +/dev +1034 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
00e27eeb75bb91 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2015-05-27 1027
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1028 static int brcmf_ops_sdio_probe(struct sdio_func *func,
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1029 const struct sdio_device_id *id)
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1030 {
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1031 int err;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1032 struct brcmf_sdio_dev *sdiodev;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1033 struct brcmf_bus *bus_if;
f0992ace680c7a drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Fu, Zhonghui 2015-05-11 @1034 struct device *dev;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1035
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1036 brcmf_dbg(SDIO, "Enter\n");
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1037 brcmf_dbg(SDIO, "Class=%x\n", func->class);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1038 brcmf_dbg(SDIO, "sdio vendor ID: 0x%04x\n", func->vendor);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1039 brcmf_dbg(SDIO, "sdio device ID: 0x%04x\n", func->device);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1040 brcmf_dbg(SDIO, "Function#: %d\n", func->num);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1041
f0992ace680c7a drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Fu, Zhonghui 2015-05-11 1042 dev = &func->dev;
508422f3695bf6 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Ian Molton 2017-12-19 1043
508422f3695bf6 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Ian Molton 2017-12-19 1044 /* Set MMC_QUIRK_LENIENT_FN0 for this card */
508422f3695bf6 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Ian Molton 2017-12-19 1045 func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
508422f3695bf6 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Ian Molton 2017-12-19 1046
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1047 /* Consume func num 1 but dont do anything with it. */
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1048 if (func->num == 1)
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1049 return 0;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1050
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1051 /* Ignore anything but func 2 */
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1052 if (func->num != 2)
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1053 return -ENODEV;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1054
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1055 bus_if = kzalloc(sizeof(struct brcmf_bus), GFP_KERNEL);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1056 if (!bus_if)
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1057 return -ENOMEM;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1058 sdiodev = kzalloc(sizeof(struct brcmf_sdio_dev), GFP_KERNEL);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1059 if (!sdiodev) {
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1060 kfree(bus_if);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1061 return -ENOMEM;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1062 }
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1063
36c4e7e4aa0605 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1064 /* store refs to functions used. mmc_card does
36c4e7e4aa0605 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1065 * not hold the F0 function pointer.
36c4e7e4aa0605 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1066 */
c9aa7a91de740c drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend Van Spriel 2018-01-09 1067 sdiodev->func1 = func->card->sdio_func[0];
c9aa7a91de740c drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend Van Spriel 2018-01-09 1068 sdiodev->func2 = func;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1069
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1070 sdiodev->bus_if = bus_if;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1071 bus_if->bus_priv.sdio = sdiodev;
943258b6a3b1fe drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Hante Meuleman 2013-12-12 1072 bus_if->proto_type = BRCMF_PROTO_BCDC;
da6d9c8ecd00e2 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2022-11-29 1073 bus_if->fwvid = id->driver_data;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1074 dev_set_drvdata(&func->dev, bus_if);
c9aa7a91de740c drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend Van Spriel 2018-01-09 1075 dev_set_drvdata(&sdiodev->func1->dev, bus_if);
c9aa7a91de740c drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend Van Spriel 2018-01-09 1076 sdiodev->dev = &sdiodev->func1->dev;
330b4e4be937bf drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Hante Meuleman 2014-10-28 1077
cc08330c25aed8 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Hans de Goede 2023-03-20 1078 brcmf_sdiod_acpi_save_power_manageable(sdiodev);
a1ce7a0d6a4f1e drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2015-02-06 1079 brcmf_sdiod_change_state(sdiodev, BRCMF_SDIOD_DOWN);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1080
a39be27b49e309 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1081 brcmf_dbg(SDIO, "F2 found, calling brcmf_sdiod_probe...\n");
a39be27b49e309 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1082 err = brcmf_sdiod_probe(sdiodev);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1083 if (err) {
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1084 brcmf_err("F2 error, probe failed %d...\n", err);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1085 goto fail;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1086 }
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1087
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1088 brcmf_dbg(SDIO, "F2 init completed...\n");
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1089 return 0;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1090
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1091 fail:
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1092 dev_set_drvdata(&func->dev, NULL);
c9aa7a91de740c drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c Arend Van Spriel 2018-01-09 1093 dev_set_drvdata(&sdiodev->func1->dev, NULL);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1094 kfree(sdiodev);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1095 kfree(bus_if);
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1096 return err;
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1097 }
e2dc9eea531ec9 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c Arend van Spriel 2013-12-12 1098
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests