net/qrtr/mhi.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-)
The call to qrtr_endpoint_register() was moved before
mhi_prepare_for_transfer_autoqueue() to prevent a case where a dl
callback can occur before the qrtr endpoint is registered.
Now the reverse can happen where qrtr will try to send a packet
before the channels are prepared. The correct sequence needs to be
prepare the mhi channel, register the qrtr endpoint, queue buffers for
receiving dl transfers.
Since qrtr will not use mhi_prepare_for_transfer_autoqueue(), qrtr must
do the buffer management and requeue the buffers in the dl_callback.
Sizing of the buffers will be inherited from the mhi controller
settings.
Fixes: 68a838b84eff ("net: qrtr: start MHI channel after endpoit creation")
Reported-by: Johan Hovold <johan@kernel.org>
Closes: https://lore.kernel.org/linux-arm-msm/ZyTtVdkCCES0lkl4@hovoldconsulting.com/
Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
---
net/qrtr/mhi.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 47 insertions(+), 5 deletions(-)
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 69f53625a049..5e7476afb6b4 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -15,6 +15,8 @@ struct qrtr_mhi_dev {
struct qrtr_endpoint ep;
struct mhi_device *mhi_dev;
struct device *dev;
+
+ size_t dl_buf_len;
};
/* From MHI to QRTR */
@@ -24,13 +26,22 @@ static void qcom_mhi_qrtr_dl_callback(struct mhi_device *mhi_dev,
struct qrtr_mhi_dev *qdev = dev_get_drvdata(&mhi_dev->dev);
int rc;
- if (!qdev || mhi_res->transaction_status)
+ if (!qdev)
+ return;
+
+ if (mhi_res->transaction_status == -ENOTCONN) {
+ devm_kfree(qdev->dev, mhi_res->buf_addr);
+ return;
+ } else if (mhi_res->transaction_status) {
return;
+ }
rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
mhi_res->bytes_xferd);
if (rc == -EINVAL)
dev_err(qdev->dev, "invalid ipcrouter packet\n");
+
+ rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, mhi_res->buf_addr, qdev->dl_buf_len, MHI_EOT);
}
/* From QRTR to MHI */
@@ -72,6 +83,30 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
return rc;
}
+static int qrtr_mhi_queue_rx(struct qrtr_mhi_dev *qdev)
+{
+ struct mhi_device *mhi_dev = qdev->mhi_dev;
+ struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+ int rc = 0;
+ int nr_el;
+
+ qdev->dl_buf_len = mhi_cntrl->buffer_len;
+ nr_el = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
+ while (nr_el--) {
+ void *buf;
+
+ buf = devm_kzalloc(qdev->dev, qdev->dl_buf_len, GFP_KERNEL);
+ if (!buf) {
+ rc = -ENOMEM;
+ break;
+ }
+ rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, qdev->dl_buf_len, MHI_EOT);
+ if (rc)
+ break;
+ }
+ return rc;
+}
+
static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
const struct mhi_device_id *id)
{
@@ -87,17 +122,24 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
qdev->ep.xmit = qcom_mhi_qrtr_send;
dev_set_drvdata(&mhi_dev->dev, qdev);
- rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
+
+ /* start channels */
+ rc = mhi_prepare_for_transfer(mhi_dev);
if (rc)
return rc;
- /* start channels */
- rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
+ rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
if (rc) {
- qrtr_endpoint_unregister(&qdev->ep);
+ mhi_unprepare_from_transfer(mhi_dev);
return rc;
}
+ rc = qrtr_mhi_queue_rx(qdev);
+ if (rc) {
+ qrtr_endpoint_unregister(&qdev->ep);
+ mhi_unprepare_from_transfer(mhi_dev);
+ }
+
dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
return 0;
---
base-commit: f48887a98b78880b7711aca311fbbbcaad6c4e3b
change-id: 20250508-qrtr_mhi_auto-3a3567456f95
Best regards,
--
Chris Lew <chris.lew@oss.qualcomm.com>On Wed, Jun 04, 2025 at 02:05:42PM -0700, Chris Lew wrote:
> The call to qrtr_endpoint_register() was moved before
> mhi_prepare_for_transfer_autoqueue() to prevent a case where a dl
> callback can occur before the qrtr endpoint is registered.
>
> Now the reverse can happen where qrtr will try to send a packet
> before the channels are prepared. The correct sequence needs to be
> prepare the mhi channel, register the qrtr endpoint, queue buffers for
> receiving dl transfers.
>
> Since qrtr will not use mhi_prepare_for_transfer_autoqueue(), qrtr must
> do the buffer management and requeue the buffers in the dl_callback.
> Sizing of the buffers will be inherited from the mhi controller
> settings.
>
> Fixes: 68a838b84eff ("net: qrtr: start MHI channel after endpoit creation")
> Reported-by: Johan Hovold <johan@kernel.org>
> Closes: https://lore.kernel.org/linux-arm-msm/ZyTtVdkCCES0lkl4@hovoldconsulting.com/
> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
Thanks for the update. I believe this one should have a stable tag as
well as it fixes a critical boot failure on Qualcomm platforms that we
hit frequently with the in-kernel pd-mapper.
And it indeed fixes the crash:
Tested-by: Johan Hovold <johan+linaro@kernel.org>
> /* From MHI to QRTR */
> @@ -24,13 +26,22 @@ static void qcom_mhi_qrtr_dl_callback(struct mhi_device *mhi_dev,
> struct qrtr_mhi_dev *qdev = dev_get_drvdata(&mhi_dev->dev);
> int rc;
>
> - if (!qdev || mhi_res->transaction_status)
> + if (!qdev)
> + return;
> +
> + if (mhi_res->transaction_status == -ENOTCONN) {
> + devm_kfree(qdev->dev, mhi_res->buf_addr);
Why do you need to free this buffer here?
AFAICS, all buffers are allocated at probe() and freed at (after)
remove().
> + return;
> + } else if (mhi_res->transaction_status) {
> return;
> + }
>
> rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> mhi_res->bytes_xferd);
> if (rc == -EINVAL)
> dev_err(qdev->dev, "invalid ipcrouter packet\n");
> +
> + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, mhi_res->buf_addr, qdev->dl_buf_len, MHI_EOT);
Please try to stay within 80 columns except when not doing so
significantly improves readability.
Also you don't do anything with rc here. Should you log an error at
least?
> }
> +static int qrtr_mhi_queue_rx(struct qrtr_mhi_dev *qdev)
> +{
> + struct mhi_device *mhi_dev = qdev->mhi_dev;
> + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> + int rc = 0;
> + int nr_el;
> +
> + qdev->dl_buf_len = mhi_cntrl->buffer_len;
> + nr_el = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> + while (nr_el--) {
> + void *buf;
> +
> + buf = devm_kzalloc(qdev->dev, qdev->dl_buf_len, GFP_KERNEL);
> + if (!buf) {
> + rc = -ENOMEM;
> + break;
> + }
> + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, qdev->dl_buf_len, MHI_EOT);
80 cols here too.
> + if (rc)
> + break;
> + }
> + return rc;
> +}
> +
> static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> const struct mhi_device_id *id)
> {
> @@ -87,17 +122,24 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> qdev->ep.xmit = qcom_mhi_qrtr_send;
>
> dev_set_drvdata(&mhi_dev->dev, qdev);
> - rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
> +
> + /* start channels */
> + rc = mhi_prepare_for_transfer(mhi_dev);
> if (rc)
> return rc;
>
> - /* start channels */
> - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
> + rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
> if (rc) {
> - qrtr_endpoint_unregister(&qdev->ep);
> + mhi_unprepare_from_transfer(mhi_dev);
> return rc;
> }
>
> + rc = qrtr_mhi_queue_rx(qdev);
> + if (rc) {
> + qrtr_endpoint_unregister(&qdev->ep);
> + mhi_unprepare_from_transfer(mhi_dev);
Jakub already pointed out the missing return here. Perhaps you should
consider adding error labels for the unwinding.
> + }
> +
> dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
>
> return 0;
Johan
On Wed, Jun 18, 2025 at 09:53:34AM +0200, Johan Hovold wrote:
> On Wed, Jun 04, 2025 at 02:05:42PM -0700, Chris Lew wrote:
> > The call to qrtr_endpoint_register() was moved before
> > mhi_prepare_for_transfer_autoqueue() to prevent a case where a dl
> > callback can occur before the qrtr endpoint is registered.
> >
> > Now the reverse can happen where qrtr will try to send a packet
> > before the channels are prepared. The correct sequence needs to be
> > prepare the mhi channel, register the qrtr endpoint, queue buffers for
> > receiving dl transfers.
> >
> > Since qrtr will not use mhi_prepare_for_transfer_autoqueue(), qrtr must
> > do the buffer management and requeue the buffers in the dl_callback.
> > Sizing of the buffers will be inherited from the mhi controller
> > settings.
> >
> > Fixes: 68a838b84eff ("net: qrtr: start MHI channel after endpoit creation")
> > Reported-by: Johan Hovold <johan@kernel.org>
> > Closes: https://lore.kernel.org/linux-arm-msm/ZyTtVdkCCES0lkl4@hovoldconsulting.com/
> > Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
>
> Thanks for the update. I believe this one should have a stable tag as
> well as it fixes a critical boot failure on Qualcomm platforms that we
> hit frequently with the in-kernel pd-mapper.
>
> And it indeed fixes the crash:
>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>
While it fixes the registration race and NULL-deref, something else is
not right with the patch.
On resume from suspend I now get a bunch of mhi errors for the ath12k
wifi:
[ 25.843963] mhi mhi1: Requested to power ON
[ 25.848766] mhi mhi1: Power on setup success
[ 25.939124] mhi mhi1: Wait for device to enter SBL or Mission mode
[ 26.325393] mhi mhi1: Error recycling buffer for chan:21
[ 26.331193] mhi mhi1: Error recycling buffer for chan:21
[ 26.336798] mhi mhi1: Error recycling buffer for chan:21
[ 26.342390] mhi mhi1: Error recycling buffer for chan:21
[ 26.347994] mhi mhi1: Error recycling buffer for chan:21
[ 26.353609] mhi mhi1: Error recycling buffer for chan:21
[ 26.359207] mhi mhi1: Error recycling buffer for chan:21
...
and after that there's a warning at shutdown when tearing down mhi:
[ 36.384573] WARNING: CPU: 5 PID: 109 at mm/slub.c:4753 free_large_kmalloc+0x13c/0x160
[ 36.552152] CPU: 5 UID: 0 PID: 109 Comm: kworker/u52:0 Not tainted 6.16.0-rc2 #10 PREEMPT
[ 36.560724] Hardware name: Qualcomm CRD, BIOS 6.0.241007.BOOT.MXF.2.4-00534.1-HAMOA-1 10/ 7/2024
[ 36.569835] Workqueue: mhi_hiprio_wq mhi_pm_st_worker [mhi]
[ 36.575648] pstate: 21400005 (nzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 36.582882] pc : free_large_kmalloc+0x13c/0x160
[ 36.587610] lr : kfree+0x208/0x32c
[ 36.591166] sp : ffff80008107b900
[ 36.594636] x29: ffff80008107b900 x28: 0000000000000000 x27: ffff800082b9d690
[ 36.602045] x26: ffff800082f681e0 x25: ffff800082f681e8 x24: 00000000ffffffff
[ 36.609454] x23: ffff00080406cd80 x22: 0000000000000001 x21: ffff0008023f2000
[ 36.616863] x20: 05a2dd88f4602478 x19: fffffdffe008fc80 x18: 00000000000c8dc0
[ 36.624272] x17: 0000000000000028 x16: ffffdd893588f02c x15: ffffdd8936a28928
[ 36.631681] x14: ffffdd8936af16e8 x13: 0000000000008000 x12: 0000000000000000
[ 36.639097] x11: ffffdd893709c968 x10: 0000000000000001 x9 : ffff0008099c95c0
[ 36.646505] x8 : 0000001000000000 x7 : ffff0008099c95c0 x6 : 00000008823f2000
[ 36.653915] x5 : ffffdd8937417f60 x4 : 0000000000000020 x3 : ffff000801c2d7e0
[ 36.661324] x2 : 0bfffe0000000000 x1 : ffff0008023f2000 x0 : 00000000000000ff
[ 36.668733] Call trace:
[ 36.671307] free_large_kmalloc+0x13c/0x160 (P)
[ 36.676036] kfree+0x208/0x32c
[ 36.679241] mhi_reset_chan+0x1d4/0x2e4 [mhi]
[ 36.683786] mhi_driver_remove+0x1bc/0x1fc [mhi]
[ 36.688597] device_remove+0x70/0x80
[ 36.692341] device_release_driver_internal+0x1e4/0x240
[ 36.697778] device_release_driver+0x18/0x24
[ 36.702233] bus_remove_device+0xd0/0x148
[ 36.706424] device_del+0x148/0x374
[ 36.710077] mhi_destroy_device+0xb0/0x13c [mhi]
[ 36.714888] device_for_each_child+0x60/0xbc
[ 36.719344] mhi_pm_disable_transition+0x154/0x510 [mhi]
[ 36.724875] mhi_pm_st_worker+0x2dc/0xb18 [mhi]
[ 36.729594] process_one_work+0x20c/0x610
[ 36.733788] worker_thread+0x244/0x388
[ 36.737711] kthread+0x150/0x220
[ 36.741093] ret_from_fork+0x10/0x20
Johan
On Wed, 04 Jun 2025 14:05:42 -0700 Chris Lew wrote:
> + rc = qrtr_mhi_queue_rx(qdev);
> + if (rc) {
> + qrtr_endpoint_unregister(&qdev->ep);
> + mhi_unprepare_from_transfer(mhi_dev);
is ignoring the rc here intentional? may be worth a comment
> + }
> +
> dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
>
> return 0;
Note that we return 0 here, not rc
--
pw-bot: cr
On Mon, Jun 9, 2025 at 4:00 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 04 Jun 2025 14:05:42 -0700 Chris Lew wrote:
> > + rc = qrtr_mhi_queue_rx(qdev);
> > + if (rc) {
> > + qrtr_endpoint_unregister(&qdev->ep);
> > + mhi_unprepare_from_transfer(mhi_dev);
>
> is ignoring the rc here intentional? may be worth a comment
>
Ah no, not intentional. We should return rc here. Will update, thanks!
> > + }
> > +
> > dev_dbg(qdev->dev, "Qualcomm MHI QRTR driver probed\n");
> >
> > return 0;
>
> Note that we return 0 here, not rc
> --
> pw-bot: cr
On Wed, Jun 4, 2025 at 11:05 PM Chris Lew <chris.lew@oss.qualcomm.com> wrote:
>
> The call to qrtr_endpoint_register() was moved before
> mhi_prepare_for_transfer_autoqueue() to prevent a case where a dl
> callback can occur before the qrtr endpoint is registered.
>
> Now the reverse can happen where qrtr will try to send a packet
> before the channels are prepared. The correct sequence needs to be
> prepare the mhi channel, register the qrtr endpoint, queue buffers for
> receiving dl transfers.
>
> Since qrtr will not use mhi_prepare_for_transfer_autoqueue(), qrtr must
> do the buffer management and requeue the buffers in the dl_callback.
> Sizing of the buffers will be inherited from the mhi controller
> settings.
>
> Fixes: 68a838b84eff ("net: qrtr: start MHI channel after endpoit creation")
> Reported-by: Johan Hovold <johan@kernel.org>
> Closes: https://lore.kernel.org/linux-arm-msm/ZyTtVdkCCES0lkl4@hovoldconsulting.com/
> Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> ---
> net/qrtr/mhi.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 47 insertions(+), 5 deletions(-)
>
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 69f53625a049..5e7476afb6b4 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -15,6 +15,8 @@ struct qrtr_mhi_dev {
> struct qrtr_endpoint ep;
> struct mhi_device *mhi_dev;
> struct device *dev;
> +
> + size_t dl_buf_len;
> };
>
> /* From MHI to QRTR */
> @@ -24,13 +26,22 @@ static void qcom_mhi_qrtr_dl_callback(struct mhi_device *mhi_dev,
> struct qrtr_mhi_dev *qdev = dev_get_drvdata(&mhi_dev->dev);
> int rc;
>
> - if (!qdev || mhi_res->transaction_status)
> + if (!qdev)
> + return;
> +
> + if (mhi_res->transaction_status == -ENOTCONN) {
> + devm_kfree(qdev->dev, mhi_res->buf_addr);
> + return;
> + } else if (mhi_res->transaction_status) {
> return;
> + }
>
> rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> mhi_res->bytes_xferd);
> if (rc == -EINVAL)
> dev_err(qdev->dev, "invalid ipcrouter packet\n");
> +
> + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, mhi_res->buf_addr, qdev->dl_buf_len, MHI_EOT);
> }
>
> /* From QRTR to MHI */
> @@ -72,6 +83,30 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
> return rc;
> }
>
> +static int qrtr_mhi_queue_rx(struct qrtr_mhi_dev *qdev)
> +{
> + struct mhi_device *mhi_dev = qdev->mhi_dev;
> + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> + int rc = 0;
> + int nr_el;
> +
> + qdev->dl_buf_len = mhi_cntrl->buffer_len;
> + nr_el = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> + while (nr_el--) {
> + void *buf;
> +
> + buf = devm_kzalloc(qdev->dev, qdev->dl_buf_len, GFP_KERNEL);
> + if (!buf) {
> + rc = -ENOMEM;
> + break;
> + }
> + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, qdev->dl_buf_len, MHI_EOT);
> + if (rc)
> + break;
> + }
> + return rc;
> +}
> +
> static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> const struct mhi_device_id *id)
> {
> @@ -87,17 +122,24 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> qdev->ep.xmit = qcom_mhi_qrtr_send;
>
> dev_set_drvdata(&mhi_dev->dev, qdev);
> - rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
> +
> + /* start channels */
> + rc = mhi_prepare_for_transfer(mhi_dev);
> if (rc)
> return rc;
>
> - /* start channels */
> - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
The autoqueue has been introduced to simplify drivers, but if it
becomes unused, should we simply remove that interface from MHI? Or
improve it with a autoqueue_prepare() and autoqueue_start()?
Regards,
Loic
On Thu, Jun 5, 2025 at 1:24 AM Loic Poulain
<loic.poulain@oss.qualcomm.com> wrote:
>
> On Wed, Jun 4, 2025 at 11:05 PM Chris Lew <chris.lew@oss.qualcomm.com> wrote:
> >
> > The call to qrtr_endpoint_register() was moved before
> > mhi_prepare_for_transfer_autoqueue() to prevent a case where a dl
> > callback can occur before the qrtr endpoint is registered.
> >
> > Now the reverse can happen where qrtr will try to send a packet
> > before the channels are prepared. The correct sequence needs to be
> > prepare the mhi channel, register the qrtr endpoint, queue buffers for
> > receiving dl transfers.
> >
> > Since qrtr will not use mhi_prepare_for_transfer_autoqueue(), qrtr must
> > do the buffer management and requeue the buffers in the dl_callback.
> > Sizing of the buffers will be inherited from the mhi controller
> > settings.
> >
> > Fixes: 68a838b84eff ("net: qrtr: start MHI channel after endpoit creation")
> > Reported-by: Johan Hovold <johan@kernel.org>
> > Closes: https://lore.kernel.org/linux-arm-msm/ZyTtVdkCCES0lkl4@hovoldconsulting.com/
> > Signed-off-by: Chris Lew <chris.lew@oss.qualcomm.com>
> > ---
> > net/qrtr/mhi.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++-----
> > 1 file changed, 47 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> > index 69f53625a049..5e7476afb6b4 100644
> > --- a/net/qrtr/mhi.c
> > +++ b/net/qrtr/mhi.c
> > @@ -15,6 +15,8 @@ struct qrtr_mhi_dev {
> > struct qrtr_endpoint ep;
> > struct mhi_device *mhi_dev;
> > struct device *dev;
> > +
> > + size_t dl_buf_len;
> > };
> >
> > /* From MHI to QRTR */
> > @@ -24,13 +26,22 @@ static void qcom_mhi_qrtr_dl_callback(struct mhi_device *mhi_dev,
> > struct qrtr_mhi_dev *qdev = dev_get_drvdata(&mhi_dev->dev);
> > int rc;
> >
> > - if (!qdev || mhi_res->transaction_status)
> > + if (!qdev)
> > + return;
> > +
> > + if (mhi_res->transaction_status == -ENOTCONN) {
> > + devm_kfree(qdev->dev, mhi_res->buf_addr);
> > + return;
> > + } else if (mhi_res->transaction_status) {
> > return;
> > + }
> >
> > rc = qrtr_endpoint_post(&qdev->ep, mhi_res->buf_addr,
> > mhi_res->bytes_xferd);
> > if (rc == -EINVAL)
> > dev_err(qdev->dev, "invalid ipcrouter packet\n");
> > +
> > + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, mhi_res->buf_addr, qdev->dl_buf_len, MHI_EOT);
> > }
> >
> > /* From QRTR to MHI */
> > @@ -72,6 +83,30 @@ static int qcom_mhi_qrtr_send(struct qrtr_endpoint *ep, struct sk_buff *skb)
> > return rc;
> > }
> >
> > +static int qrtr_mhi_queue_rx(struct qrtr_mhi_dev *qdev)
> > +{
> > + struct mhi_device *mhi_dev = qdev->mhi_dev;
> > + struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> > + int rc = 0;
> > + int nr_el;
> > +
> > + qdev->dl_buf_len = mhi_cntrl->buffer_len;
> > + nr_el = mhi_get_free_desc_count(mhi_dev, DMA_FROM_DEVICE);
> > + while (nr_el--) {
> > + void *buf;
> > +
> > + buf = devm_kzalloc(qdev->dev, qdev->dl_buf_len, GFP_KERNEL);
> > + if (!buf) {
> > + rc = -ENOMEM;
> > + break;
> > + }
> > + rc = mhi_queue_buf(mhi_dev, DMA_FROM_DEVICE, buf, qdev->dl_buf_len, MHI_EOT);
> > + if (rc)
> > + break;
> > + }
> > + return rc;
> > +}
> > +
> > static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> > const struct mhi_device_id *id)
> > {
> > @@ -87,17 +122,24 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
> > qdev->ep.xmit = qcom_mhi_qrtr_send;
> >
> > dev_set_drvdata(&mhi_dev->dev, qdev);
> > - rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
> > +
> > + /* start channels */
> > + rc = mhi_prepare_for_transfer(mhi_dev);
> > if (rc)
> > return rc;
> >
> > - /* start channels */
> > - rc = mhi_prepare_for_transfer_autoqueue(mhi_dev);
>
> The autoqueue has been introduced to simplify drivers, but if it
> becomes unused, should we simply remove that interface from MHI? Or
> improve it with a autoqueue_prepare() and autoqueue_start()?
>
Yes, I think it is reasonable to remove the autoqueue() interface from
MHI. If another driver comes along and needs something similar we can
revert the commit that removes it.
When I had more cycles I was planning on removing the related code. I
was very late in providing this patch, so I wanted to get this on the
list first.
> Regards,
> Loic
Thanks,
Chris
© 2016 - 2025 Red Hat, Inc.