[PATCH] net: qrtr: mhi: make it work again

Subsystems: networking [general], qualcomm ipc router (qrtr) driver, the rest

STALE1810d

2 messages, 2 authors, 2021-08-29 · open the first message on its own page

[PATCH] net: qrtr: mhi: make it work again

From: Dmitry Baryshkov <hidden>
Date: 2021-08-29 12:45:35

The commit ce78ffa3ef16 ("net: really fix the build...") introduced two
issues into the mhi.c driver:
 - use of initialized completion
 - calling mhi_prepare_for_transfer twice

While the first one is pretty obvious, the second one makes all devices
using mhi.c to return -EINVAL during probe. Fist
mhi_prepare_for_transfer() would change both channels state to ENABLED.
Then when second mhi_prepare_for_transfer() would be called it would
also try switching them to ENABLED again, which is forbidden by the
state machine in the mhi_update_channel_state() function (see
drivers/bus/mhi/core/main.c).
These two issues make all drivers using qcom_mhi_qrtr (e.g. ath11k) to
fail with -EINVAL.

Fix them by removing first mhi_prepare_for_transfer() call and by adding
the init_completion() call.

Fixes: ce78ffa3ef16 ("net: really fix the build...")
Signed-off-by: Dmitry Baryshkov <redacted>
---
 net/qrtr/mhi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 1dc955ca57d3..f3f4a5fdeaf3 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -83,15 +83,12 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	struct qrtr_mhi_dev *qdev;
 	int rc;
 
-	/* start channels */
-	rc = mhi_prepare_for_transfer(mhi_dev, 0);
-	if (rc)
-		return rc;
-
 	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
 	if (!qdev)
 		return -ENOMEM;
 
+	init_completion(&qdev->ready);
+
 	qdev->mhi_dev = mhi_dev;
 	qdev->dev = &mhi_dev->dev;
 	qdev->ep.xmit = qcom_mhi_qrtr_send;
-- 
2.33.0

Re: [PATCH] net: qrtr: mhi: make it work again

From: Manivannan Sadhasivam <hidden>
Date: 2021-08-29 13:49:08

Hi Dmitry,

On Sun, Aug 29, 2021 at 03:45:28PM +0300, Dmitry Baryshkov wrote:
The commit ce78ffa3ef16 ("net: really fix the build...") introduced two
issues into the mhi.c driver:
 - use of initialized completion
 - calling mhi_prepare_for_transfer twice
ce78ffa3ef16 got reverted in net-next:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/net/qrtr?id=9ebc2758d0bbed951511d1709be0717178ec2660

And you also need the below commit:
https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?h=char-misc-next&id=0dc3ad3f859d3a65b335c861ec342d31d91e8bc8

Both commits are in linux-next so the issue should be fixed there and also in
the mainline soon.

Thanks,
Mani
quoted hunk
While the first one is pretty obvious, the second one makes all devices
using mhi.c to return -EINVAL during probe. Fist
mhi_prepare_for_transfer() would change both channels state to ENABLED.
Then when second mhi_prepare_for_transfer() would be called it would
also try switching them to ENABLED again, which is forbidden by the
state machine in the mhi_update_channel_state() function (see
drivers/bus/mhi/core/main.c).
These two issues make all drivers using qcom_mhi_qrtr (e.g. ath11k) to
fail with -EINVAL.

Fix them by removing first mhi_prepare_for_transfer() call and by adding
the init_completion() call.

Fixes: ce78ffa3ef16 ("net: really fix the build...")
Signed-off-by: Dmitry Baryshkov <redacted>
---
 net/qrtr/mhi.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 1dc955ca57d3..f3f4a5fdeaf3 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -83,15 +83,12 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	struct qrtr_mhi_dev *qdev;
 	int rc;
 
-	/* start channels */
-	rc = mhi_prepare_for_transfer(mhi_dev, 0);
-	if (rc)
-		return rc;
-
 	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
 	if (!qdev)
 		return -ENOMEM;
 
+	init_completion(&qdev->ready);
+
 	qdev->mhi_dev = mhi_dev;
 	qdev->dev = &mhi_dev->dev;
 	qdev->ep.xmit = qcom_mhi_qrtr_send;
-- 
2.33.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help