[v4,4/7] soundwire: Handle multiple master instances in a stream

Message ID 1529924340-30065-5-git-send-email-shreyas.nc@intel.com
State New
Headers show
Series
  • Untitled series #13065
Related show

Commit Message

Nc, Shreyas June 25, 2018, 10:58 a.m.
From: Vinod Koul <vkoul@kernel.org>


For each SoundWire stream operation, we need to parse master
list and operate upon all master runtime.

This is a preparatory patch to do the boilerplate conversion
of stream handling from single master runtime to handle a
list of master runtime. The code to support bank switch for
multiple master instances is added in the next patch.

Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>

Signed-off-by: Vinod Koul <vkoul@kernel.org>

Signed-off-by: Shreyas NC <shreyas.nc@intel.com>

---
 drivers/soundwire/stream.c    | 309 +++++++++++++++++++++++++-----------------
 include/linux/soundwire/sdw.h |   2 -
 2 files changed, 186 insertions(+), 125 deletions(-)

-- 
2.7.4

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Comments

Pierre-Louis Bossart July 2, 2018, 8:22 p.m. | #1
>   /**

> @@ -918,13 +951,22 @@ static void sdw_release_master_stream(struct sdw_stream_runtime *stream)

>   int sdw_stream_remove_master(struct sdw_bus *bus,

>   		struct sdw_stream_runtime *stream)

>   {

> +	struct sdw_master_runtime *m_rt, *_m_rt;

> +

>   	mutex_lock(&bus->bus_lock);

>   

> -	sdw_release_master_stream(stream);

> -	sdw_master_port_release(bus, stream->m_rt);

> -	stream->state = SDW_STREAM_RELEASED;

> -	kfree(stream->m_rt);

> -	stream->m_rt = NULL;

> +	list_for_each_entry_safe(m_rt, _m_rt,

> +			&stream->master_list, stream_node) {

> +

> +		if (m_rt->bus != bus)

> +			continue;

> +

> +		sdw_master_port_release(bus, m_rt);

> +		sdw_release_master_stream(m_rt, stream);

> +	}

> +

> +	if (list_empty(&stream->master_list))

> +		stream->state = SDW_STREAM_RELEASED;

When a master is removed, there is an explicit test to make sure the 
stream state changes when there are no masters left in the list, but...

>   

>   	mutex_unlock(&bus->bus_lock);

>   

> @@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,

>   	stream->state = SDW_STREAM_CONFIGURED;

... it's not symmetrical for the add_master case. The stream state 
changes on the first added master. In addition the stream state changes 
both when a slave is added and a master is added.
Is this intentional or not - and are there side effects resulting from 
this inconsistency?

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Nc, Shreyas July 3, 2018, 1:13 a.m. | #2
On Mon, Jul 02, 2018 at 03:22:07PM -0500, Pierre-Louis Bossart wrote:
> 

> >  /**

> >@@ -918,13 +951,22 @@ static void sdw_release_master_stream(struct sdw_stream_runtime *stream)

> >  int sdw_stream_remove_master(struct sdw_bus *bus,

> >  		struct sdw_stream_runtime *stream)

> >  {

> >+	struct sdw_master_runtime *m_rt, *_m_rt;

> >+

> >  	mutex_lock(&bus->bus_lock);

> >-	sdw_release_master_stream(stream);

> >-	sdw_master_port_release(bus, stream->m_rt);

> >-	stream->state = SDW_STREAM_RELEASED;

> >-	kfree(stream->m_rt);

> >-	stream->m_rt = NULL;

> >+	list_for_each_entry_safe(m_rt, _m_rt,

> >+			&stream->master_list, stream_node) {

> >+

> >+		if (m_rt->bus != bus)

> >+			continue;

> >+

> >+		sdw_master_port_release(bus, m_rt);

> >+		sdw_release_master_stream(m_rt, stream);

> >+	}

> >+

> >+	if (list_empty(&stream->master_list))

> >+		stream->state = SDW_STREAM_RELEASED;

> When a master is removed, there is an explicit test to make sure the stream

> state changes when there are no masters left in the list, but...

>

> >  	mutex_unlock(&bus->bus_lock);

> >@@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,

> >  	stream->state = SDW_STREAM_CONFIGURED;

> ... it's not symmetrical for the add_master case. The stream state changes

> on the first added master. In addition the stream state changes both when a

> slave is added and a master is added.

> Is this intentional or not - and are there side effects resulting from this

> inconsistency?

>


For remove_master, we already know the number of Masters in the stream and
hence we change the state to RELEASED only when there are no Masters
left in stream.

But, in the add_master case, we have no idea on how many master instances
are part of the stream and hence how many times add_master would be called.
So, we change the stream state to CONFIGURED when the first Master is added.
I can add a comment if that helps :)

Yes, it is added intentionally (or rather because we could not think of any
other suitable way) and we dont see any side effects. Anything
that you could think of?

IIRC, we had this discussion when the stream series was posted. But I am
unable to find those mails :(

Thanks for the review!

--Shreyas
-- 
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Pierre-Louis Bossart July 3, 2018, 3:03 p.m. | #3
On 7/2/18 8:13 PM, Shreyas NC wrote:
> On Mon, Jul 02, 2018 at 03:22:07PM -0500, Pierre-Louis Bossart wrote:

>>

>>>   /**

>>> @@ -918,13 +951,22 @@ static void sdw_release_master_stream(struct sdw_stream_runtime *stream)

>>>   int sdw_stream_remove_master(struct sdw_bus *bus,

>>>   		struct sdw_stream_runtime *stream)

>>>   {

>>> +	struct sdw_master_runtime *m_rt, *_m_rt;

>>> +

>>>   	mutex_lock(&bus->bus_lock);

>>> -	sdw_release_master_stream(stream);

>>> -	sdw_master_port_release(bus, stream->m_rt);

>>> -	stream->state = SDW_STREAM_RELEASED;

>>> -	kfree(stream->m_rt);

>>> -	stream->m_rt = NULL;

>>> +	list_for_each_entry_safe(m_rt, _m_rt,

>>> +			&stream->master_list, stream_node) {

>>> +

>>> +		if (m_rt->bus != bus)

>>> +			continue;

>>> +

>>> +		sdw_master_port_release(bus, m_rt);

>>> +		sdw_release_master_stream(m_rt, stream);

>>> +	}

>>> +

>>> +	if (list_empty(&stream->master_list))

>>> +		stream->state = SDW_STREAM_RELEASED;

>> When a master is removed, there is an explicit test to make sure the stream

>> state changes when there are no masters left in the list, but...

>>

>>>   	mutex_unlock(&bus->bus_lock);

>>> @@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,

>>>   	stream->state = SDW_STREAM_CONFIGURED;

>> ... it's not symmetrical for the add_master case. The stream state changes

>> on the first added master. In addition the stream state changes both when a

>> slave is added and a master is added.

>> Is this intentional or not - and are there side effects resulting from this

>> inconsistency?

>>

> 

> For remove_master, we already know the number of Masters in the stream and

> hence we change the state to RELEASED only when there are no Masters

> left in stream.

> 

> But, in the add_master case, we have no idea on how many master instances

> are part of the stream and hence how many times add_master would be called.

> So, we change the stream state to CONFIGURED when the first Master is added.

> I can add a comment if that helps :)


I get the point, but you currently change the state for the first slave 
that's added, so there is an inconsistency here (even before we add the 
multi-master support).
If I wanted to split hair I'd also note it's almost like the state is 
CONFIGURING rather than CONFIGURED if you don't really control when all 
configurations are complete at the bus level and depend on external 
transitions (e.g. DAPM/ALSA initiated) to go in PREPARED mode.

> 

> Yes, it is added intentionally (or rather because we could not think of any

> other suitable way) and we dont see any side effects. Anything

> that you could think of?


We should check what happens if the stream is removed before being 
enabled, or all cases of testing stream->state.

> 

> IIRC, we had this discussion when the stream series was posted. But I am

> unable to find those mails :(


I've seen SoundWire patches for the last 2 years and don't have the 
memory of all decisions and directions either...

> 

> Thanks for the review!

> 

> --Shreyas

> 


_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Nc, Shreyas July 3, 2018, 4:03 p.m. | #4
> >>> +	struct sdw_master_runtime *m_rt, *_m_rt;

> >>> +

> >>>   	mutex_lock(&bus->bus_lock);

> >>> -	sdw_release_master_stream(stream);

> >>> -	sdw_master_port_release(bus, stream->m_rt);

> >>> -	stream->state = SDW_STREAM_RELEASED;

> >>> -	kfree(stream->m_rt);

> >>> -	stream->m_rt = NULL;

> >>> +	list_for_each_entry_safe(m_rt, _m_rt,

> >>> +			&stream->master_list, stream_node) {

> >>> +

> >>> +		if (m_rt->bus != bus)

> >>> +			continue;

> >>> +

> >>> +		sdw_master_port_release(bus, m_rt);

> >>> +		sdw_release_master_stream(m_rt, stream);

> >>> +	}

> >>> +

> >>> +	if (list_empty(&stream->master_list))

> >>> +		stream->state = SDW_STREAM_RELEASED;

> >> When a master is removed, there is an explicit test to make sure the

> >> stream state changes when there are no masters left in the list, but...

> >>

> >>>   	mutex_unlock(&bus->bus_lock);

> >>> @@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus

> *bus,

> >>>   	stream->state = SDW_STREAM_CONFIGURED;

> >> ... it's not symmetrical for the add_master case. The stream state

> >> changes on the first added master. In addition the stream state

> >> changes both when a slave is added and a master is added.

> >> Is this intentional or not - and are there side effects resulting

> >> from this inconsistency?

> >>

> >

> > For remove_master, we already know the number of Masters in the stream

> > and hence we change the state to RELEASED only when there are no

> > Masters left in stream.

> >

> > But, in the add_master case, we have no idea on how many master

> > instances are part of the stream and hence how many times add_master

> would be called.

> > So, we change the stream state to CONFIGURED when the first Master is

> added.

> > I can add a comment if that helps :)

> 

> I get the point, but you currently change the state for the first slave that's

> added, so there is an inconsistency here (even before we add the multi-master

> support).


When we add a slave, we check if there is an existing m_rt for that bus instance
and if It does not exist we create a m_rt for that master instance, add the slave
runtime and change the state to CONFIGURED. So technically we still change
the state after adding the first m_rt.

> If I wanted to split hair I'd also note it's almost like the state is CONFIGURING

> rather than CONFIGURED if you don't really control when all configurations are

> complete at the bus level and depend on external transitions (e.g. DAPM/ALSA

> initiated) to go in PREPARED mode.

> 

> >

> > Yes, it is added intentionally (or rather because we could not think

> > of any other suitable way) and we dont see any side effects. Anything

> > that you could think of?

> 

> We should check what happens if the stream is removed before being enabled,

> or all cases of testing stream->state.


While releasing the stream we unconditionally clean up things without checking
stream state. So, we should be fine that way. But, I can check these :)

--Shreyas
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Pierre-Louis Bossart July 3, 2018, 6:59 p.m. | #5
On 07/03/2018 11:03 AM, Nc, Shreyas wrote:
>>>>> +	struct sdw_master_runtime *m_rt, *_m_rt;
>>>>> +
>>>>>    	mutex_lock(&bus->bus_lock);
>>>>> -	sdw_release_master_stream(stream);
>>>>> -	sdw_master_port_release(bus, stream->m_rt);
>>>>> -	stream->state = SDW_STREAM_RELEASED;
>>>>> -	kfree(stream->m_rt);
>>>>> -	stream->m_rt = NULL;
>>>>> +	list_for_each_entry_safe(m_rt, _m_rt,
>>>>> +			&stream->master_list, stream_node) {
>>>>> +
>>>>> +		if (m_rt->bus != bus)
>>>>> +			continue;
>>>>> +
>>>>> +		sdw_master_port_release(bus, m_rt);
>>>>> +		sdw_release_master_stream(m_rt, stream);
>>>>> +	}
>>>>> +
>>>>> +	if (list_empty(&stream->master_list))
>>>>> +		stream->state = SDW_STREAM_RELEASED;
>>>> When a master is removed, there is an explicit test to make sure the
>>>> stream state changes when there are no masters left in the list, but...
>>>>
>>>>>    	mutex_unlock(&bus->bus_lock);
>>>>> @@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus
>> *bus,
>>>>>    	stream->state = SDW_STREAM_CONFIGURED;
>>>> ... it's not symmetrical for the add_master case. The stream state
>>>> changes on the first added master. In addition the stream state
>>>> changes both when a slave is added and a master is added.
>>>> Is this intentional or not - and are there side effects resulting
>>>> from this inconsistency?
>>>>
>>> For remove_master, we already know the number of Masters in the stream
>>> and hence we change the state to RELEASED only when there are no
>>> Masters left in stream.
>>>
>>> But, in the add_master case, we have no idea on how many master
>>> instances are part of the stream and hence how many times add_master
>> would be called.
>>> So, we change the stream state to CONFIGURED when the first Master is
>> added.
>>> I can add a comment if that helps :)
>> I get the point, but you currently change the state for the first slave that's
>> added, so there is an inconsistency here (even before we add the multi-master
>> support).
> When we add a slave, we check if there is an existing m_rt for that bus instance
> and if It does not exist we create a m_rt for that master instance, add the slave
> runtime and change the state to CONFIGURED. So technically we still change
> the state after adding the first m_rt.

If you made no assumption on the order in which slaves and masters are 
added then you need this assignment to CONFIGURED twice. But if you know 
the slaves are added first then the second assignment is irrelevant.
It's hard to memorize really since there are parts where you assume 
things are triggered by external events and others where there is no 
assumption on API use...

>
>> If I wanted to split hair I'd also note it's almost like the state is CONFIGURING
>> rather than CONFIGURED if you don't really control when all configurations are
>> complete at the bus level and depend on external transitions (e.g. DAPM/ALSA
>> initiated) to go in PREPARED mode.
>>
>>> Yes, it is added intentionally (or rather because we could not think
>>> of any other suitable way) and we dont see any side effects. Anything
>>> that you could think of?
>> We should check what happens if the stream is removed before being enabled,
>> or all cases of testing stream->state.
> While releasing the stream we unconditionally clean up things without checking
> stream state. So, we should be fine that way. But, I can check these :)
A quick check for stream->state gives me this:

     stream->state = SDW_STREAM_CONFIGURED;

 >>> so here it's a success case but we fallback to an error case...

stream_error:
     sdw_release_master_stream(m_rt, stream);
error:
     mutex_unlock(&bus->bus_lock);
     return ret;

You are probably missing a

     stream->state = SDW_STREAM_CONFIGURED;
     goto error; //<<< Add this

as done for the slave?

Wondering how this ever worked, you may want to triple-check the regular 
stream management before adding the multi-master stuff?

>
> --Shreyas
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Nc, Shreyas July 4, 2018, 12:17 a.m. | #6
> On 07/03/2018 11:03 AM, Nc, Shreyas wrote:

> >>>>> +	struct sdw_master_runtime *m_rt, *_m_rt;

> >>>>> +

> >>>>>    	mutex_lock(&bus->bus_lock);

> >>>>> -	sdw_release_master_stream(stream);

> >>>>> -	sdw_master_port_release(bus, stream->m_rt);

> >>>>> -	stream->state = SDW_STREAM_RELEASED;

> >>>>> -	kfree(stream->m_rt);

> >>>>> -	stream->m_rt = NULL;

> >>>>> +	list_for_each_entry_safe(m_rt, _m_rt,

> >>>>> +			&stream->master_list, stream_node) {

> >>>>> +

> >>>>> +		if (m_rt->bus != bus)

> >>>>> +			continue;

> >>>>> +

> >>>>> +		sdw_master_port_release(bus, m_rt);

> >>>>> +		sdw_release_master_stream(m_rt, stream);

> >>>>> +	}

> >>>>> +

> >>>>> +	if (list_empty(&stream->master_list))

> >>>>> +		stream->state = SDW_STREAM_RELEASED;

> >>>> When a master is removed, there is an explicit test to make sure

> >>>> the stream state changes when there are no masters left in the list, but...

> >>>>

> >>>>>    	mutex_unlock(&bus->bus_lock);

> >>>>> @@ -1127,7 +1169,7 @@ int sdw_stream_add_master(struct sdw_bus

> >> *bus,

> >>>>>    	stream->state = SDW_STREAM_CONFIGURED;

> >>>> ... it's not symmetrical for the add_master case. The stream state

> >>>> changes on the first added master. In addition the stream state

> >>>> changes both when a slave is added and a master is added.

> >>>> Is this intentional or not - and are there side effects resulting

> >>>> from this inconsistency?

> >>>>

> >>> For remove_master, we already know the number of Masters in the

> >>> stream and hence we change the state to RELEASED only when there are

> >>> no Masters left in stream.

> >>>

> >>> But, in the add_master case, we have no idea on how many master

> >>> instances are part of the stream and hence how many times add_master

> >> would be called.

> >>> So, we change the stream state to CONFIGURED when the first Master

> >>> is

> >> added.

> >>> I can add a comment if that helps :)

> >> I get the point, but you currently change the state for the first

> >> slave that's added, so there is an inconsistency here (even before we

> >> add the multi-master support).

> > When we add a slave, we check if there is an existing m_rt for that

> > bus instance and if It does not exist we create a m_rt for that master

> > instance, add the slave runtime and change the state to CONFIGURED. So

> > technically we still change the state after adding the first m_rt.

> 

> If you made no assumption on the order in which slaves and masters are added

> then you need this assignment to CONFIGURED twice. But if you know the slaves

> are added first then the second assignment is irrelevant.

> It's hard to memorize really since there are parts where you assume things are

> triggered by external events and others where there is no assumption on API

> use...


Ok, will fix this. 
Vinod, would you like to have a patch for fixing this in the same series or a
patch later on ?

> 

> >

> >> If I wanted to split hair I'd also note it's almost like the state is

> >> CONFIGURING rather than CONFIGURED if you don't really control when

> >> all configurations are complete at the bus level and depend on

> >> external transitions (e.g. DAPM/ALSA

> >> initiated) to go in PREPARED mode.

> >>

> >>> Yes, it is added intentionally (or rather because we could not think

> >>> of any other suitable way) and we dont see any side effects.

> >>> Anything that you could think of?

> >> We should check what happens if the stream is removed before being

> >> enabled, or all cases of testing stream->state.

> > While releasing the stream we unconditionally clean up things without

> > checking stream state. So, we should be fine that way. But, I can

> > check these :)

> A quick check for stream->state gives me this:

> 

>      stream->state = SDW_STREAM_CONFIGURED;

> 

>  >>> so here it's a success case but we fallback to an error case...

> 

> stream_error:

>      sdw_release_master_stream(m_rt, stream);

> error:

>      mutex_unlock(&bus->bus_lock);

>      return ret;

> 

> You are probably missing a

> 

>      stream->state = SDW_STREAM_CONFIGURED;

>      goto error; //<<< Add this

> 

> as done for the slave?

> 

> Wondering how this ever worked, you may want to triple-check the regular

> stream management before adding the multi-master stuff?


Yes, you are right. It works because of the change being a part of another change
that is yet to be up streamed :(

Wondering how this slipped, this should have been part of the stream series itself.
Will fix this up :)

--Shreyas
Vinod Koul July 4, 2018, 4:24 a.m. | #7
On 04-07-18, 00:17, Nc, Shreyas wrote:
> > On 07/03/2018 11:03 AM, Nc, Shreyas wrote:


> Ok, will fix this. 

> Vinod, would you like to have a patch for fixing this in the same series or a

> patch later on ?


It is a fix, so early on please :)
-- 
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Patch

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index eb942c6..0e8a2eb5 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -681,35 +681,45 @@  static int sdw_bank_switch(struct sdw_bus *bus)
 
 static int do_bank_switch(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
+	struct sdw_master_runtime *m_rt = NULL;
 	const struct sdw_master_ops *ops;
-	struct sdw_bus *bus = m_rt->bus;
+	struct sdw_bus *bus = NULL;
 	int ret = 0;
 
-	ops = bus->ops;
 
-	/* Pre-bank switch */
-	if (ops->pre_bank_switch) {
-		ret = ops->pre_bank_switch(bus);
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		ops = bus->ops;
+
+		/* Pre-bank switch */
+		if (ops->pre_bank_switch) {
+			ret = ops->pre_bank_switch(bus);
+			if (ret < 0) {
+				dev_err(bus->dev,
+					"Pre bank switch op failed: %d", ret);
+				return ret;
+			}
+		}
+
+		/* Bank switch */
+		ret = sdw_bank_switch(bus);
 		if (ret < 0) {
-			dev_err(bus->dev, "Pre bank switch op failed: %d", ret);
+			dev_err(bus->dev, "Bank switch failed: %d", ret);
 			return ret;
 		}
 	}
 
-	/* Bank switch */
-	ret = sdw_bank_switch(bus);
-	if (ret < 0) {
-		dev_err(bus->dev, "Bank switch failed: %d", ret);
-		return ret;
-	}
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		ops = bus->ops;
 
-	/* Post-bank switch */
-	if (ops->post_bank_switch) {
-		ret = ops->post_bank_switch(bus);
-		if (ret < 0) {
-			dev_err(bus->dev,
+		/* Post-bank switch */
+		if (ops->post_bank_switch) {
+			ret = ops->post_bank_switch(bus);
+			if (ret < 0) {
+				dev_err(bus->dev,
 					"Post bank switch op failed: %d", ret);
+			}
 		}
 	}
 
@@ -754,6 +764,21 @@  struct sdw_stream_runtime *sdw_alloc_stream(char *stream_name)
 }
 EXPORT_SYMBOL(sdw_alloc_stream);
 
+static struct sdw_master_runtime
+*sdw_find_master_rt(struct sdw_bus *bus,
+			struct sdw_stream_runtime *stream)
+{
+	struct sdw_master_runtime *m_rt = NULL;
+
+	/* Retrieve Bus handle if already available */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		if (m_rt->bus == bus)
+			return m_rt;
+	}
+
+	return NULL;
+}
+
 /**
  * sdw_alloc_master_rt() - Allocates and initialize Master runtime handle
  *
@@ -770,12 +795,11 @@  static struct sdw_master_runtime
 {
 	struct sdw_master_runtime *m_rt;
 
-	m_rt = stream->m_rt;
-
 	/*
 	 * check if Master is already allocated (as a result of Slave adding
 	 * it first), if so skip allocation and go to configure
 	 */
+	m_rt = sdw_find_master_rt(bus, stream);
 	if (m_rt)
 		goto stream_config;
 
@@ -786,7 +810,7 @@  static struct sdw_master_runtime
 	/* Initialization of Master runtime handle */
 	INIT_LIST_HEAD(&m_rt->port_list);
 	INIT_LIST_HEAD(&m_rt->slave_rt_list);
-	stream->m_rt = m_rt;
+	list_add_tail(&m_rt->stream_node, &stream->master_list);
 
 	list_add_tail(&m_rt->bus_node, &bus->m_rt_list);
 
@@ -844,17 +868,21 @@  static void sdw_slave_port_release(struct sdw_bus *bus,
 			struct sdw_stream_runtime *stream)
 {
 	struct sdw_port_runtime *p_rt, *_p_rt;
-	struct sdw_master_runtime *m_rt = stream->m_rt;
+	struct sdw_master_runtime *m_rt;
 	struct sdw_slave_runtime *s_rt;
 
-	list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
-		if (s_rt->slave != slave)
-			continue;
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		list_for_each_entry(s_rt, &m_rt->slave_rt_list, m_rt_node) {
 
-		list_for_each_entry_safe(p_rt, _p_rt,
-				&s_rt->port_list, port_node) {
-			list_del(&p_rt->port_node);
-			kfree(p_rt);
+			if (s_rt->slave != slave)
+				continue;
+
+			list_for_each_entry_safe(p_rt, _p_rt,
+					&s_rt->port_list, port_node) {
+
+				list_del(&p_rt->port_node);
+				kfree(p_rt);
+			}
 		}
 	}
 }
@@ -871,16 +899,18 @@  static void sdw_release_slave_stream(struct sdw_slave *slave,
 			struct sdw_stream_runtime *stream)
 {
 	struct sdw_slave_runtime *s_rt, *_s_rt;
-	struct sdw_master_runtime *m_rt = stream->m_rt;
-
-	/* Retrieve Slave runtime handle */
-	list_for_each_entry_safe(s_rt, _s_rt,
-			&m_rt->slave_rt_list, m_rt_node) {
+	struct sdw_master_runtime *m_rt;
 
-		if (s_rt->slave == slave) {
-			list_del(&s_rt->m_rt_node);
-			kfree(s_rt);
-			return;
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		/* Retrieve Slave runtime handle */
+		list_for_each_entry_safe(s_rt, _s_rt,
+					&m_rt->slave_rt_list, m_rt_node) {
+
+			if (s_rt->slave == slave) {
+				list_del(&s_rt->m_rt_node);
+				kfree(s_rt);
+				return;
+			}
 		}
 	}
 }
@@ -888,6 +918,7 @@  static void sdw_release_slave_stream(struct sdw_slave *slave,
 /**
  * sdw_release_master_stream() - Free Master runtime handle
  *
+ * @m_rt: Master runtime node
  * @stream: Stream runtime handle.
  *
  * This function is to be called with bus_lock held
@@ -895,16 +926,18 @@  static void sdw_release_slave_stream(struct sdw_slave *slave,
  * handle. If this is called first then sdw_release_slave_stream() will have
  * no effect as Slave(s) runtime handle would already be freed up.
  */
-static void sdw_release_master_stream(struct sdw_stream_runtime *stream)
+static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
+			struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
 	struct sdw_slave_runtime *s_rt, *_s_rt;
 
 	list_for_each_entry_safe(s_rt, _s_rt,
 			&m_rt->slave_rt_list, m_rt_node)
 		sdw_stream_remove_slave(s_rt->slave, stream);
 
+	list_del(&m_rt->stream_node);
 	list_del(&m_rt->bus_node);
+	kfree(m_rt);
 }
 
 /**
@@ -918,13 +951,22 @@  static void sdw_release_master_stream(struct sdw_stream_runtime *stream)
 int sdw_stream_remove_master(struct sdw_bus *bus,
 		struct sdw_stream_runtime *stream)
 {
+	struct sdw_master_runtime *m_rt, *_m_rt;
+
 	mutex_lock(&bus->bus_lock);
 
-	sdw_release_master_stream(stream);
-	sdw_master_port_release(bus, stream->m_rt);
-	stream->state = SDW_STREAM_RELEASED;
-	kfree(stream->m_rt);
-	stream->m_rt = NULL;
+	list_for_each_entry_safe(m_rt, _m_rt,
+			&stream->master_list, stream_node) {
+
+		if (m_rt->bus != bus)
+			continue;
+
+		sdw_master_port_release(bus, m_rt);
+		sdw_release_master_stream(m_rt, stream);
+	}
+
+	if (list_empty(&stream->master_list))
+		stream->state = SDW_STREAM_RELEASED;
 
 	mutex_unlock(&bus->bus_lock);
 
@@ -1127,7 +1169,7 @@  int sdw_stream_add_master(struct sdw_bus *bus,
 	stream->state = SDW_STREAM_CONFIGURED;
 
 stream_error:
-	sdw_release_master_stream(stream);
+	sdw_release_master_stream(m_rt, stream);
 error:
 	mutex_unlock(&bus->bus_lock);
 	return ret;
@@ -1195,7 +1237,7 @@  int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * we hit error so cleanup the stream, release all Slave(s) and
 	 * Master runtime
 	 */
-	sdw_release_master_stream(stream);
+	sdw_release_master_stream(m_rt, stream);
 error:
 	mutex_unlock(&slave->bus->bus_lock);
 	return ret;
@@ -1277,31 +1319,36 @@  static void sdw_release_bus_lock(struct sdw_stream_runtime *stream)
 
 static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
-	struct sdw_bus *bus = m_rt->bus;
+	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_bus *bus = NULL;
 	struct sdw_master_prop *prop = NULL;
 	struct sdw_bus_params params;
 	int ret;
 
-	prop = &bus->prop;
-	memcpy(&params, &bus->params, sizeof(params));
+	/* Prepare  Master(s) and Slave(s) port(s) associated with stream */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		prop = &bus->prop;
+		memcpy(&params, &bus->params, sizeof(params));
 
-	/* TODO: Support Asynchronous mode */
-	if ((prop->max_freq % stream->params.rate) != 0) {
-		dev_err(bus->dev, "Async mode not supported");
-		return -EINVAL;
-	}
+		/* TODO: Support Asynchronous mode */
+		if ((prop->max_freq % stream->params.rate) != 0) {
+			dev_err(bus->dev, "Async mode not supported");
+			return -EINVAL;
+		}
 
-	/* Increment cumulative bus bandwidth */
-	/* TODO: Update this during Device-Device support */
-	bus->params.bandwidth += m_rt->stream->params.rate *
-		m_rt->ch_count * m_rt->stream->params.bps;
+		/* Increment cumulative bus bandwidth */
+		/* TODO: Update this during Device-Device support */
+		bus->params.bandwidth += m_rt->stream->params.rate *
+			m_rt->ch_count * m_rt->stream->params.bps;
+
+		/* Program params */
+		ret = sdw_program_params(bus);
+		if (ret < 0) {
+			dev_err(bus->dev, "Program params failed: %d", ret);
+			goto restore_params;
+		}
 
-	/* Program params */
-	ret = sdw_program_params(bus);
-	if (ret < 0) {
-		dev_err(bus->dev, "Program params failed: %d", ret);
-		goto restore_params;
 	}
 
 	ret = do_bank_switch(stream);
@@ -1310,12 +1357,16 @@  static int _sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		goto restore_params;
 	}
 
-	/* Prepare port(s) on the new clock configuration */
-	ret = sdw_prep_deprep_ports(m_rt, true);
-	if (ret < 0) {
-		dev_err(bus->dev, "Prepare port(s) failed ret = %d",
-				ret);
-		return ret;
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+
+		/* Prepare port(s) on the new clock configuration */
+		ret = sdw_prep_deprep_ports(m_rt, true);
+		if (ret < 0) {
+			dev_err(bus->dev, "Prepare port(s) failed ret = %d",
+					ret);
+			return ret;
+		}
 	}
 
 	stream->state = SDW_STREAM_PREPARED;
@@ -1343,35 +1394,40 @@  int sdw_prepare_stream(struct sdw_stream_runtime *stream)
 		return -EINVAL;
 	}
 
-	mutex_lock(&stream->m_rt->bus->bus_lock);
+	sdw_acquire_bus_lock(stream);
 
 	ret = _sdw_prepare_stream(stream);
 	if (ret < 0)
 		pr_err("Prepare for stream:%s failed: %d", stream->name, ret);
 
-	mutex_unlock(&stream->m_rt->bus->bus_lock);
+	sdw_release_bus_lock(stream);
 	return ret;
 }
 EXPORT_SYMBOL(sdw_prepare_stream);
 
 static int _sdw_enable_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
-	struct sdw_bus *bus = m_rt->bus;
+	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_bus *bus = NULL;
 	int ret;
 
-	/* Program params */
-	ret = sdw_program_params(bus);
-	if (ret < 0) {
-		dev_err(bus->dev, "Program params failed: %d", ret);
-		return ret;
-	}
+	/* Enable Master(s) and Slave(s) port(s) associated with stream */
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
 
-	/* Enable port(s) */
-	ret = sdw_enable_disable_ports(m_rt, true);
-	if (ret < 0) {
-		dev_err(bus->dev, "Enable port(s) failed ret: %d", ret);
-		return ret;
+		/* Program params */
+		ret = sdw_program_params(bus);
+		if (ret < 0) {
+			dev_err(bus->dev, "Program params failed: %d", ret);
+			return ret;
+		}
+
+		/* Enable port(s) */
+		ret = sdw_enable_disable_ports(m_rt, true);
+		if (ret < 0) {
+			dev_err(bus->dev, "Enable port(s) failed ret: %d", ret);
+			return ret;
+		}
 	}
 
 	ret = do_bank_switch(stream);
@@ -1400,37 +1456,42 @@  int sdw_enable_stream(struct sdw_stream_runtime *stream)
 		return -EINVAL;
 	}
 
-	mutex_lock(&stream->m_rt->bus->bus_lock);
+	sdw_acquire_bus_lock(stream);
 
 	ret = _sdw_enable_stream(stream);
 	if (ret < 0)
 		pr_err("Enable for stream:%s failed: %d", stream->name, ret);
 
-	mutex_unlock(&stream->m_rt->bus->bus_lock);
+	sdw_release_bus_lock(stream);
 	return ret;
 }
 EXPORT_SYMBOL(sdw_enable_stream);
 
 static int _sdw_disable_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
-	struct sdw_bus *bus = m_rt->bus;
+	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_bus *bus = NULL;
 	int ret;
 
-	/* Disable port(s) */
-	ret = sdw_enable_disable_ports(m_rt, false);
-	if (ret < 0) {
-		dev_err(bus->dev, "Disable port(s) failed: %d", ret);
-		return ret;
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		/* Disable port(s) */
+		ret = sdw_enable_disable_ports(m_rt, false);
+		if (ret < 0) {
+			dev_err(bus->dev, "Disable port(s) failed: %d", ret);
+			return ret;
+		}
 	}
-
 	stream->state = SDW_STREAM_DISABLED;
 
-	/* Program params */
-	ret = sdw_program_params(bus);
-	if (ret < 0) {
-		dev_err(bus->dev, "Program params failed: %d", ret);
-		return ret;
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		/* Program params */
+		ret = sdw_program_params(bus);
+		if (ret < 0) {
+			dev_err(bus->dev, "Program params failed: %d", ret);
+			return ret;
+		}
 	}
 
 	return do_bank_switch(stream);
@@ -1452,43 +1513,46 @@  int sdw_disable_stream(struct sdw_stream_runtime *stream)
 		return -EINVAL;
 	}
 
-	mutex_lock(&stream->m_rt->bus->bus_lock);
+	sdw_acquire_bus_lock(stream);
 
 	ret = _sdw_disable_stream(stream);
 	if (ret < 0)
 		pr_err("Disable for stream:%s failed: %d", stream->name, ret);
 
-	mutex_unlock(&stream->m_rt->bus->bus_lock);
+	sdw_release_bus_lock(stream);
 	return ret;
 }
 EXPORT_SYMBOL(sdw_disable_stream);
 
 static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 {
-	struct sdw_master_runtime *m_rt = stream->m_rt;
-	struct sdw_bus *bus = m_rt->bus;
+	struct sdw_master_runtime *m_rt = NULL;
+	struct sdw_bus *bus = NULL;
 	int ret = 0;
 
-	/* De-prepare port(s) */
-	ret = sdw_prep_deprep_ports(m_rt, false);
-	if (ret < 0) {
-		dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
-		return ret;
-	}
+	list_for_each_entry(m_rt, &stream->master_list, stream_node) {
+		bus = m_rt->bus;
+		/* De-prepare port(s) */
+		ret = sdw_prep_deprep_ports(m_rt, false);
+		if (ret < 0) {
+			dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
+			return ret;
+		}
 
-	stream->state = SDW_STREAM_DEPREPARED;
+		/* TODO: Update this during Device-Device support */
+		bus->params.bandwidth -= m_rt->stream->params.rate *
+			m_rt->ch_count * m_rt->stream->params.bps;
 
-	/* TODO: Update this during Device-Device support */
-	bus->params.bandwidth -= m_rt->stream->params.rate *
-		m_rt->ch_count * m_rt->stream->params.bps;
+		/* Program params */
+		ret = sdw_program_params(bus);
+		if (ret < 0) {
+			dev_err(bus->dev, "Program params failed: %d", ret);
+			return ret;
+		}
 
-	/* Program params */
-	ret = sdw_program_params(bus);
-	if (ret < 0) {
-		dev_err(bus->dev, "Program params failed: %d", ret);
-		return ret;
 	}
 
+	stream->state = SDW_STREAM_DEPREPARED;
 	return do_bank_switch(stream);
 }
 
@@ -1508,13 +1572,12 @@  int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 		return -EINVAL;
 	}
 
-	mutex_lock(&stream->m_rt->bus->bus_lock);
-
+	sdw_acquire_bus_lock(stream);
 	ret = _sdw_deprepare_stream(stream);
 	if (ret < 0)
 		pr_err("De-prepare for stream:%d failed: %d", ret, ret);
 
-	mutex_unlock(&stream->m_rt->bus->bus_lock);
+	sdw_release_bus_lock(stream);
 	return ret;
 }
 EXPORT_SYMBOL(sdw_deprepare_stream);
diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
index ccd8dcdf..03df709 100644
--- a/include/linux/soundwire/sdw.h
+++ b/include/linux/soundwire/sdw.h
@@ -768,7 +768,6 @@  struct sdw_stream_params {
  * @params: Stream parameters
  * @state: Current state of the stream
  * @type: Stream type PCM or PDM
- * @m_rt: Master runtime
  * @master_list: List of Master runtime(s) in this stream.
  * master_list can contain only one m_rt per Master instance
  * for a stream
@@ -778,7 +777,6 @@  struct sdw_stream_runtime {
 	struct sdw_stream_params params;
 	enum sdw_stream_state state;
 	enum sdw_stream_type type;
-	struct sdw_master_runtime *m_rt;
 	struct list_head master_list;
 };