diff mbox series

[v6,1/2] bus: mhi: Add mhi_queue_is_full function

Message ID 1602840007-27140-1-git-send-email-loic.poulain@linaro.org
State New
Headers show
Series [v6,1/2] bus: mhi: Add mhi_queue_is_full function | expand

Commit Message

Loic Poulain Oct. 16, 2020, 9:20 a.m. UTC
This function can be used by client driver to determine whether it's
possible to queue new elements in a channel ring.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 v1->v5: This change is not part of the series
 v6: add this patch to the series

 drivers/bus/mhi/core/main.c | 15 +++++++++++++--
 include/linux/mhi.h         |  7 +++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

Comments

Hemant Kumar Oct. 23, 2020, 3:06 a.m. UTC | #1
Hi Loic,

On 10/16/20 2:20 AM, Loic Poulain wrote:
> This function can be used by client driver to determine whether it's

> possible to queue new elements in a channel ring.

> 

> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

[..]
> +static inline bool mhi_is_ring_full(struct mhi_controller *mhi_cntrl,

> +				    struct mhi_ring *ring)

>   {

>   	void *tmp = ring->wp + ring->el_size;

>   

> @@ -1173,6 +1173,17 @@ int mhi_queue_buf(struct mhi_device *mhi_dev, enum dma_data_direction dir,

>   }

>   EXPORT_SYMBOL_GPL(mhi_queue_buf);

>   

> +bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir)

> +{

> +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;

> +	struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?

> +					mhi_dev->ul_chan : mhi_dev->dl_chan;

> +	struct mhi_ring *tre_ring = &mhi_chan->tre_ring;

> +

> +	return mhi_is_ring_full(mhi_cntrl, tre_ring);

> +}

> +EXPORT_SYMBOL_GPL(mhi_queue_is_full);

> 

i was wondering if you can make use of mhi_get_free_desc() API (pushed 
as part of MHI UCI - User Control Interface driver) here?

Thanks,
Hemant
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Jakub Kicinski Oct. 23, 2020, 3:44 p.m. UTC | #2
On Thu, 22 Oct 2020 20:06:37 -0700 Hemant Kumar wrote:
> > @@ -1173,6 +1173,17 @@ int mhi_queue_buf(struct mhi_device *mhi_dev, enum dma_data_direction dir,

> >   }

> >   EXPORT_SYMBOL_GPL(mhi_queue_buf);

> >   

> > +bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir)

> > +{

> > +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;

> > +	struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?

> > +					mhi_dev->ul_chan : mhi_dev->dl_chan;

> > +	struct mhi_ring *tre_ring = &mhi_chan->tre_ring;

> > +

> > +	return mhi_is_ring_full(mhi_cntrl, tre_ring);

> > +}

> > +EXPORT_SYMBOL_GPL(mhi_queue_is_full);

> >   

> i was wondering if you can make use of mhi_get_free_desc() API (pushed 

> as part of MHI UCI - User Control Interface driver) here?


Let me ask you one more time. Where is this MHI UCI code you're talking
about?

linux$ git remote show linux
* remote linux
  Fetch URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
  Push  URL: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
  HEAD branch: master
  Remote branch:
    master tracked
linux$ git fetch linux
linux$ git checkout linux/master
HEAD is now at f9893351acae Merge tag 'kconfig-v5.10' of git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
linux$ git grep mhi_get_free_desc
linux$
Jeffrey Hugo Oct. 23, 2020, 4:19 p.m. UTC | #3
On 10/23/2020 9:44 AM, Jakub Kicinski wrote:
> On Thu, 22 Oct 2020 20:06:37 -0700 Hemant Kumar wrote:

>>> @@ -1173,6 +1173,17 @@ int mhi_queue_buf(struct mhi_device *mhi_dev, enum dma_data_direction dir,

>>>    }

>>>    EXPORT_SYMBOL_GPL(mhi_queue_buf);

>>>    

>>> +bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir)

>>> +{

>>> +	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;

>>> +	struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?

>>> +					mhi_dev->ul_chan : mhi_dev->dl_chan;

>>> +	struct mhi_ring *tre_ring = &mhi_chan->tre_ring;

>>> +

>>> +	return mhi_is_ring_full(mhi_cntrl, tre_ring);

>>> +}

>>> +EXPORT_SYMBOL_GPL(mhi_queue_is_full);

>>>    

>> i was wondering if you can make use of mhi_get_free_desc() API (pushed

>> as part of MHI UCI - User Control Interface driver) here?

> 

> Let me ask you one more time. Where is this MHI UCI code you're talking

> about?


https://lkml.org/lkml/2020/10/22/186

-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
Jeffrey Hugo Oct. 23, 2020, 7:24 p.m. UTC | #4
On 10/23/2020 1:11 PM, Loic Poulain wrote:
> Hi Hemant,

> 

> On Fri, 23 Oct 2020 at 05:06, Hemant Kumar <hemantk@codeaurora.org 

> <mailto:hemantk@codeaurora.org>> wrote:

> 

>     Hi Loic,

> 

>     On 10/16/20 2:20 AM, Loic Poulain wrote:

>      > This function can be used by client driver to determine whether it's

>      > possible to queue new elements in a channel ring.

>      >

>      > Signed-off-by: Loic Poulain <loic.poulain@linaro.org

>     <mailto:loic.poulain@linaro.org>>

>     [..]

>      > +static inline bool mhi_is_ring_full(struct mhi_controller

>     *mhi_cntrl,

>      > +                                 struct mhi_ring *ring)

>      >   {

>      >       void *tmp = ring->wp + ring->el_size;

>      >

>      > @@ -1173,6 +1173,17 @@ int mhi_queue_buf(struct mhi_device

>     *mhi_dev, enum dma_data_direction dir,

>      >   }

>      >   EXPORT_SYMBOL_GPL(mhi_queue_buf);

>      >

>      > +bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum

>     dma_data_direction dir)

>      > +{

>      > +     struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;

>      > +     struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?

>      > +                                     mhi_dev->ul_chan :

>     mhi_dev->dl_chan;

>      > +     struct mhi_ring *tre_ring = &mhi_chan->tre_ring;

>      > +

>      > +     return mhi_is_ring_full(mhi_cntrl, tre_ring);

>      > +}

>      > +EXPORT_SYMBOL_GPL(mhi_queue_is_full);

>      >

>     i was wondering if you can make use of mhi_get_free_desc() API (pushed

>     as part of MHI UCI - User Control Interface driver) here?

> 

> 

> I prefer not to depend on PR that is not yet merged to keep things 

> simple, though I could integrate it in my PR... I think this function is 

> good to have anyway for client drivers, and slightly faster since this 

> is just a pointer comparison.


Its a little bit more than that.  Frankly, unless you are counting 
assembly instructions for both methods, the difference is likely to be 
in the noise.

However, I wonder why core MHI changes were not copied to the proper 
list (namely linux-arm-msm)?

-- 
Jeffrey Hugo
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
diff mbox series

Patch

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index a588eac..44aa11f 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -943,8 +943,8 @@  void mhi_ctrl_ev_task(unsigned long data)
 	}
 }
 
-static bool mhi_is_ring_full(struct mhi_controller *mhi_cntrl,
-			     struct mhi_ring *ring)
+static inline bool mhi_is_ring_full(struct mhi_controller *mhi_cntrl,
+				    struct mhi_ring *ring)
 {
 	void *tmp = ring->wp + ring->el_size;
 
@@ -1173,6 +1173,17 @@  int mhi_queue_buf(struct mhi_device *mhi_dev, enum dma_data_direction dir,
 }
 EXPORT_SYMBOL_GPL(mhi_queue_buf);
 
+bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir)
+{
+	struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+	struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+					mhi_dev->ul_chan : mhi_dev->dl_chan;
+	struct mhi_ring *tre_ring = &mhi_chan->tre_ring;
+
+	return mhi_is_ring_full(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_queue_is_full);
+
 int mhi_send_cmd(struct mhi_controller *mhi_cntrl,
 		 struct mhi_chan *mhi_chan,
 		 enum mhi_cmd_type cmd)
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 9d67e75..f72c3a4 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -745,4 +745,11 @@  int mhi_queue_buf(struct mhi_device *mhi_dev, enum dma_data_direction dir,
 int mhi_queue_skb(struct mhi_device *mhi_dev, enum dma_data_direction dir,
 		  struct sk_buff *skb, size_t len, enum mhi_flags mflags);
 
+/**
+ * mhi_queue_is_full - Determine whether queueing new elements is possible
+ * @mhi_dev: Device associated with the channels
+ * @dir: DMA direction for the channel
+ */
+bool mhi_queue_is_full(struct mhi_device *mhi_dev, enum dma_data_direction dir);
+
 #endif /* _MHI_H_ */