2014-12-31 08:20:15

by Fu, Zhonghui

[permalink] [raw]
Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

>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(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
index 3c06e93..eee7818 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
@@ -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



2014-12-31 09:56:12

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

On 12/31/14 09:20, Fu, Zhonghui wrote:
> 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]>
> Signed-off-by: Zhonghui Fu<[email protected]>
> ---
> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 +++++++++++++++++--
> 1 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 3c06e93..eee7818 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -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
>


2014-12-31 11:22:45

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

Hello.

On 12/31/2014 11:20 AM, Fu, Zhonghui wrote:

> 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(-)

> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 3c06e93..eee7818 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -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;
> + }

{} not needed.

[...]
> @@ -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


2015-01-12 16:24:44

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

On 01/12/15 15:06, Sergei Shtylyov wrote:
> Hello.
>
> On 1/12/2015 9:41 AM, Fu, Zhonghui wrote:
>
>> From 8685c3c2746b4275fc808d9db23c364b2f54b52a Mon Sep 17 00:00:00 2001
>> From: Zhonghui Fu <[email protected]>
>> Date: Mon, 12 Jan 2015 14:25:46 +0800
>> Subject: [PATCH] brcmfmac: avoid duplicated suspend/resume operation
>
> The lines above are not needed.
>
>> 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.
>
>> Acked-by: Arend van Spriel<[email protected]>
>> Acked-by: Sergei Shtylyov<[email protected]>
>> Signed-off-by: Zhonghui Fu <[email protected]>
>> ---
>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 21
>> +++++++++++++++++----
>> 1 files changed, 17 insertions(+), 4 deletions(-)
>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> index 9880dae..8f71485 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> @@ -1070,7 +1070,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func
>> *func,
>> */
>> if ((sdio_get_host_pm_caps(sdiodev->func[1]) & MMC_PM_KEEP_POWER) &&
>> ((sdio_get_host_pm_caps(sdiodev->func[1]) & MMC_PM_WAKE_SDIO_IRQ) ||
>> - (sdiodev->pdata && sdiodev->pdata->oob_irq_supported)))
>> + (sdiodev->pdata->oob_irq_supported)))
>
> Inner parens not needed on this line.

Well, actually this patch should not affect those line as it would
reintroduce a recently fixed issue.

Regards,
Arend

> [...]
>
> WBR, Sergei
>


2015-01-05 10:48:15

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

On 01/05/15 03:34, Fu, Zhonghui wrote:
> 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

>
> Thanks,
> Zhonghui
>
> On 2014/12/31 17:56, Arend van Spriel wrote:
>> On 12/31/14 09:20, Fu, Zhonghui wrote:
>>> 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]>
>>> Signed-off-by: Zhonghui Fu<[email protected]>
>>> ---
>>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 +++++++++++++++++--
>>> 1 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> index 3c06e93..eee7818 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>> @@ -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
>>>
>>
>> --
>> 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
>


2015-01-15 12:52:18

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

Arend van Spriel <[email protected]> writes:

> On 01/12/15 07:41, Fu, Zhonghui wrote:
>> From 8685c3c2746b4275fc808d9db23c364b2f54b52a Mon Sep 17 00:00:00 2001
>> From: Zhonghui Fu<[email protected]>
>> Date: Mon, 12 Jan 2015 14:25:46 +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.
>>
>> Acked-by: Arend van Spriel<[email protected]>
>> Acked-by: Sergei Shtylyov<[email protected]>
>> Signed-off-by: Zhonghui Fu<[email protected]>
>
> This patch needs to be rebased.
>
> Kalle,
>
> Please drop this one.

Ok, dropped. I'll wait for the rebase.

--
Kalle Valo

2015-01-12 10:01:09

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

On 01/12/15 07:41, Fu, Zhonghui wrote:
> From 8685c3c2746b4275fc808d9db23c364b2f54b52a Mon Sep 17 00:00:00 2001
> From: Zhonghui Fu<[email protected]>
> Date: Mon, 12 Jan 2015 14:25:46 +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.
>
> Acked-by: Arend van Spriel<[email protected]>
> Acked-by: Sergei Shtylyov<[email protected]>
> Signed-off-by: Zhonghui Fu<[email protected]>

This patch needs to be rebased.

Kalle,

Please drop this one.

Regards,
Arend

> ---
> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> index 9880dae..8f71485 100644
> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
> @@ -1070,7 +1070,7 @@ static int brcmf_ops_sdio_probe(struct sdio_func *func,
> */
> if ((sdio_get_host_pm_caps(sdiodev->func[1])& MMC_PM_KEEP_POWER)&&
> ((sdio_get_host_pm_caps(sdiodev->func[1])& MMC_PM_WAKE_SDIO_IRQ) ||
> - (sdiodev->pdata&& sdiodev->pdata->oob_irq_supported)))
> + (sdiodev->pdata->oob_irq_supported)))
> bus_if->wowl_supported = true;
> #endif
>
> @@ -1139,11 +1139,17 @@ 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,10 +1170,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 (sdiodev->pdata&& sdiodev->pdata->oob_irq_supported)
> +
> + 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);
> atomic_set(&sdiodev->suspend, false);
> -- 1.7.1
>


2015-01-22 13:54:58

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

Hello.

On 1/22/2015 4:49 PM, Kalle Valo wrote:

>> >From 04d3fa673897ca4ccbea6c76836d0092dba2484a Mon Sep 17 00:00:00 2001
>> From: Zhonghui Fu <[email protected]>
>> Date: Tue, 20 Jan 2015 11:14:13 +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.

>> Acked-by: Arend van Spriel <[email protected]>
>> Acked-by: Sergei Shtylyov <[email protected]>
>> Acked-by: Kalle Valo <[email protected]>
>> Signed-off-by: Zhonghui Fu <[email protected]>

> I don't remember giving Acked-by to this (or for matter to anything for
> a long time). What about Sergei or Arend?

I haven't ACK'ed this patch either.

WBR, Sergei


2015-01-20 03:34:28

by Fu, Zhonghui

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation


On 2015/1/15 20:52, Kalle Valo wrote:
> Arend van Spriel <[email protected]> writes:
>
>> On 01/12/15 07:41, Fu, Zhonghui wrote:
>>> From 8685c3c2746b4275fc808d9db23c364b2f54b52a Mon Sep 17 00:00:00 2001
>>> From: Zhonghui Fu<[email protected]>
>>> Date: Mon, 12 Jan 2015 14:25:46 +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.
>>>
>>> Acked-by: Arend van Spriel<[email protected]>
>>> Acked-by: Sergei Shtylyov<[email protected]>
>>> Signed-off-by: Zhonghui Fu<[email protected]>
>> This patch needs to be rebased.
>>
>> Kalle,
>>
>> Please drop this one.
> Ok, dropped. I'll wait for the rebase.
I have re-based this patch and sent it in another mail with the same subject.

Thanks,
Zhonghui
>


2015-01-12 06:43:43

by Fu, Zhonghui

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation


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


Thanks,
Zhonghui


On 2015/1/5 18:48, Arend van Spriel wrote:
> On 01/05/15 03:34, Fu, Zhonghui wrote:
>> 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
>
>>
>> Thanks,
>> Zhonghui
>>
>> On 2014/12/31 17:56, Arend van Spriel wrote:
>>> On 12/31/14 09:20, Fu, Zhonghui wrote:
>>>> 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]>
>>>> Signed-off-by: Zhonghui Fu<[email protected]>
>>>> ---
>>>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 +++++++++++++++++--
>>>> 1 files changed, 17 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>>> index 3c06e93..eee7818 100644
>>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>>>> @@ -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
>>>>
>>>
>>> --
>>> 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
>>
>
> --
> 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


2015-01-05 02:37:12

by Fu, Zhonghui

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation


Hi Sergei,

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


Thanks,
Zhonghui

On 2014/12/31 19:22, Sergei Shtylyov wrote:
> Hello.
>
> On 12/31/2014 11:20 AM, Fu, Zhonghui wrote:
>
>> 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(-)
>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> index 3c06e93..eee7818 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> @@ -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;
>> + }
>
> {} not needed.
>
> [...]
>> @@ -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
>
> --
> 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


2015-01-22 20:58:24

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

On 01/22/15 14:54, Sergei Shtylyov wrote:
> Hello.
>
> On 1/22/2015 4:49 PM, Kalle Valo wrote:
>
>>> >From 04d3fa673897ca4ccbea6c76836d0092dba2484a Mon Sep 17 00:00:00 2001
>>> From: Zhonghui Fu <[email protected]>
>>> Date: Tue, 20 Jan 2015 11:14:13 +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.
>
>>> Acked-by: Arend van Spriel <[email protected]>
>>> Acked-by: Sergei Shtylyov <[email protected]>
>>> Acked-by: Kalle Valo <[email protected]>
>>> Signed-off-by: Zhonghui Fu <[email protected]>
>
>> I don't remember giving Acked-by to this (or for matter to anything for
>> a long time). What about Sergei or Arend?
>
> I haven't ACK'ed this patch either.

I did ACK the initial patch and felt it still valid for this 'V2' patch.

Regards,
Arend

2015-01-05 02:34:26

by Fu, Zhonghui

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

Hi Arend,

Where to find your patch for this?


Thanks,
Zhonghui

On 2014/12/31 17:56, Arend van Spriel wrote:
> On 12/31/14 09:20, Fu, Zhonghui wrote:
>> 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]>
>> Signed-off-by: Zhonghui Fu<[email protected]>
>> ---
>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 19 +++++++++++++++++--
>> 1 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> index 3c06e93..eee7818 100644
>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c
>> @@ -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
>>
>
> --
> 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


2015-01-23 15:29:48

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] brcmfmac: avoid duplicated suspend/resume operation

Arend van Spriel <[email protected]> writes:

> On 01/22/15 14:54, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 1/22/2015 4:49 PM, Kalle Valo wrote:
>>
>>>> >From 04d3fa673897ca4ccbea6c76836d0092dba2484a Mon Sep 17 00:00:00 2001
>>>> From: Zhonghui Fu <[email protected]>
>>>> Date: Tue, 20 Jan 2015 11:14:13 +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.
>>
>>>> Acked-by: Arend van Spriel <[email protected]>
>>>> Acked-by: Sergei Shtylyov <[email protected]>
>>>> Acked-by: Kalle Valo <[email protected]>
>>>> Signed-off-by: Zhonghui Fu <[email protected]>
>>
>>> I don't remember giving Acked-by to this (or for matter to anything for
>>> a long time). What about Sergei or Arend?
>>
>> I haven't ACK'ed this patch either.
>
> I did ACK the initial patch and felt it still valid for this 'V2' patch.

Ok, thanks. So the patch is good, Zhonghui just needs to remove the two
acked-by lines.

--
Kalle Valo