| Message ID | 54DC1D7D.5050407@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
Any comments to this patch? Can it be accepted? Thanks, Zhonghui On 2015/2/12 11:26, Fu, Zhonghui wrote: > From a05d35ab334c20970c236fb971dae88810078c88 Mon Sep 17 00:00:00 2001 > From: Fu Zhonghui <zhonghui.fu@linux.intel.com> > Date: Thu, 12 Feb 2015 10:49:35 +0800 > Subject: [PATCH v3] 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 <arend@broadcom.com> > Signed-off-by: Fu Zhonghui <zhonghui.fu@linux.intel.com> > --- > Changes in v3: > - Rebase to wireless-drivers-next/master branch > > drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 13 ++++++++++++- > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > index 7944224..b8832a7 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c > @@ -1117,9 +1117,13 @@ static int brcmf_ops_sdio_suspend(struct device *dev) > struct brcmf_bus *bus_if; > 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; > + > bus_if = dev_get_drvdata(dev); > sdiodev = bus_if->bus_priv.sdio; > > @@ -1148,9 +1152,16 @@ 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 && sdiodev->pdata->oob_irq_supported) > disable_irq_wake(sdiodev->pdata->oob_irq_nr); > brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS); > -- 1.9.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
On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: > > Any comments to this patch? Can it be accepted? > > Thanks, > Zhonghui > > On 2015/2/12 11:26, Fu, Zhonghui wrote: >> From a05d35ab334c20970c236fb971dae88810078c88 Mon Sep 17 00:00:00 2001 >> From: Fu Zhonghui <zhonghui.fu@linux.intel.com> >> Date: Thu, 12 Feb 2015 10:49:35 +0800 >> Subject: [PATCH v3] 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 <arend@broadcom.com> >> Signed-off-by: Fu Zhonghui <zhonghui.fu@linux.intel.com> >> --- >> Changes in v3: >> - Rebase to wireless-drivers-next/master branch >> >> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 13 ++++++++++++- >> 1 file changed, 12 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >> index 7944224..b8832a7 100644 >> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >> @@ -1117,9 +1117,13 @@ static int brcmf_ops_sdio_suspend(struct device *dev) >> struct brcmf_bus *bus_if; >> 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; >> + Should it be >= 2 instead of == 2 so that if, in the future, a 3+ SDIO function chip comes out, it's already handled? Not that that should hold up the patch or anything, just a curiosity. >> bus_if = dev_get_drvdata(dev); >> sdiodev = bus_if->bus_priv.sdio; >> >> @@ -1148,9 +1152,16 @@ 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 && sdiodev->pdata->oob_irq_supported) >> disable_irq_wake(sdiodev->pdata->oob_irq_nr); >> brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS); >> -- 1.9.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
On 02/15/15 04:27, Pat Erley wrote: > On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >> >> Any comments to this patch? Can it be accepted? I assume that patches are queued up until after the merge window that we are currently in. >> Thanks, >> Zhonghui >> >> On 2015/2/12 11:26, Fu, Zhonghui wrote: >>> From a05d35ab334c20970c236fb971dae88810078c88 Mon Sep 17 00:00:00 2001 >>> From: Fu Zhonghui <zhonghui.fu@linux.intel.com> >>> Date: Thu, 12 Feb 2015 10:49:35 +0800 >>> Subject: [PATCH v3] 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 <arend@broadcom.com> >>> Signed-off-by: Fu Zhonghui <zhonghui.fu@linux.intel.com> >>> --- >>> Changes in v3: >>> - Rebase to wireless-drivers-next/master branch >>> >>> drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c | 13 ++++++++++++- >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >>> b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >>> index 7944224..b8832a7 100644 >>> --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c >>> @@ -1117,9 +1117,13 @@ static int brcmf_ops_sdio_suspend(struct >>> device *dev) >>> struct brcmf_bus *bus_if; >>> 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; >>> + > > Should it be >= 2 instead of == 2 so that if, in the future, a 3+ > SDIO function chip comes out, it's already handled? Not that that > should hold up the patch or anything, just a curiosity. The driver only claims functions 1 and 2 during the probe so that assure it works for SDIO devices that have more than two functions. Regards, Arend >>> bus_if = dev_get_drvdata(dev); >>> sdiodev = bus_if->bus_priv.sdio; >>> >>> @@ -1148,9 +1152,16 @@ 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 && sdiodev->pdata->oob_irq_supported) >>> disable_irq_wake(sdiodev->pdata->oob_irq_nr); >>> brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS); >>> -- 1.9.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
Arend van Spriel <arend@broadcom.com> writes: > On 02/15/15 04:27, Pat Erley wrote: >> On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >>> >>> Any comments to this patch? Can it be accepted? > > I assume that patches are queued up until after the merge window that > we are currently in. That's right. In the future I will most likely apply patches also during the merge window, but as I'm still a greenhorn I'll be on the safe and wait for the merge window to end.
On 2015/2/15 22:54, Kalle Valo wrote: > Arend van Spriel <arend@broadcom.com> writes: > >> On 02/15/15 04:27, Pat Erley wrote: >>> On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >>>> Any comments to this patch? Can it be accepted? >> I assume that patches are queued up until after the merge window that >> we are currently in. > That's right. In the future I will most likely apply patches also during > the merge window, but as I'm still a greenhorn I'll be on the safe and > wait for the merge window to end. I am very glad to see this. Could you please tell which release candidate this patch will be likely merged into now? Thanks, Zhonghui -- 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 02/16/15 08:34, Fu, Zhonghui wrote: > > On 2015/2/15 22:54, Kalle Valo wrote: >> Arend van Spriel<arend@broadcom.com> writes: >> >>> On 02/15/15 04:27, Pat Erley wrote: >>>> On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >>>>> Any comments to this patch? Can it be accepted? >>> I assume that patches are queued up until after the merge window that >>> we are currently in. >> That's right. In the future I will most likely apply patches also during >> the merge window, but as I'm still a greenhorn I'll be on the safe and >> wait for the merge window to end. > I am very glad to see this. > Could you please tell which release candidate this patch will be likely merged into now? For which tree are you asking this? When the merge window ends and linus' tree has moved to 3.20-rc1, the wireless-drivers-next will move to that -rc1 as well and pending/accepted patches will be applied for the next kernel release. If you are asking when they will be in linus' tree than the answer is 3.21-rc1. Now if you say this patch solves a real problem for you (providing usual proof like log with stack trace) you can request it to go on the wireless-drivers tree to be fixed for 3.20. Regards, Arend -- 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
> > 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. Do you want to suspend on the first or last request? In general it might be that one function is in use and something wants to suspend the other (as inactive). If they suspend together you might need to pretend the first function is suspended but only do the real power-saving device suspend when all the functions have been suspended. David
On 2015/2/16 17:50, David Laight wrote: >>> 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. > Do you want to suspend on the first or last request? > > In general it might be that one function is in use and > something wants to suspend the other (as inactive). > > If they suspend together you might need to pretend the > first function is suspended but only do the real power-saving > device suspend when all the functions have been suspended. I was in Chinese new-year vacation these few days. So sorry for late response. Suspend/Resume entry functions of brcmfmac driver does not differentiate between two functions, performs the same operations instead. Thanks, Zhonghui > > David > > N?????r??y???b?X???v?^?)?{.n?+????{??*??,?{ay? ???,j??f???h???z? ?w??? ???j:+v???w?j?m????????zZ+??????j"??!tml= -- 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 2015/2/16 17:35, Arend van Spriel wrote: > On 02/16/15 08:34, Fu, Zhonghui wrote: >> >> On 2015/2/15 22:54, Kalle Valo wrote: >>> Arend van Spriel<arend@broadcom.com> writes: >>> >>>> On 02/15/15 04:27, Pat Erley wrote: >>>>> On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >>>>>> Any comments to this patch? Can it be accepted? >>>> I assume that patches are queued up until after the merge window that >>>> we are currently in. >>> That's right. In the future I will most likely apply patches also during >>> the merge window, but as I'm still a greenhorn I'll be on the safe and >>> wait for the merge window to end. >> I am very glad to see this. >> Could you please tell which release candidate this patch will be likely merged into now? > > For which tree are you asking this? When the merge window ends and linus' tree has moved to 3.20-rc1, the wireless-drivers-next will move to that -rc1 as well and pending/accepted patches will be applied for the next kernel release. If you are asking when they will be in linus' tree than the answer is 3.21-rc1. Now if you say this patch solves a real problem for you (providing usual proof like log with stack trace) you can request it to go on the wireless-drivers tree to be fixed for 3.20. I was in Chinese new-year vacation these few days. So sorry for late response. Many thanks for your explanation first. Now that there is not 3.20 version. My understanding is that this patch will be in linus' tree 4.1-rc1, right? Thanks, Zhonghui > > Regards, > Arend > -- > To unsubscribe from this list: send the line "unsubscribe netdev" 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 02/27/15 08:53, Fu, Zhonghui wrote: > > On 2015/2/16 17:35, Arend van Spriel wrote: >> On 02/16/15 08:34, Fu, Zhonghui wrote: >>> >>> On 2015/2/15 22:54, Kalle Valo wrote: >>>> Arend van Spriel<arend@broadcom.com> writes: >>>> >>>>> On 02/15/15 04:27, Pat Erley wrote: >>>>>> On 02/14/2015 08:40 PM, Fu, Zhonghui wrote: >>>>>>> Any comments to this patch? Can it be accepted? >>>>> I assume that patches are queued up until after the merge window that >>>>> we are currently in. >>>> That's right. In the future I will most likely apply patches also during >>>> the merge window, but as I'm still a greenhorn I'll be on the safe and >>>> wait for the merge window to end. >>> I am very glad to see this. >>> Could you please tell which release candidate this patch will be likely merged into now? >> >> For which tree are you asking this? When the merge window ends and linus' tree has moved to 3.20-rc1, the wireless-drivers-next will move to that -rc1 as well and pending/accepted patches will be applied for the next kernel release. If you are asking when they will be in linus' tree than the answer is 3.21-rc1. Now if you say this patch solves a real problem for you (providing usual proof like log with stack trace) you can request it to go on the wireless-drivers tree to be fixed for 3.20. > > I was in Chinese new-year vacation these few days. So sorry for late response. > > Many thanks for your explanation first. > > Now that there is not 3.20 version. My understanding is that this patch will be in linus' tree 4.1-rc1, right? Yes. It will go into linux-next first, which you can consider to be an incubator where all stuff for the next release is integrated. Stuff will be added there until 4.0 is released. At that moment the merge window starts which moves all the stuff from linux-next into the mainline linux repo to prepare 4.1-rc1. Now regarding your patch I have to give a heads up. Our pending patches have been applied by Kalle and includes similar fix. Regards, Arend > > Thanks, > Zhonghui > >> >> Regards, >> Arend >> -- >> To unsubscribe from this list: send the line "unsubscribe netdev" 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
Arend van Spriel <arend@broadcom.com> writes: >> Now that there is not 3.20 version. My understanding is that this >> patch will be in linus' tree 4.1-rc1, right? > > Yes. It will go into linux-next first, which you can consider to be an > incubator where all stuff for the next release is integrated. Stuff > will be added there until 4.0 is released. At that moment the merge > window starts which moves all the stuff from linux-next into the > mainline linux repo to prepare 4.1-rc1. > > Now regarding your patch I have to give a heads up. Our pending > patches have been applied by Kalle and includes similar fix. Yeah, Zhonghui's patch doesn't apply anymore. There is similar code in wireless-drivers-next but still a bit different. So what should we do? Is the driver ok now?
On 03/02/15 16:08, Kalle Valo wrote: > Arend van Spriel<arend@broadcom.com> writes: > >>> Now that there is not 3.20 version. My understanding is that this >>> patch will be in linus' tree 4.1-rc1, right? >> >> Yes. It will go into linux-next first, which you can consider to be an >> incubator where all stuff for the next release is integrated. Stuff >> will be added there until 4.0 is released. At that moment the merge >> window starts which moves all the stuff from linux-next into the >> mainline linux repo to prepare 4.1-rc1. >> >> Now regarding your patch I have to give a heads up. Our pending >> patches have been applied by Kalle and includes similar fix. > > Yeah, Zhonghui's patch doesn't apply anymore. There is similar code in > wireless-drivers-next but still a bit different. So what should we do? > Is the driver ok now? The idea of Zhonghui's patch was to avoid entering suspend and resume callbacks twice. The current behavior is same as his patch intended so yes the driver is ok (pending undiscovered bugs :-p ). Regards, Arend -- 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 7944224..b8832a7 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh.c @@ -1117,9 +1117,13 @@ static int brcmf_ops_sdio_suspend(struct device *dev) struct brcmf_bus *bus_if; 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; + bus_if = dev_get_drvdata(dev); sdiodev = bus_if->bus_priv.sdio; @@ -1148,9 +1152,16 @@ 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 && sdiodev->pdata->oob_irq_supported) disable_irq_wake(sdiodev->pdata->oob_irq_nr); brcmf_sdio_wd_timer(sdiodev->bus, BRCMF_WD_POLL_MS);