Re: [PATCH v5 6/9] soundwire: Handle multiple master instances in a stream

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

Sorry, another issue that I found while reviewing the entire section.
>   }
> @@ -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);
>   
>
So the sequence is

mutex_lock
sdw_master_port_release()
sdw_release_master_stream()
     sdw_stream_remove_slave()
         mutex_lock

Is this intentional to take the same mutex twice (not sure if it even 
works).
what you probably wanted is to replace sdw_stream_remove_slave() by the 
equivalent sequence

sdw_slave_port_release()
sdw_release_slave_stream()

which are both supposed to be called with a bus_lock held.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel