Message ID | 20211019134451.174318-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | bus: mhi: Add mhi_prepare_for_transfer_autoqueue API for DL auto queue | expand |
Context | Check | Description |
---|---|---|
netdev/tree_selection | success | Not a local patch |
On Tue, 19 Oct 2021 19:14:51 +0530 Manivannan Sadhasivam wrote: > Add a new API "mhi_prepare_for_transfer_autoqueue" for using with client > drivers like QRTR to request MHI core to autoqueue buffers for the DL > channel along with starting both UL and DL channels. > > So far, the "auto_queue" flag specified by the controller drivers in > channel definition served this purpose but this will be removed at some > point in future. > > Cc: netdev@vger.kernel.org > Co-developed-by: Loic Poulain <loic.poulain@linaro.org> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > --- > > Dave, Jakub: This patch should go through MHI tree. But since the QRTR driver > is also modified, this needs an Ack from you. CCing us wouldn't hurt. Speaking of people who aren't CCed I've seen Greg nack the flags argument. SMH
On Tue, Oct 19, 2021 at 07:49:18AM -0700, Jakub Kicinski wrote: > On Tue, 19 Oct 2021 19:14:51 +0530 Manivannan Sadhasivam wrote: > > Add a new API "mhi_prepare_for_transfer_autoqueue" for using with client > > drivers like QRTR to request MHI core to autoqueue buffers for the DL > > channel along with starting both UL and DL channels. > > > > So far, the "auto_queue" flag specified by the controller drivers in > > channel definition served this purpose but this will be removed at some > > point in future. > > > > Cc: netdev@vger.kernel.org > > Co-developed-by: Loic Poulain <loic.poulain@linaro.org> > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > > > Dave, Jakub: This patch should go through MHI tree. But since the QRTR driver > > is also modified, this needs an Ack from you. > > CCing us wouldn't hurt. > > Speaking of people who aren't CCed I've seen Greg nack the flags > argument. Yes, that type of api is not ok.
On Tue, Oct 19, 2021 at 07:49:18AM -0700, Jakub Kicinski wrote: > On Tue, 19 Oct 2021 19:14:51 +0530 Manivannan Sadhasivam wrote: > > Add a new API "mhi_prepare_for_transfer_autoqueue" for using with client > > drivers like QRTR to request MHI core to autoqueue buffers for the DL > > channel along with starting both UL and DL channels. > > > > So far, the "auto_queue" flag specified by the controller drivers in > > channel definition served this purpose but this will be removed at some > > point in future. > > > > Cc: netdev@vger.kernel.org > > Co-developed-by: Loic Poulain <loic.poulain@linaro.org> > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > --- > > > > Dave, Jakub: This patch should go through MHI tree. But since the QRTR driver > > is also modified, this needs an Ack from you. > > CCing us wouldn't hurt. > Okay. > Speaking of people who aren't CCed I've seen Greg nack the flags > argument. > I usually send patches to Greg during the PR time as MHI patches goes through char-misc tree. I don't include him during the patch reviews. And yes, Greg indeed NACK the API that had flags argument. But this one didn't. Thanks, Mani > SMH
Hi Manivannan, I love your patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.15-rc6 next-20211019] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Manivannan-Sadhasivam/bus-mhi-Add-mhi_prepare_for_transfer_autoqueue-API-for-DL-auto-queue/20211019-221447 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 519d81956ee277b4419c723adfb154603c2565ba config: hexagon-randconfig-r041-20211019 (attached as .config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project b37efed957ed0a0193d80020aefd55cb587dfc1f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/434ab9e12f5e85c6d84c1a0524d850559b058291 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Manivannan-Sadhasivam/bus-mhi-Add-mhi_prepare_for_transfer_autoqueue-API-for-DL-auto-queue/20211019-221447 git checkout 434ab9e12f5e85c6d84c1a0524d850559b058291 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=hexagon If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): >> drivers/bus/mhi/core/main.c:1615:5: warning: no previous prototype for function '__mhi_prepare_for_transfer' [-Wmissing-prototypes] int __mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags) ^ drivers/bus/mhi/core/main.c:1615:1: note: declare 'static' if the function is not intended to be used outside of this translation unit int __mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags) ^ static 1 warning generated. vim +/__mhi_prepare_for_transfer +1615 drivers/bus/mhi/core/main.c 1614 > 1615 int __mhi_prepare_for_transfer(struct mhi_device *mhi_dev, unsigned int flags) 1616 { 1617 int ret, dir; 1618 struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; 1619 struct mhi_chan *mhi_chan; 1620 1621 for (dir = 0; dir < 2; dir++) { 1622 mhi_chan = dir ? mhi_dev->dl_chan : mhi_dev->ul_chan; 1623 if (!mhi_chan) 1624 continue; 1625 1626 ret = mhi_prepare_channel(mhi_cntrl, mhi_chan, flags); 1627 if (ret) 1628 goto error_open_chan; 1629 } 1630 1631 return 0; 1632 1633 error_open_chan: 1634 for (--dir; dir >= 0; dir--) { 1635 mhi_chan = dir ? mhi_dev->dl_chan : mhi_dev->ul_chan; 1636 if (!mhi_chan) 1637 continue; 1638 1639 mhi_unprepare_channel(mhi_cntrl, mhi_chan); 1640 } 1641 1642 return ret; 1643 } 1644 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index 3a732afaf73e..597da2cce069 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -681,8 +681,12 @@ void mhi_deinit_free_irq(struct mhi_controller *mhi_cntrl); void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl, struct image_info *img_info); void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl); + +/* Automatically allocate and queue inbound buffers */ +#define MHI_CH_INBOUND_ALLOC_BUFS BIT(0) 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 b15c5bc37dd4..eaa1f62d16a5 100644 --- a/drivers/bus/mhi/core/main.c +++ b/drivers/bus/mhi/core/main.c @@ -1430,7 +1430,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; @@ -1455,6 +1455,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, @@ -1609,8 +1612,7 @@ void mhi_reset_chan(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan) read_unlock_bh(&mhi_cntrl->pm_lock); } -/* 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; @@ -1621,7 +1623,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; } @@ -1639,8 +1641,19 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev) return ret; } + +int mhi_prepare_for_transfer(struct mhi_device *mhi_dev) +{ + return __mhi_prepare_for_transfer(mhi_dev, 0); +} EXPORT_SYMBOL_GPL(mhi_prepare_for_transfer); +int mhi_prepare_for_transfer_autoqueue(struct mhi_device *mhi_dev) +{ + return __mhi_prepare_for_transfer(mhi_dev, MHI_CH_INBOUND_ALLOC_BUFS); +} +EXPORT_SYMBOL_GPL(mhi_prepare_for_transfer_autoqueue); + void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev) { struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; diff --git a/include/linux/mhi.h b/include/linux/mhi.h index 723985879035..271db1d6da85 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -717,15 +717,26 @@ void mhi_device_put(struct mhi_device *mhi_dev); /** * mhi_prepare_for_transfer - Setup UL and DL channels for data transfer. - * Allocate and initialize the channel context and - * also issue the START channel command to both - * channels. Channels can be started only if both - * host and device execution environments match and - * channels are in a DISABLED state. * @mhi_dev: Device associated with the channels + * + * Allocate and initialize the channel context and also issue the START channel + * command to both channels. Channels can be started only if both host and + * device execution environments match and channels are in a DISABLED state. */ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev); +/** + * mhi_prepare_for_transfer_autoqueue - Setup UL and DL channels with auto queue + * buffers for DL traffic + * @mhi_dev: Device associated with the channels + * + * Allocate and initialize the channel context and also issue the START channel + * command to both channels. Channels can be started only if both host and + * device execution environments match and channels are in a DISABLED state. + * The MHI core will automatically allocate and queue buffers for the DL traffic. + */ +int mhi_prepare_for_transfer_autoqueue(struct mhi_device *mhi_dev); + /** * mhi_unprepare_from_transfer - Reset UL and DL channels for data transfer. * Issue the RESET channel command and let the diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c index fa611678af05..18196e1c8c2f 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_autoqueue(mhi_dev); if (rc) return rc;