diff mbox

[v6,07/10] soundwire: keep track of Masters in a stream

Message ID 20180716184713.13356-8-sanyog.r.kale@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sanyog Kale July 16, 2018, 6:47 p.m. UTC
From: Shreyas NC <shreyas.nc@intel.com>

A multi link bankswitch can be done if the hardware supports and
the stream is handled by multiple Master(s).

This preparatory patch adds support to track m_rt in a stream.

Signed-off-by: Shreyas NC <shreyas.nc@intel.com>
Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
---
 drivers/soundwire/stream.c    | 2 ++
 include/linux/soundwire/sdw.h | 2 ++
 2 files changed, 4 insertions(+)

Comments

Pierre-Louis Bossart July 19, 2018, 3:06 p.m. UTC | #1
On 7/16/18 1:47 PM, Sanyog Kale wrote:
> From: Shreyas NC <shreyas.nc@intel.com>
> 
> A multi link bankswitch can be done if the hardware supports and
> the stream is handled by multiple Master(s).
> 
> This preparatory patch adds support to track m_rt in a stream.

The order of the patches seems off, you are adding the definition of 
m_rt_count in patch 7 but using it in patch 6, that'll break git bisect.
What am i missing?

It'd also make more sense to have the reference counts in the same 
patch, it's hard to track otherwise, so maybe one patch to add the 
definitions and inits and the second to increase/decrease+use the value 
as needed.

> 
> Signed-off-by: Shreyas NC <shreyas.nc@intel.com>
> Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
> ---
>   drivers/soundwire/stream.c    | 2 ++
>   include/linux/soundwire/sdw.h | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index 7e75a400d03e..539b98ec18d9 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -759,6 +759,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name)
>   	stream->name = stream_name;
>   	INIT_LIST_HEAD(&stream->master_list);
>   	stream->state = SDW_STREAM_ALLOCATED;
> +	stream->m_rt_count = 0;
>   
>   	return stream;
>   }
> @@ -963,6 +964,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
>   
>   		sdw_master_port_release(bus, m_rt);
>   		sdw_release_master_stream(m_rt, stream);
> +		stream->m_rt_count--;
>   	}
>   
>   	if (list_empty(&stream->master_list))
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index 03df709fb8ef..214e14604d9f 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -771,6 +771,7 @@ struct sdw_stream_params {
>    * @master_list: List of Master runtime(s) in this stream.
>    * master_list can contain only one m_rt per Master instance
>    * for a stream
> + * @m_rt_count: Count of Master runtime(s) in this stream
>    */
>   struct sdw_stream_runtime {
>   	char *name;
> @@ -778,6 +779,7 @@ struct sdw_stream_runtime {
>   	enum sdw_stream_state state;
>   	enum sdw_stream_type type;
>   	struct list_head master_list;
> +	int m_rt_count;
>   };
>   
>   struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name);
>
Vinod Koul July 20, 2018, 4:43 a.m. UTC | #2
On 19-07-18, 10:06, Pierre-Louis Bossart wrote:
> On 7/16/18 1:47 PM, Sanyog Kale wrote:
> > From: Shreyas NC <shreyas.nc@intel.com>
> > 
> > A multi link bankswitch can be done if the hardware supports and
> > the stream is handled by multiple Master(s).
> > 
> > This preparatory patch adds support to track m_rt in a stream.
> 
> The order of the patches seems off, you are adding the definition of
> m_rt_count in patch 7 but using it in patch 6, that'll break git bisect.
> What am i missing?
> 
> It'd also make more sense to have the reference counts in the same patch,
> it's hard to track otherwise, so maybe one patch to add the definitions and
> inits and the second to increase/decrease+use the value as needed.

Also it would help to have fixes first, so they go in and the bigger
changes later. That is how a series should be built typically.
Sanyog Kale July 23, 2018, 3:50 a.m. UTC | #3
On Thu, Jul 19, 2018 at 10:06:30AM -0500, Pierre-Louis Bossart wrote:
> On 7/16/18 1:47 PM, Sanyog Kale wrote:
> >From: Shreyas NC <shreyas.nc@intel.com>
> >
> >A multi link bankswitch can be done if the hardware supports and
> >the stream is handled by multiple Master(s).
> >
> >This preparatory patch adds support to track m_rt in a stream.
> 
> The order of the patches seems off, you are adding the definition of
> m_rt_count in patch 7 but using it in patch 6, that'll break git bisect.
> What am i missing?
> 
> It'd also make more sense to have the reference counts in the same patch,
> it's hard to track otherwise, so maybe one patch to add the definitions and
> inits and the second to increase/decrease+use the value as needed.
>

Ok. Will check and update accordingly.

> >
> >Signed-off-by: Shreyas NC <shreyas.nc@intel.com>
> >Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
> >---
> >  drivers/soundwire/stream.c    | 2 ++
> >  include/linux/soundwire/sdw.h | 2 ++
> >  2 files changed, 4 insertions(+)
> >
> >diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> >index 7e75a400d03e..539b98ec18d9 100644
> >--- a/drivers/soundwire/stream.c
> >+++ b/drivers/soundwire/stream.c
> >@@ -759,6 +759,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name)
> >  	stream->name = stream_name;
> >  	INIT_LIST_HEAD(&stream->master_list);
> >  	stream->state = SDW_STREAM_ALLOCATED;
> >+	stream->m_rt_count = 0;
> >  	return stream;
> >  }
> >@@ -963,6 +964,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
> >  		sdw_master_port_release(bus, m_rt);
> >  		sdw_release_master_stream(m_rt, stream);
> >+		stream->m_rt_count--;
> >  	}
> >  	if (list_empty(&stream->master_list))
> >diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> >index 03df709fb8ef..214e14604d9f 100644
> >--- a/include/linux/soundwire/sdw.h
> >+++ b/include/linux/soundwire/sdw.h
> >@@ -771,6 +771,7 @@ struct sdw_stream_params {
> >   * @master_list: List of Master runtime(s) in this stream.
> >   * master_list can contain only one m_rt per Master instance
> >   * for a stream
> >+ * @m_rt_count: Count of Master runtime(s) in this stream
> >   */
> >  struct sdw_stream_runtime {
> >  	char *name;
> >@@ -778,6 +779,7 @@ struct sdw_stream_runtime {
> >  	enum sdw_stream_state state;
> >  	enum sdw_stream_type type;
> >  	struct list_head master_list;
> >+	int m_rt_count;
> >  };
> >  struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name);
> >
>
Sanyog Kale July 23, 2018, 4 a.m. UTC | #4
On Fri, Jul 20, 2018 at 10:13:17AM +0530, Vinod wrote:
> On 19-07-18, 10:06, Pierre-Louis Bossart wrote:
> > On 7/16/18 1:47 PM, Sanyog Kale wrote:
> > > From: Shreyas NC <shreyas.nc@intel.com>
> > > 
> > > A multi link bankswitch can be done if the hardware supports and
> > > the stream is handled by multiple Master(s).
> > > 
> > > This preparatory patch adds support to track m_rt in a stream.
> > 
> > The order of the patches seems off, you are adding the definition of
> > m_rt_count in patch 7 but using it in patch 6, that'll break git bisect.
> > What am i missing?
> > 
> > It'd also make more sense to have the reference counts in the same patch,
> > it's hard to track otherwise, so maybe one patch to add the definitions and
> > inits and the second to increase/decrease+use the value as needed.
> 
> Also it would help to have fixes first, so they go in and the bigger
> changes later. That is how a series should be built typically.
>

Ok. Will take care in next update.

> -- 
> ~Vinod
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
diff mbox

Patch

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 7e75a400d03e..539b98ec18d9 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -759,6 +759,7 @@  struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name)
 	stream->name = stream_name;
 	INIT_LIST_HEAD(&stream->master_list);
 	stream->state = SDW_STREAM_ALLOCATED;
+	stream->m_rt_count = 0;
 
 	return stream;
 }
@@ -963,6 +964,7 @@  int sdw_stream_remove_master(struct sdw_bus *bus,
 
 		sdw_master_port_release(bus, m_rt);
 		sdw_release_master_stream(m_rt, stream);
+		stream->m_rt_count--;
 	}
 
 	if (list_empty(&stream->master_list))
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index 03df709fb8ef..214e14604d9f 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -771,6 +771,7 @@  struct sdw_stream_params {
  * @master_list: List of Master runtime(s) in this stream.
  * master_list can contain only one m_rt per Master instance
  * for a stream
+ * @m_rt_count: Count of Master runtime(s) in this stream
  */
 struct sdw_stream_runtime {
 	char *name;
@@ -778,6 +779,7 @@  struct sdw_stream_runtime {
 	enum sdw_stream_state state;
 	enum sdw_stream_type type;
 	struct list_head master_list;
+	int m_rt_count;
 };
 
 struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name);