| Message ID | 54A3B1B4.7090705@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kalle Valo |
| Headers | show |
On 12/31/14 09:20, Fu, Zhonghui wrote: > From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001 > From: Zhonghui Fu<zhonghui.fu@linux.intel.com> > 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 <arend@broadcom.com> > Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com> > --- > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello. On 12/31/2014 11:20 AM, Fu, Zhonghui wrote: > From e34419970a07bfcd365f9c66bdfa552188a0cd26 Mon Sep 17 00:00:00 2001 > From: Zhonghui Fu <zhonghui.fu@linux.intel.com> > 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 <zhonghui.fu@linux.intel.com> > --- > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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<zhonghui.fu@linux.intel.com> >> 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 <arend@broadcom.com> >> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com> >> --- >> 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 majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <zhonghui.fu@linux.intel.com> >> 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 <zhonghui.fu@linux.intel.com> >> --- >> 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 majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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<zhonghui.fu@linux.intel.com> >>> 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<arend@broadcom.com> >>> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com> >>> --- >>> 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 majordomo@vger.kernel.org >> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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<zhonghui.fu@linux.intel.com> >>>> 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<arend@broadcom.com> >>>> Signed-off-by: Zhonghui Fu<zhonghui.fu@linux.intel.com> >>>> --- >>>> 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 majordomo@vger.kernel.org >>> 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 majordomo@vger.kernel.org > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);