diff mbox series

[01/29] bus: mhi: Remove auto-start option

Message ID 20201202094159.107075-2-manivannan.sadhasivam@linaro.org
State Accepted
Commit ed5298c7d500abaf34ed7783969e953a1f028e5b
Headers show
Series MHI changes for v5.11 | expand

Commit Message

Manivannan Sadhasivam Dec. 2, 2020, 9:41 a.m. UTC
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>

Acked-by: Kalle Valo <kvalo@codeaurora.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(-)

-- 
2.25.1

Comments

Kalle Valo Dec. 2, 2020, 4 p.m. UTC | #1
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>

> Acked-by: Kalle Valo <kvalo@codeaurora.org>

> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> [mani: clubbed ath11k change]

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>


Hmm, didn't we apply this already? At least I pulled the immutable
branch to my tree:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=526740b495059ebbc0c3c086dceca1263820fa4f

So if there's a new version of this patch (and a new commit id) the
immutable branch I pulled is not immutable anymore.

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

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Manivannan Sadhasivam Dec. 2, 2020, 4:27 p.m. UTC | #2
On Wed, Dec 02, 2020 at 06:00:05PM +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>

> > Acked-by: Kalle Valo <kvalo@codeaurora.org>

> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> > [mani: clubbed ath11k change]

> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> 

> Hmm, didn't we apply this already? At least I pulled the immutable

> branch to my tree:

> 

> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=526740b495059ebbc0c3c086dceca1263820fa4f

> 

> So if there's a new version of this patch (and a new commit id) the

> immutable branch I pulled is not immutable anymore.


This is not a new version of the patch. The commit SHA of this patch in
immutable branch is ed5298c7d500abaf34ed7783969e953a1f028e5b and that is same
in mhi-next as well.

Now I'm funneling all patches in mhi-next to mainline through Greg, so this
patch is part of the (MHI changes for v5.11) patch series. Perhaps you might be
dealing with pull requests to Dave/Jakub but since the MHI patch flow is usually
fairly low, Greg is happy with patch series.

But since we are dealing with immutable branch I should send the pull request
to Greg now.

Thanks,
Mani

> 

> -- 

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

> 

> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Kalle Valo Dec. 2, 2020, 4:57 p.m. UTC | #3
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:

> On Wed, Dec 02, 2020 at 06:00:05PM +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>

>> > Acked-by: Kalle Valo <kvalo@codeaurora.org>

>> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

>> > [mani: clubbed ath11k change]

>> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

>> 

>> Hmm, didn't we apply this already? At least I pulled the immutable

>> branch to my tree:

>> 

>> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=526740b495059ebbc0c3c086dceca1263820fa4f

>> 

>> So if there's a new version of this patch (and a new commit id) the

>> immutable branch I pulled is not immutable anymore.

>

> This is not a new version of the patch. The commit SHA of this patch in

> immutable branch is ed5298c7d500abaf34ed7783969e953a1f028e5b and that is same

> in mhi-next as well.

>

> Now I'm funneling all patches in mhi-next to mainline through Greg, so this

> patch is part of the (MHI changes for v5.11) patch series. Perhaps you might be

> dealing with pull requests to Dave/Jakub but since the MHI patch flow is usually

> fairly low, Greg is happy with patch series.

>

> But since we are dealing with immutable branch I should send the pull request

> to Greg now.


Yes, I very much prefer you send a pull request so that we can avoid
conflicts between ath11k and mhi. If Greg would apply this patch as is,
we would have two versions of the same commit (with different commit
ids) and git would get confused.

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

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Greg KH Dec. 2, 2020, 5:48 p.m. UTC | #4
On Wed, Dec 02, 2020 at 06:57:15PM +0200, Kalle Valo wrote:
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:

> 

> > On Wed, Dec 02, 2020 at 06:00:05PM +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>

> >> > Acked-by: Kalle Valo <kvalo@codeaurora.org>

> >> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> >> > [mani: clubbed ath11k change]

> >> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

> >> 

> >> Hmm, didn't we apply this already? At least I pulled the immutable

> >> branch to my tree:

> >> 

> >> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=ath-next&id=526740b495059ebbc0c3c086dceca1263820fa4f

> >> 

> >> So if there's a new version of this patch (and a new commit id) the

> >> immutable branch I pulled is not immutable anymore.

> >

> > This is not a new version of the patch. The commit SHA of this patch in

> > immutable branch is ed5298c7d500abaf34ed7783969e953a1f028e5b and that is same

> > in mhi-next as well.

> >

> > Now I'm funneling all patches in mhi-next to mainline through Greg, so this

> > patch is part of the (MHI changes for v5.11) patch series. Perhaps you might be

> > dealing with pull requests to Dave/Jakub but since the MHI patch flow is usually

> > fairly low, Greg is happy with patch series.

> >

> > But since we are dealing with immutable branch I should send the pull request

> > to Greg now.

> 

> Yes, I very much prefer you send a pull request so that we can avoid

> conflicts between ath11k and mhi. If Greg would apply this patch as is,

> we would have two versions of the same commit (with different commit

> ids) and git would get confused.


git wouldn't get confused, we do merges like this all the time.

It would confuse developers, I get confused by this type of thing all
the time :)

I'll just do the pull instead to keep my sanity, git is fine...

thanks,

greg k-h
diff mbox series

Patch

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