diff mbox series

wifi: brcmfmac: Fix SDIO suspend/resume regression

Message ID 20230320101345.105714-1-hdegoede@redhat.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series wifi: brcmfmac: Fix SDIO suspend/resume regression | expand

Commit Message

Hans de Goede March 20, 2023, 10:13 a.m. UTC 
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 <ulf.hansson@linaro.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 .../broadcom/brcm80211/brcmfmac/bcmsdh.c      | 33 +++++++++++++++----
 .../broadcom/brcm80211/brcmfmac/sdio.h        |  2 ++
 2 files changed, 28 insertions(+), 7 deletions(-)

Comments

kernel test robot March 20, 2023, 11:51 a.m. UTC | #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/202303201958.knUoo50R-lkp@intel.com/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 <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303201958.knUoo50R-lkp@intel.com/

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
diff mbox series

Patch

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;