| Message ID | 20210829124528.507457-1-dmitry.baryshkov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net: qrtr: mhi: make it work again | expand |
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 > 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 <dmitry.baryshkov@linaro.org> > --- > 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 >
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;
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 <dmitry.baryshkov@linaro.org> --- net/qrtr/mhi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) -- 2.33.0