| Message ID | 20201118053102.13119-1-manivannan.sadhasivam@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bus: mhi: Remove auto-start option | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: > From: Loic Poulain <loic.poulain@linaro.org> > > There is really no point having an auto-start for channels. > This is confusing for the device drivers, some have to enable the > channels, others don't have... and waste resources (e.g. pre allocated > buffers) that may never be used. > > This is really up to the MHI device(channel) driver to manage the state > of its channels. > > While at it, let's also remove the auto-start option from ath11k mhi > controller. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > [mani: clubbed ath11k change] > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> Thanks and feel free to take this to the immutable branch: Acked-by: Kalle Valo <kvalo@codeaurora.org>
On Wed, Nov 18, 2020 at 07:43:48AM +0200, Kalle Valo wrote: > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: > > > From: Loic Poulain <loic.poulain@linaro.org> > > > > There is really no point having an auto-start for channels. > > This is confusing for the device drivers, some have to enable the > > channels, others don't have... and waste resources (e.g. pre allocated > > buffers) that may never be used. > > > > This is really up to the MHI device(channel) driver to manage the state > > of its channels. > > > > While at it, let's also remove the auto-start option from ath11k mhi > > controller. > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > [mani: clubbed ath11k change] > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > Thanks and feel free to take this to the immutable branch: > > Acked-by: Kalle Valo <kvalo@codeaurora.org> Patch applied to mhi-ath11k-immutable branch and merged into mhi-next. Thanks, Mani > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 2020-11-18 17:31, Manivannan Sadhasivam wrote: > On Wed, Nov 18, 2020 at 07:43:48AM +0200, Kalle Valo wrote: >> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: >> >> > From: Loic Poulain <loic.poulain@linaro.org> >> > >> > There is really no point having an auto-start for channels. >> > This is confusing for the device drivers, some have to enable the >> > channels, others don't have... and waste resources (e.g. pre allocated >> > buffers) that may never be used. >> > >> > This is really up to the MHI device(channel) driver to manage the state >> > of its channels. >> > >> > While at it, let's also remove the auto-start option from ath11k mhi >> > controller. >> > >> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> >> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> > [mani: clubbed ath11k change] >> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> >> Thanks and feel free to take this to the immutable branch: >> >> Acked-by: Kalle Valo <kvalo@codeaurora.org> > > Patch applied to mhi-ath11k-immutable branch and merged into mhi-next. > > Thanks, > Mani > Does net/qrtr/mhi.c need changes? I guess now net/qrtr/mhi.c needs to call mhi_prepare_for_transfer() before transfer. >> >> -- >> https://patchwork.kernel.org/project/linux-wireless/list/ >> >> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Wed, Nov 18, 2020 at 07:55:19PM +0800, Carl Huang wrote: > On 2020-11-18 17:31, Manivannan Sadhasivam wrote: > > On Wed, Nov 18, 2020 at 07:43:48AM +0200, Kalle Valo wrote: > > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: > > > > > > > From: Loic Poulain <loic.poulain@linaro.org> > > > > > > > > There is really no point having an auto-start for channels. > > > > This is confusing for the device drivers, some have to enable the > > > > channels, others don't have... and waste resources (e.g. pre allocated > > > > buffers) that may never be used. > > > > > > > > This is really up to the MHI device(channel) driver to manage the state > > > > of its channels. > > > > > > > > While at it, let's also remove the auto-start option from ath11k mhi > > > > controller. > > > > > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > [mani: clubbed ath11k change] > > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > > > > > Thanks and feel free to take this to the immutable branch: > > > > > > Acked-by: Kalle Valo <kvalo@codeaurora.org> > > > > Patch applied to mhi-ath11k-immutable branch and merged into mhi-next. > > > > Thanks, > > Mani > > > Does net/qrtr/mhi.c need changes? I guess now net/qrtr/mhi.c needs to call > mhi_prepare_for_transfer() before transfer. > Yes and the patch is also applied: https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/commit/?h=mhi-ath11k-immutable&id=a2e2cc0dbb1121dfa875da1c04f3dff966fec162 Thanks, Mani > > > > > > -- > > > https://patchwork.kernel.org/project/linux-wireless/list/ > > > > > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 2020-11-18 03:57 AM, Manivannan Sadhasivam wrote: > On Wed, Nov 18, 2020 at 07:55:19PM +0800, Carl Huang wrote: >> On 2020-11-18 17:31, Manivannan Sadhasivam wrote: >> > On Wed, Nov 18, 2020 at 07:43:48AM +0200, Kalle Valo wrote: >> > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: >> > > >> > > > From: Loic Poulain <loic.poulain@linaro.org> >> > > > >> > > > There is really no point having an auto-start for channels. >> > > > This is confusing for the device drivers, some have to enable the >> > > > channels, others don't have... and waste resources (e.g. pre allocated >> > > > buffers) that may never be used. >> > > > >> > > > This is really up to the MHI device(channel) driver to manage the state >> > > > of its channels. >> > > > >> > > > While at it, let's also remove the auto-start option from ath11k mhi >> > > > controller. >> > > > >> > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> >> > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> > > > [mani: clubbed ath11k change] >> > > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> > > >> > > Thanks and feel free to take this to the immutable branch: >> > > >> > > Acked-by: Kalle Valo <kvalo@codeaurora.org> >> > >> > Patch applied to mhi-ath11k-immutable branch and merged into mhi-next. >> > >> > Thanks, >> > Mani >> > >> Does net/qrtr/mhi.c need changes? I guess now net/qrtr/mhi.c needs to >> call >> mhi_prepare_for_transfer() before transfer. >> > > Yes and the patch is also applied: > https://git.kernel.org/pub/scm/linux/kernel/git/mani/mhi.git/commit/?h=mhi-ath11k-immutable&id=a2e2cc0dbb1121dfa875da1c04f3dff966fec162 > > Thanks, > Mani > >> > > >> > > -- >> > > https://patchwork.kernel.org/project/linux-wireless/list/ >> > > >> > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches It looks we forgot to add the mhi_unprepare_from_transfer() equivalent in the remove(). Will send a patch for it today. Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: > On Wed, Nov 18, 2020 at 07:43:48AM +0200, Kalle Valo wrote: >> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes: >> >> > From: Loic Poulain <loic.poulain@linaro.org> >> > >> > There is really no point having an auto-start for channels. >> > This is confusing for the device drivers, some have to enable the >> > channels, others don't have... and waste resources (e.g. pre allocated >> > buffers) that may never be used. >> > >> > This is really up to the MHI device(channel) driver to manage the state >> > of its channels. >> > >> > While at it, let's also remove the auto-start option from ath11k mhi >> > controller. >> > >> > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> >> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> > [mani: clubbed ath11k change] >> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> >> >> Thanks and feel free to take this to the immutable branch: >> >> Acked-by: Kalle Valo <kvalo@codeaurora.org> > > Patch applied to mhi-ath11k-immutable branch and merged into mhi-next. Tested on QCA6390 hw2.0 and pulled also to ath-next, thanks.
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c index 0ffdebde8265..381fdea2eb9f 100644 --- a/drivers/bus/mhi/core/init.c +++ b/drivers/bus/mhi/core/init.c @@ -758,7 +758,6 @@ static int parse_ch_cfg(struct mhi_controller *mhi_cntrl, mhi_chan->offload_ch = ch_cfg->offload_channel; mhi_chan->db_cfg.reset_req = ch_cfg->doorbell_mode_switch; mhi_chan->pre_alloc = ch_cfg->auto_queue; - mhi_chan->auto_start = ch_cfg->auto_start; /* * If MHI host allocates buffers, then the channel direction @@ -1160,11 +1159,6 @@ static int mhi_driver_probe(struct device *dev) goto exit_probe; ul_chan->xfer_cb = mhi_drv->ul_xfer_cb; - if (ul_chan->auto_start) { - ret = mhi_prepare_channel(mhi_cntrl, ul_chan); - if (ret) - goto exit_probe; - } } ret = -EINVAL; @@ -1198,9 +1192,6 @@ static int mhi_driver_probe(struct device *dev) if (ret) goto exit_probe; - if (dl_chan && dl_chan->auto_start) - mhi_prepare_channel(mhi_cntrl, dl_chan); - mhi_device_put(mhi_dev); return ret; diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h index 7989269ddd96..33c23203c531 100644 --- a/drivers/bus/mhi/core/internal.h +++ b/drivers/bus/mhi/core/internal.h @@ -563,7 +563,6 @@ struct mhi_chan { bool configured; bool offload_ch; bool pre_alloc; - bool auto_start; bool wake_capable; }; diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c index aded9a719d51..47a1ce1bee4f 100644 --- a/drivers/net/wireless/ath/ath11k/mhi.c +++ b/drivers/net/wireless/ath/ath11k/mhi.c @@ -24,7 +24,6 @@ static struct mhi_channel_config ath11k_mhi_channels[] = { .offload_channel = false, .doorbell_mode_switch = false, .auto_queue = false, - .auto_start = false, }, { .num = 1, @@ -39,7 +38,6 @@ static struct mhi_channel_config ath11k_mhi_channels[] = { .offload_channel = false, .doorbell_mode_switch = false, .auto_queue = false, - .auto_start = false, }, { .num = 20, @@ -54,7 +52,6 @@ static struct mhi_channel_config ath11k_mhi_channels[] = { .offload_channel = false, .doorbell_mode_switch = false, .auto_queue = false, - .auto_start = true, }, { .num = 21, @@ -69,7 +66,6 @@ static struct mhi_channel_config ath11k_mhi_channels[] = { .offload_channel = false, .doorbell_mode_switch = false, .auto_queue = true, - .auto_start = true, }, }; diff --git a/include/linux/mhi.h b/include/linux/mhi.h index d4841e5a5f45..6522a4adc794 100644 --- a/include/linux/mhi.h +++ b/include/linux/mhi.h @@ -214,7 +214,6 @@ enum mhi_db_brst_mode { * @offload_channel: The client manages the channel completely * @doorbell_mode_switch: Channel switches to doorbell mode on M0 transition * @auto_queue: Framework will automatically queue buffers for DL traffic - * @auto_start: Automatically start (open) this channel * @wake-capable: Channel capable of waking up the system */ struct mhi_channel_config { @@ -232,7 +231,6 @@ struct mhi_channel_config { bool offload_channel; bool doorbell_mode_switch; bool auto_queue; - bool auto_start; bool wake_capable; };