[PATCH 0/4] mwifiex: several bugfixes

[PATCH 0/4] mwifiex: several bugfixes

[Brian Norris]

Hi,

I've been stressing a few aspects of this driver on 4.11-rc1, and I've noticed
several bugs. The first 3 are probably bugfix material for the current cycle
IMO, and the 4th is some more cleanup that matches the patterns introduced in
the 1st bugfix.

I have more patches coming sometime, but they're mostly just refactoring and
cleaning up things at the moment (no further bugfixes yet), so I thought I'd
send this stack out first.

Regards,
Brian

Brian Norris (4):
  mwifiex: pcie: don't leak DMA buffers when removing
  mwifiex: set adapter->dev before starting to use mwifiex_dbg()
  mwifiex: uninit wakeup info when removing device
  mwifiex: pcie: de-duplicate buffer allocation code

 drivers/net/wireless/marvell/mwifiex/main.c |  11 +-
 drivers/net/wireless/marvell/mwifiex/pcie.c | 195 ++++++++++++----------------
 2 files changed, 92 insertions(+), 114 deletions(-)

-- 
2.12.0.246.ga2ecc84866-goog

[PATCH 1/4] mwifiex: pcie: don't leak DMA buffers when removing

[Brian Norris]

When PCIe FLR support was added, much of the remove/release code for
PCIe was migrated to ->down_dev(), but ->down_dev() is never called for
device removal. Let's refactor the cleanup to be done in both cases.

Also, drop the comments above mwifiex_cleanup_pcie(), because they were
clearly wrong, and it's better to have clear and obvious code than to
detail the code steps in comments anyway.

Fixes: 4c5dae59d2e9 ("mwifiex: add PCIe function level reset support")
Cc: <stable@vger.kernel.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
I don't think there's any code path in which we see ->down_dev() followed by
->cleanup_if(), with no ->up_dev() in between? (Or, what happens if PCIe FLR
fails?) But if so, then this could cause a double free. It could use a close
review.

 drivers/net/wireless/marvell/mwifiex/pcie.c | 38 ++++++++++++++---------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index 5438483fcefe..eae1cc58a310 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2732,6 +2732,21 @@ static void mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter)
 	schedule_work(&card->work);
 }
 
+static void mwifiex_pcie_free_buffers(struct mwifiex_adapter *adapter)
+{
+	struct pcie_service_card *card = adapter->card;
+	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
+
+	if (reg->sleep_cookie)
+		mwifiex_pcie_delete_sleep_cookie_buf(adapter);
+
+	mwifiex_pcie_delete_cmdrsp_buf(adapter);
+	mwifiex_pcie_delete_evtbd_ring(adapter);
+	mwifiex_pcie_delete_rxbd_ring(adapter);
+	mwifiex_pcie_delete_txbd_ring(adapter);
+	card->cmdrsp_buf = NULL;
+}
+
 /*
  * This function initializes the PCI-E host memory space, WCB rings, etc.
  *
@@ -2843,13 +2858,6 @@ static int mwifiex_init_pcie(struct mwifiex_adapter *adapter)
 
 /*
  * This function cleans up the allocated card buffers.
- *
- * The following are freed by this function -
- *      - TXBD ring buffers
- *      - RXBD ring buffers
- *      - Event BD ring buffers
- *      - Command response ring buffer
- *      - Sleep cookie buffer
  */
 static void mwifiex_cleanup_pcie(struct mwifiex_adapter *adapter)
 {
@@ -2868,6 +2876,8 @@ static void mwifiex_cleanup_pcie(struct mwifiex_adapter *adapter)
 				    "Failed to write driver not-ready signature\n");
 	}
 
+	mwifiex_pcie_free_buffers(adapter);
+
 	if (pdev) {
 		pci_iounmap(pdev, card->pci_mmap);
 		pci_iounmap(pdev, card->pci_mmap1);
@@ -3119,10 +3129,7 @@ static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 	pci_iounmap(pdev, card->pci_mmap1);
 }
 
-/* This function cleans up the PCI-E host memory space.
- * Some code is extracted from mwifiex_unregister_dev()
- *
- */
+/* This function cleans up the PCI-E host memory space. */
 static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
@@ -3133,14 +3140,7 @@ static void mwifiex_pcie_down_dev(struct mwifiex_adapter *adapter)
 
 	adapter->seq_num = 0;
 
-	if (reg->sleep_cookie)
-		mwifiex_pcie_delete_sleep_cookie_buf(adapter);
-
-	mwifiex_pcie_delete_cmdrsp_buf(adapter);
-	mwifiex_pcie_delete_evtbd_ring(adapter);
-	mwifiex_pcie_delete_rxbd_ring(adapter);
-	mwifiex_pcie_delete_txbd_ring(adapter);
-	card->cmdrsp_buf = NULL;
+	mwifiex_pcie_free_buffers(adapter);
 }
 
 static struct mwifiex_if_ops pcie_ops = {
-- 
2.12.0.246.ga2ecc84866-goog

Re: [1/4] mwifiex: pcie: don't leak DMA buffers when removing

[Kalle Valo]

Brian Norris <briannorris@chromium.org> wrote:
> When PCIe FLR support was added, much of the remove/release code for
> PCIe was migrated to ->down_dev(), but ->down_dev() is never called for
> device removal. Let's refactor the cleanup to be done in both cases.
> 
> Also, drop the comments above mwifiex_cleanup_pcie(), because they were
> clearly wrong, and it's better to have clear and obvious code than to
> detail the code steps in comments anyway.
> 
> Fixes: 4c5dae59d2e9 ("mwifiex: add PCIe function level reset support")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>

3 patches applied to wireless-drivers.git, thanks.

4e841d3eb929 mwifiex: pcie: don't leak DMA buffers when removing
ba1c7e45ec22 mwifiex: set adapter->dev before starting to use mwifiex_dbg()
36908c4e5b10 mwifiex: uninit wakeup info when removing device

-- 
https://patchwork.kernel.org/patch/9618297/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/4] mwifiex: set adapter->dev before starting to use mwifiex_dbg()

[Brian Norris]

The mwifiex_dbg() log handler utilizes the struct device in
adapter->dev. Without it, it decides not to print anything.

As of commit 2e02b5814217 ("mwifiex: Allow mwifiex early access to device
structure"), we started assigning that pointer only after we finished
mwifiex_register() -- this effectively neuters any mwifiex_dbg() logging
done before this point.

Let's move the device assignment into mwifiex_register().

Fixes: 2e02b5814217 ("mwifiex: Allow mwifiex early access to device structure")
Cc: Rajat Jain <rajatja@google.com>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/net/wireless/marvell/mwifiex/main.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 5ebca1d0cfc7..43d040e02e4d 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -57,8 +57,8 @@ MODULE_PARM_DESC(mfg_mode, "manufacturing mode enable:1, disable:0");
  * In case of any errors during inittialization, this function also ensures
  * proper cleanup before exiting.
  */
-static int mwifiex_register(void *card, struct mwifiex_if_ops *if_ops,
-			    void **padapter)
+static int mwifiex_register(void *card, struct device *dev,
+			    struct mwifiex_if_ops *if_ops, void **padapter)
 {
 	struct mwifiex_adapter *adapter;
 	int i;
@@ -68,6 +68,7 @@ static int mwifiex_register(void *card, struct mwifiex_if_ops *if_ops,
 		return -ENOMEM;
 
 	*padapter = adapter;
+	adapter->dev = dev;
 	adapter->card = card;
 
 	/* Save interface specific operations in adapter */
@@ -1568,12 +1569,11 @@ mwifiex_add_card(void *card, struct completion *fw_done,
 {
 	struct mwifiex_adapter *adapter;
 
-	if (mwifiex_register(card, if_ops, (void **)&adapter)) {
+	if (mwifiex_register(card, dev, if_ops, (void **)&adapter)) {
 		pr_err("%s: software init failed\n", __func__);
 		goto err_init_sw;
 	}
 
-	adapter->dev = dev;
 	mwifiex_probe_of(adapter);
 
 	adapter->iface_type = iface_type;
-- 
2.12.0.246.ga2ecc84866-goog

[PATCH 3/4] mwifiex: uninit wakeup info when removing device

[Brian Norris]

We manually init wakeup info, but we don't detach it on device removal.
This means that if we (for example) rmmod + modprobe the driver, the
device framework might return -EEXIST the second time, and we'll
complain in the logs:

[  839.311881] mwifiex_pcie 0000:01:00.0: fail to init wakeup for mwifiex

AFAICT, there's no other negative effect.

But we can fix this by disabling wakeup on remove, similar to what a few
other drivers do (e.g., the power supply framework).

This code (and bug) has existed on SDIO for a while, but it got moved
around and enabled for PCIe with commit 853402a00823 ("mwifiex: Enable
WoWLAN for both sdio and pcie").

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/net/wireless/marvell/mwifiex/main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 43d040e02e4d..b62e03d11c2e 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -1718,6 +1718,9 @@ int mwifiex_remove_card(struct mwifiex_adapter *adapter)
 	wiphy_unregister(adapter->wiphy);
 	wiphy_free(adapter->wiphy);
 
+	if (adapter->irq_wakeup >= 0)
+		device_init_wakeup(adapter->dev, false);
+
 	/* Unregister device */
 	mwifiex_dbg(adapter, INFO,
 		    "info: unregister device\n");
-- 
2.12.0.246.ga2ecc84866-goog

[PATCH 4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Brian Norris]

This code was duplicated as part of the PCIe FLR code added to this
driver. Let's de-duplicate it to:

 * make things easier to read (mwifiex_pcie_free_buffers() now has a
   corresponding mwifiex_pcie_alloc_buffers())
 * reduce likelihood of bugs
 * make error logging equally verbose
 * save lines of code!

Also drop some of the commentary that isn't really needed.

Signed-off-by: Brian Norris <briannorris@chromium.org>
---
 drivers/net/wireless/marvell/mwifiex/pcie.c | 157 ++++++++++++----------------
 1 file changed, 66 insertions(+), 91 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c b/drivers/net/wireless/marvell/mwifiex/pcie.c
index eae1cc58a310..889d87086913 100644
--- a/drivers/net/wireless/marvell/mwifiex/pcie.c
+++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
@@ -2732,6 +2732,61 @@ static void mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter)
 	schedule_work(&card->work);
 }
 
+static int mwifiex_pcie_alloc_buffers(struct mwifiex_adapter *adapter)
+{
+	struct pcie_service_card *card = adapter->card;
+	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
+	int ret;
+
+	card->cmdrsp_buf = NULL;
+	ret = mwifiex_pcie_create_txbd_ring(adapter);
+	if (ret) {
+		mwifiex_dbg(adapter, ERROR, "Failed to create txbd ring\n");
+		goto err_cre_txbd;
+	}
+
+	ret = mwifiex_pcie_create_rxbd_ring(adapter);
+	if (ret) {
+		mwifiex_dbg(adapter, ERROR, "Failed to create rxbd ring\n");
+		goto err_cre_rxbd;
+	}
+
+	ret = mwifiex_pcie_create_evtbd_ring(adapter);
+	if (ret) {
+		mwifiex_dbg(adapter, ERROR, "Failed to create evtbd ring\n");
+		goto err_cre_evtbd;
+	}
+
+	ret = mwifiex_pcie_alloc_cmdrsp_buf(adapter);
+	if (ret) {
+		mwifiex_dbg(adapter, ERROR, "Failed to allocate cmdbuf buffer\n");
+		goto err_alloc_cmdbuf;
+	}
+
+	if (reg->sleep_cookie) {
+		ret = mwifiex_pcie_alloc_sleep_cookie_buf(adapter);
+		if (ret) {
+			mwifiex_dbg(adapter, ERROR, "Failed to allocate sleep_cookie buffer\n");
+			goto err_alloc_cookie;
+		}
+	} else {
+		card->sleep_cookie_vbase = NULL;
+	}
+
+	return 0;
+
+err_alloc_cookie:
+	mwifiex_pcie_delete_cmdrsp_buf(adapter);
+err_alloc_cmdbuf:
+	mwifiex_pcie_delete_evtbd_ring(adapter);
+err_cre_evtbd:
+	mwifiex_pcie_delete_rxbd_ring(adapter);
+err_cre_rxbd:
+	mwifiex_pcie_delete_txbd_ring(adapter);
+err_cre_txbd:
+	return ret;
+}
+
 static void mwifiex_pcie_free_buffers(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
@@ -2749,20 +2804,12 @@ static void mwifiex_pcie_free_buffers(struct mwifiex_adapter *adapter)
 
 /*
  * This function initializes the PCI-E host memory space, WCB rings, etc.
- *
- * The following initializations steps are followed -
- *      - Allocate TXBD ring buffers
- *      - Allocate RXBD ring buffers
- *      - Allocate event BD ring buffers
- *      - Allocate command response ring buffer
- *      - Allocate sleep cookie buffer
  */
 static int mwifiex_init_pcie(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
 	int ret;
 	struct pci_dev *pdev = card->dev;
-	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
 
 	pci_set_drvdata(pdev, card);
 
@@ -2811,37 +2858,13 @@ static int mwifiex_init_pcie(struct mwifiex_adapter *adapter)
 	pr_notice("PCI memory map Virt0: %p PCI memory map Virt2: %p\n",
 		  card->pci_mmap, card->pci_mmap1);
 
-	card->cmdrsp_buf = NULL;
-	ret = mwifiex_pcie_create_txbd_ring(adapter);
+	ret = mwifiex_pcie_alloc_buffers(adapter);
 	if (ret)
-		goto err_cre_txbd;
-	ret = mwifiex_pcie_create_rxbd_ring(adapter);
-	if (ret)
-		goto err_cre_rxbd;
-	ret = mwifiex_pcie_create_evtbd_ring(adapter);
-	if (ret)
-		goto err_cre_evtbd;
-	ret = mwifiex_pcie_alloc_cmdrsp_buf(adapter);
-	if (ret)
-		goto err_alloc_cmdbuf;
-	if (reg->sleep_cookie) {
-		ret = mwifiex_pcie_alloc_sleep_cookie_buf(adapter);
-		if (ret)
-			goto err_alloc_cookie;
-	} else {
-		card->sleep_cookie_vbase = NULL;
-	}
-	return ret;
+		goto err_alloc_buffers;
 
-err_alloc_cookie:
-	mwifiex_pcie_delete_cmdrsp_buf(adapter);
-err_alloc_cmdbuf:
-	mwifiex_pcie_delete_evtbd_ring(adapter);
-err_cre_evtbd:
-	mwifiex_pcie_delete_rxbd_ring(adapter);
-err_cre_rxbd:
-	mwifiex_pcie_delete_txbd_ring(adapter);
-err_cre_txbd:
+	return 0;
+
+err_alloc_buffers:
 	pci_iounmap(pdev, card->pci_mmap1);
 err_iomap2:
 	pci_release_region(pdev, 2);
@@ -3053,22 +3076,15 @@ static void mwifiex_unregister_dev(struct mwifiex_adapter *adapter)
 	card->adapter = NULL;
 }
 
-/* This function initializes the PCI-E host memory space, WCB rings, etc.
- *
- * The following initializations steps are followed -
- *      - Allocate TXBD ring buffers
- *      - Allocate RXBD ring buffers
- *      - Allocate event BD ring buffers
- *      - Allocate command response ring buffer
- *      - Allocate sleep cookie buffer
- * Part of mwifiex_init_pcie(), not reset the PCIE registers
+/*
+ * This function initializes the PCI-E host memory space, WCB rings, etc.,
+ * similar to mwifiex_init_pcie(), but without resetting PCI-E state.
  */
 static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 {
 	struct pcie_service_card *card = adapter->card;
 	int ret;
 	struct pci_dev *pdev = card->dev;
-	const struct mwifiex_pcie_card_reg *reg = card->pcie.reg;
 
 	/* Bluetooth is not on pcie interface. Download Wifi only firmware
 	 * during pcie FLR, so that bluetooth part of firmware which is
@@ -3081,51 +3097,10 @@ static void mwifiex_pcie_up_dev(struct mwifiex_adapter *adapter)
 	 */
 	adapter->tx_buf_size = card->pcie.tx_buf_size;
 
-	card->cmdrsp_buf = NULL;
-	ret = mwifiex_pcie_create_txbd_ring(adapter);
-	if (ret) {
-		mwifiex_dbg(adapter, ERROR, "Failed to create txbd ring\n");
-		goto err_cre_txbd;
-	}
-
-	ret = mwifiex_pcie_create_rxbd_ring(adapter);
-	if (ret) {
-		mwifiex_dbg(adapter, ERROR, "Failed to create rxbd ring\n");
-		goto err_cre_rxbd;
-	}
-
-	ret = mwifiex_pcie_create_evtbd_ring(adapter);
-	if (ret) {
-		mwifiex_dbg(adapter, ERROR, "Failed to create evtbd ring\n");
-		goto err_cre_evtbd;
-	}
-
-	ret = mwifiex_pcie_alloc_cmdrsp_buf(adapter);
-	if (ret) {
-		mwifiex_dbg(adapter, ERROR, "Failed to allocate cmdbuf buffer\n");
-		goto err_alloc_cmdbuf;
-	}
-
-	if (reg->sleep_cookie) {
-		ret = mwifiex_pcie_alloc_sleep_cookie_buf(adapter);
-		if (ret) {
-			mwifiex_dbg(adapter, ERROR, "Failed to allocate sleep_cookie buffer\n");
-			goto err_alloc_cookie;
-		}
-	} else {
-		card->sleep_cookie_vbase = NULL;
-	}
-	return;
+	ret = mwifiex_pcie_alloc_buffers(adapter);
+	if (!ret)
+		return;
 
-err_alloc_cookie:
-	mwifiex_pcie_delete_cmdrsp_buf(adapter);
-err_alloc_cmdbuf:
-	mwifiex_pcie_delete_evtbd_ring(adapter);
-err_cre_evtbd:
-	mwifiex_pcie_delete_rxbd_ring(adapter);
-err_cre_rxbd:
-	mwifiex_pcie_delete_txbd_ring(adapter);
-err_cre_txbd:
 	pci_iounmap(pdev, card->pci_mmap1);
 }
 
-- 
2.12.0.246.ga2ecc84866-goog

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Kalle Valo]

Brian Norris <briannorris@chromium.org> wrote:
> This code was duplicated as part of the PCIe FLR code added to this
> driver. Let's de-duplicate it to:
> 
>  * make things easier to read (mwifiex_pcie_free_buffers() now has a
>    corresponding mwifiex_pcie_alloc_buffers())
>  * reduce likelihood of bugs
>  * make error logging equally verbose
>  * save lines of code!
> 
> Also drop some of the commentary that isn't really needed.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Failed to apply:

fatal: sha1 information is lacking or useless (drivers/net/wireless/marvell/mwifiex/pcie.c).
error: could not build fake ancestor
Applying: mwifiex: pcie: de-duplicate buffer allocation code
Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/9618309/

Documentation about submitting wireless patches and checking status
from patchwork:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Brian Norris]

Hi Kalle,

On Mon, Mar 20, 2017 at 05:08:35PM +0000, Kalle Valo wrote:
> Brian Norris <briannorris@chromium.org> wrote:
> > This code was duplicated as part of the PCIe FLR code added to this
> > driver. Let's de-duplicate it to:
> > 
> >  * make things easier to read (mwifiex_pcie_free_buffers() now has a
> >    corresponding mwifiex_pcie_alloc_buffers())
> >  * reduce likelihood of bugs
> >  * make error logging equally verbose
> >  * save lines of code!
> > 
> > Also drop some of the commentary that isn't really needed.
> > 
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> 
> Failed to apply:
> 
> fatal: sha1 information is lacking or useless (drivers/net/wireless/marvell/mwifiex/pcie.c).
> error: could not build fake ancestor
> Applying: mwifiex: pcie: de-duplicate buffer allocation code
> Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
> The copy of the patch that failed is found in: .git/rebase-apply/patch
> 
> Patch set to Changes Requested.

This applies fine to your wireless-drivers/master branch for me, where
patches 1-3 were applied. Are you applying this to
wireless-drivers-next? It's quite understandable that patch 4 wouldn't
apply there, as you've stripped out the previous patches...

Brian

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Kalle Valo]

Brian Norris <briannorris@chromium.org> writes:

> On Mon, Mar 20, 2017 at 05:08:35PM +0000, Kalle Valo wrote:
>> Brian Norris <briannorris@chromium.org> wrote:
>> > This code was duplicated as part of the PCIe FLR code added to this
>> > driver. Let's de-duplicate it to:
>> > 
>> >  * make things easier to read (mwifiex_pcie_free_buffers() now has a
>> >    corresponding mwifiex_pcie_alloc_buffers())
>> >  * reduce likelihood of bugs
>> >  * make error logging equally verbose
>> >  * save lines of code!
>> > 
>> > Also drop some of the commentary that isn't really needed.
>> > 
>> > Signed-off-by: Brian Norris <briannorris@chromium.org>
>> 
>> Failed to apply:
>> 
>> fatal: sha1 information is lacking or useless
>> (drivers/net/wireless/marvell/mwifiex/pcie.c).
>> error: could not build fake ancestor
>> Applying: mwifiex: pcie: de-duplicate buffer allocation code
>> Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
>> The copy of the patch that failed is found in: .git/rebase-apply/patch
>> 
>> Patch set to Changes Requested.
>
> This applies fine to your wireless-drivers/master branch for me, where
> patches 1-3 were applied. Are you applying this to
> wireless-drivers-next? It's quite understandable that patch 4 wouldn't
> apply there, as you've stripped out the previous patches...

I (wrongly) understood that patches 1-3 are for 4.11 and patch 4 is for
4.12, don't remember anymore how I got that impression. But I don't
think a cleanup patch like this is justified for 4.11 so I'm not
comfortable applying this to wireless-drivers (which should only contain
fixes to important bugs or regressions).

What I could do is to wait for the patches 1-3 trickle down to w-d-next
and then apply this patch. It usually takes few weeks, but with bad luck
it might happen only after the merge window. Would that work?

-- 
Kalle Valo

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Brian Norris]

On Tue, Mar 21, 2017 at 02:14:05PM +0200, Kalle Valo wrote:
> Brian Norris <briannorris@chromium.org> writes:
> > On Mon, Mar 20, 2017 at 05:08:35PM +0000, Kalle Valo wrote:
> >> Failed to apply:
> >> 
> >> fatal: sha1 information is lacking or useless
> >> (drivers/net/wireless/marvell/mwifiex/pcie.c).
> >> error: could not build fake ancestor
> >> Applying: mwifiex: pcie: de-duplicate buffer allocation code
> >> Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
> >> The copy of the patch that failed is found in: .git/rebase-apply/patch
> >> 
> >> Patch set to Changes Requested.
> >
> > This applies fine to your wireless-drivers/master branch for me, where
> > patches 1-3 were applied. Are you applying this to
> > wireless-drivers-next? It's quite understandable that patch 4 wouldn't
> > apply there, as you've stripped out the previous patches...
> 
> I (wrongly) understood that patches 1-3 are for 4.11 and patch 4 is for
> 4.12, don't remember anymore how I got that impression. But I don't

Well, you're not exactly wrong. I mentioned in the cover letter that the
first 3 are bugfixes (probably for 4.11) and the 4th is not.

> think a cleanup patch like this is justified for 4.11 so I'm not
> comfortable applying this to wireless-drivers (which should only contain
> fixes to important bugs or regressions).

Right.

> What I could do is to wait for the patches 1-3 trickle down to w-d-next
> and then apply this patch. It usually takes few weeks, but with bad luck
> it might happen only after the merge window. Would that work?

Yeah, I figured something like that would happen. Seems fine to me.

Brian

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Brian Norris]

On Tue, Mar 21, 2017 at 02:14:05PM +0200, Kalle Valo wrote:
> What I could do is to wait for the patches 1-3 trickle down to w-d-next
> and then apply this patch. It usually takes few weeks, but with bad luck
> it might happen only after the merge window. Would that work?

Is this going to get picked up?

Brian

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Kalle Valo]

Brian Norris <briannorris@chromium.org> writes:

> On Tue, Mar 21, 2017 at 02:14:05PM +0200, Kalle Valo wrote:
>> What I could do is to wait for the patches 1-3 trickle down to w-d-next
>> and then apply this patch. It usually takes few weeks, but with bad luck
>> it might happen only after the merge window. Would that work?
>
> Is this going to get picked up?

Oh man, I thought I had changed the state of this patch from Changes
Requested to Awaiting Upstream but apparently I didn't do that and hence
missed the patch. It's now back in active my queue in Under Review
state:

https://patchwork.kernel.org/patch/9618309/

Thanks for reminding me and sorry for the screwup.

-- 
Kalle Valo

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Kalle Valo]

Brian Norris <briannorris@chromium.org> wrote:
> This code was duplicated as part of the PCIe FLR code added to this
> driver. Let's de-duplicate it to:
> 
>  * make things easier to read (mwifiex_pcie_free_buffers() now has a
>    corresponding mwifiex_pcie_alloc_buffers())
>  * reduce likelihood of bugs
>  * make error logging equally verbose
>  * save lines of code!
> 
> Also drop some of the commentary that isn't really needed.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Failed to apply:

fatal: sha1 information is lacking or useless (drivers/net/wireless/marvell/mwifiex/pcie.c).
error: could not build fake ancestor
Applying: mwifiex: pcie: de-duplicate buffer allocation code
Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

-- 
https://patchwork.kernel.org/patch/9618309/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [4/4] mwifiex: pcie: de-duplicate buffer allocation code

[Brian Norris]

On Thu, May 18, 2017 at 01:33:48PM +0000, Kalle Valo wrote:
> Brian Norris <briannorris@chromium.org> wrote:
> > This code was duplicated as part of the PCIe FLR code added to this
> > driver. Let's de-duplicate it to:
> > 
> >  * make things easier to read (mwifiex_pcie_free_buffers() now has a
> >    corresponding mwifiex_pcie_alloc_buffers())
> >  * reduce likelihood of bugs
> >  * make error logging equally verbose
> >  * save lines of code!
> > 
> > Also drop some of the commentary that isn't really needed.
> > 
> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> 
> Failed to apply:
> 
> fatal: sha1 information is lacking or useless (drivers/net/wireless/marvell/mwifiex/pcie.c).
> error: could not build fake ancestor
> Applying: mwifiex: pcie: de-duplicate buffer allocation code
> Patch failed at 0001 mwifiex: pcie: de-duplicate buffer allocation code
> The copy of the patch that failed is found in: .git/rebase-apply/patch

Hmm, well it still applies for me, but it does require a 3-way merge...
and I guess I have local history (because the 3 previous patches *used*
to be together...):

$ pwclient git-am -p linux-wireless 9618309
Applying patch #9618309 using u'git am'
Description: [4/4] mwifiex: pcie: de-duplicate buffer allocation code
Applying: mwifiex: pcie: de-duplicate buffer allocation code
Using index info to reconstruct a base tree...
M	drivers/net/wireless/marvell/mwifiex/pcie.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/net/wireless/marvell/mwifiex/pcie.c

Anyway, I'll resend.

Brian

> Patch set to Changes Requested.


> https://patchwork.kernel.org/patch/9618309/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>