[PATCH 00/10] orinoco: Remove in_interrupt() usage.

[PATCH 00/10] orinoco: Remove in_interrupt() usage.

[Sebastian Andrzej Siewior]

Hi,

this series removes the in_interrupt() usage in the orinoco driver which
is the last one in drivers/net.

The series preserves the current status based on the context I saw
during the review.

The proper way dealing with this would be to use a mutex_t as another
lock and split the current usage of orinoco_lock() and push some of the
callers into a worker where an immediate answer is not needed. This
would allow to always block on the completion and so avoid the polling.

The problem with the current approach / polling in BH context is that
USB HCDs which marked with HCD_BH (EHCI/XHCI) will complete the URB in
tasklet/BH context. The URB's callback (that part that completes
completion which is being polled for) will obviously not be invoked if
the driver polls with the orinoco_private::lock acquired which disables
BH. The only way it may work is if the USB HCD does not set HCD_BH or
the HCD IRQ fires on a different CPU.

A lot of the "BH" usage comes from the orinoco_lock(). The other callers
are fully preemtible. I didn't look how many of the callers, that
disable BH via orinoco_lock(), are already in atomic context due to
another lock, timer or so. I'm afraid I make it worse by missing
something subtle. This might still work on a OHCI/UHCI controller.

Sebastian

[PATCH 01/10] orinoco: Move context allocation after processing the skb

[Sebastian Andrzej Siewior]

ezusb_xmit() allocates a context which is leaked if
orinoco_process_xmit_skb() returns an error.

Move ezusb_alloc_ctx() after the invocation of
orinoco_process_xmit_skb() because the context is not needed so early.
ezusb_access_ltv() will cleanup the context in case of an error.

Fixes: bac6fafd4d6a0 ("orinoco: refactor xmit path")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../net/wireless/intersil/orinoco/orinoco_usb.c    | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 046f2453ad5d9..8419e13a79e2a 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1221,13 +1221,6 @@ static netdev_tx_t ezusb_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (skb->len < ETH_HLEN)
 		goto drop;
 
-	ctx = ezusb_alloc_ctx(upriv, EZUSB_RID_TX, 0);
-	if (!ctx)
-		goto busy;
-
-	memset(ctx->buf, 0, BULK_BUF_SIZE);
-	buf = ctx->buf->data;
-
 	tx_control = 0;
 
 	err = orinoco_process_xmit_skb(skb, dev, priv, &tx_control,
@@ -1235,6 +1228,13 @@ static netdev_tx_t ezusb_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (err)
 		goto drop;
 
+	ctx = ezusb_alloc_ctx(upriv, EZUSB_RID_TX, 0);
+	if (!ctx)
+		goto drop;
+
+	memset(ctx->buf, 0, BULK_BUF_SIZE);
+	buf = ctx->buf->data;
+
 	{
 		__le16 *tx_cntl = (__le16 *)buf;
 		*tx_cntl = cpu_to_le16(tx_control);
-- 
2.29.2

[PATCH 02/10] orinoco: Prepare stubs for in_interrupt() removal

[Sebastian Andrzej Siewior]

ezusb_access_ltv() sends the prepared request to the USB device.
Requests which have ->in_rid set expect an answer from the USB device
and the function has to wait until the URB with the answer arrives.
The function uses in_interrupt() to determine if it can simply sleep on
the completion and be woken up once the answer arrives or if it needs to
poll on the completion.

The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be separated or the context be conveyed in an argument passed by the
caller, which usually knows the context.

Aside of that in_interrupt() is not correct as it does not catch preempt
disabled regions in which sleeping is also not allowed.

Provide stubs which can be used as a replacement. The current default is
the current behaviour which sleeps/polls depending on in_interrupt().
The goal is to audit all callers and use either the poll or sleep
version.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../wireless/intersil/orinoco/orinoco_usb.c   | 81 ++++++++++++++++---
 1 file changed, 69 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 8419e13a79e2a..6642948809e1c 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -665,6 +665,7 @@ static void ezusb_request_in_callback(struct ezusb_priv *upriv,
 	}			/* switch */
 }
 
+typedef void (*ezusb_ctx_wait)(struct ezusb_priv *, struct request_context *);
 
 static void ezusb_req_ctx_wait(struct ezusb_priv *upriv,
 			       struct request_context *ctx)
@@ -692,6 +693,54 @@ static void ezusb_req_ctx_wait(struct ezusb_priv *upriv,
 	}
 }
 
+static void ezusb_req_ctx_wait_compl(struct ezusb_priv *upriv,
+				     struct request_context *ctx)
+{
+	switch (ctx->state) {
+	case EZUSB_CTX_QUEUED:
+	case EZUSB_CTX_REQ_SUBMITTED:
+	case EZUSB_CTX_REQ_COMPLETE:
+	case EZUSB_CTX_RESP_RECEIVED:
+		wait_for_completion(&ctx->done);
+		break;
+	default:
+		/* Done or failed - nothing to wait for */
+		break;
+	}
+}
+
+static void ezusb_req_ctx_wait_poll(struct ezusb_priv *upriv,
+				    struct request_context *ctx)
+{
+	int msecs;
+
+	switch (ctx->state) {
+	case EZUSB_CTX_QUEUED:
+	case EZUSB_CTX_REQ_SUBMITTED:
+	case EZUSB_CTX_REQ_COMPLETE:
+	case EZUSB_CTX_RESP_RECEIVED:
+		/* If we get called from a timer or with our lock acquired, then
+		 * we can't wait for the completion and have to poll. This won't
+		 * happen if the USB controller completes the URB requests in
+		 * BH.
+		 */
+		msecs = DEF_TIMEOUT * (1000 / HZ);
+
+		while (!try_wait_for_completion(&ctx->done) && msecs--)
+			udelay(1000);
+		break;
+	default:
+		/* Done or failed - nothing to wait for */
+		break;
+	}
+}
+
+static void ezusb_req_ctx_wait_skip(struct ezusb_priv *upriv,
+				    struct request_context *ctx)
+{
+	WARN(1, "Shouldn't be invoked for in_rid\n");
+}
+
 static inline u16 build_crc(struct ezusb_packet *data)
 {
 	u16 crc = 0;
@@ -853,7 +902,8 @@ static int ezusb_firmware_download(struct ezusb_priv *upriv,
 static int ezusb_access_ltv(struct ezusb_priv *upriv,
 			    struct request_context *ctx,
 			    u16 length, const void *data, u16 frame_type,
-			    void *ans_buff, unsigned ans_size, u16 *ans_length)
+			    void *ans_buff, unsigned ans_size, u16 *ans_length,
+			    ezusb_ctx_wait ezusb_ctx_wait_func)
 {
 	int req_size;
 	int retval = 0;
@@ -883,7 +933,7 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv,
 	spin_unlock_bh(&upriv->reply_count_lock);
 
 	if (ctx->in_rid)
-		ezusb_req_ctx_wait(upriv, ctx);
+		ezusb_ctx_wait_func(upriv, ctx);
 
 	state = ctx->state;
 	switch (state) {
@@ -971,7 +1021,7 @@ static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
 		frame_type = EZUSB_FRAME_CONTROL;
 
 	return ezusb_access_ltv(upriv, ctx, length, data, frame_type,
-				NULL, 0, NULL);
+				NULL, 0, NULL, ezusb_req_ctx_wait);
 }
 
 static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
@@ -988,7 +1038,7 @@ static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, 0, NULL, EZUSB_FRAME_CONTROL,
-				buf, bufsize, length);
+				buf, bufsize, length, ezusb_req_ctx_wait);
 }
 
 static int ezusb_doicmd_wait(struct hermes *hw, u16 cmd, u16 parm0, u16 parm1,
@@ -1011,7 +1061,8 @@ static int ezusb_doicmd_wait(struct hermes *hw, u16 cmd, u16 parm0, u16 parm1,
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+				ezusb_req_ctx_wait);
 }
 
 static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
@@ -1032,7 +1083,8 @@ static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+				ezusb_req_ctx_wait);
 }
 
 static int ezusb_bap_pread(struct hermes *hw, int bap,
@@ -1090,7 +1142,7 @@ static int ezusb_read_pda(struct hermes *hw, __le16 *pda,
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
 				EZUSB_FRAME_CONTROL, &pda[2], pda_len - 4,
-				NULL);
+				NULL, ezusb_req_ctx_wait);
 }
 
 static int ezusb_program_init(struct hermes *hw, u32 entry_point)
@@ -1104,7 +1156,8 @@ static int ezusb_program_init(struct hermes *hw, u32 entry_point)
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+				ezusb_req_ctx_wait);
 }
 
 static int ezusb_program_end(struct hermes *hw)
@@ -1117,7 +1170,8 @@ static int ezusb_program_end(struct hermes *hw)
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, 0, NULL,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+				ezusb_req_ctx_wait);
 }
 
 static int ezusb_program_bytes(struct hermes *hw, const char *buf,
@@ -1133,7 +1187,8 @@ static int ezusb_program_bytes(struct hermes *hw, const char *buf,
 		return -ENOMEM;
 
 	err = ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
-			       EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+			       EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+			       ezusb_req_ctx_wait);
 	if (err)
 		return err;
 
@@ -1142,7 +1197,8 @@ static int ezusb_program_bytes(struct hermes *hw, const char *buf,
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, len, buf,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL);
+				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
+				ezusb_req_ctx_wait);
 }
 
 static int ezusb_program(struct hermes *hw, const char *buf,
@@ -1262,7 +1318,8 @@ static netdev_tx_t ezusb_xmit(struct sk_buff *skb, struct net_device *dev)
 	tx_size = ALIGN(buf - ctx->buf->data, 2);
 
 	err = ezusb_access_ltv(upriv, ctx, tx_size, NULL,
-			       EZUSB_FRAME_DATA, NULL, 0, NULL);
+			       EZUSB_FRAME_DATA, NULL, 0, NULL,
+			       ezusb_req_ctx_wait);
 
 	if (err) {
 		netif_start_queue(dev);
-- 
2.29.2

[PATCH 03/10] orinoco: Annotate ezusb_xmit()

[Sebastian Andrzej Siewior]

ezusb_xmit() sets ->in_rid in its request which means it does not wait
for an answer.

Use the ezusb_req_ctx_wait_skip() to denote that an answer is not
expected.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 6642948809e1c..66d77af91d22c 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1319,7 +1319,7 @@ static netdev_tx_t ezusb_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	err = ezusb_access_ltv(upriv, ctx, tx_size, NULL,
 			       EZUSB_FRAME_DATA, NULL, 0, NULL,
-			       ezusb_req_ctx_wait);
+			       ezusb_req_ctx_wait_skip);
 
 	if (err) {
 		netif_start_queue(dev);
-- 
2.29.2

[PATCH 04/10] orinoco: Annotate ezusb_init()

[Sebastian Andrzej Siewior]

ezusb_init() is always invoked in preemptible context during device
probe. Only orinoco_up() -> orinoco_reinit_firmware() may invoke the
function from atomic context but this is never used for the USB
interface.

Use ezusb_req_ctx_wait_compl() for the ezusb_write_ltv() and
ezusb_docmd_wait() invocations from within ezusb_init().
Preserve the generic versions which have still other user via the
callback.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../wireless/intersil/orinoco/orinoco_usb.c   | 35 ++++++++++++++-----
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 66d77af91d22c..3f49e15967a48 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -994,8 +994,9 @@ static int ezusb_access_ltv(struct ezusb_priv *upriv,
 	return retval;
 }
 
-static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
-			   u16 length, const void *data)
+static int __ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
+			   u16 length, const void *data,
+			   ezusb_ctx_wait ezusb_ctx_wait_func)
 {
 	struct ezusb_priv *upriv = hw->priv;
 	u16 frame_type;
@@ -1021,7 +1022,14 @@ static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
 		frame_type = EZUSB_FRAME_CONTROL;
 
 	return ezusb_access_ltv(upriv, ctx, length, data, frame_type,
-				NULL, 0, NULL, ezusb_req_ctx_wait);
+				NULL, 0, NULL, ezusb_ctx_wait_func);
+}
+
+static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
+			   u16 length, const void *data)
+{
+	return __ezusb_write_ltv(hw, bap, rid, length, data,
+				 ezusb_req_ctx_wait);
 }
 
 static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
@@ -1065,8 +1073,9 @@ static int ezusb_doicmd_wait(struct hermes *hw, u16 cmd, u16 parm0, u16 parm1,
 				ezusb_req_ctx_wait);
 }
 
-static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
-			    struct hermes_response *resp)
+static int __ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
+			    struct hermes_response *resp,
+			    ezusb_ctx_wait ezusb_ctx_wait_func)
 {
 	struct ezusb_priv *upriv = hw->priv;
 	struct request_context *ctx;
@@ -1084,7 +1093,13 @@ static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
 				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-				ezusb_req_ctx_wait);
+				ezusb_ctx_wait_func);
+}
+
+static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
+			    struct hermes_response *resp)
+{
+	return __ezusb_docmd_wait(hw, cmd, parm0, resp, ezusb_req_ctx_wait);
 }
 
 static int ezusb_bap_pread(struct hermes *hw, int bap,
@@ -1416,14 +1431,16 @@ static int ezusb_init(struct hermes *hw)
 	usb_kill_urb(upriv->read_urb);
 	ezusb_submit_in_urb(upriv);
 
-	retval = ezusb_write_ltv(hw, 0, EZUSB_RID_INIT1,
-				 HERMES_BYTES_TO_RECLEN(2), "\x10\x00");
+	retval = __ezusb_write_ltv(hw, 0, EZUSB_RID_INIT1,
+				 HERMES_BYTES_TO_RECLEN(2), "\x10\x00",
+				 ezusb_req_ctx_wait_compl);
 	if (retval < 0) {
 		printk(KERN_ERR PFX "EZUSB_RID_INIT1 error %d\n", retval);
 		return retval;
 	}
 
-	retval = ezusb_docmd_wait(hw, HERMES_CMD_INIT, 0, NULL);
+	retval = __ezusb_docmd_wait(hw, HERMES_CMD_INIT, 0, NULL,
+				    ezusb_req_ctx_wait_compl);
 	if (retval < 0) {
 		printk(KERN_ERR PFX "HERMES_CMD_INIT error %d\n", retval);
 		return retval;
-- 
2.29.2

[PATCH 05/10] orinoco: Annotate firmware loading

[Sebastian Andrzej Siewior]

The ezusb_program() is invoked via ->program() in preemptible
context during firmware loading. This is also true for the
->program_init() and ->program_end() callback.

Use ezusb_req_ctx_wait_compl() in ezusb_program_init(),
ezusb_program_bytes(), ezusb_program_end() which are part of firmware
loading during device probe.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 3f49e15967a48..2c9c55569e8f4 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1172,7 +1172,7 @@ static int ezusb_program_init(struct hermes *hw, u32 entry_point)
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
 				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-				ezusb_req_ctx_wait);
+				ezusb_req_ctx_wait_compl);
 }
 
 static int ezusb_program_end(struct hermes *hw)
@@ -1186,7 +1186,7 @@ static int ezusb_program_end(struct hermes *hw)
 
 	return ezusb_access_ltv(upriv, ctx, 0, NULL,
 				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-				ezusb_req_ctx_wait);
+				ezusb_req_ctx_wait_compl);
 }
 
 static int ezusb_program_bytes(struct hermes *hw, const char *buf,
@@ -1203,7 +1203,7 @@ static int ezusb_program_bytes(struct hermes *hw, const char *buf,
 
 	err = ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
 			       EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-			       ezusb_req_ctx_wait);
+			       ezusb_req_ctx_wait_compl);
 	if (err)
 		return err;
 
@@ -1213,7 +1213,7 @@ static int ezusb_program_bytes(struct hermes *hw, const char *buf,
 
 	return ezusb_access_ltv(upriv, ctx, len, buf,
 				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-				ezusb_req_ctx_wait);
+				ezusb_req_ctx_wait_compl);
 }
 
 static int ezusb_program(struct hermes *hw, const char *buf,
-- 
2.29.2

[PATCH 06/10] orinoco: Annotate ezusb_read_pda()

[Sebastian Andrzej Siewior]

ezusb_read_pda() is invoked via ->read_pda() while firmware is loaded in
preemtible context.

Use ezusb_req_ctx_wait_compl() in ezusb_read_pda().

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 2c9c55569e8f4..7178889f9ec4c 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1157,7 +1157,7 @@ static int ezusb_read_pda(struct hermes *hw, __le16 *pda,
 
 	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
 				EZUSB_FRAME_CONTROL, &pda[2], pda_len - 4,
-				NULL, ezusb_req_ctx_wait);
+				NULL, ezusb_req_ctx_wait_compl);
 }
 
 static int ezusb_program_init(struct hermes *hw, u32 entry_point)
-- 
2.29.2

[PATCH 07/10] orinoco: Annotate ezusb_write_ltv()

[Sebastian Andrzej Siewior]

All invocation of ezusb_write_ltv() happen via ->write_ltv() and are
performed under the orinoco_lock() which disables BH.

Use ezusb_req_ctx_wait_poll() for ezusb_write_ltv() because it must not
sleep.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 7178889f9ec4c..d12bd5be4cde6 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1029,7 +1029,7 @@ static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
 			   u16 length, const void *data)
 {
 	return __ezusb_write_ltv(hw, bap, rid, length, data,
-				 ezusb_req_ctx_wait);
+				 ezusb_req_ctx_wait_poll);
 }
 
 static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
-- 
2.29.2

[PATCH 08/10] orinoco: Remove ezusb_doicmd_wait()

[Sebastian Andrzej Siewior]

ezusb_doicmd_wait() is invoked via ->init_cmd_wait() callback.
This callback is only invoked hermesi_program_init() and
hermesi_program_end() which are the ->program_init() and ->program_end()
callbacks as assigned by `hermes_ops_local'. They are never used by the
USB interface since the USB interface provides its own set of callbacks
by `ezusb_ops'.

Replace ezusb_doicmd_wait() with a warning in case I missed the obvious.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../wireless/intersil/orinoco/orinoco_usb.c   | 21 ++-----------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index d12bd5be4cde6..c8508a3d652b0 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1052,25 +1052,8 @@ static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
 static int ezusb_doicmd_wait(struct hermes *hw, u16 cmd, u16 parm0, u16 parm1,
 			     u16 parm2, struct hermes_response *resp)
 {
-	struct ezusb_priv *upriv = hw->priv;
-	struct request_context *ctx;
-
-	__le16 data[4] = {
-		cpu_to_le16(cmd),
-		cpu_to_le16(parm0),
-		cpu_to_le16(parm1),
-		cpu_to_le16(parm2),
-	};
-	netdev_dbg(upriv->dev,
-		   "0x%04X, parm0 0x%04X, parm1 0x%04X, parm2 0x%04X\n", cmd,
-		   parm0, parm1, parm2);
-	ctx = ezusb_alloc_ctx(upriv, EZUSB_RID_DOCMD, EZUSB_RID_ACK);
-	if (!ctx)
-		return -ENOMEM;
-
-	return ezusb_access_ltv(upriv, ctx, sizeof(data), &data,
-				EZUSB_FRAME_CONTROL, NULL, 0, NULL,
-				ezusb_req_ctx_wait);
+	WARN_ON_ONCE(1);
+	return -EINVAL;
 }
 
 static int __ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
-- 
2.29.2

[PATCH 09/10] orinoco: Annotate ezusb_docmd_wait()

[Sebastian Andrzej Siewior]

All invocations of ezusb_docmd_wait() happen via ->cmd_wait(). This
callback is always invoked under the orinoco_lock() which disables BH.

Use ezusb_req_ctx_wait_poll() for ezusb_docmd_wait() because it must not
sleep.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/net/wireless/intersil/orinoco/orinoco_usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index c8508a3d652b0..4c60b48214637 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -1082,7 +1082,7 @@ static int __ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
 static int ezusb_docmd_wait(struct hermes *hw, u16 cmd, u16 parm0,
 			    struct hermes_response *resp)
 {
-	return __ezusb_docmd_wait(hw, cmd, parm0, resp, ezusb_req_ctx_wait);
+	return __ezusb_docmd_wait(hw, cmd, parm0, resp, ezusb_req_ctx_wait_poll);
 }
 
 static int ezusb_bap_pread(struct hermes *hw, int bap,
-- 
2.29.2

[PATCH 10/10] orinoco: Annotate ezusb_read_ltv()

[Sebastian Andrzej Siewior]

ezusb_read_ltv() is always invoked via the ->read_ltv() callback. This
callback is mostly invoked under orinoco_lock() which disables BH.

There are a few invocations during probe which occur in preemptible
context via:
   ezusb_probe() -> orinoco_init() -> determine_fw_capabilities()

Extend `hermes_ops' with the ->read_ltv_pr callback which is implemented
with the same callback like ->read_ltv on `hermes_ops_local'.
On `ezusb_ops' ->read_ltv is used for callbacks under the lock which
need to poll.
The new ->read_ltv_pr() is used in the preemptible context in which it
is possible to wait for the completion. Provide HERMES_READ_RECORD_PR()
and hermes_read_wordrec_pr() which behave like their non _pr equivalents
and invoke ->read_ltv_pr().

This removes the last user of ezusb_req_ctx_wait() and can now be
removed.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 .../net/wireless/intersil/orinoco/hermes.c    |  1 +
 .../net/wireless/intersil/orinoco/hermes.h    | 15 ++++++
 drivers/net/wireless/intersil/orinoco/hw.c    | 32 ++++++------
 .../wireless/intersil/orinoco/orinoco_usb.c   | 49 ++++++++-----------
 4 files changed, 52 insertions(+), 45 deletions(-)

diff --git a/drivers/net/wireless/intersil/orinoco/hermes.c b/drivers/net/wireless/intersil/orinoco/hermes.c
index 43790fbea0e00..6d4b7f64efcf5 100644
--- a/drivers/net/wireless/intersil/orinoco/hermes.c
+++ b/drivers/net/wireless/intersil/orinoco/hermes.c
@@ -763,6 +763,7 @@ static const struct hermes_ops hermes_ops_local = {
 	.init_cmd_wait = hermes_doicmd_wait,
 	.allocate = hermes_allocate,
 	.read_ltv = hermes_read_ltv,
+	.read_ltv_pr = hermes_read_ltv,
 	.write_ltv = hermes_write_ltv,
 	.bap_pread = hermes_bap_pread,
 	.bap_pwrite = hermes_bap_pwrite,
diff --git a/drivers/net/wireless/intersil/orinoco/hermes.h b/drivers/net/wireless/intersil/orinoco/hermes.h
index 9f668185b7d28..3dc561a5cb7ae 100644
--- a/drivers/net/wireless/intersil/orinoco/hermes.h
+++ b/drivers/net/wireless/intersil/orinoco/hermes.h
@@ -386,6 +386,8 @@ struct hermes_ops {
 	int (*allocate)(struct hermes *hw, u16 size, u16 *fid);
 	int (*read_ltv)(struct hermes *hw, int bap, u16 rid, unsigned buflen,
 			u16 *length, void *buf);
+	int (*read_ltv_pr)(struct hermes *hw, int bap, u16 rid,
+			      unsigned buflen, u16 *length, void *buf);
 	int (*write_ltv)(struct hermes *hw, int bap, u16 rid,
 			 u16 length, const void *value);
 	int (*bap_pread)(struct hermes *hw, int bap, void *buf, int len,
@@ -494,6 +496,8 @@ static inline void hermes_clear_words(struct hermes *hw, int off,
 
 #define HERMES_READ_RECORD(hw, bap, rid, buf) \
 	(hw->ops->read_ltv((hw), (bap), (rid), sizeof(*buf), NULL, (buf)))
+#define HERMES_READ_RECORD_PR(hw, bap, rid, buf) \
+	(hw->ops->read_ltv_pr((hw), (bap), (rid), sizeof(*buf), NULL, (buf)))
 #define HERMES_WRITE_RECORD(hw, bap, rid, buf) \
 	(hw->ops->write_ltv((hw), (bap), (rid), \
 			    HERMES_BYTES_TO_RECLEN(sizeof(*buf)), (buf)))
@@ -509,6 +513,17 @@ static inline int hermes_read_wordrec(struct hermes *hw, int bap, u16 rid,
 	return err;
 }
 
+static inline int hermes_read_wordrec_pr(struct hermes *hw, int bap, u16 rid,
+					 u16 *word)
+{
+	__le16 rec;
+	int err;
+
+	err = HERMES_READ_RECORD_PR(hw, bap, rid, &rec);
+	*word = le16_to_cpu(rec);
+	return err;
+}
+
 static inline int hermes_write_wordrec(struct hermes *hw, int bap, u16 rid,
 				       u16 word)
 {
diff --git a/drivers/net/wireless/intersil/orinoco/hw.c b/drivers/net/wireless/intersil/orinoco/hw.c
index 61af5a28f269f..2c7adb4be1003 100644
--- a/drivers/net/wireless/intersil/orinoco/hw.c
+++ b/drivers/net/wireless/intersil/orinoco/hw.c
@@ -78,7 +78,7 @@ int determine_fw_capabilities(struct orinoco_private *priv,
 	char tmp[SYMBOL_MAX_VER_LEN + 1] __attribute__((aligned(2)));
 
 	/* Get the hardware version */
-	err = HERMES_READ_RECORD(hw, USER_BAP, HERMES_RID_NICID, &nic_id);
+	err = HERMES_READ_RECORD_PR(hw, USER_BAP, HERMES_RID_NICID, &nic_id);
 	if (err) {
 		dev_err(dev, "Cannot read hardware identity: error %d\n",
 			err);
@@ -101,7 +101,7 @@ int determine_fw_capabilities(struct orinoco_private *priv,
 	priv->firmware_type = determine_firmware_type(&nic_id);
 
 	/* Get the firmware version */
-	err = HERMES_READ_RECORD(hw, USER_BAP, HERMES_RID_STAID, &sta_id);
+	err = HERMES_READ_RECORD_PR(hw, USER_BAP, HERMES_RID_STAID, &sta_id);
 	if (err) {
 		dev_err(dev, "Cannot read station identity: error %d\n",
 			err);
@@ -177,7 +177,7 @@ int determine_fw_capabilities(struct orinoco_private *priv,
 		/* 3Com MAC : 00:50:DA:* */
 		memset(tmp, 0, sizeof(tmp));
 		/* Get the Symbol firmware version */
-		err = hw->ops->read_ltv(hw, USER_BAP,
+		err = hw->ops->read_ltv_pr(hw, USER_BAP,
 					HERMES_RID_SECONDARYVERSION_SYMBOL,
 					SYMBOL_MAX_VER_LEN, NULL, &tmp);
 		if (err) {
@@ -286,7 +286,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	u16 reclen;
 
 	/* Get the MAC address */
-	err = hw->ops->read_ltv(hw, USER_BAP, HERMES_RID_CNFOWNMACADDR,
+	err = hw->ops->read_ltv_pr(hw, USER_BAP, HERMES_RID_CNFOWNMACADDR,
 				ETH_ALEN, NULL, dev_addr);
 	if (err) {
 		dev_warn(dev, "Failed to read MAC address!\n");
@@ -296,7 +296,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	dev_dbg(dev, "MAC address %pM\n", dev_addr);
 
 	/* Get the station name */
-	err = hw->ops->read_ltv(hw, USER_BAP, HERMES_RID_CNFOWNNAME,
+	err = hw->ops->read_ltv_pr(hw, USER_BAP, HERMES_RID_CNFOWNNAME,
 				sizeof(nickbuf), &reclen, &nickbuf);
 	if (err) {
 		dev_err(dev, "failed to read station name\n");
@@ -312,7 +312,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	dev_dbg(dev, "Station name \"%s\"\n", priv->nick);
 
 	/* Get allowed channels */
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_CHANNELLIST,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_CHANNELLIST,
 				  &priv->channel_mask);
 	if (err) {
 		dev_err(dev, "Failed to read channel list!\n");
@@ -320,13 +320,13 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	}
 
 	/* Get initial AP density */
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_CNFSYSTEMSCALE,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_CNFSYSTEMSCALE,
 				  &priv->ap_density);
 	if (err || priv->ap_density < 1 || priv->ap_density > 3)
 		priv->has_sensitivity = 0;
 
 	/* Get initial RTS threshold */
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_CNFRTSTHRESHOLD,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_CNFRTSTHRESHOLD,
 				  &priv->rts_thresh);
 	if (err) {
 		dev_err(dev, "Failed to read RTS threshold!\n");
@@ -335,11 +335,11 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 
 	/* Get initial fragmentation settings */
 	if (priv->has_mwo)
-		err = hermes_read_wordrec(hw, USER_BAP,
+		err = hermes_read_wordrec_pr(hw, USER_BAP,
 					  HERMES_RID_CNFMWOROBUST_AGERE,
 					  &priv->mwo_robust);
 	else
-		err = hermes_read_wordrec(hw, USER_BAP,
+		err = hermes_read_wordrec_pr(hw, USER_BAP,
 					  HERMES_RID_CNFFRAGMENTATIONTHRESHOLD,
 					  &priv->frag_thresh);
 	if (err) {
@@ -351,7 +351,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	if (priv->has_pm) {
 		priv->pm_on = 0;
 		priv->pm_mcast = 1;
-		err = hermes_read_wordrec(hw, USER_BAP,
+		err = hermes_read_wordrec_pr(hw, USER_BAP,
 					  HERMES_RID_CNFMAXSLEEPDURATION,
 					  &priv->pm_period);
 		if (err) {
@@ -359,7 +359,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 				"period!\n");
 			goto out;
 		}
-		err = hermes_read_wordrec(hw, USER_BAP,
+		err = hermes_read_wordrec_pr(hw, USER_BAP,
 					  HERMES_RID_CNFPMHOLDOVERDURATION,
 					  &priv->pm_timeout);
 		if (err) {
@@ -371,7 +371,7 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 
 	/* Preamble setup */
 	if (priv->has_preamble) {
-		err = hermes_read_wordrec(hw, USER_BAP,
+		err = hermes_read_wordrec_pr(hw, USER_BAP,
 					  HERMES_RID_CNFPREAMBLE_SYMBOL,
 					  &priv->preamble);
 		if (err) {
@@ -381,21 +381,21 @@ int orinoco_hw_read_card_settings(struct orinoco_private *priv, u8 *dev_addr)
 	}
 
 	/* Retry settings */
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_SHORTRETRYLIMIT,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_SHORTRETRYLIMIT,
 				  &priv->short_retry_limit);
 	if (err) {
 		dev_err(dev, "Failed to read short retry limit\n");
 		goto out;
 	}
 
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_LONGRETRYLIMIT,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_LONGRETRYLIMIT,
 				  &priv->long_retry_limit);
 	if (err) {
 		dev_err(dev, "Failed to read long retry limit\n");
 		goto out;
 	}
 
-	err = hermes_read_wordrec(hw, USER_BAP, HERMES_RID_MAXTRANSMITLIFETIME,
+	err = hermes_read_wordrec_pr(hw, USER_BAP, HERMES_RID_MAXTRANSMITLIFETIME,
 				  &priv->retry_lifetime);
 	if (err) {
 		dev_err(dev, "Failed to read max retry lifetime\n");
diff --git a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
index 4c60b48214637..dd31929261ab9 100644
--- a/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
+++ b/drivers/net/wireless/intersil/orinoco/orinoco_usb.c
@@ -667,32 +667,6 @@ static void ezusb_request_in_callback(struct ezusb_priv *upriv,
 
 typedef void (*ezusb_ctx_wait)(struct ezusb_priv *, struct request_context *);
 
-static void ezusb_req_ctx_wait(struct ezusb_priv *upriv,
-			       struct request_context *ctx)
-{
-	switch (ctx->state) {
-	case EZUSB_CTX_QUEUED:
-	case EZUSB_CTX_REQ_SUBMITTED:
-	case EZUSB_CTX_REQ_COMPLETE:
-	case EZUSB_CTX_RESP_RECEIVED:
-		if (in_softirq()) {
-			/* If we get called from a timer, timeout timers don't
-			 * get the chance to run themselves. So we make sure
-			 * that we don't sleep for ever */
-			int msecs = DEF_TIMEOUT * (1000 / HZ);
-
-			while (!try_wait_for_completion(&ctx->done) && msecs--)
-				udelay(1000);
-		} else {
-			wait_for_completion(&ctx->done);
-		}
-		break;
-	default:
-		/* Done or failed - nothing to wait for */
-		break;
-	}
-}
-
 static void ezusb_req_ctx_wait_compl(struct ezusb_priv *upriv,
 				     struct request_context *ctx)
 {
@@ -1032,8 +1006,10 @@ static int ezusb_write_ltv(struct hermes *hw, int bap, u16 rid,
 				 ezusb_req_ctx_wait_poll);
 }
 
-static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
-			  unsigned bufsize, u16 *length, void *buf)
+static int __ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
+			    unsigned bufsize, u16 *length, void *buf,
+			    ezusb_ctx_wait ezusb_ctx_wait_func)
+
 {
 	struct ezusb_priv *upriv = hw->priv;
 	struct request_context *ctx;
@@ -1046,7 +1022,21 @@ static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
 		return -ENOMEM;
 
 	return ezusb_access_ltv(upriv, ctx, 0, NULL, EZUSB_FRAME_CONTROL,
-				buf, bufsize, length, ezusb_req_ctx_wait);
+				buf, bufsize, length, ezusb_req_ctx_wait_poll);
+}
+
+static int ezusb_read_ltv(struct hermes *hw, int bap, u16 rid,
+			    unsigned bufsize, u16 *length, void *buf)
+{
+	return __ezusb_read_ltv(hw, bap, rid, bufsize, length, buf,
+				ezusb_req_ctx_wait_poll);
+}
+
+static int ezusb_read_ltv_preempt(struct hermes *hw, int bap, u16 rid,
+				  unsigned bufsize, u16 *length, void *buf)
+{
+	return __ezusb_read_ltv(hw, bap, rid, bufsize, length, buf,
+				ezusb_req_ctx_wait_compl);
 }
 
 static int ezusb_doicmd_wait(struct hermes *hw, u16 cmd, u16 parm0, u16 parm1,
@@ -1586,6 +1576,7 @@ static const struct hermes_ops ezusb_ops = {
 	.init_cmd_wait = ezusb_doicmd_wait,
 	.allocate = ezusb_allocate,
 	.read_ltv = ezusb_read_ltv,
+	.read_ltv_pr = ezusb_read_ltv_preempt,
 	.write_ltv = ezusb_write_ltv,
 	.bap_pread = ezusb_bap_pread,
 	.read_pda = ezusb_read_pda,
-- 
2.29.2

Re: [PATCH 01/10] orinoco: Move context allocation after processing the skb

[Kalle Valo]

Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:

> ezusb_xmit() allocates a context which is leaked if
> orinoco_process_xmit_skb() returns an error.
> 
> Move ezusb_alloc_ctx() after the invocation of
> orinoco_process_xmit_skb() because the context is not needed so early.
> ezusb_access_ltv() will cleanup the context in case of an error.
> 
> Fixes: bac6fafd4d6a0 ("orinoco: refactor xmit path")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

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

a31eb615646a orinoco: Move context allocation after processing the skb
915fd9db418f orinoco: Prepare stubs for in_interrupt() removal
29b2fe3f6f20 orinoco: Annotate ezusb_xmit()
46233f778604 orinoco: Annotate ezusb_init()
69476b25a4ba orinoco: Annotate firmware loading
154f2bae9bed orinoco: Annotate ezusb_read_pda()
39628fe48cb3 orinoco: Annotate ezusb_write_ltv()
47ab2a951eb9 orinoco: Remove ezusb_doicmd_wait()
a9a2678abacb orinoco: Annotate ezusb_docmd_wait()
a3d8a2599d47 orinoco: Annotate ezusb_read_ltv()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20201113212252.2243570-2-bigeasy@linutronix.de/

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