linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] bus: mhi: Add inbound buffers allocation flag
@ 2021-06-24 20:28 Loic Poulain
  2021-06-24 20:30 ` Bjorn Andersson
  0 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2021-06-24 20:28 UTC (permalink / raw)
  To: mani; +Cc: linux-arm-msm, Loic Poulain, Manivannan Sadhasivam

Currently, the MHI controller driver defines which channels should
have their inbound buffers allocated and queued. But ideally, this is
something that should be decided by the MHI device driver instead,
which actually deals with that buffers.

Add a flag parameter to mhi_prepare_for_transfer allowing to specify
if buffers have to be allocated and queued by the MHI stack.

Keep auto_queue flag for now, but should be removed at some point.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Acked-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 v2: Update API in mhi_wwan_ctrl driver
 v3: Do not use enum for flags

 drivers/bus/mhi/core/internal.h  | 2 +-
 drivers/bus/mhi/core/main.c      | 9 ++++++---
 drivers/net/mhi/net.c            | 2 +-
 drivers/net/wwan/mhi_wwan_ctrl.c | 2 +-
 include/linux/mhi.h              | 7 ++++++-
 net/qrtr/mhi.c                   | 2 +-
 6 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 5b9ea66..bc239a1 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -682,7 +682,7 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
 		      struct image_info *img_info);
 void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl);
 int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
-			struct mhi_chan *mhi_chan);
+			struct mhi_chan *mhi_chan, unsigned int flags);
 int mhi_init_chan_ctxt(struct mhi_controller *mhi_cntrl,
 		       struct mhi_chan *mhi_chan);
 void mhi_deinit_chan_ctxt(struct mhi_controller *mhi_cntrl,
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 050381d..594fe37 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1433,7 +1433,7 @@ static void mhi_unprepare_channel(struct mhi_controller *mhi_cntrl,
 }
 
 int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
-			struct mhi_chan *mhi_chan)
+			struct mhi_chan *mhi_chan, unsigned int flags)
 {
 	int ret = 0;
 	struct device *dev = &mhi_chan->mhi_dev->dev;
@@ -1458,6 +1458,9 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
 	if (ret)
 		goto error_pm_state;
 
+	if (mhi_chan->dir == DMA_FROM_DEVICE)
+		mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
+
 	/* Pre-allocate buffer for xfer ring */
 	if (mhi_chan->pre_alloc) {
 		int nr_el = get_nr_avail_ring_elements(mhi_cntrl,
@@ -1613,7 +1616,7 @@ void mhi_reset_chan(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan)
 }
 
 /* Move channel to start state */
-int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
+int mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags)
 {
 	int ret, dir;
 	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
@@ -1624,7 +1627,7 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
 		if (!mhi_chan)
 			continue;
 
-		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan);
+		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan, flags);
 		if (ret)
 			goto error_open_chan;
 	}
diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
index 0d8293a..774e329 100644
--- a/drivers/net/mhi/net.c
+++ b/drivers/net/mhi/net.c
@@ -327,7 +327,7 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
 	u64_stats_init(&mhi_netdev->stats.tx_syncp);
 
 	/* Start MHI channels */
-	err = mhi_prepare_for_transfer(mhi_dev);
+	err = mhi_prepare_for_transfer(mhi_dev, 0);
 	if (err)
 		goto out_err;
 
diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
index 1bc6b69..1e18420 100644
--- a/drivers/net/wwan/mhi_wwan_ctrl.c
+++ b/drivers/net/wwan/mhi_wwan_ctrl.c
@@ -110,7 +110,7 @@ static int mhi_wwan_ctrl_start(struct wwan_port *port)
 	int ret;
 
 	/* Start mhi device's channel(s) */
-	ret = mhi_prepare_for_transfer(mhiwwan->mhi_dev);
+	ret = mhi_prepare_for_transfer(mhiwwan->mhi_dev, 0);
 	if (ret)
 		return ret;
 
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 5eb626a..eed949c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -719,8 +719,13 @@ void mhi_device_put(struct mhi_device *mhi_dev);
  *                            host and device execution environments match and
  *                            channels are in a DISABLED state.
  * @mhi_dev: Device associated with the channels
+ * @flags: MHI channel flags
  */
-int mhi_prepare_for_transfer(struct mhi_device *mhi_dev);
+int mhi_prepare_for_transfer(struct mhi_device *mhi_dev,
+			     unsigned int flags);
+
+/* Automatically allocate and queue inbound buffers */
+#define MHI_CH_INBOUND_ALLOC_BUFS BIT(0)
 
 /**
  * mhi_unprepare_from_transfer - Reset UL and DL channels for data transfer.
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index fa61167..29b4fa3 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -79,7 +79,7 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	int rc;
 
 	/* start channels */
-	rc = mhi_prepare_for_transfer(mhi_dev);
+	rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
 	if (rc)
 		return rc;
 
-- 
2.7.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] bus: mhi: Add inbound buffers allocation flag
  2021-06-24 20:28 [PATCH v3] bus: mhi: Add inbound buffers allocation flag Loic Poulain
@ 2021-06-24 20:30 ` Bjorn Andersson
  2021-06-24 22:45   ` Loic Poulain
  0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Andersson @ 2021-06-24 20:30 UTC (permalink / raw)
  To: Loic Poulain; +Cc: mani, linux-arm-msm, Manivannan Sadhasivam

On Thu 24 Jun 15:28 CDT 2021, Loic Poulain wrote:

> Currently, the MHI controller driver defines which channels should
> have their inbound buffers allocated and queued. But ideally, this is
> something that should be decided by the MHI device driver instead,
> which actually deals with that buffers.
> 
> Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> if buffers have to be allocated and queued by the MHI stack.
> 
> Keep auto_queue flag for now, but should be removed at some point.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

What's the intention with this patch? Why is Mani the last S-o-b when
you're the one sending it and why is it sent only to linux-arm-msm@?


And completely separate of the practical matters, is it true that qrtr
is the only client that use this pre-allocation feature? Would it be a
net gain to simplify the core and add buffer allocation to qrtr instead?

Regards,
Bjorn

> ---
>  v2: Update API in mhi_wwan_ctrl driver
>  v3: Do not use enum for flags
> 
>  drivers/bus/mhi/core/internal.h  | 2 +-
>  drivers/bus/mhi/core/main.c      | 9 ++++++---
>  drivers/net/mhi/net.c            | 2 +-
>  drivers/net/wwan/mhi_wwan_ctrl.c | 2 +-
>  include/linux/mhi.h              | 7 ++++++-
>  net/qrtr/mhi.c                   | 2 +-
>  6 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
> index 5b9ea66..bc239a1 100644
> --- a/drivers/bus/mhi/core/internal.h
> +++ b/drivers/bus/mhi/core/internal.h
> @@ -682,7 +682,7 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
>  		      struct image_info *img_info);
>  void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl);
>  int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
> -			struct mhi_chan *mhi_chan);
> +			struct mhi_chan *mhi_chan, unsigned int flags);
>  int mhi_init_chan_ctxt(struct mhi_controller *mhi_cntrl,
>  		       struct mhi_chan *mhi_chan);
>  void mhi_deinit_chan_ctxt(struct mhi_controller *mhi_cntrl,
> diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
> index 050381d..594fe37 100644
> --- a/drivers/bus/mhi/core/main.c
> +++ b/drivers/bus/mhi/core/main.c
> @@ -1433,7 +1433,7 @@ static void mhi_unprepare_channel(struct mhi_controller *mhi_cntrl,
>  }
>  
>  int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
> -			struct mhi_chan *mhi_chan)
> +			struct mhi_chan *mhi_chan, unsigned int flags)
>  {
>  	int ret = 0;
>  	struct device *dev = &mhi_chan->mhi_dev->dev;
> @@ -1458,6 +1458,9 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
>  	if (ret)
>  		goto error_pm_state;
>  
> +	if (mhi_chan->dir == DMA_FROM_DEVICE)
> +		mhi_chan->pre_alloc = !!(flags & MHI_CH_INBOUND_ALLOC_BUFS);
> +
>  	/* Pre-allocate buffer for xfer ring */
>  	if (mhi_chan->pre_alloc) {
>  		int nr_el = get_nr_avail_ring_elements(mhi_cntrl,
> @@ -1613,7 +1616,7 @@ void mhi_reset_chan(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan)
>  }
>  
>  /* Move channel to start state */
> -int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
> +int mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags)
>  {
>  	int ret, dir;
>  	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
> @@ -1624,7 +1627,7 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev)
>  		if (!mhi_chan)
>  			continue;
>  
> -		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan);
> +		ret = mhi_prepare_channel(mhi_cntrl, mhi_chan, flags);
>  		if (ret)
>  			goto error_open_chan;
>  	}
> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
> index 0d8293a..774e329 100644
> --- a/drivers/net/mhi/net.c
> +++ b/drivers/net/mhi/net.c
> @@ -327,7 +327,7 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
>  	u64_stats_init(&mhi_netdev->stats.tx_syncp);
>  
>  	/* Start MHI channels */
> -	err = mhi_prepare_for_transfer(mhi_dev);
> +	err = mhi_prepare_for_transfer(mhi_dev, 0);
>  	if (err)
>  		goto out_err;
>  
> diff --git a/drivers/net/wwan/mhi_wwan_ctrl.c b/drivers/net/wwan/mhi_wwan_ctrl.c
> index 1bc6b69..1e18420 100644
> --- a/drivers/net/wwan/mhi_wwan_ctrl.c
> +++ b/drivers/net/wwan/mhi_wwan_ctrl.c
> @@ -110,7 +110,7 @@ static int mhi_wwan_ctrl_start(struct wwan_port *port)
>  	int ret;
>  
>  	/* Start mhi device's channel(s) */
> -	ret = mhi_prepare_for_transfer(mhiwwan->mhi_dev);
> +	ret = mhi_prepare_for_transfer(mhiwwan->mhi_dev, 0);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 5eb626a..eed949c 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -719,8 +719,13 @@ void mhi_device_put(struct mhi_device *mhi_dev);
>   *                            host and device execution environments match and
>   *                            channels are in a DISABLED state.
>   * @mhi_dev: Device associated with the channels
> + * @flags: MHI channel flags
>   */
> -int mhi_prepare_for_transfer(struct mhi_device *mhi_dev);
> +int mhi_prepare_for_transfer(struct mhi_device *mhi_dev,
> +			     unsigned int flags);
> +
> +/* Automatically allocate and queue inbound buffers */
> +#define MHI_CH_INBOUND_ALLOC_BUFS BIT(0)
>  
>  /**
>   * mhi_unprepare_from_transfer - Reset UL and DL channels for data transfer.
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index fa61167..29b4fa3 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -79,7 +79,7 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
>  	int rc;
>  
>  	/* start channels */
> -	rc = mhi_prepare_for_transfer(mhi_dev);
> +	rc = mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS);
>  	if (rc)
>  		return rc;
>  
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] bus: mhi: Add inbound buffers allocation flag
  2021-06-24 22:45   ` Loic Poulain
@ 2021-06-24 21:00     ` Bhaumik Bhatt
  2021-06-24 21:09     ` Bjorn Andersson
  2021-06-25  5:08     ` Manivannan Sadhasivam
  2 siblings, 0 replies; 6+ messages in thread
From: Bhaumik Bhatt @ 2021-06-24 21:00 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Bjorn Andersson, Manivannan Sadhasivam, linux-arm-msm,
	Manivannan Sadhasivam, clew

On 2021-06-24 03:45 PM, Loic Poulain wrote:
> Hi Bjorn,
> 
> On Thu, 24 Jun 2021 at 22:30, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> 
>> On Thu 24 Jun 15:28 CDT 2021, Loic Poulain wrote:
>> 
>> > Currently, the MHI controller driver defines which channels should
>> > have their inbound buffers allocated and queued. But ideally, this is
>> > something that should be decided by the MHI device driver instead,
>> > which actually deals with that buffers.
>> >
>> > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
>> > if buffers have to be allocated and queued by the MHI stack.
>> >
>> > Keep auto_queue flag for now, but should be removed at some point.
>> >
>> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>> > Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> > Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> > Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> > Acked-by: Jakub Kicinski <kuba@kernel.org>
>> > Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
>> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> 
>> What's the intention with this patch? Why is Mani the last S-o-b when
>> you're the one sending it and why is it sent only to linux-arm-msm@?
> 
> Actually the previous version of that patch has already been applied
> to mhi-next, but has been nacked as part of Mani's PR, so it's a quick
> follow-up fix to address the issue.
> 
>> And completely separate of the practical matters, is it true that qrtr
>> is the only client that use this pre-allocation feature?
> 
> yes.
> 
>> Would it be a net gain to simplify the core and add buffer allocation 
>> to qrtr instead?
> 
> Yes, I 100% agree, but I however would prefer to keep that for a
> follow-up series since this patch fixes a real issue for MHI/PCI
> modems (no inbound QRTR buffers allocated).
> 
> Regards,
> Loic
I had discussed this with Chris Lew a while ago and he also agreed qrtr 
can
queue inbound buffers.

This patch is good and we can continue to have flexibility for clients 
since
this allows MHI controller to not have to bother about some of the 
unnecessary
plugins/booleans we maintain on behalf of clients.

They can have more control this way and less headache for us.

Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] bus: mhi: Add inbound buffers allocation flag
  2021-06-24 22:45   ` Loic Poulain
  2021-06-24 21:00     ` Bhaumik Bhatt
@ 2021-06-24 21:09     ` Bjorn Andersson
  2021-06-25  5:08     ` Manivannan Sadhasivam
  2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2021-06-24 21:09 UTC (permalink / raw)
  To: Loic Poulain; +Cc: Manivannan Sadhasivam, linux-arm-msm, Manivannan Sadhasivam

On Thu 24 Jun 17:45 CDT 2021, Loic Poulain wrote:

> Hi Bjorn,
> 
> On Thu, 24 Jun 2021 at 22:30, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Thu 24 Jun 15:28 CDT 2021, Loic Poulain wrote:
> >
> > > Currently, the MHI controller driver defines which channels should
> > > have their inbound buffers allocated and queued. But ideally, this is
> > > something that should be decided by the MHI device driver instead,
> > > which actually deals with that buffers.
> > >
> > > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> > > if buffers have to be allocated and queued by the MHI stack.
> > >
> > > Keep auto_queue flag for now, but should be removed at some point.
> > >
> > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > > Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > > Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > > Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > > Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > What's the intention with this patch? Why is Mani the last S-o-b when
> > you're the one sending it and why is it sent only to linux-arm-msm@?
> 
> Actually the previous version of that patch has already been applied
> to mhi-next, but has been nacked as part of Mani's PR, so it's a quick
> follow-up fix to address the issue.
> 

Thanks, that makes sense.

> > And completely separate of the practical matters, is it true that qrtr
> > is the only client that use this pre-allocation feature?
> 
> yes.
> 

Then I think we should fix qrtr instead.

> > Would it be a net gain to simplify the core and add buffer allocation to qrtr instead?
> 
> Yes, I 100% agree, but I however would prefer to keep that for a
> follow-up series since this patch fixes a real issue for MHI/PCI
> modems (no inbound QRTR buffers allocated).
> 

I certainly don't mind this patch going upstream while we put such
refactoring in the backlog.

Thanks,
Bjorn

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] bus: mhi: Add inbound buffers allocation flag
  2021-06-24 20:30 ` Bjorn Andersson
@ 2021-06-24 22:45   ` Loic Poulain
  2021-06-24 21:00     ` Bhaumik Bhatt
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Loic Poulain @ 2021-06-24 22:45 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: Manivannan Sadhasivam, linux-arm-msm, Manivannan Sadhasivam

Hi Bjorn,

On Thu, 24 Jun 2021 at 22:30, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Thu 24 Jun 15:28 CDT 2021, Loic Poulain wrote:
>
> > Currently, the MHI controller driver defines which channels should
> > have their inbound buffers allocated and queued. But ideally, this is
> > something that should be decided by the MHI device driver instead,
> > which actually deals with that buffers.
> >
> > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> > if buffers have to be allocated and queued by the MHI stack.
> >
> > Keep auto_queue flag for now, but should be removed at some point.
> >
> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> What's the intention with this patch? Why is Mani the last S-o-b when
> you're the one sending it and why is it sent only to linux-arm-msm@?

Actually the previous version of that patch has already been applied
to mhi-next, but has been nacked as part of Mani's PR, so it's a quick
follow-up fix to address the issue.

> And completely separate of the practical matters, is it true that qrtr
> is the only client that use this pre-allocation feature?

yes.

> Would it be a net gain to simplify the core and add buffer allocation to qrtr instead?

Yes, I 100% agree, but I however would prefer to keep that for a
follow-up series since this patch fixes a real issue for MHI/PCI
modems (no inbound QRTR buffers allocated).

Regards,
Loic

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] bus: mhi: Add inbound buffers allocation flag
  2021-06-24 22:45   ` Loic Poulain
  2021-06-24 21:00     ` Bhaumik Bhatt
  2021-06-24 21:09     ` Bjorn Andersson
@ 2021-06-25  5:08     ` Manivannan Sadhasivam
  2 siblings, 0 replies; 6+ messages in thread
From: Manivannan Sadhasivam @ 2021-06-25  5:08 UTC (permalink / raw)
  To: Loic Poulain; +Cc: Bjorn Andersson, linux-arm-msm

On Fri, Jun 25, 2021 at 12:45:25AM +0200, Loic Poulain wrote:
> Hi Bjorn,
> 
> On Thu, 24 Jun 2021 at 22:30, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
> >
> > On Thu 24 Jun 15:28 CDT 2021, Loic Poulain wrote:
> >
> > > Currently, the MHI controller driver defines which channels should
> > > have their inbound buffers allocated and queued. But ideally, this is
> > > something that should be decided by the MHI device driver instead,
> > > which actually deals with that buffers.
> > >
> > > Add a flag parameter to mhi_prepare_for_transfer allowing to specify
> > > if buffers have to be allocated and queued by the MHI stack.
> > >
> > > Keep auto_queue flag for now, but should be removed at some point.
> > >
> > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> > > Tested-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > > Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > > Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > Acked-by: Jakub Kicinski <kuba@kernel.org>
> > > Link: https://lore.kernel.org/r/1621603519-16773-1-git-send-email-loic.poulain@linaro.org
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> >
> > What's the intention with this patch? Why is Mani the last S-o-b when
> > you're the one sending it and why is it sent only to linux-arm-msm@?
> 
> Actually the previous version of that patch has already been applied
> to mhi-next, but has been nacked as part of Mani's PR, so it's a quick
> follow-up fix to address the issue.
> 

Thanks but you could've removed my s-o-b and "Link" as my script will
attach them again while applying ;)

But no worries, I'll deal with it.

> > And completely separate of the practical matters, is it true that qrtr
> > is the only client that use this pre-allocation feature?
> 
> yes.
> 
> > Would it be a net gain to simplify the core and add buffer allocation to qrtr instead?
> 
> Yes, I 100% agree, but I however would prefer to keep that for a
> follow-up series since this patch fixes a real issue for MHI/PCI
> modems (no inbound QRTR buffers allocated).
> 

Yeah, let's do that in 5.15.

Thanks,
Mani

> Regards,
> Loic

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-06-25  5:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 20:28 [PATCH v3] bus: mhi: Add inbound buffers allocation flag Loic Poulain
2021-06-24 20:30 ` Bjorn Andersson
2021-06-24 22:45   ` Loic Poulain
2021-06-24 21:00     ` Bhaumik Bhatt
2021-06-24 21:09     ` Bjorn Andersson
2021-06-25  5:08     ` Manivannan Sadhasivam

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).