[PATCH] bus: mhi: Remove auto-start option

[PATCH] bus: mhi: Remove auto-start option

[Loic Poulain]

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.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/bus/mhi/core/init.c     | 9 ---------
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h             | 2 --
 net/qrtr/mhi.c                  | 5 +++++
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index ac19067..7aef6b7 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -727,7 +727,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
@@ -1125,11 +1124,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;
@@ -1163,9 +1157,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 5a81a42..73b52a0 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/include/linux/mhi.h b/include/linux/mhi.h
index 008b8f6..742dabe 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;
 };
 
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index ff0c414..7100f0b 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -76,6 +76,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	struct qrtr_mhi_dev *qdev;
 	int rc;
 
+	/* start channels */
+	rc = mhi_prepare_for_transfer(mhi_dev);
+	if (rc)
+		return rc;
+
 	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
 	if (!qdev)
 		return -ENOMEM;
-- 
2.7.4

Re: [PATCH] bus: mhi: Remove auto-start option

[bbhatt]

On 2020-09-18 03:16, Loic Poulain wrote:
> 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.
> 
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> ---
>  drivers/bus/mhi/core/init.c     | 9 ---------
>  drivers/bus/mhi/core/internal.h | 1 -
>  include/linux/mhi.h             | 2 --
>  net/qrtr/mhi.c                  | 5 +++++
>  4 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index ac19067..7aef6b7 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -727,7 +727,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
> @@ -1125,11 +1124,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;
> @@ -1163,9 +1157,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 5a81a42..73b52a0 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/include/linux/mhi.h b/include/linux/mhi.h
> index 008b8f6..742dabe 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;
>  };
> 
> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index ff0c414..7100f0b 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c
> @@ -76,6 +76,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device 
> *mhi_dev,
>  	struct qrtr_mhi_dev *qdev;
>  	int rc;
> 
> +	/* start channels */
> +	rc = mhi_prepare_for_transfer(mhi_dev);
> +	if (rc)
> +		return rc;
> +
>  	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
>  	if (!qdev)
>  		return -ENOMEM;
Hi Loic,

Thank you for your patch. Hemant and I discussed and we are OK with this 
patch.

Adding Chris to ensure that QRTR changes are acceptable.

Thanks,
Bhaumik

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, a Linux Foundation Collaborative Project'

Re: [PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

Hi Loic, 

On 18 September 2020 3:46:59 PM IST, Loic Poulain <loic.poulain@linaro.org> wrote:
>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.
>
>Signed-off-by: Loic Poulain <loic.poulain@linaro.org>

The patch content looks good but you should split the qrtr change to a separate patch. I can't queue the networking change through mhi tree. 

Thanks, 
Mani

>---
> drivers/bus/mhi/core/init.c     | 9 ---------
> drivers/bus/mhi/core/internal.h | 1 -
> include/linux/mhi.h             | 2 --
> net/qrtr/mhi.c                  | 5 +++++
> 4 files changed, 5 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
>index ac19067..7aef6b7 100644
>--- a/drivers/bus/mhi/core/init.c
>+++ b/drivers/bus/mhi/core/init.c
>@@ -727,7 +727,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
>@@ -1125,11 +1124,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;
>@@ -1163,9 +1157,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 5a81a42..73b52a0 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/include/linux/mhi.h b/include/linux/mhi.h
>index 008b8f6..742dabe 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;
> };
> 
>diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
>index ff0c414..7100f0b 100644
>--- a/net/qrtr/mhi.c
>+++ b/net/qrtr/mhi.c
>@@ -76,6 +76,11 @@ static int qcom_mhi_qrtr_probe(struct mhi_device
>*mhi_dev,
> 	struct qrtr_mhi_dev *qdev;
> 	int rc;
> 
>+	/* start channels */
>+	rc = mhi_prepare_for_transfer(mhi_dev);
>+	if (rc)
>+		return rc;
>+
> 	qdev = devm_kzalloc(&mhi_dev->dev, sizeof(*qdev), GFP_KERNEL);
> 	if (!qdev)
> 		return -ENOMEM;

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH] bus: mhi: Remove auto-start option

[Loic Poulain]

On Sat, 19 Sep 2020 at 13:27, Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> Hi Loic,
>
> On 18 September 2020 3:46:59 PM IST, Loic Poulain <loic.poulain@linaro.org> wrote:
> >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.
> >
> >Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>
> The patch content looks good but you should split the qrtr change to a separate patch. I can't queue the networking change through mhi tree.

Ok going to split that.

Thanks,
Loic

Re: [PATCH] bus: mhi: Remove auto-start option

[patchwork-bot+linux-arm-msm]

Hello:

This patch was applied to qcom/linux.git (refs/heads/for-next):

On Wed, 18 Nov 2020 11:01:02 +0530 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - bus: mhi: Remove auto-start option
    https://git.kernel.org/qcom/c/ed5298c7d500

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH] bus: mhi: Remove auto-start option

[Kalle Valo]

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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] bus: mhi: Remove auto-start option

[Kalle Valo]

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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] bus: mhi: Remove auto-start option

[Bhaumik Bhatt]

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

Re: [PATCH] bus: mhi: Remove auto-start option

[Bhaumik Bhatt]

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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

Re: [PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] bus: mhi: Remove auto-start option

[Carl Huang]

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

Re: [PATCH] bus: mhi: Remove auto-start option

[Carl Huang]

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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

Re: [PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

Re: [PATCH] bus: mhi: Remove auto-start option

[Kalle Valo]

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>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] bus: mhi: Remove auto-start option

[Kalle Valo]

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>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

[PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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>
---
 drivers/bus/mhi/core/init.c           | 9 ---------
 drivers/bus/mhi/core/internal.h       | 1 -
 drivers/net/wireless/ath/ath11k/mhi.c | 4 ----
 include/linux/mhi.h                   | 2 --
 4 files changed, 16 deletions(-)

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;
 };
 
-- 
2.17.1

[PATCH] bus: mhi: Remove auto-start option

[Manivannan Sadhasivam]

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>
---
 drivers/bus/mhi/core/init.c           | 9 ---------
 drivers/bus/mhi/core/internal.h       | 1 -
 drivers/net/wireless/ath/ath11k/mhi.c | 4 ----
 include/linux/mhi.h                   | 2 --
 4 files changed, 16 deletions(-)

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;
 };
 
-- 
2.17.1


-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k