| Message ID | 1600874218-10815-1-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | bus: mhi: Add inbound buffers allocation flag | expand |
Hi Loic, On 2020-09-23 08:16, 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. We could say, MHI device driver can be given the option to choose if it wants to have inbound buffers queued for them during channel preparation instead of having it pre-defined one-time in their configuration. > > Add a flag parameter to mhi_prepare_for_transfer allowing to specify mhi_prepare_for_transfer()? > 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. > I feel there is no need to mention this line here. We can do it as a follow-up patch to this one or address it later. > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> With some of these suggestions picked up, Reviewed-by: Bhaumik Bhatt <bbhatt@codeaurora.org> > --- > drivers/bus/mhi/core/internal.h | 2 +- > drivers/bus/mhi/core/main.c | 11 ++++++++--- > include/linux/mhi.h | 12 +++++++++++- > 3 files changed, 20 insertions(+), 5 deletions(-) > > diff --git a/drivers/bus/mhi/core/internal.h > b/drivers/bus/mhi/core/internal.h > index 33c2320..5b636e2 100644 > --- a/drivers/bus/mhi/core/internal.h > +++ b/drivers/bus/mhi/core/internal.h > @@ -666,7 +666,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, enum mhi_chan_flags 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 a44fedb..fb71a0e 100644 > --- a/drivers/bus/mhi/core/main.c > +++ b/drivers/bus/mhi/core/main.c > @@ -1274,7 +1274,8 @@ 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, > + enum mhi_chan_flags flags) > { > int ret = 0; > struct device *dev = &mhi_cntrl->mhi_dev->dev; > @@ -1334,6 +1335,9 @@ int mhi_prepare_channel(struct mhi_controller > *mhi_cntrl, > mhi_chan->ch_state = MHI_CH_STATE_ENABLED; > write_unlock_irq(&mhi_chan->lock); > > + 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, > @@ -1480,7 +1484,8 @@ 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, > + enum mhi_chan_flags flags) > { > int ret, dir; > struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; > @@ -1491,7 +1496,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/include/linux/mhi.h b/include/linux/mhi.h > index 6522a4a..9d67e75 100644 > --- a/include/linux/mhi.h > +++ b/include/linux/mhi.h > @@ -60,6 +60,14 @@ enum mhi_flags { > }; > > /** > + * enum mhi_chan_flags - MHI channel flags > + * @MHI_CH_INBOUND_ALLOC_BUFS: Automatically allocate and queue > inbound buffers > + */ > +enum mhi_chan_flags { > + MHI_CH_INBOUND_ALLOC_BUFS = BIT(0), > +}; > + > +/** > * enum mhi_device_type - Device types > * @MHI_DEVICE_XFER: Handles data transfer > * @MHI_DEVICE_CONTROLLER: Control device > @@ -684,8 +692,10 @@ void mhi_device_put(struct mhi_device *mhi_dev); > /** > * mhi_prepare_for_transfer - Setup channel for data transfer > * @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, > + enum mhi_chan_flags flags); > > /** > * mhi_unprepare_from_transfer - Unprepare the channels Thanks, Bhaumik -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Hi Loic, On Tue, Sep 29, 2020 at 03:37:12PM +0200, Loic Poulain wrote: > On Wed, 23 Sep 2020 at 17:11, Loic Poulain <loic.poulain@linaro.org> 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> > > > > Please, discard this one, since it will request ath11k and qrtr update as > well. > I'll send that in a new series. > Is this part of any series? Thanks, Mani > Regards, > Loic
Hi Mani, On Mon, 9 Nov 2020 at 11:36, Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> wrote: > > Hi Loic, > > On Tue, Sep 29, 2020 at 03:37:12PM +0200, Loic Poulain wrote: > > On Wed, 23 Sep 2020 at 17:11, Loic Poulain <loic.poulain@linaro.org> 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> > > > > > > > Please, discard this one, since it will request ath11k and qrtr update as > > well. > > I'll send that in a new series. > > > > Is this part of any series? I need to rebase and submit a new one. Regards, Loic
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index 33c2320..5b636e2 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -666,7 +666,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, enum mhi_chan_flags 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 a44fedb..fb71a0e 100644 --- a/drivers/bus/mhi/core/main.c +++ b/drivers/bus/mhi/core/main.c @@ -1274,7 +1274,8 @@ 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, + enum mhi_chan_flags flags) { int ret = 0; struct device *dev = &mhi_cntrl->mhi_dev->dev; @@ -1334,6 +1335,9 @@ int mhi_prepare_channel(struct mhi_controller *mhi_cntrl, mhi_chan->ch_state = MHI_CH_STATE_ENABLED; write_unlock_irq(&mhi_chan->lock); + 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, @@ -1480,7 +1484,8 @@ 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, + enum mhi_chan_flags flags) { int ret, dir; struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl; @@ -1491,7 +1496,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/include/linux/mhi.h b/include/linux/mhi.h index 6522a4a..9d67e75 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -60,6 +60,14 @@ enum mhi_flags { }; /** + * enum mhi_chan_flags - MHI channel flags + * @MHI_CH_INBOUND_ALLOC_BUFS: Automatically allocate and queue inbound buffers + */ +enum mhi_chan_flags { + MHI_CH_INBOUND_ALLOC_BUFS = BIT(0), +}; + +/** * enum mhi_device_type - Device types * @MHI_DEVICE_XFER: Handles data transfer * @MHI_DEVICE_CONTROLLER: Control device @@ -684,8 +692,10 @@ void mhi_device_put(struct mhi_device *mhi_dev); /** * mhi_prepare_for_transfer - Setup channel for data transfer * @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, + enum mhi_chan_flags flags); /** * mhi_unprepare_from_transfer - Unprepare the channels
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> --- drivers/bus/mhi/core/internal.h | 2 +- drivers/bus/mhi/core/main.c | 11 ++++++++--- include/linux/mhi.h | 12 +++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-)