Skip to contentSkip to conversation

brcmfmac: avoid duplicated suspend/resume operation

PATCH In review Fu, Zhonghui 3 participants 7 messages Dec 31, 2014 – Jan 12, 2015

Subsystems (3)
Networking Network drivers Networking drivers (wireless)
lore
Share & tools
lore

Fu, Zhonghui

author
#0 lore raw

PATCH 1/1 brcmfmac: avoid duplicated suspend/resume operation

Quoted 1 line

From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001

From: Zhonghui Fu <[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

Signed-off-by Zhonghui Fu <[email protected]>
---
 drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +17 -2
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
 static int brcmf_ops_sdio_suspend(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
 	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	atomic_set(&sdiodev->suspend, true);
 
 	if (sdiodev->wowl_enabled) {

@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
 static int brcmf_ops_sdio_resume(struct device *dev)
 {
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
 	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
 	if (sdiodev->pdata->oob_irq_supported)
 		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
 	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);

-- 1.7.1

Acked-by on v1 1/1
#1 lore raw
Quoted 8 lines · Zhonghui

From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu<[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

We have a patch queued up for this as well, but this one looks good enough although I personally prefer container_of() instead of dev_to_sdio_func().

Acked-by Arend van Spriel <[email protected]>
Quoted 50 lines
Signed-off-by Zhonghui Fu<[email protected]>
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
  1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +17 -3
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
  static int brcmf_ops_sdio_suspend(struct device *dev)
  {
  	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
  	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
  	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
  	atomic_set(&sdiodev->suspend, true);
 
  	if (sdiodev->wowl_enabled) {

@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
  static int brcmf_ops_sdio_resume(struct device *dev)
  {
  	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
  	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2) {
+		return 0;
+	}
+
+	sdiodev = bus_if->bus_priv.sdio;
+
  	if (sdiodev->pdata->oob_irq_supported)
  		disable_irq_wake(sdiodev->pdata->oob_irq_nr);
  	brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1
author
#2 lore raw

Hi Arend,

Where to find your patch for this?

Thanks,
Zhonghui

Quoted 68 lines · Arend van Spriel
Quoted 8 lines · Zhonghui

From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu<[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

We have a patch queued up for this as well, but this one looks good enough although I personally prefer container_of() instead of dev_to_sdio_func().

Acked-by Arend van Spriel <[email protected]>
Quoted 50 lines
Signed-off-by Zhonghui Fu<[email protected]>
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
  1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +17 -3
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
  static int brcmf_ops_sdio_suspend(struct device *dev)
  {
      struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
      mmc_pm_flag_t sdio_flags;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
      brcmf_dbg(SDIO, "Enter\n");
 
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
      atomic_set(&sdiodev->suspend, true);
 
      if (sdiodev->wowl_enabled) {

@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
  static int brcmf_ops_sdio_resume(struct device *dev)
  {
      struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
      brcmf_dbg(SDIO, "Enter\n");
+
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
      if (sdiodev->pdata->oob_irq_supported)
          disable_irq_wake(sdiodev->pdata->oob_irq_nr);
      brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#3 lore raw
Quoted 3 lines · Zhonghui

Hi Arend,

Where to find your patch for this?

Well, we did not submit it. Hence my "Acked-by:" to your patch below.

Regards,
Arend

Quoted 74 lines

Thanks,
Zhonghui

Quoted 68 lines · Arend van Spriel
Quoted 8 lines · Zhonghui
   From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001

From: Zhonghui Fu<[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

We have a patch queued up for this as well, but this one looks good enough although I personally prefer container_of() instead of dev_to_sdio_func().

Acked-by Arend van Spriel<[email protected]>
Quoted 40 lines
Signed-off-by Zhonghui Fu<[email protected]>
---
   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
   1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +11 -2
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
   static int brcmf_ops_sdio_suspend(struct device *dev)
   {
       struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
       mmc_pm_flag_t sdio_flags;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
       brcmf_dbg(SDIO, "Enter\n");
 
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
       atomic_set(&sdiodev->suspend, true);
 
       if (sdiodev->wowl_enabled) {

@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
   static int brcmf_ops_sdio_resume(struct device *dev)
   {
       struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
       brcmf_dbg(SDIO, "Enter\n");
+

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c static int brcmf_ops_sdio_suspend(struct device *dev)

Quoted 10 lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +6 -1
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
       if (sdiodev->pdata->oob_irq_supported)
           disable_irq_wake(sdiodev->pdata->oob_irq_nr);
       brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#4 lore raw

I sent a new version of this patch in another mail.

Thanks,
Zhonghui

Quoted 89 lines · Arend van Spriel
Quoted 3 lines · Zhonghui

Hi Arend,

Where to find your patch for this?

Well, we did not submit it. Hence my "Acked-by:" to your patch below.

Regards,
Arend

Quoted 74 lines

Thanks,
Zhonghui

Quoted 68 lines · Arend van Spriel
Quoted 8 lines · Zhonghui
   From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001

From: Zhonghui Fu<[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

We have a patch queued up for this as well, but this one looks good enough although I personally prefer container_of() instead of dev_to_sdio_func().

Acked-by Arend van Spriel<[email protected]>
Quoted 50 lines
Signed-off-by Zhonghui Fu<[email protected]>
---
   drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
   1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +17 -3
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
   static int brcmf_ops_sdio_suspend(struct device *dev)
   {
       struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
       mmc_pm_flag_t sdio_flags;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
       brcmf_dbg(SDIO, "Enter\n");
 
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
       atomic_set(&sdiodev->suspend, true);
 
       if (sdiodev->wowl_enabled) {

@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
   static int brcmf_ops_sdio_resume(struct device *dev)
   {
       struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
       brcmf_dbg(SDIO, "Enter\n");
+
+    if (func->num == 2) {
+        return 0;
+    }
+
+    sdiodev = bus_if->bus_priv.sdio;
+
       if (sdiodev->pdata->oob_irq_supported)
           disable_irq_wake(sdiodev->pdata->oob_irq_nr);
       brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);
-- 1.7.1
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
In reply to Fu, Zhonghui

Hello.

Quoted 22 lines · Zhonghui

From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

Signed-off-by Zhonghui Fu <[email protected]>
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
  1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +0 -0
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
  static int brcmf_ops_sdio_suspend(struct device *dev)
  {
  	struct brcmf_bus *bus_if = dev_get_drvdata(dev);

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c void brcmf_sdio_wowl_config(struct device *dev, bool enabled) · line 1150

Quoted 10 lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +5 -1
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
  	mmc_pm_flag_t sdio_flags;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
  	brcmf_dbg(SDIO, "Enter\n");
 
+	if (func->num == 2) {
+		return 0;
+	}

{} not needed.

[...]

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c static int brcmf_ops_sdio_suspend(struct device *dev) · line 1181

Quoted 13 lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +6 -1
@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
  static int brcmf_ops_sdio_resume(struct device *dev)
  {
  	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-	struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+	struct brcmf_sdio_dev *sdiodev;
+	struct sdio_func *func = dev_to_sdio_func(dev);
 
  	brcmf_dbg(SDIO, "Enter\n");
+
+	if (func->num == 2) {
+		return 0;
+	}

Same here.

[...]

WBR, Sergei

author
#6 lore raw

Hi Sergei,

You are right, I will apply your advice in next version of this patch.

Thanks,
Zhonghui

Quoted 64 lines · Sergei Shtylyov

Hello.

Quoted 22 lines · Zhonghui

From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001
From: Zhonghui Fu <[email protected]>
Date: Mon, 29 Dec 2014 21:25:31 +0800
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

WiFi chip has 2 SDIO functions, and PM core will trigger twice suspend/resume operations for one WiFi chip to do the same things. This patch avoid this case.

Signed-off-by Zhonghui Fu <[email protected]>
---
  drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c |   19 +++++++++++++++++--
  1 files changed, 17 insertions(+), 2 deletions(-)
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +0 -0
index 3c06e93..eee7818 100644
@@ -1139,11 +1139,18 @@ void brcmf_sdio_wowl_config(struct device *dev, bool enabled)
  static int brcmf_ops_sdio_suspend(struct device *dev)
  {
      struct brcmf_bus *bus_if = dev_get_drvdata(dev);

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c void brcmf_sdio_wowl_config(struct device *dev, bool enabled) · line 1150

Quoted 10 lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +5 -1
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
      mmc_pm_flag_t sdio_flags;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
      brcmf_dbg(SDIO, "Enter\n");
 
+    if (func->num == 2) {
+        return 0;
+    }

{} not needed.

[...]

drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c static int brcmf_ops_sdio_suspend(struct device *dev) · line 1181

Quoted 13 lines
drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +6 -1
@@ -1164,9 +1171,17 @@ static int brcmf_ops_sdio_suspend(struct device *dev)
  static int brcmf_ops_sdio_resume(struct device *dev)
  {
      struct brcmf_bus *bus_if = dev_get_drvdata(dev);
-    struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
+    struct brcmf_sdio_dev *sdiodev;
+    struct sdio_func *func = dev_to_sdio_func(dev);
 
      brcmf_dbg(SDIO, "Enter\n");
+
+    if (func->num == 2) {
+        return 0;
+    }

Same here.

[...]

WBR, Sergei

Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html