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 (mailing list archive)
State Not Applicable
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
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 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)?
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_ */