Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Arend Van Spriel @ 2019-04-14 12:34 UTC (permalink / raw)
  To: Felix Fietkau, Toke Høiland-Jørgensen, linux-wireless
  Cc: johannes, Herbert Xu
In-Reply-To: <e912d736-a3ad-1e21-0cdd-6a065df96fbb@nbd.name>

On April 14, 2019 1:19:49 PM Felix Fietkau <nbd@nbd.name> wrote:

> On 2019-04-14 11:44, Arend Van Spriel wrote:
>> + Herbert
>> 
>> On 3/16/2019 7:14 PM, Toke Høiland-Jørgensen wrote:
>>> Felix Fietkau <nbd@nbd.name> writes:
>>> 
>>>> When using iTXQ, tx sequence number allocation and statistics are run at
>>>> dequeue time. Because of that, it is safe to enable NETIF_F_LLTX, which
>>>> allows tx handlers to run on multiple CPUs in parallel.
>>> 
>>> Cool, didn't know about that flag.
>> 
>> It is water under the bridge as this patch got applied already, but I
>> stumbled upon it just recently and didn't know about that flag either.
>> So I looked for more information about it and found the definition [1],
>> but the comment seemed important enough to send this reply.
>> 
>> 	NETIF_F_LLTX_BIT,	/* LockLess TX - deprecated. Please */
>> 				/* do not use LLTX in new drivers */
>> 
>> Here is the commit that marked it deprecated:
>> 
>> commit e24eb521fbf2a350ce879dfc1d8e56d4ffa2aa22
>> Author: Christian Borntraeger <borntraeger@de.ibm.com>
>> Date:   Tue Sep 25 19:42:02 2007 -0700
>> 
>>      [NET]: note that NETIF_F_LLTX is deprecated
>> 
>> So I am not sure we should really do this in mac80211. Maybe Herbert can
>> comment although it has been over a decade ago.
> There is a lot of comparable code that also uses this flag, e.g.
> batman-adv, bridge, vlan, various tunnel implementations. I think
> mac80211 fits well with those kinds of use cases.

Ok. As said I was not sure so I can/will not argue.

> If I remember correctly, the deprecation was added to avoid quirky
> custom locking schemes in ethernet drivers.

What do you mean by "quirky custom locking schemes"? You mean that TX path 
would use driver data that should actually be accessed under some lock?

When seeing the deprecated comment I wanted to know the why and was hoping 
the commit message would divulge. It just mentions it is not needed. So now 
I am curious as to why it wouldn't be needed especially as you say there 
are (valid) use-cases in the kernel today.

Regards,
Arend



^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Felix Fietkau @ 2019-04-14 11:19 UTC (permalink / raw)
  To: Arend Van Spriel, Toke Høiland-Jørgensen,
	linux-wireless
  Cc: johannes, Herbert Xu
In-Reply-To: <773e4dff-29fd-22b4-e4bc-cd5a94c66dc2@broadcom.com>

On 2019-04-14 11:44, Arend Van Spriel wrote:
> + Herbert
> 
> On 3/16/2019 7:14 PM, Toke Høiland-Jørgensen wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>> 
>>> When using iTXQ, tx sequence number allocation and statistics are run at
>>> dequeue time. Because of that, it is safe to enable NETIF_F_LLTX, which
>>> allows tx handlers to run on multiple CPUs in parallel.
>> 
>> Cool, didn't know about that flag.
> 
> It is water under the bridge as this patch got applied already, but I 
> stumbled upon it just recently and didn't know about that flag either. 
> So I looked for more information about it and found the definition [1], 
> but the comment seemed important enough to send this reply.
> 
> 	NETIF_F_LLTX_BIT,	/* LockLess TX - deprecated. Please */
> 				/* do not use LLTX in new drivers */
> 
> Here is the commit that marked it deprecated:
> 
> commit e24eb521fbf2a350ce879dfc1d8e56d4ffa2aa22
> Author: Christian Borntraeger <borntraeger@de.ibm.com>
> Date:   Tue Sep 25 19:42:02 2007 -0700
> 
>      [NET]: note that NETIF_F_LLTX is deprecated
> 
> So I am not sure we should really do this in mac80211. Maybe Herbert can 
> comment although it has been over a decade ago.
There is a lot of comparable code that also uses this flag, e.g.
batman-adv, bridge, vlan, various tunnel implementations. I think
mac80211 fits well with those kinds of use cases.
If I remember correctly, the deprecation was added to avoid quirky
custom locking schemes in ethernet drivers.

- Felix

^ permalink raw reply

* Re: [PATCH 5/5] mac80211: set NETIF_F_LLTX when using intermediate tx queues
From: Arend Van Spriel @ 2019-04-14  9:44 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Felix Fietkau, linux-wireless
  Cc: johannes, Herbert Xu
In-Reply-To: <87sgvmvg9g.fsf@toke.dk>

+ Herbert

On 3/16/2019 7:14 PM, Toke Høiland-Jørgensen wrote:
> Felix Fietkau <nbd@nbd.name> writes:
> 
>> When using iTXQ, tx sequence number allocation and statistics are run at
>> dequeue time. Because of that, it is safe to enable NETIF_F_LLTX, which
>> allows tx handlers to run on multiple CPUs in parallel.
> 
> Cool, didn't know about that flag.

It is water under the bridge as this patch got applied already, but I 
stumbled upon it just recently and didn't know about that flag either. 
So I looked for more information about it and found the definition [1], 
but the comment seemed important enough to send this reply.

	NETIF_F_LLTX_BIT,	/* LockLess TX - deprecated. Please */
				/* do not use LLTX in new drivers */

Here is the commit that marked it deprecated:

commit e24eb521fbf2a350ce879dfc1d8e56d4ffa2aa22
Author: Christian Borntraeger <borntraeger@de.ibm.com>
Date:   Tue Sep 25 19:42:02 2007 -0700

     [NET]: note that NETIF_F_LLTX is deprecated

So I am not sure we should really do this in mac80211. Maybe Herbert can 
comment although it has been over a decade ago.

Regards,
Arend

[1] 
https://elixir.bootlin.com/linux/latest/source/include/linux/netdev_features.h#L32

^ permalink raw reply

* Re: [PATCH] mwifiex: check for null return from skb_copy
From: Dan Carpenter @ 2019-04-13 19:27 UTC (permalink / raw)
  To: Colin King
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190413161438.6376-1-colin.king@canonical.com>

On Sat, Apr 13, 2019 at 05:14:38PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> It is possible for skb_copy to return a null pointer and currently
> this will cause a null pointer dereference when the function
> mwifiex_uap_queue_bridged_pkt is called.  Fix this by checking for
> a null return from skb_copy and return -ENOMEM.
> 
> Addresses-Coverity: ("Dereference null return")
> Fixes: 838e4f449297 ("mwifiex: improve uAP RX handling")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/net/wireless/marvell/mwifiex/uap_txrx.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> index 5ce85d5727e4..b262dc78d638 100644
> --- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> +++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
> @@ -256,6 +256,8 @@ int mwifiex_handle_uap_rx_forward(struct mwifiex_private *priv,
>  
>  	if (is_multicast_ether_addr(ra)) {
>  		skb_uap = skb_copy(skb, GFP_ATOMIC);
> +		if (!skb_uap)
> +			return -ENOMEM;

I think we would want to free dev_kfree_skb_any(skb) before returning.

>  		mwifiex_uap_queue_bridged_pkt(priv, skb_uap);
>  	} else {
>  		if (mwifiex_get_sta_entry(priv, ra)) {

regards,
dan carpenter

^ permalink raw reply

* [PATCH] mwifiex: check for null return from skb_copy
From: Colin King @ 2019-04-13 16:14 UTC (permalink / raw)
  To: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	Kalle Valo, David S . Miller, linux-wireless, netdev
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

It is possible for skb_copy to return a null pointer and currently
this will cause a null pointer dereference when the function
mwifiex_uap_queue_bridged_pkt is called.  Fix this by checking for
a null return from skb_copy and return -ENOMEM.

Addresses-Coverity: ("Dereference null return")
Fixes: 838e4f449297 ("mwifiex: improve uAP RX handling")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/net/wireless/marvell/mwifiex/uap_txrx.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
index 5ce85d5727e4..b262dc78d638 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_txrx.c
@@ -256,6 +256,8 @@ int mwifiex_handle_uap_rx_forward(struct mwifiex_private *priv,
 
 	if (is_multicast_ether_addr(ra)) {
 		skb_uap = skb_copy(skb, GFP_ATOMIC);
+		if (!skb_uap)
+			return -ENOMEM;
 		mwifiex_uap_queue_bridged_pkt(priv, skb_uap);
 	} else {
 		if (mwifiex_get_sta_entry(priv, ra)) {
-- 
2.20.1


^ permalink raw reply related

* [PATCH 5/5] mt7603: init mcu_restart function pointer
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1555163776.git.lorenzo@kernel.org>

Use common function wrapper in mt7603_mcu_exit since the code is shared
with mt7615 driver

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt7603/mcu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
index c52ae301062c..7ebfcb021d40 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
@@ -161,9 +161,9 @@ mt7603_mcu_start_firmware(struct mt7603_dev *dev, u32 addr)
 }
 
 static int
-mt7603_mcu_restart(struct mt7603_dev *dev)
+mt7603_mcu_restart(struct mt76_dev *dev)
 {
-	return __mt76_mcu_send_msg(&dev->mt76, -MCU_CMD_RESTART_DL_REQ,
+	return __mt76_mcu_send_msg(dev, -MCU_CMD_RESTART_DL_REQ,
 				   NULL, 0, true);
 }
 
@@ -269,6 +269,7 @@ int mt7603_mcu_init(struct mt7603_dev *dev)
 {
 	static const struct mt76_mcu_ops mt7603_mcu_ops = {
 		.mcu_send_msg = mt7603_mcu_msg_send,
+		.mcu_restart = mt7603_mcu_restart,
 	};
 
 	dev->mt76.mcu_ops = &mt7603_mcu_ops;
@@ -277,7 +278,7 @@ int mt7603_mcu_init(struct mt7603_dev *dev)
 
 void mt7603_mcu_exit(struct mt7603_dev *dev)
 {
-	mt7603_mcu_restart(dev);
+	__mt76_mcu_restart(&dev->mt76);
 	skb_queue_purge(&dev->mt76.mmio.mcu.res_q);
 }
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 4/5] mt76: introduce mt76_mcu_restart macro
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1555163776.git.lorenzo@kernel.org>

Use common function wrapper in mt76x02_watchdog_reset

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h         | 2 ++
 drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index cff9c57d2838..4eed7b489349 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -537,6 +537,8 @@ struct mt76_rx_status {
 
 #define mt76_mcu_send_msg(dev, ...)	(dev)->mt76.mcu_ops->mcu_send_msg(&((dev)->mt76), __VA_ARGS__)
 #define __mt76_mcu_send_msg(dev, ...)	(dev)->mcu_ops->mcu_send_msg((dev), __VA_ARGS__)
+#define mt76_mcu_restart(dev, ...)	(dev)->mt76.mcu_ops->mcu_restart(&((dev)->mt76))
+#define __mt76_mcu_restart(dev, ...)	(dev)->mcu_ops->mcu_restart((dev))
 
 #define mt76_set(dev, offset, val)	mt76_rmw(dev, offset, 0, val)
 #define mt76_clear(dev, offset, val)	mt76_rmw(dev, offset, val, 0)
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index 705c0939d10b..88aa992aa679 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -454,7 +454,7 @@ static void mt76x02_watchdog_reset(struct mt76x02_dev *dev)
 	mt76_set(dev, 0x734, 0x3);
 
 	if (restart)
-		dev->mt76.mcu_ops->mcu_restart(&dev->mt76);
+		mt76_mcu_restart(dev);
 
 	for (i = 0; i < ARRAY_SIZE(dev->mt76.q_tx); i++)
 		mt76_queue_tx_cleanup(dev, i, true);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 3/5] mt7603: initialize mt76_mcu_ops data structure
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1555163776.git.lorenzo@kernel.org>

Use __mt76_mcu_send_msg wrapper instead of mt7603_mcu_msg_send.
This is a preliminary patch for mt7615-mt7603 mcu code unification

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt76.h     |  1 +
 .../net/wireless/mediatek/mt76/mt7603/init.c  |  2 +-
 .../net/wireless/mediatek/mt76/mt7603/mcu.c   | 28 +++++++++++++------
 .../wireless/mediatek/mt76/mt7603/mt7603.h    |  2 +-
 4 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index f0d34901c825..cff9c57d2838 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -536,6 +536,7 @@ struct mt76_rx_status {
 #define mt76_rd_rp(dev, ...)	(dev)->mt76.bus->rd_rp(&((dev)->mt76), __VA_ARGS__)
 
 #define mt76_mcu_send_msg(dev, ...)	(dev)->mt76.mcu_ops->mcu_send_msg(&((dev)->mt76), __VA_ARGS__)
+#define __mt76_mcu_send_msg(dev, ...)	(dev)->mcu_ops->mcu_send_msg((dev), __VA_ARGS__)
 
 #define mt76_set(dev, offset, val)	mt76_rmw(dev, offset, 0, val)
 #define mt76_clear(dev, offset, val)	mt76_rmw(dev, offset, val, 0)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/init.c b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
index 569c61b9b5ae..849261dc0e0b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
@@ -282,7 +282,7 @@ mt7603_init_hardware(struct mt7603_dev *dev)
 		mt76_poll(dev, MT_PSE_RTA, MT_PSE_RTA_BUSY, 0, 5000);
 	}
 
-	ret = mt7603_load_firmware(dev);
+	ret = mt7603_mcu_init(dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
index a978305cc969..c52ae301062c 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
@@ -54,10 +54,10 @@ __mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb,
 }
 
 static int
-mt7603_mcu_msg_send(struct mt7603_dev *dev, int cmd, const void *data,
+mt7603_mcu_msg_send(struct mt76_dev *mdev, int cmd, const void *data,
 		    int len, bool wait_resp)
 {
-	struct mt76_dev *mdev = &dev->mt76;
+	struct mt7603_dev *dev = container_of(mdev, struct mt7603_dev, mt76);
 	unsigned long expires = jiffies + 3 * HZ;
 	struct mt7603_mcu_rxd *rxd;
 	struct sk_buff *skb;
@@ -115,7 +115,7 @@ mt7603_mcu_init_download(struct mt7603_dev *dev, u32 addr, u32 len)
 		.mode = cpu_to_le32(BIT(31)),
 	};
 
-	return mt7603_mcu_msg_send(dev, -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
+	return __mt76_mcu_send_msg(&dev->mt76, -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
 				   &req, sizeof(req), true);
 }
 
@@ -156,18 +156,18 @@ mt7603_mcu_start_firmware(struct mt7603_dev *dev, u32 addr)
 		.addr = cpu_to_le32(addr),
 	};
 
-	return mt7603_mcu_msg_send(dev, -MCU_CMD_FW_START_REQ,
+	return __mt76_mcu_send_msg(&dev->mt76, -MCU_CMD_FW_START_REQ,
 				   &req, sizeof(req), true);
 }
 
 static int
 mt7603_mcu_restart(struct mt7603_dev *dev)
 {
-	return mt7603_mcu_msg_send(dev, -MCU_CMD_RESTART_DL_REQ,
+	return __mt76_mcu_send_msg(&dev->mt76, -MCU_CMD_RESTART_DL_REQ,
 				   NULL, 0, true);
 }
 
-int mt7603_load_firmware(struct mt7603_dev *dev)
+static int mt7603_load_firmware(struct mt7603_dev *dev)
 {
 	const struct firmware *fw;
 	const struct mt7603_fw_trailer *hdr;
@@ -265,6 +265,16 @@ int mt7603_load_firmware(struct mt7603_dev *dev)
 	return ret;
 }
 
+int mt7603_mcu_init(struct mt7603_dev *dev)
+{
+	static const struct mt76_mcu_ops mt7603_mcu_ops = {
+		.mcu_send_msg = mt7603_mcu_msg_send,
+	};
+
+	dev->mt76.mcu_ops = &mt7603_mcu_ops;
+	return mt7603_load_firmware(dev);
+}
+
 void mt7603_mcu_exit(struct mt7603_dev *dev)
 {
 	mt7603_mcu_restart(dev);
@@ -361,7 +371,7 @@ int mt7603_mcu_set_eeprom(struct mt7603_dev *dev)
 		req.data[i].pad = 0;
 	}
 
-	return mt7603_mcu_msg_send(dev, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
+	return __mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
 				   &req, sizeof(req), true);
 }
 
@@ -405,7 +415,7 @@ static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
 	memcpy(req.temp_comp_power, eep + MT_EE_STEP_NUM_NEG_6_7,
 	       sizeof(req.temp_comp_power));
 
-	return mt7603_mcu_msg_send(dev, MCU_EXT_CMD_SET_TX_POWER_CTRL,
+	return __mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD_SET_TX_POWER_CTRL,
 				   &req, sizeof(req), true);
 }
 
@@ -451,7 +461,7 @@ int mt7603_mcu_set_channel(struct mt7603_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(req.txpower); i++)
 		req.txpower[i] = tx_power;
 
-	ret = mt7603_mcu_msg_send(dev, MCU_EXT_CMD_CHANNEL_SWITCH,
+	ret = __mt76_mcu_send_msg(&dev->mt76, MCU_EXT_CMD_CHANNEL_SWITCH,
 				  &req, sizeof(req), true);
 	if (ret)
 		return ret;
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mt7603.h b/drivers/net/wireless/mediatek/mt76/mt7603/mt7603.h
index 9cc8ca7a4d1c..f414ff2a5279 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mt7603.h
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mt7603.h
@@ -176,7 +176,7 @@ void mt7603_unregister_device(struct mt7603_dev *dev);
 int mt7603_eeprom_init(struct mt7603_dev *dev);
 int mt7603_dma_init(struct mt7603_dev *dev);
 void mt7603_dma_cleanup(struct mt7603_dev *dev);
-int mt7603_load_firmware(struct mt7603_dev *dev);
+int mt7603_mcu_init(struct mt7603_dev *dev);
 void mt7603_init_debugfs(struct mt7603_dev *dev);
 
 static inline void mt7603_irq_enable(struct mt7603_dev *dev, u32 mask)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/5] mt7603: use standard signature for mt7603_mcu_msg_send
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1555163776.git.lorenzo@kernel.org>

Use mt76 common signature for mt7603_mcu_msg_send. Move skb allocation
in mt7603_mcu_msg_send and remove duplicated code. This is a preliminary
patch for mt7615-mt7603 mcu code unification

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../net/wireless/mediatek/mt76/mt7603/mcu.c   | 61 ++++++++-----------
 1 file changed, 27 insertions(+), 34 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
index 868a4b601a6f..a978305cc969 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
@@ -22,9 +22,6 @@ __mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb,
 	struct mt7603_mcu_txd *txd;
 	u8 seq;
 
-	if (!skb)
-		return -EINVAL;
-
 	seq = ++mdev->mmio.mcu.msg_seq & 0xf;
 	if (!seq)
 		seq = ++mdev->mmio.mcu.msg_seq & 0xf;
@@ -57,20 +54,26 @@ __mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb,
 }
 
 static int
-mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd)
+mt7603_mcu_msg_send(struct mt7603_dev *dev, int cmd, const void *data,
+		    int len, bool wait_resp)
 {
 	struct mt76_dev *mdev = &dev->mt76;
 	unsigned long expires = jiffies + 3 * HZ;
 	struct mt7603_mcu_rxd *rxd;
+	struct sk_buff *skb;
 	int ret, seq;
 
+	skb = mt7603_mcu_msg_alloc(data, len);
+	if (!skb)
+		return -ENOMEM;
+
 	mutex_lock(&mdev->mmio.mcu.mutex);
 
 	ret = __mt7603_mcu_msg_send(dev, skb, cmd, &seq);
 	if (ret)
 		goto out;
 
-	while (1) {
+	while (wait_resp) {
 		bool check_seq = false;
 
 		skb = mt76_mcu_get_response(&dev->mt76, expires);
@@ -111,9 +114,9 @@ mt7603_mcu_init_download(struct mt7603_dev *dev, u32 addr, u32 len)
 		.len = cpu_to_le32(len),
 		.mode = cpu_to_le32(BIT(31)),
 	};
-	struct sk_buff *skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
 
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_TARGET_ADDRESS_LEN_REQ);
+	return mt7603_mcu_msg_send(dev, -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
+				   &req, sizeof(req), true);
 }
 
 static int
@@ -152,17 +155,16 @@ mt7603_mcu_start_firmware(struct mt7603_dev *dev, u32 addr)
 		.override = cpu_to_le32(addr ? 1 : 0),
 		.addr = cpu_to_le32(addr),
 	};
-	struct sk_buff *skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
 
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ);
+	return mt7603_mcu_msg_send(dev, -MCU_CMD_FW_START_REQ,
+				   &req, sizeof(req), true);
 }
 
 static int
 mt7603_mcu_restart(struct mt7603_dev *dev)
 {
-	struct sk_buff *skb = mt7603_mcu_msg_alloc(NULL, 0);
-
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ);
+	return mt7603_mcu_msg_send(dev, -MCU_CMD_RESTART_DL_REQ,
+				   NULL, 0, true);
 }
 
 int mt7603_load_firmware(struct mt7603_dev *dev)
@@ -343,30 +345,24 @@ int mt7603_mcu_set_eeprom(struct mt7603_dev *dev)
 		u8 buffer_mode;
 		u8 len;
 		u8 pad[2];
-	} req_hdr = {
+		struct req_data data[255];
+	} req = {
 		.buffer_mode = 1,
 		.len = ARRAY_SIZE(req_fields) - 1,
 	};
-	struct sk_buff *skb;
-	struct req_data *data;
-	const int size = 0xff * sizeof(struct req_data);
 	u8 *eep = (u8 *)dev->mt76.eeprom.data;
 	int i;
 
-	BUILD_BUG_ON(ARRAY_SIZE(req_fields) * sizeof(*data) > size);
-
-	skb = mt7603_mcu_msg_alloc(NULL, size + sizeof(req_hdr));
-	memcpy(skb_put(skb, sizeof(req_hdr)), &req_hdr, sizeof(req_hdr));
-	data = (struct req_data *)skb_put(skb, size);
-	memset(data, 0, size);
+	BUILD_BUG_ON(ARRAY_SIZE(req_fields) > ARRAY_SIZE(req.data));
 
 	for (i = 0; i < ARRAY_SIZE(req_fields); i++) {
-		data[i].addr = cpu_to_le16(req_fields[i]);
-		data[i].val = eep[req_fields[i]];
-		data[i].pad = 0;
+		req.data[i].addr = cpu_to_le16(req_fields[i]);
+		req.data[i].val = eep[req_fields[i]];
+		req.data[i].pad = 0;
 	}
 
-	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_EFUSE_BUFFER_MODE);
+	return mt7603_mcu_msg_send(dev, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
+				   &req, sizeof(req), true);
 }
 
 static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
@@ -401,7 +397,6 @@ static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
 		},
 #undef EEP_VAL
 	};
-	struct sk_buff *skb;
 	u8 *eep = (u8 *)dev->mt76.eeprom.data;
 
 	memcpy(req.rate_power_delta, eep + MT_EE_TX_POWER_CCK,
@@ -410,8 +405,8 @@ static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
 	memcpy(req.temp_comp_power, eep + MT_EE_STEP_NUM_NEG_6_7,
 	       sizeof(req.temp_comp_power));
 
-	skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
-	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_TX_POWER_CTRL);
+	return mt7603_mcu_msg_send(dev, MCU_EXT_CMD_SET_TX_POWER_CTRL,
+				   &req, sizeof(req), true);
 }
 
 int mt7603_mcu_set_channel(struct mt7603_dev *dev)
@@ -435,10 +430,8 @@ int mt7603_mcu_set_channel(struct mt7603_dev *dev)
 		.tx_streams = n_chains,
 		.rx_streams = n_chains,
 	};
-	struct sk_buff *skb;
 	s8 tx_power;
-	int ret;
-	int i;
+	int i, ret;
 
 	if (dev->mt76.chandef.width == NL80211_CHAN_WIDTH_40) {
 		req.bw = MT_BW_40;
@@ -458,8 +451,8 @@ int mt7603_mcu_set_channel(struct mt7603_dev *dev)
 	for (i = 0; i < ARRAY_SIZE(req.txpower); i++)
 		req.txpower[i] = tx_power;
 
-	skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
-	ret = mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH);
+	ret = mt7603_mcu_msg_send(dev, MCU_EXT_CMD_CHANNEL_SWITCH,
+				  &req, sizeof(req), true);
 	if (ret)
 		return ret;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/5] mt7603: remove query from mt7603_mcu_msg_send signature
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless
In-Reply-To: <cover.1555163776.git.lorenzo@kernel.org>

Remove query parameter from mt7603_mcu_msg_send/__mt7603_mcu_msg_send
routine signature since it can be obtained from cmd value. This is a
preliminary patch for mcu code unification between mt7615 and mt7603
drivers

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 .../net/wireless/mediatek/mt76/mt7603/mcu.c   | 36 ++++++++-----------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
index 57481012ee47..868a4b601a6f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mcu.c
@@ -14,8 +14,8 @@ struct mt7603_fw_trailer {
 } __packed;
 
 static int
-__mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd,
-		      int query, int *wait_seq)
+__mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb,
+		      int cmd, int *wait_seq)
 {
 	int hdrlen = dev->mcu_running ? sizeof(struct mt7603_mcu_txd) : 12;
 	struct mt76_dev *mdev = &dev->mt76;
@@ -42,15 +42,14 @@ __mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd,
 
 	if (cmd < 0) {
 		txd->cid = -cmd;
+		txd->set_query = MCU_Q_NA;
 	} else {
 		txd->cid = MCU_CMD_EXT_CID;
 		txd->ext_cid = cmd;
-		if (query != MCU_Q_NA)
-			txd->ext_cid_ack = 1;
+		txd->set_query = MCU_Q_SET;
+		txd->ext_cid_ack = 1;
 	}
 
-	txd->set_query = query;
-
 	if (wait_seq)
 		*wait_seq = seq;
 
@@ -58,8 +57,7 @@ __mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd,
 }
 
 static int
-mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd,
-		    int query)
+mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd)
 {
 	struct mt76_dev *mdev = &dev->mt76;
 	unsigned long expires = jiffies + 3 * HZ;
@@ -68,7 +66,7 @@ mt7603_mcu_msg_send(struct mt7603_dev *dev, struct sk_buff *skb, int cmd,
 
 	mutex_lock(&mdev->mmio.mcu.mutex);
 
-	ret = __mt7603_mcu_msg_send(dev, skb, cmd, query, &seq);
+	ret = __mt7603_mcu_msg_send(dev, skb, cmd, &seq);
 	if (ret)
 		goto out;
 
@@ -115,8 +113,7 @@ mt7603_mcu_init_download(struct mt7603_dev *dev, u32 addr, u32 len)
 	};
 	struct sk_buff *skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
 
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_TARGET_ADDRESS_LEN_REQ,
-				   MCU_Q_NA);
+	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_TARGET_ADDRESS_LEN_REQ);
 }
 
 static int
@@ -134,7 +131,7 @@ mt7603_mcu_send_firmware(struct mt7603_dev *dev, const void *data, int len)
 			return -ENOMEM;
 
 		ret = __mt7603_mcu_msg_send(dev, skb, -MCU_CMD_FW_SCATTER,
-					    MCU_Q_NA, NULL);
+					    NULL);
 		if (ret)
 			break;
 
@@ -157,8 +154,7 @@ mt7603_mcu_start_firmware(struct mt7603_dev *dev, u32 addr)
 	};
 	struct sk_buff *skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
 
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ,
-				   MCU_Q_NA);
+	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_FW_START_REQ);
 }
 
 static int
@@ -166,8 +162,7 @@ mt7603_mcu_restart(struct mt7603_dev *dev)
 {
 	struct sk_buff *skb = mt7603_mcu_msg_alloc(NULL, 0);
 
-	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ,
-				   MCU_Q_NA);
+	return mt7603_mcu_msg_send(dev, skb, -MCU_CMD_RESTART_DL_REQ);
 }
 
 int mt7603_load_firmware(struct mt7603_dev *dev)
@@ -371,8 +366,7 @@ int mt7603_mcu_set_eeprom(struct mt7603_dev *dev)
 		data[i].pad = 0;
 	}
 
-	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_EFUSE_BUFFER_MODE,
-				   MCU_Q_SET);
+	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_EFUSE_BUFFER_MODE);
 }
 
 static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
@@ -417,8 +411,7 @@ static int mt7603_mcu_set_tx_power(struct mt7603_dev *dev)
 	       sizeof(req.temp_comp_power));
 
 	skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
-	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_TX_POWER_CTRL,
-				   MCU_Q_SET);
+	return mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_SET_TX_POWER_CTRL);
 }
 
 int mt7603_mcu_set_channel(struct mt7603_dev *dev)
@@ -466,8 +459,7 @@ int mt7603_mcu_set_channel(struct mt7603_dev *dev)
 		req.txpower[i] = tx_power;
 
 	skb = mt7603_mcu_msg_alloc(&req, sizeof(req));
-	ret = mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH,
-				  MCU_Q_SET);
+	ret = mt7603_mcu_msg_send(dev, skb, MCU_EXT_CMD_CHANNEL_SWITCH);
 	if (ret)
 		return ret;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 0/5] use standard signature for mt7603 mcu api
From: Lorenzo Bianconi @ 2019-04-13 14:01 UTC (permalink / raw)
  To: nbd; +Cc: lorenzo.bianconi, linux-wireless

Introduce mt76_mcu_ops data structure in mt7603 mcu code in order to
unify mcu code between mt7603 and mt7615 drivers

Lorenzo Bianconi (5):
  mt7603: remove query from mt7603_mcu_msg_send signature
  mt7603: use standard signature for mt7603_mcu_msg_send
  mt7603: initialize mt76_mcu_ops data structure
  mt76: introduce mt76_mcu_restart macro
  mt7603: init mcu_restart function pointer

 drivers/net/wireless/mediatek/mt76/mt76.h     |   3 +
 .../net/wireless/mediatek/mt76/mt7603/init.c  |   2 +-
 .../net/wireless/mediatek/mt76/mt7603/mcu.c   | 102 +++++++++---------
 .../wireless/mediatek/mt76/mt7603/mt7603.h    |   2 +-
 .../net/wireless/mediatek/mt76/mt76x02_mmio.c |   2 +-
 5 files changed, 55 insertions(+), 56 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCH v3 1/4] mt76: add mac80211 driver for MT7615 PCIe-based chipsets
From: Felix Fietkau @ 2019-04-13 13:06 UTC (permalink / raw)
  To: Ryder Lee, Lorenzo Bianconi
  Cc: Roy Luo, Sean Wang, Nelson Chang, YF Luo, Kai Lin, Rorscha Yang,
	Kyle Lee, Miller Shen, Haipin Liang, KL Huang, YH chen,
	linux-wireless, linux-mediatek, linux-kernel, Lorenzo Bianconi
In-Reply-To: <4e974271e93b719e0463aa0269b4b970cc2b267f.1554102584.git.ryder.lee@mediatek.com>

On 2019-04-01 09:16, Ryder Lee wrote:
> This driver is for a newer generation of MediaTek MT7615 4x4 802.11ac
> PCIe-based chipsets, which support wave2 MU-MIMO up to 4 users/group
> and also support up to 160MHz bandwidth.
> 
> The driver fully supports AP, station and monitor mode.
> 
> Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
> Signed-off-by: Roy Luo <royluo@google.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>I'll fold in the following missing bit when I apply this patch.

- Felix

---
--- a/mt76.h
+++ b/mt76.h
@@ -69,6 +69,7 @@ enum mt76_txq_id {
 	MT_TXQ_MCU,
 	MT_TXQ_BEACON,
 	MT_TXQ_CAB,
+	MT_TXQ_FWDL,
 	__MT_TXQ_MAX
 };



^ permalink raw reply

* [PATCH] mt76: use readl/writel instead of ioread32/iowrite32
From: Felix Fietkau @ 2019-04-13 11:39 UTC (permalink / raw)
  To: linux-wireless

Switching to readl/writel is faster because it gets rid of an unnecessary
wrapper with extra checks.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 drivers/net/wireless/mediatek/mt76/dma.c      | 22 +++++++++----------
 drivers/net/wireless/mediatek/mt76/mmio.c     |  4 ++--
 .../net/wireless/mediatek/mt76/mt7603/mac.c   |  4 ++--
 .../net/wireless/mediatek/mt76/mt76x02_mmio.c |  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
index 7b8a998103d7..cdeca22bf3a7 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -49,10 +49,10 @@ mt76_dma_alloc_queue(struct mt76_dev *dev, struct mt76_queue *q,
 	for (i = 0; i < q->ndesc; i++)
 		q->desc[i].ctrl = cpu_to_le32(MT_DMA_CTL_DMA_DONE);
 
-	iowrite32(q->desc_dma, &q->regs->desc_base);
-	iowrite32(0, &q->regs->cpu_idx);
-	iowrite32(0, &q->regs->dma_idx);
-	iowrite32(q->ndesc, &q->regs->ring_size);
+	writel(q->desc_dma, &q->regs->desc_base);
+	writel(0, &q->regs->cpu_idx);
+	writel(0, &q->regs->dma_idx);
+	writel(q->ndesc, &q->regs->ring_size);
 
 	return 0;
 }
@@ -136,11 +136,11 @@ mt76_dma_tx_cleanup_idx(struct mt76_dev *dev, struct mt76_queue *q, int idx,
 static void
 mt76_dma_sync_idx(struct mt76_dev *dev, struct mt76_queue *q)
 {
-	iowrite32(q->desc_dma, &q->regs->desc_base);
-	iowrite32(q->ndesc, &q->regs->ring_size);
-	q->head = ioread32(&q->regs->dma_idx);
+	writel(q->desc_dma, &q->regs->desc_base);
+	writel(q->ndesc, &q->regs->ring_size);
+	q->head = readl(&q->regs->dma_idx);
 	q->tail = q->head;
-	iowrite32(q->head, &q->regs->cpu_idx);
+	writel(q->head, &q->regs->cpu_idx);
 }
 
 static void
@@ -159,7 +159,7 @@ mt76_dma_tx_cleanup(struct mt76_dev *dev, enum mt76_txq_id qid, bool flush)
 	if (flush)
 		last = -1;
 	else
-		last = ioread32(&q->regs->dma_idx);
+		last = readl(&q->regs->dma_idx);
 
 	while (q->queued && q->tail != last) {
 		mt76_dma_tx_cleanup_idx(dev, q, q->tail, &entry);
@@ -181,7 +181,7 @@ mt76_dma_tx_cleanup(struct mt76_dev *dev, enum mt76_txq_id qid, bool flush)
 		}
 
 		if (!flush && q->tail == last)
-			last = ioread32(&q->regs->dma_idx);
+			last = readl(&q->regs->dma_idx);
 	}
 
 	if (!flush)
@@ -251,7 +251,7 @@ mt76_dma_dequeue(struct mt76_dev *dev, struct mt76_queue *q, bool flush,
 static void
 mt76_dma_kick_queue(struct mt76_dev *dev, struct mt76_queue *q)
 {
-	iowrite32(q->head, &q->regs->cpu_idx);
+	writel(q->head, &q->regs->cpu_idx);
 }
 
 static int
diff --git a/drivers/net/wireless/mediatek/mt76/mmio.c b/drivers/net/wireless/mediatek/mt76/mmio.c
index 059f13bf9dff..38368d19aa6f 100644
--- a/drivers/net/wireless/mediatek/mt76/mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mmio.c
@@ -21,7 +21,7 @@ static u32 mt76_mmio_rr(struct mt76_dev *dev, u32 offset)
 {
 	u32 val;
 
-	val = ioread32(dev->mmio.regs + offset);
+	val = readl(dev->mmio.regs + offset);
 	trace_reg_rr(dev, offset, val);
 
 	return val;
@@ -30,7 +30,7 @@ static u32 mt76_mmio_rr(struct mt76_dev *dev, u32 offset)
 static void mt76_mmio_wr(struct mt76_dev *dev, u32 offset, u32 val)
 {
 	trace_reg_wr(dev, offset, val);
-	iowrite32(val, dev->mmio.regs + offset);
+	writel(val, dev->mmio.regs + offset);
 }
 
 static u32 mt76_mmio_rmw(struct mt76_dev *dev, u32 offset, u32 mask, u32 val)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
index 5f800467c628..5a5337e2dad4 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/mac.c
@@ -1417,11 +1417,11 @@ static bool mt7603_tx_hang(struct mt7603_dev *dev)
 			continue;
 
 		prev_dma_idx = dev->tx_dma_idx[i];
-		dma_idx = ioread32(&q->regs->dma_idx);
+		dma_idx = readl(&q->regs->dma_idx);
 		dev->tx_dma_idx[i] = dma_idx;
 
 		if (dma_idx == prev_dma_idx &&
-		    dma_idx != ioread32(&q->regs->cpu_idx))
+		    dma_idx != readl(&q->regs->cpu_idx))
 			break;
 	}
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
index ca8320711bc2..705c0939d10b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x02_mmio.c
@@ -349,7 +349,7 @@ static bool mt76x02_tx_hang(struct mt76x02_dev *dev)
 			continue;
 
 		prev_dma_idx = dev->mt76.tx_dma_idx[i];
-		dma_idx = ioread32(&q->regs->dma_idx);
+		dma_idx = readl(&q->regs->dma_idx);
 		dev->mt76.tx_dma_idx[i] = dma_idx;
 
 		if (prev_dma_idx == dma_idx)
-- 
2.17.0


^ permalink raw reply related

* Re: [PATCH] rtlwifi: rtl8723ae: Make rtl8723e_dm_refresh_rate_adaptive_mask static
From: Kalle Valo @ 2019-04-13 11:18 UTC (permalink / raw)
  To: Yue Haibing
  Cc: davem, pkshih, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190319151927.19392-1-yuehaibing@huawei.com>

Yue Haibing <yuehaibing@huawei.com> wrote:

> From: YueHaibing <yuehaibing@huawei.com>
> 
> Fix sparse warning:
> 
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/dm.c:666:6:
>  warning: symbol 'rtl8723e_dm_refresh_rate_adaptive_mask' was not declared. Should it be static?
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>

Patch applied to wireless-drivers-next.git, thanks.

bdfc4027de15 rtlwifi: rtl8723ae: Make rtl8723e_dm_refresh_rate_adaptive_mask static

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

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


^ permalink raw reply

* Re: [PATCH v2 1/2] rtlwifi: Fix duplicate tests of one of the RX descriptors
From: Kalle Valo @ 2019-04-13 11:17 UTC (permalink / raw)
  To: Larry Finger; +Cc: linux-wireless, pkshih, Larry Finger
In-Reply-To: <20190404182931.9477-2-Larry.Finger@lwfinger.net>

Larry Finger <Larry.Finger@lwfinger.net> wrote:

> In drivers rtl8188ee, rtl8821ae, rtl8723be, and rtl8192ee, the reason
> for a wake-up is returned in the fourth RX descriptor in bits 29-31. Due
> to typographical errors, all but rtl8821ae test bit 31 twice and fail to
> test bit 29.
> 
> This error causes no problems as the tests are only used to set bits in
> the output of an optional debugging statement.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>

2 patches applied to wireless-drivers-next.git, thanks.

0961d9874a2e rtlwifi: Fix duplicate tests of one of the RX descriptors
ddab2eee7949 rtlwifi: Convert the wake_match variable to local

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

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


^ permalink raw reply

* Re: [PATCH][V2] brcmfmac: fix leak of mypkt on error return path
From: Kalle Valo @ 2019-04-13 11:08 UTC (permalink / raw)
  To: Colin King
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, David S . Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190409114333.24342-1-colin.king@canonical.com>

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Currently if the call to brcmf_sdiod_set_backplane_window fails then
> error return path leaks mypkt. Fix this by returning by a new
> error path labelled 'out' that calls brcmu_pkt_buf_free_skb to free
> mypkt.  Also remove redundant check on err before calling
> brcmf_sdiod_skbuff_write.
> 
> Addresses-Coverity: ("Resource Leak")
> Fixes: a7c3aa1509e2 ("brcmfmac: Remove brcmf_sdiod_addrprep()")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>

Patch applied to wireless-drivers-next.git, thanks.

a927e8d8ab57 brcmfmac: fix leak of mypkt on error return path

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

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


^ permalink raw reply

* Re: [PATCH 11/12] brcmfmac: Loading the correct firmware for brcm43456
From: Kalle Valo @ 2019-04-13 11:06 UTC (permalink / raw)
  To: megous
  Cc: linux-sunxi, Maxime Ripard, Chen-Yu Tsai, Rob Herring,
	Linus Walleij, Ondrej Jirman, David Airlie, Daniel Vetter,
	Mark Rutland, Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu,
	David S. Miller, Maxime Coquelin, Arend van Spriel, Franky Lin,
	Hante Meuleman, Chi-Hsien Lin, Wright Feng, Naveen Gupta,
	dri-devel, devicetree, linux-arm-kernel, linux-kernel, netdev,
	linux-stm32, linux-wireless, brcm80211-dev-list.pdl,
	brcm80211-dev-list, linux-gpio
In-Reply-To: <20190405234514.6183-12-megous@megous.com>

megous@megous.com wrote:

> From: Ondrej Jirman <megous@megous.com>
> 
> SDIO based brcm43456 is currently misdetected as brcm43455 and the wrong
> firmware name is used. Correct the detection and load the correct
> firmware file. Chiprev for brcm43456 is "9".
> 
> Signed-off-by: Ondrej Jirman <megous@megous.com>

Patch applied to wireless-drivers-next.git, thanks.

e3062e05e1cf brcmfmac: Loading the correct firmware for brcm43456

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

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


^ permalink raw reply

* Re: [PATCH][next] brcmfmac: Use struct_size() in kzalloc()
From: Kalle Valo @ 2019-04-13 11:03 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
	Wright Feng, David S. Miller, linux-wireless,
	brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel,
	Gustavo A. R. Silva
In-Reply-To: <20190403164611.GA29107@embeddedor>

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     struct boo entry[];
> };
> 
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = kzalloc(size, GFP_KERNEL)
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
> 
> Notice that, in this case, variable reqsz is not necessary,
> hence it is removed.
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Patch applied to wireless-drivers-next.git, thanks.

0cf83903aad0 brcmfmac: Use struct_size() in kzalloc()

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

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


^ permalink raw reply

* Re: pull-request: iwlwifi-next 2019-04-03
From: Kalle Valo @ 2019-04-13 10:47 UTC (permalink / raw)
  To: Luca Coelho; +Cc: linux-wireless, linuxwifi, david.e.box, joe.konno
In-Reply-To: <713ec14872e6f0b9ced02d2d523286d522e0eab1.camel@coelho.fi>

Luca Coelho <luca@coelho.fi> writes:

> Hi Kalle,
>
> This is the second batch of patches intended for v5.2.  This includes
> the last patchset I sent.  Usual development work.  More details about
> the contents in the tag description.
>
> I have sent this out before and kbuildbot reported success.
>
> Please let me know if there are any issues.
>
> Cheers,
> Luca.
>
>
> The following changes since commit 8db32fa400916333c20184793d1400bd74e1f740:
>
>   Merge tag 'iwlwifi-next-for-kalle-2019-03-22' of git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next (2019-03-30 08:37:38 +0200)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-next.git tags/iwlwifi-next-for-kalle-2019-04-03
>
> for you to fetch changes up to ef8a913766cd5e341da68c011b4a0ff5e2c0f5d1:
>
>   iwlwifi: remove misconfigured pci ids from 22260 series (2019-04-03 11:20:05 +0300)
>
> ----------------------------------------------------------------
> Second batch of patches intended for v5.2
>
> * Work on the new debugging infra continues;
> * Fixes for the 22000 series;
> * Support for some new FW API changes;
> * Work on new hardware continues;
> * Some debugfs cleanups by Greg-KH;
> * General bugfixes;
> * Other cleanups;
>
> ----------------------------------------------------------------

Pulled, thanks Luca.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH] mt76: usb: fix possible memory leak during suspend/resume
From: Lorenzo Bianconi @ 2019-04-13 10:10 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190413083050.GA7434@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 8194 bytes --]

> On Fri, Apr 12, 2019 at 06:27:48PM +0200, Lorenzo Bianconi wrote:
> > > > On Fri, Apr 12, 2019 at 02:27:16PM +0200, Lorenzo Bianconi wrote:
> > > > > Disable mt76u_tx_tasklet at the end of mt76u_stop_queues in order to
> > > > > properly deallocate all pending skbs during suspend/resume phase
> > > > 
> > > > On suspend/resume tx skb's are processed after tasklet_enable()
> > > > in resume callback. There is issue with device removal though
> > > > (during suspend or otherwise).
> > > 
> > > Hi Stanislaw,
> > > 
> > > I guess the right moment to deallocate the skbs is during suspend since resume
> > > can happen in very far future
> 
> Yes, it's better to free on suspend, but in practice does not really matter since
> system is disabled till resume.
> 
> > > > > Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> > > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > > ---
> > > > >  drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> > > > > index a3acc070063a..575207133775 100644
> > > > > --- a/drivers/net/wireless/mediatek/mt76/usb.c
> > > > > +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> > > > > @@ -842,10 +842,10 @@ static void mt76u_stop_tx(struct mt76_dev *dev)
> > > > >  void mt76u_stop_queues(struct mt76_dev *dev)
> > > > >  {
> > > > >  	tasklet_disable(&dev->usb.rx_tasklet);
> > > > > -	tasklet_disable(&dev->usb.tx_tasklet);
> > > > > -
> > > > >  	mt76u_stop_rx(dev);
> > > > > +
> > > > >  	mt76u_stop_tx(dev);
> > > > > +	tasklet_disable(&dev->usb.tx_tasklet);
> > > > 
> > > > If tasklet is scheduled and we disable it and never enable, we end up
> > > > with infinite loop in tasklet_action_common(). This patch make the
> > > > problem less reproducible since tasklet_disable() is moved after
> > > > usb_kill_urb() -> tasklet_schedule(), but it is still possible.
> > > 
> > > I can see the point here. Maybe we can just run tasklet_kill instead of
> > > tasklet_disable here (at least for tx one)
> 
> I think you have right as tasklet_kill() will wait for scheduled tasklet .
> Originally in my patch (see below) I used wait_event as I thought
> tasklet_kill() may prevent scheduled tasklet to be executed (hence cause
> leak) but that seems to be not true.

I agree with rx side (good catch!!), but on tx one I guess usb_kill_urb()
is already waiting for tx pending so we just need to use tasklet_kill
at the end of mt76u_stop_queues, in this way we will free pending skbs during
suspend

Regards,
Lorenzo

> 
> > I think it is enough even for rx side since usb_kill_urb() will wait the end of
> > mt76u_complete_rx() and tasklet_kill() will wait for the completion of
> > mt76u_rx_tasklet(). We will need to remove tasklet_enable in resume routines.
> 
> No, because rx urb's are resubmitted on mt76u_rx_tasklet(). I use usb_poison_urb()
> to prevent that in my patch.
> 
> Stanislaw
> 
> commit 088b1209302e17cda688c9b562e18970c92823ac
> Author: Stanislaw Gruszka <sgruszka@redhat.com>
> Date:   Thu Apr 4 13:37:43 2019 +0200
> 
>     mt76usb: fix tx/rx stop
>     
>     Disabling tasklets on stopping rx/tx is wrong. If blocked tasklet
>     is scheduled and we remove device we get 100% cpu usage:
>     
>       PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
>         9 root      20   0       0      0      0 R  93.8   0.0   1:47.19 ksoftirqd/0
>     
>     by using memory after free and eventually crash:
>     
>     [ 2068.591964] RIP: 0010:tasklet_action_common.isra.17+0x66/0x100
>     [ 2068.591966] Code: 41 89 f5 eb 25 f0 48 0f ba 33 00 0f 83 b1 00 00 00 48 8b 7a 20 48 8b 42 18 e8 56 a3 b5 00 f0 80 23 fd 48 89 ea 48 85 ed 74 53 <48> 8b 2a 48 8d 5a 08 f0 48 0f ba 6a 08 01 72 0b 8b 42 10 85 c0 74
>     [ 2068.591968] RSP: 0018:ffff98758c34be58 EFLAGS: 00010206
>     [ 2068.591969] RAX: ffff98758e6966d0 RBX: ffff98756e69aef8 RCX: 0000000000000006
>     [ 2068.591970] RDX: 01060a053d060305 RSI: 0000000000000006 RDI: ffff98758e6966d0
>     [ 2068.591971] RBP: 01060a053d060305 R08: 0000000000000000 R09: 00000000000203c0
>     [ 2068.591971] R10: 000003ff65b34f08 R11: 0000000000000001 R12: ffff98758e6966d0
>     [ 2068.591972] R13: 0000000000000006 R14: 0000000000000040 R15: 0000000000000006
>     [ 2068.591974] FS:  0000000000000000(0000) GS:ffff98758e680000(0000) knlGS:0000000000000000
>     [ 2068.591975] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>     [ 2068.591975] CR2: 00002c5f73a6cc20 CR3: 00000002f920a001 CR4: 00000000003606e0
>     [ 2068.591977] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>     [ 2068.591978] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>     [ 2068.591978] Call Trace:
>     [ 2068.591985]  __do_softirq+0xe3/0x30a
>     [ 2068.591989]  ? sort_range+0x20/0x20
>     [ 2068.591990]  run_ksoftirqd+0x26/0x40
>     [ 2068.591992]  smpboot_thread_fn+0xc5/0x160
>     [ 2068.591995]  kthread+0x112/0x130
>     [ 2068.591997]  ? kthread_create_on_node+0x40/0x40
>     [ 2068.591998]  ret_from_fork+0x35/0x40
>     [ 2068.591999] Modules linked in: ccm arc4 fuse rfcomm cmac bnep sunrpc snd_hda_codec_hdmi snd_soc_skl snd_soc_core snd_soc_acpi_intel_match snd_hda_codec_realtek snd_soc_acpi snd_hda_codec_generic snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core iTCO_wdt snd_hda_intel intel_rapl iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp btusb mei_wdt coretemp btrtl snd_hda_codec btbcm btintel intel_cstate snd_hwdep intel_uncore uvcvideo snd_hda_core videobuf2_vmalloc videobuf2_memops intel_rapl_perf wmi_bmof videobuf2_v4l2 intel_wmi_thunderbolt snd_seq bluetooth joydev videobuf2_common snd_seq_device snd_pcm videodev media i2c_i801 snd_timer idma64 ecdh_generic intel_lpss_pci intel_lpss mei_me mei ucsi_acpi typec_ucsi processor_thermal_device intel_soc_dts_iosf intel_pch_thermal typec thinkpad_acpi wmi snd soundcore rfkill int3403_thermal int340x_thermal_zone int3400_thermal acpi_thermal_rel acpi_pad pcc_cpufreq uas usb_storage crc32c_intel i915 i2c_algo_bit nvme serio_raw
>     [ 2068.592033]  drm_kms_helper e1000e nvme_core drm video ipv6 [last unloaded: cfg80211]
>     
>     Fortunate thing is that this not happen frequently, as scheduling
>     tasklet on blocked state is very exceptional, though might happen.
>     
>     Due to different RX/TX tasklet processing fix is different for those.
>     For TX just wait until queues are empty. For RX assure rx_tasklet
>     do fail to resubmit buffers by poisoning urb's and kill the tasklet.
>     
>     Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
> index 15825e9a458f..7bf91566e212 100644

[...]

> @@ -830,20 +831,35 @@ static void mt76u_free_tx(struct mt76_dev *dev)
>  static void mt76u_stop_tx(struct mt76_dev *dev)
>  {
>  	struct mt76_queue *q;
> -	int i, j;
> +	int i, j, ret;
>  
>  	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
>  		q = dev->q_tx[i].q;
>  		for (j = 0; j < q->ndesc; j++)
>  			usb_kill_urb(q->entry[j].urb);
>  	}
> +
> +	ret = wait_event_timeout(dev->tx_wait, !mt76_has_tx_pending(dev), HZ/5);
> +	if (!ret)
> +		dev_err(dev->dev, "timed out waiting for pending tx\n");
>  }
>  
> -void mt76u_stop_queues(struct mt76_dev *dev)
> +int mt76u_start_queues(struct mt76_dev *dev)

maybe mt76u_resume_rx_queue would be more clear

>  {
> -	tasklet_disable(&dev->usb.rx_tasklet);
> -	tasklet_disable(&dev->usb.tx_tasklet);
> +	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
> +	int i;
> +
> +	/* Nothing to do for TX */
>  
> +	for (i = 0; i < q->ndesc; i++)
> +		usb_unpoison_urb(q->entry[i].urb);
> +
> +	return mt76u_submit_rx_buffers(dev);
> +}
> +EXPORT_SYMBOL_GPL(mt76u_start_queues);
> +
> +void mt76u_stop_queues(struct mt76_dev *dev)
> +{
>  	mt76u_stop_rx(dev);
>  	mt76u_stop_tx(dev);
>  }


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply

* Re: [PATCH] mt76: usb: fix possible memory leak during suspend/resume
From: Stanislaw Gruszka @ 2019-04-13  8:30 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190412162746.GC3156@localhost.localdomain>

On Fri, Apr 12, 2019 at 06:27:48PM +0200, Lorenzo Bianconi wrote:
> > > On Fri, Apr 12, 2019 at 02:27:16PM +0200, Lorenzo Bianconi wrote:
> > > > Disable mt76u_tx_tasklet at the end of mt76u_stop_queues in order to
> > > > properly deallocate all pending skbs during suspend/resume phase
> > > 
> > > On suspend/resume tx skb's are processed after tasklet_enable()
> > > in resume callback. There is issue with device removal though
> > > (during suspend or otherwise).
> > 
> > Hi Stanislaw,
> > 
> > I guess the right moment to deallocate the skbs is during suspend since resume
> > can happen in very far future

Yes, it's better to free on suspend, but in practice does not really matter since
system is disabled till resume.

> > > > Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> > > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > > ---
> > > >  drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> > > > index a3acc070063a..575207133775 100644
> > > > --- a/drivers/net/wireless/mediatek/mt76/usb.c
> > > > +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> > > > @@ -842,10 +842,10 @@ static void mt76u_stop_tx(struct mt76_dev *dev)
> > > >  void mt76u_stop_queues(struct mt76_dev *dev)
> > > >  {
> > > >  	tasklet_disable(&dev->usb.rx_tasklet);
> > > > -	tasklet_disable(&dev->usb.tx_tasklet);
> > > > -
> > > >  	mt76u_stop_rx(dev);
> > > > +
> > > >  	mt76u_stop_tx(dev);
> > > > +	tasklet_disable(&dev->usb.tx_tasklet);
> > > 
> > > If tasklet is scheduled and we disable it and never enable, we end up
> > > with infinite loop in tasklet_action_common(). This patch make the
> > > problem less reproducible since tasklet_disable() is moved after
> > > usb_kill_urb() -> tasklet_schedule(), but it is still possible.
> > 
> > I can see the point here. Maybe we can just run tasklet_kill instead of
> > tasklet_disable here (at least for tx one)

I think you have right as tasklet_kill() will wait for scheduled tasklet .
Originally in my patch (see below) I used wait_event as I thought
tasklet_kill() may prevent scheduled tasklet to be executed (hence cause
leak) but that seems to be not true.

> I think it is enough even for rx side since usb_kill_urb() will wait the end of
> mt76u_complete_rx() and tasklet_kill() will wait for the completion of
> mt76u_rx_tasklet(). We will need to remove tasklet_enable in resume routines.

No, because rx urb's are resubmitted on mt76u_rx_tasklet(). I use usb_poison_urb()
to prevent that in my patch.

Stanislaw

commit 088b1209302e17cda688c9b562e18970c92823ac
Author: Stanislaw Gruszka <sgruszka@redhat.com>
Date:   Thu Apr 4 13:37:43 2019 +0200

    mt76usb: fix tx/rx stop
    
    Disabling tasklets on stopping rx/tx is wrong. If blocked tasklet
    is scheduled and we remove device we get 100% cpu usage:
    
      PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
        9 root      20   0       0      0      0 R  93.8   0.0   1:47.19 ksoftirqd/0
    
    by using memory after free and eventually crash:
    
    [ 2068.591964] RIP: 0010:tasklet_action_common.isra.17+0x66/0x100
    [ 2068.591966] Code: 41 89 f5 eb 25 f0 48 0f ba 33 00 0f 83 b1 00 00 00 48 8b 7a 20 48 8b 42 18 e8 56 a3 b5 00 f0 80 23 fd 48 89 ea 48 85 ed 74 53 <48> 8b 2a 48 8d 5a 08 f0 48 0f ba 6a 08 01 72 0b 8b 42 10 85 c0 74
    [ 2068.591968] RSP: 0018:ffff98758c34be58 EFLAGS: 00010206
    [ 2068.591969] RAX: ffff98758e6966d0 RBX: ffff98756e69aef8 RCX: 0000000000000006
    [ 2068.591970] RDX: 01060a053d060305 RSI: 0000000000000006 RDI: ffff98758e6966d0
    [ 2068.591971] RBP: 01060a053d060305 R08: 0000000000000000 R09: 00000000000203c0
    [ 2068.591971] R10: 000003ff65b34f08 R11: 0000000000000001 R12: ffff98758e6966d0
    [ 2068.591972] R13: 0000000000000006 R14: 0000000000000040 R15: 0000000000000006
    [ 2068.591974] FS:  0000000000000000(0000) GS:ffff98758e680000(0000) knlGS:0000000000000000
    [ 2068.591975] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 2068.591975] CR2: 00002c5f73a6cc20 CR3: 00000002f920a001 CR4: 00000000003606e0
    [ 2068.591977] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
    [ 2068.591978] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
    [ 2068.591978] Call Trace:
    [ 2068.591985]  __do_softirq+0xe3/0x30a
    [ 2068.591989]  ? sort_range+0x20/0x20
    [ 2068.591990]  run_ksoftirqd+0x26/0x40
    [ 2068.591992]  smpboot_thread_fn+0xc5/0x160
    [ 2068.591995]  kthread+0x112/0x130
    [ 2068.591997]  ? kthread_create_on_node+0x40/0x40
    [ 2068.591998]  ret_from_fork+0x35/0x40
    [ 2068.591999] Modules linked in: ccm arc4 fuse rfcomm cmac bnep sunrpc snd_hda_codec_hdmi snd_soc_skl snd_soc_core snd_soc_acpi_intel_match snd_hda_codec_realtek snd_soc_acpi snd_hda_codec_generic snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core iTCO_wdt snd_hda_intel intel_rapl iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp btusb mei_wdt coretemp btrtl snd_hda_codec btbcm btintel intel_cstate snd_hwdep intel_uncore uvcvideo snd_hda_core videobuf2_vmalloc videobuf2_memops intel_rapl_perf wmi_bmof videobuf2_v4l2 intel_wmi_thunderbolt snd_seq bluetooth joydev videobuf2_common snd_seq_device snd_pcm videodev media i2c_i801 snd_timer idma64 ecdh_generic intel_lpss_pci intel_lpss mei_me mei ucsi_acpi typec_ucsi processor_thermal_device intel_soc_dts_iosf intel_pch_thermal typec thinkpad_acpi wmi snd soundcore rfkill int3403_thermal int340x_thermal_zone int3400_thermal acpi_thermal_rel acpi_pad pcc_cpufreq uas usb_storage crc32c_intel i915 i2c_algo_bit nvme 
 serio_raw
    [ 2068.592033]  drm_kms_helper e1000e nvme_core drm video ipv6 [last unloaded: cfg80211]
    
    Fortunate thing is that this not happen frequently, as scheduling
    tasklet on blocked state is very exceptional, though might happen.
    
    Due to different RX/TX tasklet processing fix is different for those.
    For TX just wait until queues are empty. For RX assure rx_tasklet
    do fail to resubmit buffers by poisoning urb's and kill the tasklet.
    
    Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index 15825e9a458f..7bf91566e212 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -384,7 +384,7 @@ void mt76_rx(struct mt76_dev *dev, enum mt76_rxq_id q, struct sk_buff *skb)
 }
 EXPORT_SYMBOL_GPL(mt76_rx);
 
-static bool mt76_has_tx_pending(struct mt76_dev *dev)
+bool mt76_has_tx_pending(struct mt76_dev *dev)
 {
 	struct mt76_queue *q;
 	int i;
@@ -397,6 +397,7 @@ static bool mt76_has_tx_pending(struct mt76_dev *dev)
 
 	return false;
 }
+EXPORT_SYMBOL_GPL(mt76_has_tx_pending);
 
 void mt76_set_channel(struct mt76_dev *dev)
 {
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index b432da3f55c7..19d03294e678 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -682,6 +682,7 @@ void mt76_release_buffered_frames(struct ieee80211_hw *hw,
 				  u16 tids, int nframes,
 				  enum ieee80211_frame_release_type reason,
 				  bool more_data);
+bool mt76_has_tx_pending(struct mt76_dev *dev);
 void mt76_set_channel(struct mt76_dev *dev);
 int mt76_get_survey(struct ieee80211_hw *hw, int idx,
 		    struct survey_info *survey);
@@ -776,9 +777,9 @@ int mt76u_vendor_request(struct mt76_dev *dev, u8 req,
 void mt76u_single_wr(struct mt76_dev *dev, const u8 req,
 		     const u16 offset, const u32 val);
 int mt76u_init(struct mt76_dev *dev, struct usb_interface *intf);
-int mt76u_submit_rx_buffers(struct mt76_dev *dev);
 int mt76u_alloc_queues(struct mt76_dev *dev);
 void mt76u_stop_queues(struct mt76_dev *dev);
+int mt76u_start_queues(struct mt76_dev *dev);
 void mt76u_stop_stat_wk(struct mt76_dev *dev);
 void mt76u_queues_deinit(struct mt76_dev *dev);
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
index 1ef00e971cfa..a27a3ae3dbf5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/usb.c
@@ -327,13 +327,10 @@ static int __maybe_unused mt76x0_resume(struct usb_interface *usb_intf)
 	struct mt76_usb *usb = &dev->mt76.usb;
 	int ret;
 
-	ret = mt76u_submit_rx_buffers(&dev->mt76);
+	ret = mt76u_start_queues(&dev->mt76);
 	if (ret < 0)
 		goto err;
 
-	tasklet_enable(&usb->rx_tasklet);
-	tasklet_enable(&usb->tx_tasklet);
-
 	ret = mt76x0u_init_hardware(dev);
 	if (ret)
 		goto err;
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
index d08bb964966b..5450fb1c3f55 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/usb.c
@@ -119,13 +119,10 @@ static int __maybe_unused mt76x2u_resume(struct usb_interface *intf)
 	struct mt76_usb *usb = &dev->mt76.usb;
 	int err;
 
-	err = mt76u_submit_rx_buffers(&dev->mt76);
-	if (err < 0)
+	err = mt76u_start_queues(&dev->mt76);
+	if (err)
 		goto err;
 
-	tasklet_enable(&usb->rx_tasklet);
-	tasklet_enable(&usb->tx_tasklet);
-
 	err = mt76x2u_init_hardware(dev);
 	if (err < 0)
 		goto err;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index a3acc070063a..8e32a60ac1c3 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -540,7 +540,7 @@ static void mt76u_rx_tasklet(unsigned long data)
 	rcu_read_unlock();
 }
 
-int mt76u_submit_rx_buffers(struct mt76_dev *dev)
+static int mt76u_submit_rx_buffers(struct mt76_dev *dev)
 {
 	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
 	unsigned long flags;
@@ -558,7 +558,6 @@ int mt76u_submit_rx_buffers(struct mt76_dev *dev)
 
 	return err;
 }
-EXPORT_SYMBOL_GPL(mt76u_submit_rx_buffers);
 
 static int mt76u_alloc_rx(struct mt76_dev *dev)
 {
@@ -611,7 +610,9 @@ static void mt76u_stop_rx(struct mt76_dev *dev)
 	int i;
 
 	for (i = 0; i < q->ndesc; i++)
-		usb_kill_urb(q->entry[i].urb);
+		usb_poison_urb(q->entry[i].urb);
+
+	tasklet_kill(&dev->usb.rx_tasklet);
 }
 
 static void mt76u_tx_tasklet(unsigned long data)
@@ -830,20 +831,35 @@ static void mt76u_free_tx(struct mt76_dev *dev)
 static void mt76u_stop_tx(struct mt76_dev *dev)
 {
 	struct mt76_queue *q;
-	int i, j;
+	int i, j, ret;
 
 	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
 		q = dev->q_tx[i].q;
 		for (j = 0; j < q->ndesc; j++)
 			usb_kill_urb(q->entry[j].urb);
 	}
+
+	ret = wait_event_timeout(dev->tx_wait, !mt76_has_tx_pending(dev), HZ/5);
+	if (!ret)
+		dev_err(dev->dev, "timed out waiting for pending tx\n");
 }
 
-void mt76u_stop_queues(struct mt76_dev *dev)
+int mt76u_start_queues(struct mt76_dev *dev)
 {
-	tasklet_disable(&dev->usb.rx_tasklet);
-	tasklet_disable(&dev->usb.tx_tasklet);
+	struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
+	int i;
+
+	/* Nothing to do for TX */
 
+	for (i = 0; i < q->ndesc; i++)
+		usb_unpoison_urb(q->entry[i].urb);
+
+	return mt76u_submit_rx_buffers(dev);
+}
+EXPORT_SYMBOL_GPL(mt76u_start_queues);
+
+void mt76u_stop_queues(struct mt76_dev *dev)
+{
 	mt76u_stop_rx(dev);
 	mt76u_stop_tx(dev);
 }

^ permalink raw reply related

* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Arnd Bergmann @ 2019-04-13  8:04 UTC (permalink / raw)
  To: Kirtika Ruchandani; +Cc: Ben Greear, linux-wireless, Tina Ruchandani
In-Reply-To: <CAK8P3a2DVgneKY9XMQe_LFuECNR7qvLtJV-g-skC4EUDA40+-g@mail.gmail.com>

On Sat, Apr 13, 2019 at 10:00 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sat, Apr 13, 2019 at 12:07 AM Kirtika Ruchandani <kirtika@google.com> wrote:
> >
> > On Fri, Apr 12, 2019 at 2:49 PM Ben Greear <greearb@candelatech.com> wrote:
> > >
> > > On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> > > > On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
> > > >>
> > > >> From: Ben Greear <greearb@candelatech.com>
> > > >>
> > > >> This lets us more precisely calculate the absolute timestamp
> > > >> of last-rix (ie, now - idle).
> > > >
> > > > Can you use 64-bit timestamps? struct timeval suffers from the
> > > > overflow after 2038 problem.
> > >
> > > What is the preferred API to do this?  Whatever it is, it would need
> > > to compile on old crufty systems as well.
> >
> > I am not sure what the guidance for userspace is. The kernel uses
> > 'struct timespec64' I think.
> > Arnd (cc-ed) who has mostly led the 2038 problem in the kernel might
> > have more input on the
> > "old crufty systems" part.
>
> I'm not sure what you are trying to do, and there are different
> answers depending on the usecase.
>
> For getting the time in the kernel, see Documentation/core-api/timekeeping.rst
> do_gettimeofday() is going away for many reasons, so don't use that.
> It sounds like you want "ktime_to_ms(ktime_get())" here.
>
> In userspace interfaces, you should pass 64-bit nanoseconds as returned
> by ktime_get_ns().
>
> If you want to pretty-print the current wall-clock, use the %pt format string
> on a 'struct rtc_time'.

Ah, I see now this was just userspace code. In that case, using gettimeofday()
works fine, it will end up using a 64-bit version of 'timeval', and converting
that to 64-bit milliseconds is safe.

Using clock_gettime() is generally preferred over gettimeofday() since it
avoids the conversion from nanoseconds to microseconds (which you then
convert to milliseconds).

       Arnd

^ permalink raw reply

* Re: [PATCH 2/2] iw: Print current time in station info dump
From: Arnd Bergmann @ 2019-04-13  8:00 UTC (permalink / raw)
  To: Kirtika Ruchandani; +Cc: Ben Greear, linux-wireless, Tina Ruchandani
In-Reply-To: <CAOXD1icnusnOZ0b0n6eTD6PRoEcf_tHVgivmCEPM1KsWu1uEng@mail.gmail.com>

On Sat, Apr 13, 2019 at 12:07 AM Kirtika Ruchandani <kirtika@google.com> wrote:
>
> On Fri, Apr 12, 2019 at 2:49 PM Ben Greear <greearb@candelatech.com> wrote:
> >
> > On 4/12/19 2:43 PM, Kirtika Ruchandani wrote:
> > > On Fri, Apr 12, 2019 at 2:40 PM <greearb@candelatech.com> wrote:
> > >>
> > >> From: Ben Greear <greearb@candelatech.com>
> > >>
> > >> This lets us more precisely calculate the absolute timestamp
> > >> of last-rix (ie, now - idle).
> > >
> > > Can you use 64-bit timestamps? struct timeval suffers from the
> > > overflow after 2038 problem.
> >
> > What is the preferred API to do this?  Whatever it is, it would need
> > to compile on old crufty systems as well.
>
> I am not sure what the guidance for userspace is. The kernel uses
> 'struct timespec64' I think.
> Arnd (cc-ed) who has mostly led the 2038 problem in the kernel might
> have more input on the
> "old crufty systems" part.

I'm not sure what you are trying to do, and there are different
answers depending on the usecase.

For getting the time in the kernel, see Documentation/core-api/timekeeping.rst
do_gettimeofday() is going away for many reasons, so don't use that.
It sounds like you want "ktime_to_ms(ktime_get())" here.

In userspace interfaces, you should pass 64-bit nanoseconds as returned
by ktime_get_ns().

If you want to pretty-print the current wall-clock, use the %pt format string
on a 'struct rtc_time'.

      Arnd

^ permalink raw reply

* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-13  7:55 UTC (permalink / raw)
  To: Harald Welte
  Cc: Johannes Berg, openbsc, radiotap, linux-wireless,
	Subash Abhinov Kasiviswanathan, Dan Williams, Bjørn Mork,
	netdev, Sean Tranchetti, Aleksander Morgado
In-Reply-To: <20190413073505.GD24451@nataraja>

On Apr 13, 2019, at 12:35 AM, Harald Welte <laforge@gnumonks.org> wrote:

> the "physical link info" is present in GSMTAP, but the granularity of
> GSMTAP frames is not user-IP frames, but "MAC blocks".  So your user IP
> frame might not be visible as it's still compressed, encrypted,
> fragmented, etc.

The granularity of Ethernet frames is not user IP frames, but Ethernet datagrams, so you user IP frame might not be visible as it's fragmented (the fragments might still be IP datagrams, but they would have to be reassembled - which Wireshark, for example, does, for those people still doing NFS-over-UDP :-)).

"Encrypted" may not apply there, but it *does* apply for 802.11 frames on a protected network (which Wireshark can decrypt, if you have 1) the network password for WEP or WPA-Personal and 2) the EAPOL handshake for WPA-Personal).

^ permalink raw reply

* Re: gsmtap design/extensions?
From: Guy Harris @ 2019-04-13  7:49 UTC (permalink / raw)
  To: Harald Welte
  Cc: Johannes Berg, Vadim Yanitskiy, OpenBSC Mailing List,
	Sean Tranchetti, radiotap, Dan Williams, netdev, linux-wireless,
	Aleksander Morgado, Subash Abhinov Kasiviswanathan,
	Bjørn Mork
In-Reply-To: <20190413071227.GC24451@nataraja>

On Apr 13, 2019, at 12:12 AM, Harald Welte <laforge@gnumonks.org> wrote:

> On Fri, Apr 12, 2019 at 07:15:56PM +0200, Johannes Berg wrote:
> 
>> Agree. Sorry about that. No disrespect was intended, but I'm still not
>> sure I understand the need for UDP encapsulation *as part of the
>> protocol*. I guess saying "GSMTAP can optionally be encapsulated in UDP
>> with the well-known port xyz" would be something else, and it'd make
>> more sense to me than saying it has to be.
> 
> Sure, like with most protocols you can wrap them in anything you want.
> 
> Let me put it like this:
> You don't have to run RTP inside UDP, you could equally put the RTP
> frames in to SCTP or DCTP.   It's just not what the original users of
> the protocol/spec had envisioned, but it can for sure be done, and has
> no side-effect other than not being interoperable with existing
> implementations.

Or you can just have LINKTYPE_RTP/DLT_RTP and supply them inside nothing.

However, unlike RTP, there is no reason *not* to do that for GSMTAP - it's not as if the IP or UDP headers in a packet from a host supplying GSMTAP-encapsulated packets provide any information necessary or even useful for dissecting the encapsulated packets.

Whether it's useful, or possible, to have any interfaces on a *host* with cellular modem connectivity supply the cellular-network traffic as packets with GSMTAP headers - which appears to be what Johannes is thinking of - is another matter (but even if the answer is no, there is, as per my other message, a use for a LINKTYPE_GSMTAP/DLT_GSMTAP header type).  That might not be possible, as cellular modems, as you note, tend to hide a lot of lower-layer details from the host.


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox