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 |
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
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$
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
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 --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_ */
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(-)