All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/19] soundwire: stream: cleanup of 'stream' support
@ 2022-01-26  1:16 ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

This series revisits the SoundWire 'sdw_stream' support to split allocation
and configuration steps. This is necessary if for example the routines are
called multiple times from the hw_params stage. This also helps with better
error handling.

Pierre-Louis Bossart (19):
  soundwire: stream: remove unused parameter in sdw_stream_add_slave
  soundwire: stream: add slave runtime to list earlier
  soundwire: stream: simplify check on port range
  soundwire: stream: add alloc/config/free helpers for ports
  soundwire: stream: split port allocation and configuration loops
  soundwire: stream: split alloc and config in two functions
  soundwire: stream: add 'slave' prefix for port range checks
  soundwire: stream: group sdw_port and sdw_master/slave_port functions
  soundwire: stream: simplify sdw_alloc_master_rt()
  soundwire: stream: split sdw_alloc_master_rt() in alloc and config
  soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers
  soundwire: stream: split sdw_alloc_slave_rt() in alloc and config
  soundwire: stream: group sdw_stream_ functions
  soundwire: stream: rename and move master/slave_rt_free routines
  soundwire: stream: move list addition to sdw_slave_alloc_rt()
  soundwire: stream: separate alloc and config within
    sdw_stream_add_xxx()
  soundwire: stream: introduce sdw_slave_rt_find() helper
  soundwire: stream: sdw_stream_add_ functions can be called multiple
    times
  soundwire: stream: make enable/disable/deprepare idempotent

 drivers/soundwire/stream.c | 960 +++++++++++++++++++++----------------
 1 file changed, 547 insertions(+), 413 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 00/19] soundwire: stream: cleanup of 'stream' support
@ 2022-01-26  1:16 ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

This series revisits the SoundWire 'sdw_stream' support to split allocation
and configuration steps. This is necessary if for example the routines are
called multiple times from the hw_params stage. This also helps with better
error handling.

Pierre-Louis Bossart (19):
  soundwire: stream: remove unused parameter in sdw_stream_add_slave
  soundwire: stream: add slave runtime to list earlier
  soundwire: stream: simplify check on port range
  soundwire: stream: add alloc/config/free helpers for ports
  soundwire: stream: split port allocation and configuration loops
  soundwire: stream: split alloc and config in two functions
  soundwire: stream: add 'slave' prefix for port range checks
  soundwire: stream: group sdw_port and sdw_master/slave_port functions
  soundwire: stream: simplify sdw_alloc_master_rt()
  soundwire: stream: split sdw_alloc_master_rt() in alloc and config
  soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers
  soundwire: stream: split sdw_alloc_slave_rt() in alloc and config
  soundwire: stream: group sdw_stream_ functions
  soundwire: stream: rename and move master/slave_rt_free routines
  soundwire: stream: move list addition to sdw_slave_alloc_rt()
  soundwire: stream: separate alloc and config within
    sdw_stream_add_xxx()
  soundwire: stream: introduce sdw_slave_rt_find() helper
  soundwire: stream: sdw_stream_add_ functions can be called multiple
    times
  soundwire: stream: make enable/disable/deprepare idempotent

 drivers/soundwire/stream.c | 960 +++++++++++++++++++++----------------
 1 file changed, 547 insertions(+), 413 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 01/19] soundwire: stream: remove unused parameter in sdw_stream_add_slave
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:16   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The stream parameter is not used, remove before further simplifications.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 980f26d49b66..a30d0fb4871b 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -968,14 +968,12 @@ static struct sdw_master_runtime
  *
  * @slave: Slave handle
  * @stream_config: Stream configuration
- * @stream: Stream runtime handle
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
 *sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config,
-		    struct sdw_stream_runtime *stream)
+		    struct sdw_stream_config *stream_config)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1367,7 +1365,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto error;
 	}
 
-	s_rt = sdw_alloc_slave_rt(slave, stream_config, stream);
+	s_rt = sdw_alloc_slave_rt(slave, stream_config);
 	if (!s_rt) {
 		dev_err(&slave->dev,
 			"Slave runtime config failed for stream:%s\n",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 01/19] soundwire: stream: remove unused parameter in sdw_stream_add_slave
@ 2022-01-26  1:16   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The stream parameter is not used, remove before further simplifications.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 980f26d49b66..a30d0fb4871b 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -968,14 +968,12 @@ static struct sdw_master_runtime
  *
  * @slave: Slave handle
  * @stream_config: Stream configuration
- * @stream: Stream runtime handle
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
 *sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config,
-		    struct sdw_stream_runtime *stream)
+		    struct sdw_stream_config *stream_config)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1367,7 +1365,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto error;
 	}
 
-	s_rt = sdw_alloc_slave_rt(slave, stream_config, stream);
+	s_rt = sdw_alloc_slave_rt(slave, stream_config);
 	if (!s_rt) {
 		dev_err(&slave->dev,
 			"Slave runtime config failed for stream:%s\n",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 02/19] soundwire: stream: add slave runtime to list earlier
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:16   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

sdw_config_stream() only verifies the compatibility between
information provided by the Slave driver and the stream configuration.

There is no problem if we add the slave runtime to the list earlier.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a30d0fb4871b..a75d3576bfcf 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1373,21 +1373,12 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		ret = -ENOMEM;
 		goto stream_error;
 	}
-
-	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
-	if (ret) {
-		/*
-		 * sdw_release_master_stream will release s_rt in slave_rt_list in
-		 * stream_error case, but s_rt is only added to slave_rt_list
-		 * when sdw_config_stream is successful, so free s_rt explicitly
-		 * when sdw_config_stream is failed.
-		 */
-		kfree(s_rt);
-		goto stream_error;
-	}
-
 	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
+	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
+	if (ret)
+		goto stream_error;
+
 	ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports);
 	if (ret)
 		goto stream_error;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 02/19] soundwire: stream: add slave runtime to list earlier
@ 2022-01-26  1:16   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

sdw_config_stream() only verifies the compatibility between
information provided by the Slave driver and the stream configuration.

There is no problem if we add the slave runtime to the list earlier.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a30d0fb4871b..a75d3576bfcf 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1373,21 +1373,12 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		ret = -ENOMEM;
 		goto stream_error;
 	}
-
-	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
-	if (ret) {
-		/*
-		 * sdw_release_master_stream will release s_rt in slave_rt_list in
-		 * stream_error case, but s_rt is only added to slave_rt_list
-		 * when sdw_config_stream is successful, so free s_rt explicitly
-		 * when sdw_config_stream is failed.
-		 */
-		kfree(s_rt);
-		goto stream_error;
-	}
-
 	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
+	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
+	if (ret)
+		goto stream_error;
+
 	ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports);
 	if (ret)
 		goto stream_error;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 03/19] soundwire: stream: simplify check on port range
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:16   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Pass the index directly to sdw_is_valid_port_range(), this will be
useful for further simplifications.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a75d3576bfcf..3ac2e5a66700 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1177,12 +1177,10 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_is_valid_port_range(struct device *dev,
-				   struct sdw_port_runtime *p_rt)
+static int sdw_is_valid_port_range(struct device *dev, int num)
 {
-	if (!SDW_VALID_PORT_RANGE(p_rt->num)) {
-		dev_err(dev,
-			"SoundWire: Invalid port number :%d\n", p_rt->num);
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
 		return -EINVAL;
 	}
 
@@ -1249,7 +1247,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
 		 */
-		ret = sdw_is_valid_port_range(&slave->dev, p_rt);
+		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
 		if (ret < 0) {
 			kfree(p_rt);
 			return ret;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 03/19] soundwire: stream: simplify check on port range
@ 2022-01-26  1:16   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:16 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Pass the index directly to sdw_is_valid_port_range(), this will be
useful for further simplifications.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a75d3576bfcf..3ac2e5a66700 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1177,12 +1177,10 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_is_valid_port_range(struct device *dev,
-				   struct sdw_port_runtime *p_rt)
+static int sdw_is_valid_port_range(struct device *dev, int num)
 {
-	if (!SDW_VALID_PORT_RANGE(p_rt->num)) {
-		dev_err(dev,
-			"SoundWire: Invalid port number :%d\n", p_rt->num);
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
 		return -EINVAL;
 	}
 
@@ -1249,7 +1247,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
 		 */
-		ret = sdw_is_valid_port_range(&slave->dev, p_rt);
+		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
 		if (ret < 0) {
 			kfree(p_rt);
 			return ret;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 04/19] soundwire: stream: add alloc/config/free helpers for ports
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The existing code only has a config helper that allocates memory,
start adding alloc/config/free for ports, as a first step in the
simplification of the stream API.

This change removes a kfree() on a configuration error, this should
have not impact on existing platforms and error handling will be
revisited in follow-up patches to make sure invalid configurations
have not impact on memory allocation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 83 +++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 3ac2e5a66700..49d3a8d2fa31 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -865,6 +865,39 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 	return ret;
 }
 
+static struct sdw_port_runtime *sdw_port_alloc(struct list_head *port_list)
+{
+	struct sdw_port_runtime *p_rt;
+
+	p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL);
+	if (!p_rt)
+		return NULL;
+
+	list_add_tail(&p_rt->port_node, port_list);
+
+	return p_rt;
+}
+
+static int sdw_port_config(struct sdw_port_runtime *p_rt,
+			   struct sdw_port_config *port_config,
+			   int port_index)
+{
+	p_rt->ch_mask = port_config[port_index].ch_mask;
+	p_rt->num = port_config[port_index].num;
+
+	/*
+	 * TODO: Check port capabilities for requested configuration
+	 */
+
+	return 0;
+}
+
+static void sdw_port_free(struct sdw_port_runtime *p_rt)
+{
+	list_del(&p_rt->port_node);
+	kfree(p_rt);
+}
+
 /**
  * sdw_release_stream() - Free the assigned stream runtime
  *
@@ -995,8 +1028,7 @@ static void sdw_master_port_release(struct sdw_bus *bus,
 	struct sdw_port_runtime *p_rt, *_p_rt;
 
 	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
-		list_del(&p_rt->port_node);
-		kfree(p_rt);
+		sdw_port_free(p_rt);
 	}
 }
 
@@ -1015,8 +1047,7 @@ static void sdw_slave_port_release(struct sdw_bus *bus,
 
 			list_for_each_entry_safe(p_rt, _p_rt,
 						 &s_rt->port_list, port_node) {
-				list_del(&p_rt->port_node);
-				kfree(p_rt);
+				sdw_port_free(p_rt);
 			}
 		}
 	}
@@ -1187,43 +1218,24 @@ static int sdw_is_valid_port_range(struct device *dev, int num)
 	return 0;
 }
 
-static struct sdw_port_runtime
-*sdw_port_alloc(struct device *dev,
-		struct sdw_port_config *port_config,
-		int port_index)
-{
-	struct sdw_port_runtime *p_rt;
-
-	p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL);
-	if (!p_rt)
-		return NULL;
-
-	p_rt->ch_mask = port_config[port_index].ch_mask;
-	p_rt->num = port_config[port_index].num;
-
-	return p_rt;
-}
-
 static int sdw_master_port_config(struct sdw_bus *bus,
 				  struct sdw_master_runtime *m_rt,
 				  struct sdw_port_config *port_config,
 				  unsigned int num_ports)
 {
 	struct sdw_port_runtime *p_rt;
+	int ret;
 	int i;
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_ports; i++) {
-		p_rt = sdw_port_alloc(bus->dev, port_config, i);
+		p_rt = sdw_port_alloc(&m_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
 
-		/*
-		 * TODO: Check port capabilities for requested
-		 * configuration (audio mode support)
-		 */
-
-		list_add_tail(&p_rt->port_node, &m_rt->port_list);
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -1239,7 +1251,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_config; i++) {
-		p_rt = sdw_port_alloc(&slave->dev, port_config, i);
+		p_rt = sdw_port_alloc(&s_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
 
@@ -1248,17 +1260,12 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * slave
 		 */
 		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
-		if (ret < 0) {
-			kfree(p_rt);
+		if (ret < 0)
 			return ret;
-		}
 
-		/*
-		 * TODO: Check port capabilities for requested
-		 * configuration (audio mode support)
-		 */
-
-		list_add_tail(&p_rt->port_node, &s_rt->port_list);
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 04/19] soundwire: stream: add alloc/config/free helpers for ports
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The existing code only has a config helper that allocates memory,
start adding alloc/config/free for ports, as a first step in the
simplification of the stream API.

This change removes a kfree() on a configuration error, this should
have not impact on existing platforms and error handling will be
revisited in follow-up patches to make sure invalid configurations
have not impact on memory allocation.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 83 +++++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 3ac2e5a66700..49d3a8d2fa31 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -865,6 +865,39 @@ static int do_bank_switch(struct sdw_stream_runtime *stream)
 	return ret;
 }
 
+static struct sdw_port_runtime *sdw_port_alloc(struct list_head *port_list)
+{
+	struct sdw_port_runtime *p_rt;
+
+	p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL);
+	if (!p_rt)
+		return NULL;
+
+	list_add_tail(&p_rt->port_node, port_list);
+
+	return p_rt;
+}
+
+static int sdw_port_config(struct sdw_port_runtime *p_rt,
+			   struct sdw_port_config *port_config,
+			   int port_index)
+{
+	p_rt->ch_mask = port_config[port_index].ch_mask;
+	p_rt->num = port_config[port_index].num;
+
+	/*
+	 * TODO: Check port capabilities for requested configuration
+	 */
+
+	return 0;
+}
+
+static void sdw_port_free(struct sdw_port_runtime *p_rt)
+{
+	list_del(&p_rt->port_node);
+	kfree(p_rt);
+}
+
 /**
  * sdw_release_stream() - Free the assigned stream runtime
  *
@@ -995,8 +1028,7 @@ static void sdw_master_port_release(struct sdw_bus *bus,
 	struct sdw_port_runtime *p_rt, *_p_rt;
 
 	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
-		list_del(&p_rt->port_node);
-		kfree(p_rt);
+		sdw_port_free(p_rt);
 	}
 }
 
@@ -1015,8 +1047,7 @@ static void sdw_slave_port_release(struct sdw_bus *bus,
 
 			list_for_each_entry_safe(p_rt, _p_rt,
 						 &s_rt->port_list, port_node) {
-				list_del(&p_rt->port_node);
-				kfree(p_rt);
+				sdw_port_free(p_rt);
 			}
 		}
 	}
@@ -1187,43 +1218,24 @@ static int sdw_is_valid_port_range(struct device *dev, int num)
 	return 0;
 }
 
-static struct sdw_port_runtime
-*sdw_port_alloc(struct device *dev,
-		struct sdw_port_config *port_config,
-		int port_index)
-{
-	struct sdw_port_runtime *p_rt;
-
-	p_rt = kzalloc(sizeof(*p_rt), GFP_KERNEL);
-	if (!p_rt)
-		return NULL;
-
-	p_rt->ch_mask = port_config[port_index].ch_mask;
-	p_rt->num = port_config[port_index].num;
-
-	return p_rt;
-}
-
 static int sdw_master_port_config(struct sdw_bus *bus,
 				  struct sdw_master_runtime *m_rt,
 				  struct sdw_port_config *port_config,
 				  unsigned int num_ports)
 {
 	struct sdw_port_runtime *p_rt;
+	int ret;
 	int i;
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_ports; i++) {
-		p_rt = sdw_port_alloc(bus->dev, port_config, i);
+		p_rt = sdw_port_alloc(&m_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
 
-		/*
-		 * TODO: Check port capabilities for requested
-		 * configuration (audio mode support)
-		 */
-
-		list_add_tail(&p_rt->port_node, &m_rt->port_list);
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
@@ -1239,7 +1251,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_config; i++) {
-		p_rt = sdw_port_alloc(&slave->dev, port_config, i);
+		p_rt = sdw_port_alloc(&s_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
 
@@ -1248,17 +1260,12 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * slave
 		 */
 		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
-		if (ret < 0) {
-			kfree(p_rt);
+		if (ret < 0)
 			return ret;
-		}
 
-		/*
-		 * TODO: Check port capabilities for requested
-		 * configuration (audio mode support)
-		 */
-
-		list_add_tail(&p_rt->port_node, &s_rt->port_list);
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 05/19] soundwire: stream: split port allocation and configuration loops
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split loops before moving the allocation and configuration to separate
functions.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 49d3a8d2fa31..b97c59e71bdb 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1232,10 +1232,14 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 		p_rt = sdw_port_alloc(&m_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
+	}
 
+	i = 0;
+	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
 		ret = sdw_port_config(p_rt, port_config, i);
 		if (ret < 0)
 			return ret;
+		i++;
 	}
 
 	return 0;
@@ -1254,7 +1258,10 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		p_rt = sdw_port_alloc(&s_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
+	}
 
+	i = 0;
+	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 		/*
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
@@ -1266,6 +1273,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		ret = sdw_port_config(p_rt, port_config, i);
 		if (ret < 0)
 			return ret;
+		i++;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 05/19] soundwire: stream: split port allocation and configuration loops
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split loops before moving the allocation and configuration to separate
functions.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 49d3a8d2fa31..b97c59e71bdb 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1232,10 +1232,14 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 		p_rt = sdw_port_alloc(&m_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
+	}
 
+	i = 0;
+	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
 		ret = sdw_port_config(p_rt, port_config, i);
 		if (ret < 0)
 			return ret;
+		i++;
 	}
 
 	return 0;
@@ -1254,7 +1258,10 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		p_rt = sdw_port_alloc(&s_rt->port_list);
 		if (!p_rt)
 			return -ENOMEM;
+	}
 
+	i = 0;
+	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 		/*
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
@@ -1266,6 +1273,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		ret = sdw_port_config(p_rt, port_config, i);
 		if (ret < 0)
 			return ret;
+		i++;
 	}
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 06/19] soundwire: stream: split alloc and config in two functions
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Continue the split with two functions for master and slave, and remove
unused arguments.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 49 ++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b97c59e71bdb..e3cb55de0d12 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1218,13 +1218,10 @@ static int sdw_is_valid_port_range(struct device *dev, int num)
 	return 0;
 }
 
-static int sdw_master_port_config(struct sdw_bus *bus,
-				  struct sdw_master_runtime *m_rt,
-				  struct sdw_port_config *port_config,
-				  unsigned int num_ports)
+static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
+				 unsigned int num_ports)
 {
 	struct sdw_port_runtime *p_rt;
-	int ret;
 	int i;
 
 	/* Iterate for number of ports to perform initialization */
@@ -1234,6 +1231,16 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 			return -ENOMEM;
 	}
 
+	return 0;
+}
+
+static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
+				  struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
 	i = 0;
 	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
 		ret = sdw_port_config(p_rt, port_config, i);
@@ -1245,13 +1252,12 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 	return 0;
 }
 
-static int sdw_slave_port_config(struct sdw_slave *slave,
-				 struct sdw_slave_runtime *s_rt,
-				 struct sdw_port_config *port_config,
-				 unsigned int num_config)
+static int sdw_slave_port_alloc(struct sdw_slave *slave,
+				struct sdw_slave_runtime *s_rt,
+				unsigned int num_config)
 {
 	struct sdw_port_runtime *p_rt;
-	int i, ret;
+	int i;
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_config; i++) {
@@ -1260,6 +1266,17 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 			return -ENOMEM;
 	}
 
+	return 0;
+}
+
+static int sdw_slave_port_config(struct sdw_slave *slave,
+				 struct sdw_slave_runtime *s_rt,
+				 struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
 	i = 0;
 	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 		/*
@@ -1324,7 +1341,11 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	if (ret)
 		goto stream_error;
 
-	ret = sdw_master_port_config(bus, m_rt, port_config, num_ports);
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_config(m_rt, port_config);
 	if (ret)
 		goto stream_error;
 
@@ -1392,7 +1413,11 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	if (ret)
 		goto stream_error;
 
-	ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports);
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_config(slave, s_rt, port_config);
 	if (ret)
 		goto stream_error;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 06/19] soundwire: stream: split alloc and config in two functions
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Continue the split with two functions for master and slave, and remove
unused arguments.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 49 ++++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b97c59e71bdb..e3cb55de0d12 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1218,13 +1218,10 @@ static int sdw_is_valid_port_range(struct device *dev, int num)
 	return 0;
 }
 
-static int sdw_master_port_config(struct sdw_bus *bus,
-				  struct sdw_master_runtime *m_rt,
-				  struct sdw_port_config *port_config,
-				  unsigned int num_ports)
+static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
+				 unsigned int num_ports)
 {
 	struct sdw_port_runtime *p_rt;
-	int ret;
 	int i;
 
 	/* Iterate for number of ports to perform initialization */
@@ -1234,6 +1231,16 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 			return -ENOMEM;
 	}
 
+	return 0;
+}
+
+static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
+				  struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
 	i = 0;
 	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
 		ret = sdw_port_config(p_rt, port_config, i);
@@ -1245,13 +1252,12 @@ static int sdw_master_port_config(struct sdw_bus *bus,
 	return 0;
 }
 
-static int sdw_slave_port_config(struct sdw_slave *slave,
-				 struct sdw_slave_runtime *s_rt,
-				 struct sdw_port_config *port_config,
-				 unsigned int num_config)
+static int sdw_slave_port_alloc(struct sdw_slave *slave,
+				struct sdw_slave_runtime *s_rt,
+				unsigned int num_config)
 {
 	struct sdw_port_runtime *p_rt;
-	int i, ret;
+	int i;
 
 	/* Iterate for number of ports to perform initialization */
 	for (i = 0; i < num_config; i++) {
@@ -1260,6 +1266,17 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 			return -ENOMEM;
 	}
 
+	return 0;
+}
+
+static int sdw_slave_port_config(struct sdw_slave *slave,
+				 struct sdw_slave_runtime *s_rt,
+				 struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
 	i = 0;
 	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 		/*
@@ -1324,7 +1341,11 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	if (ret)
 		goto stream_error;
 
-	ret = sdw_master_port_config(bus, m_rt, port_config, num_ports);
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_config(m_rt, port_config);
 	if (ret)
 		goto stream_error;
 
@@ -1392,7 +1413,11 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	if (ret)
 		goto stream_error;
 
-	ret = sdw_slave_port_config(slave, s_rt, port_config, num_ports);
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_config(slave, s_rt, port_config);
 	if (ret)
 		goto stream_error;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 07/19] soundwire: stream: add 'slave' prefix for port range checks
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

We can only check for Slave port ranges, the ports are not defined at
the Master level. Also move the function to the 'slave port' block.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e3cb55de0d12..c326298a0fe2 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1208,16 +1208,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_is_valid_port_range(struct device *dev, int num)
-{
-	if (!SDW_VALID_PORT_RANGE(num)) {
-		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
 				 unsigned int num_ports)
 {
@@ -1269,6 +1259,16 @@ static int sdw_slave_port_alloc(struct sdw_slave *slave,
 	return 0;
 }
 
+static int sdw_slave_port_is_valid_range(struct device *dev, int num)
+{
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int sdw_slave_port_config(struct sdw_slave *slave,
 				 struct sdw_slave_runtime *s_rt,
 				 struct sdw_port_config *port_config)
@@ -1283,7 +1283,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
 		 */
-		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
+		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
 		if (ret < 0)
 			return ret;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 07/19] soundwire: stream: add 'slave' prefix for port range checks
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

We can only check for Slave port ranges, the ports are not defined at
the Master level. Also move the function to the 'slave port' block.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e3cb55de0d12..c326298a0fe2 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1208,16 +1208,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_is_valid_port_range(struct device *dev, int num)
-{
-	if (!SDW_VALID_PORT_RANGE(num)) {
-		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
 static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
 				 unsigned int num_ports)
 {
@@ -1269,6 +1259,16 @@ static int sdw_slave_port_alloc(struct sdw_slave *slave,
 	return 0;
 }
 
+static int sdw_slave_port_is_valid_range(struct device *dev, int num)
+{
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int sdw_slave_port_config(struct sdw_slave *slave,
 				 struct sdw_slave_runtime *s_rt,
 				 struct sdw_port_config *port_config)
@@ -1283,7 +1283,7 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 		 * TODO: Check valid port range as defined by DisCo/
 		 * slave
 		 */
-		ret = sdw_is_valid_port_range(&slave->dev, port_config[i].num);
+		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
 		if (ret < 0)
 			return ret;
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 08/19] soundwire: stream: group sdw_port and sdw_master/slave_port functions
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

re-group all the helpers in one location with a code move. For
consistency the 'slave' helpers are placed before the 'master'
helpers.

Also remove unused arguments and rename the 'release' function to
'free' for consistency.

No functional change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 242 ++++++++++++++++++-------------------
 1 file changed, 120 insertions(+), 122 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index c326298a0fe2..5e2d29448aaf 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -898,6 +898,123 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt)
 	kfree(p_rt);
 }
 
+static void sdw_slave_port_free(struct sdw_slave *slave,
+				struct sdw_stream_runtime *stream)
+{
+	struct sdw_port_runtime *p_rt, *_p_rt;
+	struct sdw_master_runtime *m_rt;
+	struct sdw_slave_runtime *s_rt;
+
+	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) {
+			if (s_rt->slave != slave)
+				continue;
+
+			list_for_each_entry_safe(p_rt, _p_rt,
+						 &s_rt->port_list, port_node) {
+				sdw_port_free(p_rt);
+			}
+		}
+	}
+}
+
+static int sdw_slave_port_alloc(struct sdw_slave *slave,
+				struct sdw_slave_runtime *s_rt,
+				unsigned int num_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int i;
+
+	/* Iterate for number of ports to perform initialization */
+	for (i = 0; i < num_config; i++) {
+		p_rt = sdw_port_alloc(&s_rt->port_list);
+		if (!p_rt)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int sdw_slave_port_is_valid_range(struct device *dev, int num)
+{
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int sdw_slave_port_config(struct sdw_slave *slave,
+				 struct sdw_slave_runtime *s_rt,
+				 struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
+	i = 0;
+	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
+		/*
+		 * TODO: Check valid port range as defined by DisCo/
+		 * slave
+		 */
+		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
+		if (ret < 0)
+			return ret;
+
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
+		i++;
+	}
+
+	return 0;
+}
+
+static void sdw_master_port_free(struct sdw_master_runtime *m_rt)
+{
+	struct sdw_port_runtime *p_rt, *_p_rt;
+
+	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
+		sdw_port_free(p_rt);
+	}
+}
+
+static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
+				 unsigned int num_ports)
+{
+	struct sdw_port_runtime *p_rt;
+	int i;
+
+	/* Iterate for number of ports to perform initialization */
+	for (i = 0; i < num_ports; i++) {
+		p_rt = sdw_port_alloc(&m_rt->port_list);
+		if (!p_rt)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
+				  struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
+	i = 0;
+	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
+		i++;
+	}
+
+	return 0;
+}
+
 /**
  * sdw_release_stream() - Free the assigned stream runtime
  *
@@ -1022,37 +1139,6 @@ static struct sdw_slave_runtime
 	return s_rt;
 }
 
-static void sdw_master_port_release(struct sdw_bus *bus,
-				    struct sdw_master_runtime *m_rt)
-{
-	struct sdw_port_runtime *p_rt, *_p_rt;
-
-	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
-		sdw_port_free(p_rt);
-	}
-}
-
-static void sdw_slave_port_release(struct sdw_bus *bus,
-				   struct sdw_slave *slave,
-				   struct sdw_stream_runtime *stream)
-{
-	struct sdw_port_runtime *p_rt, *_p_rt;
-	struct sdw_master_runtime *m_rt;
-	struct sdw_slave_runtime *s_rt;
-
-	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) {
-			if (s_rt->slave != slave)
-				continue;
-
-			list_for_each_entry_safe(p_rt, _p_rt,
-						 &s_rt->port_list, port_node) {
-				sdw_port_free(p_rt);
-			}
-		}
-	}
-}
-
 /**
  * sdw_release_slave_stream() - Free Slave(s) runtime handle
  *
@@ -1097,7 +1183,7 @@ static void sdw_release_master_stream(struct sdw_master_runtime *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_slave_port_release(s_rt->slave->bus, s_rt->slave, stream);
+		sdw_slave_port_free(s_rt->slave, stream);
 		sdw_release_slave_stream(s_rt->slave, stream);
 	}
 
@@ -1126,7 +1212,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
 		if (m_rt->bus != bus)
 			continue;
 
-		sdw_master_port_release(bus, m_rt);
+		sdw_master_port_free(m_rt);
 		sdw_release_master_stream(m_rt, stream);
 		stream->m_rt_count--;
 	}
@@ -1153,7 +1239,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 {
 	mutex_lock(&slave->bus->bus_lock);
 
-	sdw_slave_port_release(slave->bus, slave, stream);
+	sdw_slave_port_free(slave, stream);
 	sdw_release_slave_stream(slave, stream);
 
 	mutex_unlock(&slave->bus->bus_lock);
@@ -1208,94 +1294,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
-				 unsigned int num_ports)
-{
-	struct sdw_port_runtime *p_rt;
-	int i;
-
-	/* Iterate for number of ports to perform initialization */
-	for (i = 0; i < num_ports; i++) {
-		p_rt = sdw_port_alloc(&m_rt->port_list);
-		if (!p_rt)
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
-				  struct sdw_port_config *port_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int ret;
-	int i;
-
-	i = 0;
-	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
-		ret = sdw_port_config(p_rt, port_config, i);
-		if (ret < 0)
-			return ret;
-		i++;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_alloc(struct sdw_slave *slave,
-				struct sdw_slave_runtime *s_rt,
-				unsigned int num_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int i;
-
-	/* Iterate for number of ports to perform initialization */
-	for (i = 0; i < num_config; i++) {
-		p_rt = sdw_port_alloc(&s_rt->port_list);
-		if (!p_rt)
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_is_valid_range(struct device *dev, int num)
-{
-	if (!SDW_VALID_PORT_RANGE(num)) {
-		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_config(struct sdw_slave *slave,
-				 struct sdw_slave_runtime *s_rt,
-				 struct sdw_port_config *port_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int ret;
-	int i;
-
-	i = 0;
-	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
-		/*
-		 * TODO: Check valid port range as defined by DisCo/
-		 * slave
-		 */
-		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
-		if (ret < 0)
-			return ret;
-
-		ret = sdw_port_config(p_rt, port_config, i);
-		if (ret < 0)
-			return ret;
-		i++;
-	}
-
-	return 0;
-}
-
 /**
  * sdw_stream_add_master() - Allocate and add master runtime to a stream
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 08/19] soundwire: stream: group sdw_port and sdw_master/slave_port functions
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

re-group all the helpers in one location with a code move. For
consistency the 'slave' helpers are placed before the 'master'
helpers.

Also remove unused arguments and rename the 'release' function to
'free' for consistency.

No functional change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 242 ++++++++++++++++++-------------------
 1 file changed, 120 insertions(+), 122 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index c326298a0fe2..5e2d29448aaf 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -898,6 +898,123 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt)
 	kfree(p_rt);
 }
 
+static void sdw_slave_port_free(struct sdw_slave *slave,
+				struct sdw_stream_runtime *stream)
+{
+	struct sdw_port_runtime *p_rt, *_p_rt;
+	struct sdw_master_runtime *m_rt;
+	struct sdw_slave_runtime *s_rt;
+
+	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) {
+			if (s_rt->slave != slave)
+				continue;
+
+			list_for_each_entry_safe(p_rt, _p_rt,
+						 &s_rt->port_list, port_node) {
+				sdw_port_free(p_rt);
+			}
+		}
+	}
+}
+
+static int sdw_slave_port_alloc(struct sdw_slave *slave,
+				struct sdw_slave_runtime *s_rt,
+				unsigned int num_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int i;
+
+	/* Iterate for number of ports to perform initialization */
+	for (i = 0; i < num_config; i++) {
+		p_rt = sdw_port_alloc(&s_rt->port_list);
+		if (!p_rt)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int sdw_slave_port_is_valid_range(struct device *dev, int num)
+{
+	if (!SDW_VALID_PORT_RANGE(num)) {
+		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int sdw_slave_port_config(struct sdw_slave *slave,
+				 struct sdw_slave_runtime *s_rt,
+				 struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
+	i = 0;
+	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
+		/*
+		 * TODO: Check valid port range as defined by DisCo/
+		 * slave
+		 */
+		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
+		if (ret < 0)
+			return ret;
+
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
+		i++;
+	}
+
+	return 0;
+}
+
+static void sdw_master_port_free(struct sdw_master_runtime *m_rt)
+{
+	struct sdw_port_runtime *p_rt, *_p_rt;
+
+	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
+		sdw_port_free(p_rt);
+	}
+}
+
+static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
+				 unsigned int num_ports)
+{
+	struct sdw_port_runtime *p_rt;
+	int i;
+
+	/* Iterate for number of ports to perform initialization */
+	for (i = 0; i < num_ports; i++) {
+		p_rt = sdw_port_alloc(&m_rt->port_list);
+		if (!p_rt)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
+				  struct sdw_port_config *port_config)
+{
+	struct sdw_port_runtime *p_rt;
+	int ret;
+	int i;
+
+	i = 0;
+	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
+		ret = sdw_port_config(p_rt, port_config, i);
+		if (ret < 0)
+			return ret;
+		i++;
+	}
+
+	return 0;
+}
+
 /**
  * sdw_release_stream() - Free the assigned stream runtime
  *
@@ -1022,37 +1139,6 @@ static struct sdw_slave_runtime
 	return s_rt;
 }
 
-static void sdw_master_port_release(struct sdw_bus *bus,
-				    struct sdw_master_runtime *m_rt)
-{
-	struct sdw_port_runtime *p_rt, *_p_rt;
-
-	list_for_each_entry_safe(p_rt, _p_rt, &m_rt->port_list, port_node) {
-		sdw_port_free(p_rt);
-	}
-}
-
-static void sdw_slave_port_release(struct sdw_bus *bus,
-				   struct sdw_slave *slave,
-				   struct sdw_stream_runtime *stream)
-{
-	struct sdw_port_runtime *p_rt, *_p_rt;
-	struct sdw_master_runtime *m_rt;
-	struct sdw_slave_runtime *s_rt;
-
-	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) {
-			if (s_rt->slave != slave)
-				continue;
-
-			list_for_each_entry_safe(p_rt, _p_rt,
-						 &s_rt->port_list, port_node) {
-				sdw_port_free(p_rt);
-			}
-		}
-	}
-}
-
 /**
  * sdw_release_slave_stream() - Free Slave(s) runtime handle
  *
@@ -1097,7 +1183,7 @@ static void sdw_release_master_stream(struct sdw_master_runtime *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_slave_port_release(s_rt->slave->bus, s_rt->slave, stream);
+		sdw_slave_port_free(s_rt->slave, stream);
 		sdw_release_slave_stream(s_rt->slave, stream);
 	}
 
@@ -1126,7 +1212,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
 		if (m_rt->bus != bus)
 			continue;
 
-		sdw_master_port_release(bus, m_rt);
+		sdw_master_port_free(m_rt);
 		sdw_release_master_stream(m_rt, stream);
 		stream->m_rt_count--;
 	}
@@ -1153,7 +1239,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 {
 	mutex_lock(&slave->bus->bus_lock);
 
-	sdw_slave_port_release(slave->bus, slave, stream);
+	sdw_slave_port_free(slave, stream);
 	sdw_release_slave_stream(slave, stream);
 
 	mutex_unlock(&slave->bus->bus_lock);
@@ -1208,94 +1294,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-static int sdw_master_port_alloc(struct sdw_master_runtime *m_rt,
-				 unsigned int num_ports)
-{
-	struct sdw_port_runtime *p_rt;
-	int i;
-
-	/* Iterate for number of ports to perform initialization */
-	for (i = 0; i < num_ports; i++) {
-		p_rt = sdw_port_alloc(&m_rt->port_list);
-		if (!p_rt)
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
-				  struct sdw_port_config *port_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int ret;
-	int i;
-
-	i = 0;
-	list_for_each_entry(p_rt, &m_rt->port_list, port_node) {
-		ret = sdw_port_config(p_rt, port_config, i);
-		if (ret < 0)
-			return ret;
-		i++;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_alloc(struct sdw_slave *slave,
-				struct sdw_slave_runtime *s_rt,
-				unsigned int num_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int i;
-
-	/* Iterate for number of ports to perform initialization */
-	for (i = 0; i < num_config; i++) {
-		p_rt = sdw_port_alloc(&s_rt->port_list);
-		if (!p_rt)
-			return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_is_valid_range(struct device *dev, int num)
-{
-	if (!SDW_VALID_PORT_RANGE(num)) {
-		dev_err(dev, "SoundWire: Invalid port number :%d\n", num);
-		return -EINVAL;
-	}
-
-	return 0;
-}
-
-static int sdw_slave_port_config(struct sdw_slave *slave,
-				 struct sdw_slave_runtime *s_rt,
-				 struct sdw_port_config *port_config)
-{
-	struct sdw_port_runtime *p_rt;
-	int ret;
-	int i;
-
-	i = 0;
-	list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
-		/*
-		 * TODO: Check valid port range as defined by DisCo/
-		 * slave
-		 */
-		ret = sdw_slave_port_is_valid_range(&slave->dev, port_config[i].num);
-		if (ret < 0)
-			return ret;
-
-		ret = sdw_port_config(p_rt, port_config, i);
-		if (ret < 0)
-			return ret;
-		i++;
-	}
-
-	return 0;
-}
-
 /**
  * sdw_stream_add_master() - Allocate and add master runtime to a stream
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 09/19] soundwire: stream: simplify sdw_alloc_master_rt()
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Only do the allocation in that function, and move check for allocation
in the caller. This will it easier to split allocation and
configuration.

No functionality change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 5e2d29448aaf..263b76230f8f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1085,14 +1085,6 @@ static struct sdw_master_runtime
 {
 	struct sdw_master_runtime *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;
-
 	m_rt = kzalloc(sizeof(*m_rt), GFP_KERNEL);
 	if (!m_rt)
 		return NULL;
@@ -1104,7 +1096,6 @@ static struct sdw_master_runtime
 
 	list_add_tail(&m_rt->bus_node, &bus->m_rt_list);
 
-stream_config:
 	m_rt->ch_count = stream_config->ch_count;
 	m_rt->bus = bus;
 	m_rt->stream = stream;
@@ -1326,6 +1317,14 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		goto unlock;
 	}
 
+	/*
+	 * check if Master is already allocated (e.g. as a result of Slave adding
+	 * it first), if so skip allocation and go to configuration
+	 */
+	m_rt = sdw_find_master_rt(bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
 	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(bus->dev,
@@ -1335,6 +1334,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		goto unlock;
 	}
 
+skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
 		goto stream_error;
@@ -1384,6 +1384,14 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 
 	mutex_lock(&slave->bus->bus_lock);
 
+	/*
+	 * check if Master is already allocated, if so skip allocation
+	 * and go to configuration
+	 */
+	m_rt = sdw_find_master_rt(slave->bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
 	/*
 	 * If this API is invoked by Slave first then m_rt is not valid.
 	 * So, allocate m_rt and add Slave to it.
@@ -1397,6 +1405,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto error;
 	}
 
+skip_alloc_master_rt:
 	s_rt = sdw_alloc_slave_rt(slave, stream_config);
 	if (!s_rt) {
 		dev_err(&slave->dev,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 09/19] soundwire: stream: simplify sdw_alloc_master_rt()
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Only do the allocation in that function, and move check for allocation
in the caller. This will it easier to split allocation and
configuration.

No functionality change in this patch.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 5e2d29448aaf..263b76230f8f 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1085,14 +1085,6 @@ static struct sdw_master_runtime
 {
 	struct sdw_master_runtime *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;
-
 	m_rt = kzalloc(sizeof(*m_rt), GFP_KERNEL);
 	if (!m_rt)
 		return NULL;
@@ -1104,7 +1096,6 @@ static struct sdw_master_runtime
 
 	list_add_tail(&m_rt->bus_node, &bus->m_rt_list);
 
-stream_config:
 	m_rt->ch_count = stream_config->ch_count;
 	m_rt->bus = bus;
 	m_rt->stream = stream;
@@ -1326,6 +1317,14 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		goto unlock;
 	}
 
+	/*
+	 * check if Master is already allocated (e.g. as a result of Slave adding
+	 * it first), if so skip allocation and go to configuration
+	 */
+	m_rt = sdw_find_master_rt(bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
 	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
 	if (!m_rt) {
 		dev_err(bus->dev,
@@ -1335,6 +1334,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		goto unlock;
 	}
 
+skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
 		goto stream_error;
@@ -1384,6 +1384,14 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 
 	mutex_lock(&slave->bus->bus_lock);
 
+	/*
+	 * check if Master is already allocated, if so skip allocation
+	 * and go to configuration
+	 */
+	m_rt = sdw_find_master_rt(slave->bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
 	/*
 	 * If this API is invoked by Slave first then m_rt is not valid.
 	 * So, allocate m_rt and add Slave to it.
@@ -1397,6 +1405,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto error;
 	}
 
+skip_alloc_master_rt:
 	s_rt = sdw_alloc_slave_rt(slave, stream_config);
 	if (!s_rt) {
 		dev_err(&slave->dev,
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 10/19] soundwire: stream: split sdw_alloc_master_rt() in alloc and config
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split the two parts so that we can do multiple configurations during
ALSA/ASoC hw_params stage. Also follow existing convention
sdw_<object>_<action> used at lower level.

No functionality change here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 51 +++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 263b76230f8f..e38c9208c77b 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1055,7 +1055,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 EXPORT_SYMBOL(sdw_alloc_stream);
 
 static struct sdw_master_runtime
-*sdw_find_master_rt(struct sdw_bus *bus,
+*sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
@@ -1070,17 +1070,15 @@ static struct sdw_master_runtime
 }
 
 /**
- * sdw_alloc_master_rt() - Allocates and initialize Master runtime handle
+ * sdw_master_rt_alloc() - Allocates a Master runtime handle
  *
  * @bus: SDW bus instance
- * @stream_config: Stream configuration
  * @stream: Stream runtime handle.
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_master_runtime
-*sdw_alloc_master_rt(struct sdw_bus *bus,
-		     struct sdw_stream_config *stream_config,
+*sdw_master_rt_alloc(struct sdw_bus *bus,
 		     struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
@@ -1096,14 +1094,30 @@ static struct sdw_master_runtime
 
 	list_add_tail(&m_rt->bus_node, &bus->m_rt_list);
 
-	m_rt->ch_count = stream_config->ch_count;
 	m_rt->bus = bus;
 	m_rt->stream = stream;
-	m_rt->direction = stream_config->direction;
 
 	return m_rt;
 }
 
+/**
+ * sdw_master_rt_config() - Configure Master runtime handle
+ *
+ * @m_rt: Master runtime handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+
+static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
+				struct sdw_stream_config *stream_config)
+{
+	m_rt->ch_count = stream_config->ch_count;
+	m_rt->direction = stream_config->direction;
+
+	return 0;
+}
+
 /**
  * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
  *
@@ -1321,19 +1335,21 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	 * check if Master is already allocated (e.g. as a result of Slave adding
 	 * it first), if so skip allocation and go to configuration
 	 */
-	m_rt = sdw_find_master_rt(bus, stream);
+	m_rt = sdw_master_rt_find(bus, stream);
 	if (m_rt)
 		goto skip_alloc_master_rt;
 
-	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
+	m_rt = sdw_master_rt_alloc(bus, stream);
 	if (!m_rt) {
-		dev_err(bus->dev,
-			"Master runtime config failed for stream:%s\n",
-			stream->name);
+		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto unlock;
 	}
 
+	ret = sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto unlock;
+
 skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
@@ -1388,7 +1404,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * check if Master is already allocated, if so skip allocation
 	 * and go to configuration
 	 */
-	m_rt = sdw_find_master_rt(slave->bus, stream);
+	m_rt = sdw_master_rt_find(slave->bus, stream);
 	if (m_rt)
 		goto skip_alloc_master_rt;
 
@@ -1396,14 +1412,15 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * If this API is invoked by Slave first then m_rt is not valid.
 	 * So, allocate m_rt and add Slave to it.
 	 */
-	m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream);
+	m_rt = sdw_master_rt_alloc(slave->bus, stream);
 	if (!m_rt) {
-		dev_err(&slave->dev,
-			"alloc master runtime failed for stream:%s\n",
-			stream->name);
+		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto error;
 	}
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto stream_error;
 
 skip_alloc_master_rt:
 	s_rt = sdw_alloc_slave_rt(slave, stream_config);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 10/19] soundwire: stream: split sdw_alloc_master_rt() in alloc and config
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split the two parts so that we can do multiple configurations during
ALSA/ASoC hw_params stage. Also follow existing convention
sdw_<object>_<action> used at lower level.

No functionality change here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 51 +++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 263b76230f8f..e38c9208c77b 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1055,7 +1055,7 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 EXPORT_SYMBOL(sdw_alloc_stream);
 
 static struct sdw_master_runtime
-*sdw_find_master_rt(struct sdw_bus *bus,
+*sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
@@ -1070,17 +1070,15 @@ static struct sdw_master_runtime
 }
 
 /**
- * sdw_alloc_master_rt() - Allocates and initialize Master runtime handle
+ * sdw_master_rt_alloc() - Allocates a Master runtime handle
  *
  * @bus: SDW bus instance
- * @stream_config: Stream configuration
  * @stream: Stream runtime handle.
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_master_runtime
-*sdw_alloc_master_rt(struct sdw_bus *bus,
-		     struct sdw_stream_config *stream_config,
+*sdw_master_rt_alloc(struct sdw_bus *bus,
 		     struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
@@ -1096,14 +1094,30 @@ static struct sdw_master_runtime
 
 	list_add_tail(&m_rt->bus_node, &bus->m_rt_list);
 
-	m_rt->ch_count = stream_config->ch_count;
 	m_rt->bus = bus;
 	m_rt->stream = stream;
-	m_rt->direction = stream_config->direction;
 
 	return m_rt;
 }
 
+/**
+ * sdw_master_rt_config() - Configure Master runtime handle
+ *
+ * @m_rt: Master runtime handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+
+static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
+				struct sdw_stream_config *stream_config)
+{
+	m_rt->ch_count = stream_config->ch_count;
+	m_rt->direction = stream_config->direction;
+
+	return 0;
+}
+
 /**
  * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
  *
@@ -1321,19 +1335,21 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	 * check if Master is already allocated (e.g. as a result of Slave adding
 	 * it first), if so skip allocation and go to configuration
 	 */
-	m_rt = sdw_find_master_rt(bus, stream);
+	m_rt = sdw_master_rt_find(bus, stream);
 	if (m_rt)
 		goto skip_alloc_master_rt;
 
-	m_rt = sdw_alloc_master_rt(bus, stream_config, stream);
+	m_rt = sdw_master_rt_alloc(bus, stream);
 	if (!m_rt) {
-		dev_err(bus->dev,
-			"Master runtime config failed for stream:%s\n",
-			stream->name);
+		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto unlock;
 	}
 
+	ret = sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto unlock;
+
 skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
@@ -1388,7 +1404,7 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * check if Master is already allocated, if so skip allocation
 	 * and go to configuration
 	 */
-	m_rt = sdw_find_master_rt(slave->bus, stream);
+	m_rt = sdw_master_rt_find(slave->bus, stream);
 	if (m_rt)
 		goto skip_alloc_master_rt;
 
@@ -1396,14 +1412,15 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * If this API is invoked by Slave first then m_rt is not valid.
 	 * So, allocate m_rt and add Slave to it.
 	 */
-	m_rt = sdw_alloc_master_rt(slave->bus, stream_config, stream);
+	m_rt = sdw_master_rt_alloc(slave->bus, stream);
 	if (!m_rt) {
-		dev_err(&slave->dev,
-			"alloc master runtime failed for stream:%s\n",
-			stream->name);
+		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto error;
 	}
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto stream_error;
 
 skip_alloc_master_rt:
 	s_rt = sdw_alloc_slave_rt(slave, stream_config);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 11/19] soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Code move before splitting the function in two.
No functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 52 +++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e38c9208c77b..eef2e5fd245e 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1054,6 +1054,32 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 }
 EXPORT_SYMBOL(sdw_alloc_stream);
 
+/**
+ * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
+ *
+ * @slave: Slave handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+static struct sdw_slave_runtime
+*sdw_alloc_slave_rt(struct sdw_slave *slave,
+		    struct sdw_stream_config *stream_config)
+{
+	struct sdw_slave_runtime *s_rt;
+
+	s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
+	if (!s_rt)
+		return NULL;
+
+	INIT_LIST_HEAD(&s_rt->port_list);
+	s_rt->ch_count = stream_config->ch_count;
+	s_rt->direction = stream_config->direction;
+	s_rt->slave = slave;
+
+	return s_rt;
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1118,32 +1144,6 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
 	return 0;
 }
 
-/**
- * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
- *
- * @slave: Slave handle
- * @stream_config: Stream configuration
- *
- * This function is to be called with bus_lock held.
- */
-static struct sdw_slave_runtime
-*sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config)
-{
-	struct sdw_slave_runtime *s_rt;
-
-	s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
-	if (!s_rt)
-		return NULL;
-
-	INIT_LIST_HEAD(&s_rt->port_list);
-	s_rt->ch_count = stream_config->ch_count;
-	s_rt->direction = stream_config->direction;
-	s_rt->slave = slave;
-
-	return s_rt;
-}
-
 /**
  * sdw_release_slave_stream() - Free Slave(s) runtime handle
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 11/19] soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Code move before splitting the function in two.
No functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 52 +++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e38c9208c77b..eef2e5fd245e 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1054,6 +1054,32 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 }
 EXPORT_SYMBOL(sdw_alloc_stream);
 
+/**
+ * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
+ *
+ * @slave: Slave handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+static struct sdw_slave_runtime
+*sdw_alloc_slave_rt(struct sdw_slave *slave,
+		    struct sdw_stream_config *stream_config)
+{
+	struct sdw_slave_runtime *s_rt;
+
+	s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
+	if (!s_rt)
+		return NULL;
+
+	INIT_LIST_HEAD(&s_rt->port_list);
+	s_rt->ch_count = stream_config->ch_count;
+	s_rt->direction = stream_config->direction;
+	s_rt->slave = slave;
+
+	return s_rt;
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1118,32 +1144,6 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
 	return 0;
 }
 
-/**
- * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
- *
- * @slave: Slave handle
- * @stream_config: Stream configuration
- *
- * This function is to be called with bus_lock held.
- */
-static struct sdw_slave_runtime
-*sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config)
-{
-	struct sdw_slave_runtime *s_rt;
-
-	s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
-	if (!s_rt)
-		return NULL;
-
-	INIT_LIST_HEAD(&s_rt->port_list);
-	s_rt->ch_count = stream_config->ch_count;
-	s_rt->direction = stream_config->direction;
-	s_rt->slave = slave;
-
-	return s_rt;
-}
-
 /**
  * sdw_release_slave_stream() - Free Slave(s) runtime handle
  *
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 12/19] soundwire: stream: split sdw_alloc_slave_rt() in alloc and config
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split the two parts so that we can do multiple configurations during
ALSA/ASoC hw_params stage. Also follow existing convention
sdw_<object>_<action> used at lower level.

No functionality change here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index eef2e5fd245e..b7ccfa5a9cfc 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1055,16 +1055,14 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 EXPORT_SYMBOL(sdw_alloc_stream);
 
 /**
- * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
+ * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
  * @slave: Slave handle
- * @stream_config: Stream configuration
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
-*sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config)
+*sdw_slave_rt_alloc(struct sdw_slave *slave)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1073,13 +1071,28 @@ static struct sdw_slave_runtime
 		return NULL;
 
 	INIT_LIST_HEAD(&s_rt->port_list);
-	s_rt->ch_count = stream_config->ch_count;
-	s_rt->direction = stream_config->direction;
 	s_rt->slave = slave;
 
 	return s_rt;
 }
 
+/**
+ * sdw_slave_rt_config() - Configure a Slave runtime handle.
+ *
+ * @s_rt: Slave runtime handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
+			       struct sdw_stream_config *stream_config)
+{
+	s_rt->ch_count = stream_config->ch_count;
+	s_rt->direction = stream_config->direction;
+
+	return 0;
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1423,16 +1436,18 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto stream_error;
 
 skip_alloc_master_rt:
-	s_rt = sdw_alloc_slave_rt(slave, stream_config);
+	s_rt = sdw_slave_rt_alloc(slave);
 	if (!s_rt) {
-		dev_err(&slave->dev,
-			"Slave runtime config failed for stream:%s\n",
-			stream->name);
+		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
 	}
 	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
+	ret = sdw_slave_rt_config(s_rt, stream_config);
+	if (ret)
+		goto stream_error;
+
 	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
 	if (ret)
 		goto stream_error;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 12/19] soundwire: stream: split sdw_alloc_slave_rt() in alloc and config
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Split the two parts so that we can do multiple configurations during
ALSA/ASoC hw_params stage. Also follow existing convention
sdw_<object>_<action> used at lower level.

No functionality change here.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index eef2e5fd245e..b7ccfa5a9cfc 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1055,16 +1055,14 @@ struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
 EXPORT_SYMBOL(sdw_alloc_stream);
 
 /**
- * sdw_alloc_slave_rt() - Allocate and initialize Slave runtime handle.
+ * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
  * @slave: Slave handle
- * @stream_config: Stream configuration
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
-*sdw_alloc_slave_rt(struct sdw_slave *slave,
-		    struct sdw_stream_config *stream_config)
+*sdw_slave_rt_alloc(struct sdw_slave *slave)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1073,13 +1071,28 @@ static struct sdw_slave_runtime
 		return NULL;
 
 	INIT_LIST_HEAD(&s_rt->port_list);
-	s_rt->ch_count = stream_config->ch_count;
-	s_rt->direction = stream_config->direction;
 	s_rt->slave = slave;
 
 	return s_rt;
 }
 
+/**
+ * sdw_slave_rt_config() - Configure a Slave runtime handle.
+ *
+ * @s_rt: Slave runtime handle
+ * @stream_config: Stream configuration
+ *
+ * This function is to be called with bus_lock held.
+ */
+static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
+			       struct sdw_stream_config *stream_config)
+{
+	s_rt->ch_count = stream_config->ch_count;
+	s_rt->direction = stream_config->direction;
+
+	return 0;
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1423,16 +1436,18 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto stream_error;
 
 skip_alloc_master_rt:
-	s_rt = sdw_alloc_slave_rt(slave, stream_config);
+	s_rt = sdw_slave_rt_alloc(slave);
 	if (!s_rt) {
-		dev_err(&slave->dev,
-			"Slave runtime config failed for stream:%s\n",
-			stream->name);
+		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
 	}
 	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
+	ret = sdw_slave_rt_config(s_rt, stream_config);
+	if (ret)
+		goto stream_error;
+
 	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
 	if (ret)
 		goto stream_error;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 13/19] soundwire: stream: group sdw_stream_ functions
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Group all exported functions prior to split of add in alloc/config
stages necessary for support of multiple calls to hw_params() by
ALSA/ASoC core.

Pure code move, no functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 529 +++++++++++++++++++------------------
 1 file changed, 265 insertions(+), 264 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b7ccfa5a9cfc..b0f21f2ca599 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1015,45 +1015,6 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
 	return 0;
 }
 
-/**
- * sdw_release_stream() - Free the assigned stream runtime
- *
- * @stream: SoundWire stream runtime
- *
- * sdw_release_stream should be called only once per stream
- */
-void sdw_release_stream(struct sdw_stream_runtime *stream)
-{
-	kfree(stream);
-}
-EXPORT_SYMBOL(sdw_release_stream);
-
-/**
- * sdw_alloc_stream() - Allocate and return stream runtime
- *
- * @stream_name: SoundWire stream name
- *
- * Allocates a SoundWire stream runtime instance.
- * sdw_alloc_stream should be called only once per stream. Typically
- * invoked from ALSA/ASoC machine/platform driver.
- */
-struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
-{
-	struct sdw_stream_runtime *stream;
-
-	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
-	if (!stream)
-		return NULL;
-
-	stream->name = stream_name;
-	INIT_LIST_HEAD(&stream->master_list);
-	stream->state = SDW_STREAM_ALLOCATED;
-	stream->m_rt_count = 0;
-
-	return stream;
-}
-EXPORT_SYMBOL(sdw_alloc_stream);
-
 /**
  * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
@@ -1210,62 +1171,6 @@ static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
 	kfree(m_rt);
 }
 
-/**
- * sdw_stream_remove_master() - Remove master from sdw_stream
- *
- * @bus: SDW Bus instance
- * @stream: SoundWire stream
- *
- * This removes and frees port_rt and master_rt from a 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);
-
-	list_for_each_entry_safe(m_rt, _m_rt,
-				 &stream->master_list, stream_node) {
-		if (m_rt->bus != bus)
-			continue;
-
-		sdw_master_port_free(m_rt);
-		sdw_release_master_stream(m_rt, stream);
-		stream->m_rt_count--;
-	}
-
-	if (list_empty(&stream->master_list))
-		stream->state = SDW_STREAM_RELEASED;
-
-	mutex_unlock(&bus->bus_lock);
-
-	return 0;
-}
-EXPORT_SYMBOL(sdw_stream_remove_master);
-
-/**
- * sdw_stream_remove_slave() - Remove slave from sdw_stream
- *
- * @slave: SDW Slave instance
- * @stream: SoundWire stream
- *
- * This removes and frees port_rt and slave_rt from a stream
- */
-int sdw_stream_remove_slave(struct sdw_slave *slave,
-			    struct sdw_stream_runtime *stream)
-{
-	mutex_lock(&slave->bus->bus_lock);
-
-	sdw_slave_port_free(slave, stream);
-	sdw_release_slave_stream(slave, stream);
-
-	mutex_unlock(&slave->bus->bus_lock);
-
-	return 0;
-}
-EXPORT_SYMBOL(sdw_stream_remove_slave);
-
 /**
  * sdw_config_stream() - Configure the allocated stream
  *
@@ -1312,175 +1217,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-/**
- * sdw_stream_add_master() - Allocate and add master runtime to a stream
- *
- * @bus: SDW Bus instance
- * @stream_config: Stream configuration for audio stream
- * @port_config: Port configuration for audio stream
- * @num_ports: Number of ports
- * @stream: SoundWire stream
- */
-int sdw_stream_add_master(struct sdw_bus *bus,
-			  struct sdw_stream_config *stream_config,
-			  struct sdw_port_config *port_config,
-			  unsigned int num_ports,
-			  struct sdw_stream_runtime *stream)
-{
-	struct sdw_master_runtime *m_rt;
-	int ret;
-
-	mutex_lock(&bus->bus_lock);
-
-	/*
-	 * For multi link streams, add the second master only if
-	 * the bus supports it.
-	 * Check if bus->multi_link is set
-	 */
-	if (!bus->multi_link && stream->m_rt_count > 0) {
-		dev_err(bus->dev,
-			"Multilink not supported, link %d\n", bus->link_id);
-		ret = -EINVAL;
-		goto unlock;
-	}
-
-	/*
-	 * check if Master is already allocated (e.g. as a result of Slave adding
-	 * it first), if so skip allocation and go to configuration
-	 */
-	m_rt = sdw_master_rt_find(bus, stream);
-	if (m_rt)
-		goto skip_alloc_master_rt;
-
-	m_rt = sdw_master_rt_alloc(bus, stream);
-	if (!m_rt) {
-		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto unlock;
-	}
-
-	ret = sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto unlock;
-
-skip_alloc_master_rt:
-	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_alloc(m_rt, num_ports);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_config(m_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	stream->m_rt_count++;
-
-	goto unlock;
-
-stream_error:
-	sdw_release_master_stream(m_rt, stream);
-unlock:
-	mutex_unlock(&bus->bus_lock);
-	return ret;
-}
-EXPORT_SYMBOL(sdw_stream_add_master);
-
-/**
- * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream
- *
- * @slave: SDW Slave instance
- * @stream_config: Stream configuration for audio stream
- * @stream: SoundWire stream
- * @port_config: Port configuration for audio stream
- * @num_ports: Number of ports
- *
- * It is expected that Slave is added before adding Master
- * to the Stream.
- *
- */
-int sdw_stream_add_slave(struct sdw_slave *slave,
-			 struct sdw_stream_config *stream_config,
-			 struct sdw_port_config *port_config,
-			 unsigned int num_ports,
-			 struct sdw_stream_runtime *stream)
-{
-	struct sdw_slave_runtime *s_rt;
-	struct sdw_master_runtime *m_rt;
-	int ret;
-
-	mutex_lock(&slave->bus->bus_lock);
-
-	/*
-	 * check if Master is already allocated, if so skip allocation
-	 * and go to configuration
-	 */
-	m_rt = sdw_master_rt_find(slave->bus, stream);
-	if (m_rt)
-		goto skip_alloc_master_rt;
-
-	/*
-	 * If this API is invoked by Slave first then m_rt is not valid.
-	 * So, allocate m_rt and add Slave to it.
-	 */
-	m_rt = sdw_master_rt_alloc(slave->bus, stream);
-	if (!m_rt) {
-		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto error;
-	}
-	ret =  sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto stream_error;
-
-skip_alloc_master_rt:
-	s_rt = sdw_slave_rt_alloc(slave);
-	if (!s_rt) {
-		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto stream_error;
-	}
-	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
-
-	ret = sdw_slave_rt_config(s_rt, stream_config);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_config(slave, s_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	/*
-	 * Change stream state to CONFIGURED on first Slave add.
-	 * Bus is not aware of number of Slave(s) in a stream at this
-	 * point so cannot depend on all Slave(s) to be added in order to
-	 * change stream state to CONFIGURED.
-	 */
-	stream->state = SDW_STREAM_CONFIGURED;
-	goto error;
-
-stream_error:
-	/*
-	 * we hit error so cleanup the stream, release all Slave(s) and
-	 * Master runtime
-	 */
-	sdw_release_master_stream(m_rt, stream);
-error:
-	mutex_unlock(&slave->bus->bus_lock);
-	return ret;
-}
-EXPORT_SYMBOL(sdw_stream_add_slave);
-
 /**
  * sdw_get_slave_dpn_prop() - Get Slave port capabilities
  *
@@ -1939,6 +1675,32 @@ static int set_stream(struct snd_pcm_substream *substream,
 	return ret;
 }
 
+/**
+ * sdw_alloc_stream() - Allocate and return stream runtime
+ *
+ * @stream_name: SoundWire stream name
+ *
+ * Allocates a SoundWire stream runtime instance.
+ * sdw_alloc_stream should be called only once per stream. Typically
+ * invoked from ALSA/ASoC machine/platform driver.
+ */
+struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
+{
+	struct sdw_stream_runtime *stream;
+
+	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+	if (!stream)
+		return NULL;
+
+	stream->name = stream_name;
+	INIT_LIST_HEAD(&stream->master_list);
+	stream->state = SDW_STREAM_ALLOCATED;
+	stream->m_rt_count = 0;
+
+	return stream;
+}
+EXPORT_SYMBOL(sdw_alloc_stream);
+
 /**
  * sdw_startup_stream() - Startup SoundWire stream
  *
@@ -2015,3 +1777,242 @@ void sdw_shutdown_stream(void *sdw_substream)
 	set_stream(substream, NULL);
 }
 EXPORT_SYMBOL(sdw_shutdown_stream);
+
+/**
+ * sdw_release_stream() - Free the assigned stream runtime
+ *
+ * @stream: SoundWire stream runtime
+ *
+ * sdw_release_stream should be called only once per stream
+ */
+void sdw_release_stream(struct sdw_stream_runtime *stream)
+{
+	kfree(stream);
+}
+EXPORT_SYMBOL(sdw_release_stream);
+
+/**
+ * sdw_stream_add_master() - Allocate and add master runtime to a stream
+ *
+ * @bus: SDW Bus instance
+ * @stream_config: Stream configuration for audio stream
+ * @port_config: Port configuration for audio stream
+ * @num_ports: Number of ports
+ * @stream: SoundWire stream
+ */
+int sdw_stream_add_master(struct sdw_bus *bus,
+			  struct sdw_stream_config *stream_config,
+			  struct sdw_port_config *port_config,
+			  unsigned int num_ports,
+			  struct sdw_stream_runtime *stream)
+{
+	struct sdw_master_runtime *m_rt;
+	int ret;
+
+	mutex_lock(&bus->bus_lock);
+
+	/*
+	 * For multi link streams, add the second master only if
+	 * the bus supports it.
+	 * Check if bus->multi_link is set
+	 */
+	if (!bus->multi_link && stream->m_rt_count > 0) {
+		dev_err(bus->dev,
+			"Multilink not supported, link %d\n", bus->link_id);
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	/*
+	 * check if Master is already allocated (e.g. as a result of Slave adding
+	 * it first), if so skip allocation and go to configuration
+	 */
+	m_rt = sdw_master_rt_find(bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
+	m_rt = sdw_master_rt_alloc(bus, stream);
+	if (!m_rt) {
+		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	ret = sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto unlock;
+
+skip_alloc_master_rt:
+	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_config(m_rt, port_config);
+	if (ret)
+		goto stream_error;
+
+	stream->m_rt_count++;
+
+	goto unlock;
+
+stream_error:
+	sdw_release_master_stream(m_rt, stream);
+unlock:
+	mutex_unlock(&bus->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL(sdw_stream_add_master);
+
+/**
+ * sdw_stream_remove_master() - Remove master from sdw_stream
+ *
+ * @bus: SDW Bus instance
+ * @stream: SoundWire stream
+ *
+ * This removes and frees port_rt and master_rt from a 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);
+
+	list_for_each_entry_safe(m_rt, _m_rt,
+				 &stream->master_list, stream_node) {
+		if (m_rt->bus != bus)
+			continue;
+
+		sdw_master_port_free(m_rt);
+		sdw_release_master_stream(m_rt, stream);
+		stream->m_rt_count--;
+	}
+
+	if (list_empty(&stream->master_list))
+		stream->state = SDW_STREAM_RELEASED;
+
+	mutex_unlock(&bus->bus_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_stream_remove_master);
+
+/**
+ * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream
+ *
+ * @slave: SDW Slave instance
+ * @stream_config: Stream configuration for audio stream
+ * @stream: SoundWire stream
+ * @port_config: Port configuration for audio stream
+ * @num_ports: Number of ports
+ *
+ * It is expected that Slave is added before adding Master
+ * to the Stream.
+ *
+ */
+int sdw_stream_add_slave(struct sdw_slave *slave,
+			 struct sdw_stream_config *stream_config,
+			 struct sdw_port_config *port_config,
+			 unsigned int num_ports,
+			 struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt;
+	struct sdw_master_runtime *m_rt;
+	int ret;
+
+	mutex_lock(&slave->bus->bus_lock);
+
+	/*
+	 * check if Master is already allocated, if so skip allocation
+	 * and go to configuration
+	 */
+	m_rt = sdw_master_rt_find(slave->bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
+	/*
+	 * If this API is invoked by Slave first then m_rt is not valid.
+	 * So, allocate m_rt and add Slave to it.
+	 */
+	m_rt = sdw_master_rt_alloc(slave->bus, stream);
+	if (!m_rt) {
+		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto error;
+	}
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto stream_error;
+
+skip_alloc_master_rt:
+	s_rt = sdw_slave_rt_alloc(slave);
+	if (!s_rt) {
+		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto stream_error;
+	}
+	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
+
+	ret = sdw_slave_rt_config(s_rt, stream_config);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_config(slave, s_rt, port_config);
+	if (ret)
+		goto stream_error;
+
+	/*
+	 * Change stream state to CONFIGURED on first Slave add.
+	 * Bus is not aware of number of Slave(s) in a stream at this
+	 * point so cannot depend on all Slave(s) to be added in order to
+	 * change stream state to CONFIGURED.
+	 */
+	stream->state = SDW_STREAM_CONFIGURED;
+	goto error;
+
+stream_error:
+	/*
+	 * we hit error so cleanup the stream, release all Slave(s) and
+	 * Master runtime
+	 */
+	sdw_release_master_stream(m_rt, stream);
+error:
+	mutex_unlock(&slave->bus->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL(sdw_stream_add_slave);
+
+/**
+ * sdw_stream_remove_slave() - Remove slave from sdw_stream
+ *
+ * @slave: SDW Slave instance
+ * @stream: SoundWire stream
+ *
+ * This removes and frees port_rt and slave_rt from a stream
+ */
+int sdw_stream_remove_slave(struct sdw_slave *slave,
+			    struct sdw_stream_runtime *stream)
+{
+	mutex_lock(&slave->bus->bus_lock);
+
+	sdw_slave_port_free(slave, stream);
+	sdw_release_slave_stream(slave, stream);
+
+	mutex_unlock(&slave->bus->bus_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_stream_remove_slave);
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 13/19] soundwire: stream: group sdw_stream_ functions
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Group all exported functions prior to split of add in alloc/config
stages necessary for support of multiple calls to hw_params() by
ALSA/ASoC core.

Pure code move, no functionality change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 529 +++++++++++++++++++------------------
 1 file changed, 265 insertions(+), 264 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b7ccfa5a9cfc..b0f21f2ca599 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1015,45 +1015,6 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
 	return 0;
 }
 
-/**
- * sdw_release_stream() - Free the assigned stream runtime
- *
- * @stream: SoundWire stream runtime
- *
- * sdw_release_stream should be called only once per stream
- */
-void sdw_release_stream(struct sdw_stream_runtime *stream)
-{
-	kfree(stream);
-}
-EXPORT_SYMBOL(sdw_release_stream);
-
-/**
- * sdw_alloc_stream() - Allocate and return stream runtime
- *
- * @stream_name: SoundWire stream name
- *
- * Allocates a SoundWire stream runtime instance.
- * sdw_alloc_stream should be called only once per stream. Typically
- * invoked from ALSA/ASoC machine/platform driver.
- */
-struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
-{
-	struct sdw_stream_runtime *stream;
-
-	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
-	if (!stream)
-		return NULL;
-
-	stream->name = stream_name;
-	INIT_LIST_HEAD(&stream->master_list);
-	stream->state = SDW_STREAM_ALLOCATED;
-	stream->m_rt_count = 0;
-
-	return stream;
-}
-EXPORT_SYMBOL(sdw_alloc_stream);
-
 /**
  * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
@@ -1210,62 +1171,6 @@ static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
 	kfree(m_rt);
 }
 
-/**
- * sdw_stream_remove_master() - Remove master from sdw_stream
- *
- * @bus: SDW Bus instance
- * @stream: SoundWire stream
- *
- * This removes and frees port_rt and master_rt from a 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);
-
-	list_for_each_entry_safe(m_rt, _m_rt,
-				 &stream->master_list, stream_node) {
-		if (m_rt->bus != bus)
-			continue;
-
-		sdw_master_port_free(m_rt);
-		sdw_release_master_stream(m_rt, stream);
-		stream->m_rt_count--;
-	}
-
-	if (list_empty(&stream->master_list))
-		stream->state = SDW_STREAM_RELEASED;
-
-	mutex_unlock(&bus->bus_lock);
-
-	return 0;
-}
-EXPORT_SYMBOL(sdw_stream_remove_master);
-
-/**
- * sdw_stream_remove_slave() - Remove slave from sdw_stream
- *
- * @slave: SDW Slave instance
- * @stream: SoundWire stream
- *
- * This removes and frees port_rt and slave_rt from a stream
- */
-int sdw_stream_remove_slave(struct sdw_slave *slave,
-			    struct sdw_stream_runtime *stream)
-{
-	mutex_lock(&slave->bus->bus_lock);
-
-	sdw_slave_port_free(slave, stream);
-	sdw_release_slave_stream(slave, stream);
-
-	mutex_unlock(&slave->bus->bus_lock);
-
-	return 0;
-}
-EXPORT_SYMBOL(sdw_stream_remove_slave);
-
 /**
  * sdw_config_stream() - Configure the allocated stream
  *
@@ -1312,175 +1217,6 @@ static int sdw_config_stream(struct device *dev,
 	return 0;
 }
 
-/**
- * sdw_stream_add_master() - Allocate and add master runtime to a stream
- *
- * @bus: SDW Bus instance
- * @stream_config: Stream configuration for audio stream
- * @port_config: Port configuration for audio stream
- * @num_ports: Number of ports
- * @stream: SoundWire stream
- */
-int sdw_stream_add_master(struct sdw_bus *bus,
-			  struct sdw_stream_config *stream_config,
-			  struct sdw_port_config *port_config,
-			  unsigned int num_ports,
-			  struct sdw_stream_runtime *stream)
-{
-	struct sdw_master_runtime *m_rt;
-	int ret;
-
-	mutex_lock(&bus->bus_lock);
-
-	/*
-	 * For multi link streams, add the second master only if
-	 * the bus supports it.
-	 * Check if bus->multi_link is set
-	 */
-	if (!bus->multi_link && stream->m_rt_count > 0) {
-		dev_err(bus->dev,
-			"Multilink not supported, link %d\n", bus->link_id);
-		ret = -EINVAL;
-		goto unlock;
-	}
-
-	/*
-	 * check if Master is already allocated (e.g. as a result of Slave adding
-	 * it first), if so skip allocation and go to configuration
-	 */
-	m_rt = sdw_master_rt_find(bus, stream);
-	if (m_rt)
-		goto skip_alloc_master_rt;
-
-	m_rt = sdw_master_rt_alloc(bus, stream);
-	if (!m_rt) {
-		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto unlock;
-	}
-
-	ret = sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto unlock;
-
-skip_alloc_master_rt:
-	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_alloc(m_rt, num_ports);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_config(m_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	stream->m_rt_count++;
-
-	goto unlock;
-
-stream_error:
-	sdw_release_master_stream(m_rt, stream);
-unlock:
-	mutex_unlock(&bus->bus_lock);
-	return ret;
-}
-EXPORT_SYMBOL(sdw_stream_add_master);
-
-/**
- * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream
- *
- * @slave: SDW Slave instance
- * @stream_config: Stream configuration for audio stream
- * @stream: SoundWire stream
- * @port_config: Port configuration for audio stream
- * @num_ports: Number of ports
- *
- * It is expected that Slave is added before adding Master
- * to the Stream.
- *
- */
-int sdw_stream_add_slave(struct sdw_slave *slave,
-			 struct sdw_stream_config *stream_config,
-			 struct sdw_port_config *port_config,
-			 unsigned int num_ports,
-			 struct sdw_stream_runtime *stream)
-{
-	struct sdw_slave_runtime *s_rt;
-	struct sdw_master_runtime *m_rt;
-	int ret;
-
-	mutex_lock(&slave->bus->bus_lock);
-
-	/*
-	 * check if Master is already allocated, if so skip allocation
-	 * and go to configuration
-	 */
-	m_rt = sdw_master_rt_find(slave->bus, stream);
-	if (m_rt)
-		goto skip_alloc_master_rt;
-
-	/*
-	 * If this API is invoked by Slave first then m_rt is not valid.
-	 * So, allocate m_rt and add Slave to it.
-	 */
-	m_rt = sdw_master_rt_alloc(slave->bus, stream);
-	if (!m_rt) {
-		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto error;
-	}
-	ret =  sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto stream_error;
-
-skip_alloc_master_rt:
-	s_rt = sdw_slave_rt_alloc(slave);
-	if (!s_rt) {
-		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
-		ret = -ENOMEM;
-		goto stream_error;
-	}
-	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
-
-	ret = sdw_slave_rt_config(s_rt, stream_config);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
-	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_config(slave, s_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	/*
-	 * Change stream state to CONFIGURED on first Slave add.
-	 * Bus is not aware of number of Slave(s) in a stream at this
-	 * point so cannot depend on all Slave(s) to be added in order to
-	 * change stream state to CONFIGURED.
-	 */
-	stream->state = SDW_STREAM_CONFIGURED;
-	goto error;
-
-stream_error:
-	/*
-	 * we hit error so cleanup the stream, release all Slave(s) and
-	 * Master runtime
-	 */
-	sdw_release_master_stream(m_rt, stream);
-error:
-	mutex_unlock(&slave->bus->bus_lock);
-	return ret;
-}
-EXPORT_SYMBOL(sdw_stream_add_slave);
-
 /**
  * sdw_get_slave_dpn_prop() - Get Slave port capabilities
  *
@@ -1939,6 +1675,32 @@ static int set_stream(struct snd_pcm_substream *substream,
 	return ret;
 }
 
+/**
+ * sdw_alloc_stream() - Allocate and return stream runtime
+ *
+ * @stream_name: SoundWire stream name
+ *
+ * Allocates a SoundWire stream runtime instance.
+ * sdw_alloc_stream should be called only once per stream. Typically
+ * invoked from ALSA/ASoC machine/platform driver.
+ */
+struct sdw_stream_runtime *sdw_alloc_stream(const char *stream_name)
+{
+	struct sdw_stream_runtime *stream;
+
+	stream = kzalloc(sizeof(*stream), GFP_KERNEL);
+	if (!stream)
+		return NULL;
+
+	stream->name = stream_name;
+	INIT_LIST_HEAD(&stream->master_list);
+	stream->state = SDW_STREAM_ALLOCATED;
+	stream->m_rt_count = 0;
+
+	return stream;
+}
+EXPORT_SYMBOL(sdw_alloc_stream);
+
 /**
  * sdw_startup_stream() - Startup SoundWire stream
  *
@@ -2015,3 +1777,242 @@ void sdw_shutdown_stream(void *sdw_substream)
 	set_stream(substream, NULL);
 }
 EXPORT_SYMBOL(sdw_shutdown_stream);
+
+/**
+ * sdw_release_stream() - Free the assigned stream runtime
+ *
+ * @stream: SoundWire stream runtime
+ *
+ * sdw_release_stream should be called only once per stream
+ */
+void sdw_release_stream(struct sdw_stream_runtime *stream)
+{
+	kfree(stream);
+}
+EXPORT_SYMBOL(sdw_release_stream);
+
+/**
+ * sdw_stream_add_master() - Allocate and add master runtime to a stream
+ *
+ * @bus: SDW Bus instance
+ * @stream_config: Stream configuration for audio stream
+ * @port_config: Port configuration for audio stream
+ * @num_ports: Number of ports
+ * @stream: SoundWire stream
+ */
+int sdw_stream_add_master(struct sdw_bus *bus,
+			  struct sdw_stream_config *stream_config,
+			  struct sdw_port_config *port_config,
+			  unsigned int num_ports,
+			  struct sdw_stream_runtime *stream)
+{
+	struct sdw_master_runtime *m_rt;
+	int ret;
+
+	mutex_lock(&bus->bus_lock);
+
+	/*
+	 * For multi link streams, add the second master only if
+	 * the bus supports it.
+	 * Check if bus->multi_link is set
+	 */
+	if (!bus->multi_link && stream->m_rt_count > 0) {
+		dev_err(bus->dev,
+			"Multilink not supported, link %d\n", bus->link_id);
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	/*
+	 * check if Master is already allocated (e.g. as a result of Slave adding
+	 * it first), if so skip allocation and go to configuration
+	 */
+	m_rt = sdw_master_rt_find(bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
+	m_rt = sdw_master_rt_alloc(bus, stream);
+	if (!m_rt) {
+		dev_err(bus->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto unlock;
+	}
+
+	ret = sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto unlock;
+
+skip_alloc_master_rt:
+	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_master_port_config(m_rt, port_config);
+	if (ret)
+		goto stream_error;
+
+	stream->m_rt_count++;
+
+	goto unlock;
+
+stream_error:
+	sdw_release_master_stream(m_rt, stream);
+unlock:
+	mutex_unlock(&bus->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL(sdw_stream_add_master);
+
+/**
+ * sdw_stream_remove_master() - Remove master from sdw_stream
+ *
+ * @bus: SDW Bus instance
+ * @stream: SoundWire stream
+ *
+ * This removes and frees port_rt and master_rt from a 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);
+
+	list_for_each_entry_safe(m_rt, _m_rt,
+				 &stream->master_list, stream_node) {
+		if (m_rt->bus != bus)
+			continue;
+
+		sdw_master_port_free(m_rt);
+		sdw_release_master_stream(m_rt, stream);
+		stream->m_rt_count--;
+	}
+
+	if (list_empty(&stream->master_list))
+		stream->state = SDW_STREAM_RELEASED;
+
+	mutex_unlock(&bus->bus_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_stream_remove_master);
+
+/**
+ * sdw_stream_add_slave() - Allocate and add master/slave runtime to a stream
+ *
+ * @slave: SDW Slave instance
+ * @stream_config: Stream configuration for audio stream
+ * @stream: SoundWire stream
+ * @port_config: Port configuration for audio stream
+ * @num_ports: Number of ports
+ *
+ * It is expected that Slave is added before adding Master
+ * to the Stream.
+ *
+ */
+int sdw_stream_add_slave(struct sdw_slave *slave,
+			 struct sdw_stream_config *stream_config,
+			 struct sdw_port_config *port_config,
+			 unsigned int num_ports,
+			 struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt;
+	struct sdw_master_runtime *m_rt;
+	int ret;
+
+	mutex_lock(&slave->bus->bus_lock);
+
+	/*
+	 * check if Master is already allocated, if so skip allocation
+	 * and go to configuration
+	 */
+	m_rt = sdw_master_rt_find(slave->bus, stream);
+	if (m_rt)
+		goto skip_alloc_master_rt;
+
+	/*
+	 * If this API is invoked by Slave first then m_rt is not valid.
+	 * So, allocate m_rt and add Slave to it.
+	 */
+	m_rt = sdw_master_rt_alloc(slave->bus, stream);
+	if (!m_rt) {
+		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto error;
+	}
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret < 0)
+		goto stream_error;
+
+skip_alloc_master_rt:
+	s_rt = sdw_slave_rt_alloc(slave);
+	if (!s_rt) {
+		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
+		ret = -ENOMEM;
+		goto stream_error;
+	}
+	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
+
+	ret = sdw_slave_rt_config(s_rt, stream_config);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto stream_error;
+
+	ret = sdw_slave_port_config(slave, s_rt, port_config);
+	if (ret)
+		goto stream_error;
+
+	/*
+	 * Change stream state to CONFIGURED on first Slave add.
+	 * Bus is not aware of number of Slave(s) in a stream at this
+	 * point so cannot depend on all Slave(s) to be added in order to
+	 * change stream state to CONFIGURED.
+	 */
+	stream->state = SDW_STREAM_CONFIGURED;
+	goto error;
+
+stream_error:
+	/*
+	 * we hit error so cleanup the stream, release all Slave(s) and
+	 * Master runtime
+	 */
+	sdw_release_master_stream(m_rt, stream);
+error:
+	mutex_unlock(&slave->bus->bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL(sdw_stream_add_slave);
+
+/**
+ * sdw_stream_remove_slave() - Remove slave from sdw_stream
+ *
+ * @slave: SDW Slave instance
+ * @stream: SoundWire stream
+ *
+ * This removes and frees port_rt and slave_rt from a stream
+ */
+int sdw_stream_remove_slave(struct sdw_slave *slave,
+			    struct sdw_stream_runtime *stream)
+{
+	mutex_lock(&slave->bus->bus_lock);
+
+	sdw_slave_port_free(slave, stream);
+	sdw_release_slave_stream(slave, stream);
+
+	mutex_unlock(&slave->bus->bus_lock);
+
+	return 0;
+}
+EXPORT_SYMBOL(sdw_stream_remove_slave);
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 14/19] soundwire: stream: rename and move master/slave_rt_free routines
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The naming is rather inconsistent, use the sdw_<object>_<action>
convention, and move the free routine after alloc/config.

No functionality change beyond rename/move.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 72 +++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b0f21f2ca599..e57920ee4c55 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1054,6 +1054,33 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 	return 0;
 }
 
+/**
+ * sdw_slave_rt_free() - Free Slave(s) runtime handle
+ *
+ * @slave: Slave handle.
+ * @stream: Stream runtime handle.
+ *
+ * This function is to be called with bus_lock held.
+ */
+static void sdw_slave_rt_free(struct sdw_slave *slave,
+			      struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt, *_s_rt;
+	struct sdw_master_runtime *m_rt;
+
+	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;
+			}
+		}
+	}
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1119,51 +1146,24 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
 }
 
 /**
- * sdw_release_slave_stream() - Free Slave(s) runtime handle
- *
- * @slave: Slave handle.
- * @stream: Stream runtime handle.
- *
- * This function is to be called with bus_lock held.
- */
-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;
-
-	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;
-			}
-		}
-	}
-}
-
-/**
- * sdw_release_master_stream() - Free Master runtime handle
+ * sdw_master_rt_free() - Free Master runtime handle
  *
  * @m_rt: Master runtime node
  * @stream: Stream runtime handle.
  *
  * This function is to be called with bus_lock held
  * It frees the Master runtime handle and associated Slave(s) runtime
- * handle. If this is called first then sdw_release_slave_stream() will have
+ * handle. If this is called first then sdw_slave_rt_free() will have
  * no effect as Slave(s) runtime handle would already be freed up.
  */
-static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
-				      struct sdw_stream_runtime *stream)
+static void sdw_master_rt_free(struct sdw_master_runtime *m_rt,
+			       struct sdw_stream_runtime *stream)
 {
 	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_slave_port_free(s_rt->slave, stream);
-		sdw_release_slave_stream(s_rt->slave, stream);
+		sdw_slave_rt_free(s_rt->slave, stream);
 	}
 
 	list_del(&m_rt->stream_node);
@@ -1860,7 +1860,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	goto unlock;
 
 stream_error:
-	sdw_release_master_stream(m_rt, stream);
+	sdw_master_rt_free(m_rt, stream);
 unlock:
 	mutex_unlock(&bus->bus_lock);
 	return ret;
@@ -1888,7 +1888,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
 			continue;
 
 		sdw_master_port_free(m_rt);
-		sdw_release_master_stream(m_rt, stream);
+		sdw_master_rt_free(m_rt, stream);
 		stream->m_rt_count--;
 	}
 
@@ -1987,7 +1987,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(m_rt, stream);
+	sdw_master_rt_free(m_rt, stream);
 error:
 	mutex_unlock(&slave->bus->bus_lock);
 	return ret;
@@ -2008,7 +2008,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 	mutex_lock(&slave->bus->bus_lock);
 
 	sdw_slave_port_free(slave, stream);
-	sdw_release_slave_stream(slave, stream);
+	sdw_slave_rt_free(slave, stream);
 
 	mutex_unlock(&slave->bus->bus_lock);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 14/19] soundwire: stream: rename and move master/slave_rt_free routines
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The naming is rather inconsistent, use the sdw_<object>_<action>
convention, and move the free routine after alloc/config.

No functionality change beyond rename/move.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 72 +++++++++++++++++++-------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index b0f21f2ca599..e57920ee4c55 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1054,6 +1054,33 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 	return 0;
 }
 
+/**
+ * sdw_slave_rt_free() - Free Slave(s) runtime handle
+ *
+ * @slave: Slave handle.
+ * @stream: Stream runtime handle.
+ *
+ * This function is to be called with bus_lock held.
+ */
+static void sdw_slave_rt_free(struct sdw_slave *slave,
+			      struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt, *_s_rt;
+	struct sdw_master_runtime *m_rt;
+
+	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;
+			}
+		}
+	}
+}
+
 static struct sdw_master_runtime
 *sdw_master_rt_find(struct sdw_bus *bus,
 		    struct sdw_stream_runtime *stream)
@@ -1119,51 +1146,24 @@ static int sdw_master_rt_config(struct sdw_master_runtime *m_rt,
 }
 
 /**
- * sdw_release_slave_stream() - Free Slave(s) runtime handle
- *
- * @slave: Slave handle.
- * @stream: Stream runtime handle.
- *
- * This function is to be called with bus_lock held.
- */
-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;
-
-	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;
-			}
-		}
-	}
-}
-
-/**
- * sdw_release_master_stream() - Free Master runtime handle
+ * sdw_master_rt_free() - Free Master runtime handle
  *
  * @m_rt: Master runtime node
  * @stream: Stream runtime handle.
  *
  * This function is to be called with bus_lock held
  * It frees the Master runtime handle and associated Slave(s) runtime
- * handle. If this is called first then sdw_release_slave_stream() will have
+ * handle. If this is called first then sdw_slave_rt_free() will have
  * no effect as Slave(s) runtime handle would already be freed up.
  */
-static void sdw_release_master_stream(struct sdw_master_runtime *m_rt,
-				      struct sdw_stream_runtime *stream)
+static void sdw_master_rt_free(struct sdw_master_runtime *m_rt,
+			       struct sdw_stream_runtime *stream)
 {
 	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_slave_port_free(s_rt->slave, stream);
-		sdw_release_slave_stream(s_rt->slave, stream);
+		sdw_slave_rt_free(s_rt->slave, stream);
 	}
 
 	list_del(&m_rt->stream_node);
@@ -1860,7 +1860,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	goto unlock;
 
 stream_error:
-	sdw_release_master_stream(m_rt, stream);
+	sdw_master_rt_free(m_rt, stream);
 unlock:
 	mutex_unlock(&bus->bus_lock);
 	return ret;
@@ -1888,7 +1888,7 @@ int sdw_stream_remove_master(struct sdw_bus *bus,
 			continue;
 
 		sdw_master_port_free(m_rt);
-		sdw_release_master_stream(m_rt, stream);
+		sdw_master_rt_free(m_rt, stream);
 		stream->m_rt_count--;
 	}
 
@@ -1987,7 +1987,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(m_rt, stream);
+	sdw_master_rt_free(m_rt, stream);
 error:
 	mutex_unlock(&slave->bus->bus_lock);
 	return ret;
@@ -2008,7 +2008,7 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 	mutex_lock(&slave->bus->bus_lock);
 
 	sdw_slave_port_free(slave, stream);
-	sdw_release_slave_stream(slave, stream);
+	sdw_slave_rt_free(slave, stream);
 
 	mutex_unlock(&slave->bus->bus_lock);
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 15/19] soundwire: stream: move list addition to sdw_slave_alloc_rt()
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Simplify sdw_stream_add_slave() by moving the linked list management
inside of the sdw_slave_alloc_rt_free() helper, this also makes the
alloc/free helpers more symmetrical.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e57920ee4c55..8a76d6605f93 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1019,11 +1019,13 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
  * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
  * @slave: Slave handle
+ * @m_rt: Master runtime handle
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
-*sdw_slave_rt_alloc(struct sdw_slave *slave)
+*sdw_slave_rt_alloc(struct sdw_slave *slave,
+		    struct sdw_master_runtime *m_rt)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1034,6 +1036,8 @@ static struct sdw_slave_runtime
 	INIT_LIST_HEAD(&s_rt->port_list);
 	s_rt->slave = slave;
 
+	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
+
 	return s_rt;
 }
 
@@ -1949,13 +1953,12 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto stream_error;
 
 skip_alloc_master_rt:
-	s_rt = sdw_slave_rt_alloc(slave);
+	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
 	}
-	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
 	ret = sdw_slave_rt_config(s_rt, stream_config);
 	if (ret)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 15/19] soundwire: stream: move list addition to sdw_slave_alloc_rt()
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Simplify sdw_stream_add_slave() by moving the linked list management
inside of the sdw_slave_alloc_rt_free() helper, this also makes the
alloc/free helpers more symmetrical.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index e57920ee4c55..8a76d6605f93 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1019,11 +1019,13 @@ static int sdw_master_port_config(struct sdw_master_runtime *m_rt,
  * sdw_slave_rt_alloc() - Allocate a Slave runtime handle.
  *
  * @slave: Slave handle
+ * @m_rt: Master runtime handle
  *
  * This function is to be called with bus_lock held.
  */
 static struct sdw_slave_runtime
-*sdw_slave_rt_alloc(struct sdw_slave *slave)
+*sdw_slave_rt_alloc(struct sdw_slave *slave,
+		    struct sdw_master_runtime *m_rt)
 {
 	struct sdw_slave_runtime *s_rt;
 
@@ -1034,6 +1036,8 @@ static struct sdw_slave_runtime
 	INIT_LIST_HEAD(&s_rt->port_list);
 	s_rt->slave = slave;
 
+	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
+
 	return s_rt;
 }
 
@@ -1949,13 +1953,12 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto stream_error;
 
 skip_alloc_master_rt:
-	s_rt = sdw_slave_rt_alloc(slave);
+	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
 		goto stream_error;
 	}
-	list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
 
 	ret = sdw_slave_rt_config(s_rt, stream_config);
 	if (ret)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 16/19] soundwire: stream: separate alloc and config within sdw_stream_add_xxx()
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Separate alloc and config parts so that follow-up patches can allow
for multiple calls to sdw_stream_add_slave/master. This is a feature
from the ALSA/ASoC frameworks which is not supported today.

This is an invasive patch which modifies the error handling flow, with
cleanups only done when an allocation fails. Configuration failures
only return an error code.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 81 ++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 8a76d6605f93..03cfac0129af 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1811,6 +1811,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 			  struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
+	bool alloc_master_rt = true;
 	int ret;
 
 	mutex_lock(&bus->bus_lock);
@@ -1832,8 +1833,10 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	 * it first), if so skip allocation and go to configuration
 	 */
 	m_rt = sdw_master_rt_find(bus, stream);
-	if (m_rt)
+	if (m_rt) {
+		alloc_master_rt = false;
 		goto skip_alloc_master_rt;
+	}
 
 	m_rt = sdw_master_rt_alloc(bus, stream);
 	if (!m_rt) {
@@ -1841,30 +1844,32 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		ret = -ENOMEM;
 		goto unlock;
 	}
+skip_alloc_master_rt:
+
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto alloc_error;
+
+	stream->m_rt_count++;
 
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
 		goto unlock;
 
-skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_alloc(m_rt, num_ports);
-	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_master_port_config(m_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	stream->m_rt_count++;
 
 	goto unlock;
 
-stream_error:
-	sdw_master_rt_free(m_rt, stream);
+alloc_error:
+	/*
+	 * we only cleanup what was allocated in this routine
+	 */
+	if (alloc_master_rt)
+		sdw_master_rt_free(m_rt, stream);
 unlock:
 	mutex_unlock(&bus->bus_lock);
 	return ret;
@@ -1926,6 +1931,9 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 {
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_master_runtime *m_rt;
+	bool alloc_master_rt = true;
+	bool alloc_slave_rt = true;
+
 	int ret;
 
 	mutex_lock(&slave->bus->bus_lock);
@@ -1935,8 +1943,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * and go to configuration
 	 */
 	m_rt = sdw_master_rt_find(slave->bus, stream);
-	if (m_rt)
+	if (m_rt) {
+		alloc_master_rt = false;
 		goto skip_alloc_master_rt;
+	}
 
 	/*
 	 * If this API is invoked by Slave first then m_rt is not valid.
@@ -1946,35 +1956,37 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	if (!m_rt) {
 		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
-		goto error;
+		goto unlock;
 	}
-	ret =  sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto stream_error;
 
 skip_alloc_master_rt:
 	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
+		alloc_slave_rt = false;
 		ret = -ENOMEM;
-		goto stream_error;
+		goto alloc_error;
 	}
 
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto alloc_error;
+
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret)
+		goto unlock;
+
 	ret = sdw_slave_rt_config(s_rt, stream_config);
 	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
 	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
-	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_slave_port_config(slave, s_rt, port_config);
 	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	/*
 	 * Change stream state to CONFIGURED on first Slave add.
@@ -1983,15 +1995,19 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * change stream state to CONFIGURED.
 	 */
 	stream->state = SDW_STREAM_CONFIGURED;
-	goto error;
+	goto unlock;
 
-stream_error:
+alloc_error:
 	/*
-	 * we hit error so cleanup the stream, release all Slave(s) and
-	 * Master runtime
+	 * we only cleanup what was allocated in this routine. The 'else if'
+	 * is intentional, the 'master_rt_free' will call sdw_slave_rt_free()
+	 * internally.
 	 */
-	sdw_master_rt_free(m_rt, stream);
-error:
+	if (alloc_master_rt)
+		sdw_master_rt_free(m_rt, stream);
+	else if (alloc_slave_rt)
+		sdw_slave_rt_free(slave, stream);
+unlock:
 	mutex_unlock(&slave->bus->bus_lock);
 	return ret;
 }
@@ -2018,4 +2034,3 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 	return 0;
 }
 EXPORT_SYMBOL(sdw_stream_remove_slave);
-
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 16/19] soundwire: stream: separate alloc and config within sdw_stream_add_xxx()
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Separate alloc and config parts so that follow-up patches can allow
for multiple calls to sdw_stream_add_slave/master. This is a feature
from the ALSA/ASoC frameworks which is not supported today.

This is an invasive patch which modifies the error handling flow, with
cleanups only done when an allocation fails. Configuration failures
only return an error code.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 81 ++++++++++++++++++++++----------------
 1 file changed, 48 insertions(+), 33 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 8a76d6605f93..03cfac0129af 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1811,6 +1811,7 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 			  struct sdw_stream_runtime *stream)
 {
 	struct sdw_master_runtime *m_rt;
+	bool alloc_master_rt = true;
 	int ret;
 
 	mutex_lock(&bus->bus_lock);
@@ -1832,8 +1833,10 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	 * it first), if so skip allocation and go to configuration
 	 */
 	m_rt = sdw_master_rt_find(bus, stream);
-	if (m_rt)
+	if (m_rt) {
+		alloc_master_rt = false;
 		goto skip_alloc_master_rt;
+	}
 
 	m_rt = sdw_master_rt_alloc(bus, stream);
 	if (!m_rt) {
@@ -1841,30 +1844,32 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 		ret = -ENOMEM;
 		goto unlock;
 	}
+skip_alloc_master_rt:
+
+	ret = sdw_master_port_alloc(m_rt, num_ports);
+	if (ret)
+		goto alloc_error;
+
+	stream->m_rt_count++;
 
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
 		goto unlock;
 
-skip_alloc_master_rt:
 	ret = sdw_config_stream(bus->dev, stream, stream_config, false);
 	if (ret)
-		goto stream_error;
-
-	ret = sdw_master_port_alloc(m_rt, num_ports);
-	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_master_port_config(m_rt, port_config);
-	if (ret)
-		goto stream_error;
-
-	stream->m_rt_count++;
 
 	goto unlock;
 
-stream_error:
-	sdw_master_rt_free(m_rt, stream);
+alloc_error:
+	/*
+	 * we only cleanup what was allocated in this routine
+	 */
+	if (alloc_master_rt)
+		sdw_master_rt_free(m_rt, stream);
 unlock:
 	mutex_unlock(&bus->bus_lock);
 	return ret;
@@ -1926,6 +1931,9 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 {
 	struct sdw_slave_runtime *s_rt;
 	struct sdw_master_runtime *m_rt;
+	bool alloc_master_rt = true;
+	bool alloc_slave_rt = true;
+
 	int ret;
 
 	mutex_lock(&slave->bus->bus_lock);
@@ -1935,8 +1943,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * and go to configuration
 	 */
 	m_rt = sdw_master_rt_find(slave->bus, stream);
-	if (m_rt)
+	if (m_rt) {
+		alloc_master_rt = false;
 		goto skip_alloc_master_rt;
+	}
 
 	/*
 	 * If this API is invoked by Slave first then m_rt is not valid.
@@ -1946,35 +1956,37 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	if (!m_rt) {
 		dev_err(&slave->dev, "Master runtime alloc failed for stream:%s\n", stream->name);
 		ret = -ENOMEM;
-		goto error;
+		goto unlock;
 	}
-	ret =  sdw_master_rt_config(m_rt, stream_config);
-	if (ret < 0)
-		goto stream_error;
 
 skip_alloc_master_rt:
 	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
+		alloc_slave_rt = false;
 		ret = -ENOMEM;
-		goto stream_error;
+		goto alloc_error;
 	}
 
+	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
+	if (ret)
+		goto alloc_error;
+
+	ret =  sdw_master_rt_config(m_rt, stream_config);
+	if (ret)
+		goto unlock;
+
 	ret = sdw_slave_rt_config(s_rt, stream_config);
 	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
 	if (ret)
-		goto stream_error;
-
-	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
-	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	ret = sdw_slave_port_config(slave, s_rt, port_config);
 	if (ret)
-		goto stream_error;
+		goto unlock;
 
 	/*
 	 * Change stream state to CONFIGURED on first Slave add.
@@ -1983,15 +1995,19 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	 * change stream state to CONFIGURED.
 	 */
 	stream->state = SDW_STREAM_CONFIGURED;
-	goto error;
+	goto unlock;
 
-stream_error:
+alloc_error:
 	/*
-	 * we hit error so cleanup the stream, release all Slave(s) and
-	 * Master runtime
+	 * we only cleanup what was allocated in this routine. The 'else if'
+	 * is intentional, the 'master_rt_free' will call sdw_slave_rt_free()
+	 * internally.
 	 */
-	sdw_master_rt_free(m_rt, stream);
-error:
+	if (alloc_master_rt)
+		sdw_master_rt_free(m_rt, stream);
+	else if (alloc_slave_rt)
+		sdw_slave_rt_free(slave, stream);
+unlock:
 	mutex_unlock(&slave->bus->bus_lock);
 	return ret;
 }
@@ -2018,4 +2034,3 @@ int sdw_stream_remove_slave(struct sdw_slave *slave,
 	return 0;
 }
 EXPORT_SYMBOL(sdw_stream_remove_slave);
-
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 17/19] soundwire: stream: introduce sdw_slave_rt_find() helper
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Before we split the alloc and config steps, we need a helper to find
the Slave runtime for a stream. The helper is based on the search loop
in sdw_slave_rt_free(), which can now be simplified.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 03cfac0129af..a52a9ab0eea1 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1058,6 +1058,23 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 	return 0;
 }
 
+static struct sdw_slave_runtime *sdw_slave_rt_find(struct sdw_slave *slave,
+						   struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt, *_s_rt;
+	struct sdw_master_runtime *m_rt;
+
+	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)
+				return s_rt;
+		}
+	}
+	return NULL;
+}
+
 /**
  * sdw_slave_rt_free() - Free Slave(s) runtime handle
  *
@@ -1069,19 +1086,12 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 static void sdw_slave_rt_free(struct sdw_slave *slave,
 			      struct sdw_stream_runtime *stream)
 {
-	struct sdw_slave_runtime *s_rt, *_s_rt;
-	struct sdw_master_runtime *m_rt;
+	struct sdw_slave_runtime *s_rt;
 
-	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;
-			}
-		}
+	s_rt = sdw_slave_rt_find(slave, stream);
+	if (s_rt) {
+		list_del(&s_rt->m_rt_node);
+		kfree(s_rt);
 	}
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 17/19] soundwire: stream: introduce sdw_slave_rt_find() helper
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

Before we split the alloc and config steps, we need a helper to find
the Slave runtime for a stream. The helper is based on the search loop
in sdw_slave_rt_free(), which can now be simplified.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index 03cfac0129af..a52a9ab0eea1 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1058,6 +1058,23 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 	return 0;
 }
 
+static struct sdw_slave_runtime *sdw_slave_rt_find(struct sdw_slave *slave,
+						   struct sdw_stream_runtime *stream)
+{
+	struct sdw_slave_runtime *s_rt, *_s_rt;
+	struct sdw_master_runtime *m_rt;
+
+	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)
+				return s_rt;
+		}
+	}
+	return NULL;
+}
+
 /**
  * sdw_slave_rt_free() - Free Slave(s) runtime handle
  *
@@ -1069,19 +1086,12 @@ static int sdw_slave_rt_config(struct sdw_slave_runtime *s_rt,
 static void sdw_slave_rt_free(struct sdw_slave *slave,
 			      struct sdw_stream_runtime *stream)
 {
-	struct sdw_slave_runtime *s_rt, *_s_rt;
-	struct sdw_master_runtime *m_rt;
+	struct sdw_slave_runtime *s_rt;
 
-	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;
-			}
-		}
+	s_rt = sdw_slave_rt_find(slave, stream);
+	if (s_rt) {
+		list_del(&s_rt->m_rt_node);
+		kfree(s_rt);
 	}
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 18/19] soundwire: stream: sdw_stream_add_ functions can be called multiple times
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The sdw_stream_add_slave/master() functions are called from the
.hw_params stage. We need to make sure the functions can be called
multiple times.

In this version, we assume that only 'audio' parameters provide in the
hw_params() can change. If the number of ports could change
dynamically depending on the stream configuration (number of channels,
etc), we would need to free-up all the stream resources and reallocate
them.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a52a9ab0eea1..ccf3c99dd579 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -898,6 +898,11 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt)
 	kfree(p_rt);
 }
 
+static bool sdw_slave_port_allocated(struct sdw_slave_runtime *s_rt)
+{
+	return !list_empty(&s_rt->port_list);
+}
+
 static void sdw_slave_port_free(struct sdw_slave *slave,
 				struct sdw_stream_runtime *stream)
 {
@@ -972,6 +977,11 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 	return 0;
 }
 
+static bool sdw_master_port_allocated(struct sdw_master_runtime *m_rt)
+{
+	return !list_empty(&m_rt->port_list);
+}
+
 static void sdw_master_port_free(struct sdw_master_runtime *m_rt)
 {
 	struct sdw_port_runtime *p_rt, *_p_rt;
@@ -1856,12 +1866,17 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	}
 skip_alloc_master_rt:
 
+	if (sdw_master_port_allocated(m_rt))
+		goto skip_alloc_master_port;
+
 	ret = sdw_master_port_alloc(m_rt, num_ports);
 	if (ret)
 		goto alloc_error;
 
 	stream->m_rt_count++;
 
+skip_alloc_master_port:
+
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
 		goto unlock;
@@ -1970,6 +1985,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	}
 
 skip_alloc_master_rt:
+	s_rt = sdw_slave_rt_find(slave, stream);
+	if (s_rt)
+		goto skip_alloc_slave_rt;
+
 	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
@@ -1978,10 +1997,15 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto alloc_error;
 	}
 
+skip_alloc_slave_rt:
+	if (sdw_slave_port_allocated(s_rt))
+		goto skip_port_alloc;
+
 	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
 	if (ret)
 		goto alloc_error;
 
+skip_port_alloc:
 	ret =  sdw_master_rt_config(m_rt, stream_config);
 	if (ret)
 		goto unlock;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 18/19] soundwire: stream: sdw_stream_add_ functions can be called multiple times
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The sdw_stream_add_slave/master() functions are called from the
.hw_params stage. We need to make sure the functions can be called
multiple times.

In this version, we assume that only 'audio' parameters provide in the
hw_params() can change. If the number of ports could change
dynamically depending on the stream configuration (number of channels,
etc), we would need to free-up all the stream resources and reallocate
them.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index a52a9ab0eea1..ccf3c99dd579 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -898,6 +898,11 @@ static void sdw_port_free(struct sdw_port_runtime *p_rt)
 	kfree(p_rt);
 }
 
+static bool sdw_slave_port_allocated(struct sdw_slave_runtime *s_rt)
+{
+	return !list_empty(&s_rt->port_list);
+}
+
 static void sdw_slave_port_free(struct sdw_slave *slave,
 				struct sdw_stream_runtime *stream)
 {
@@ -972,6 +977,11 @@ static int sdw_slave_port_config(struct sdw_slave *slave,
 	return 0;
 }
 
+static bool sdw_master_port_allocated(struct sdw_master_runtime *m_rt)
+{
+	return !list_empty(&m_rt->port_list);
+}
+
 static void sdw_master_port_free(struct sdw_master_runtime *m_rt)
 {
 	struct sdw_port_runtime *p_rt, *_p_rt;
@@ -1856,12 +1866,17 @@ int sdw_stream_add_master(struct sdw_bus *bus,
 	}
 skip_alloc_master_rt:
 
+	if (sdw_master_port_allocated(m_rt))
+		goto skip_alloc_master_port;
+
 	ret = sdw_master_port_alloc(m_rt, num_ports);
 	if (ret)
 		goto alloc_error;
 
 	stream->m_rt_count++;
 
+skip_alloc_master_port:
+
 	ret = sdw_master_rt_config(m_rt, stream_config);
 	if (ret < 0)
 		goto unlock;
@@ -1970,6 +1985,10 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 	}
 
 skip_alloc_master_rt:
+	s_rt = sdw_slave_rt_find(slave, stream);
+	if (s_rt)
+		goto skip_alloc_slave_rt;
+
 	s_rt = sdw_slave_rt_alloc(slave, m_rt);
 	if (!s_rt) {
 		dev_err(&slave->dev, "Slave runtime alloc failed for stream:%s\n", stream->name);
@@ -1978,10 +1997,15 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
 		goto alloc_error;
 	}
 
+skip_alloc_slave_rt:
+	if (sdw_slave_port_allocated(s_rt))
+		goto skip_port_alloc;
+
 	ret = sdw_slave_port_alloc(slave, s_rt, num_ports);
 	if (ret)
 		goto alloc_error;
 
+skip_port_alloc:
 	ret =  sdw_master_rt_config(m_rt, stream_config);
 	if (ret)
 		goto unlock;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 19/19] soundwire: stream: make enable/disable/deprepare idempotent
  2022-01-26  1:16 ` Bard Liao
@ 2022-01-26  1:17   ` Bard Liao
  -1 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The stream management currently flags an 'inconsistent state' error
when a change is requested multiple times. This was added on purpose
to identify programming mistakes.

In hindsight, there was no real reason to fail if the logic at the
ASoC-DPCM level invokes the same callback multiple times. It's
perfectly acceptable to just return and not flag an error when there
is nothing to do. The main concern with the state management is to
trap errors such as trying to enable a stream that was not prepared
first.

This patch suggests allowing the stream functions to be idempotent,
i.e. they can be called multiple times.

Note that the prepare case was already handling multiple calls, this
was added in commit c32464c9393d ("soundwire: stream: only prepare
stream when it is configured.")

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index ccf3c99dd579..f273459b2023 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1505,6 +1505,11 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_ENABLED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_PREPARED &&
 	    stream->state != SDW_STREAM_DISABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
@@ -1588,6 +1593,11 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_DISABLED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_ENABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
 		       __func__, stream->name, stream->state);
@@ -1663,6 +1673,11 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_DEPREPARED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_PREPARED &&
 	    stream->state != SDW_STREAM_DISABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* [PATCH 19/19] soundwire: stream: make enable/disable/deprepare idempotent
@ 2022-01-26  1:17   ` Bard Liao
  0 siblings, 0 replies; 42+ messages in thread
From: Bard Liao @ 2022-01-26  1:17 UTC (permalink / raw)
  To: alsa-devel, vkoul
  Cc: vinod.koul, gregkh, pierre-louis.bossart, linux-kernel,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

The stream management currently flags an 'inconsistent state' error
when a change is requested multiple times. This was added on purpose
to identify programming mistakes.

In hindsight, there was no real reason to fail if the logic at the
ASoC-DPCM level invokes the same callback multiple times. It's
perfectly acceptable to just return and not flag an error when there
is nothing to do. The main concern with the state management is to
trap errors such as trying to enable a stream that was not prepared
first.

This patch suggests allowing the stream functions to be idempotent,
i.e. they can be called multiple times.

Note that the prepare case was already handling multiple calls, this
was added in commit c32464c9393d ("soundwire: stream: only prepare
stream when it is configured.")

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
---
 drivers/soundwire/stream.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index ccf3c99dd579..f273459b2023 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1505,6 +1505,11 @@ int sdw_enable_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_ENABLED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_PREPARED &&
 	    stream->state != SDW_STREAM_DISABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
@@ -1588,6 +1593,11 @@ int sdw_disable_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_DISABLED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_ENABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
 		       __func__, stream->name, stream->state);
@@ -1663,6 +1673,11 @@ int sdw_deprepare_stream(struct sdw_stream_runtime *stream)
 
 	sdw_acquire_bus_lock(stream);
 
+	if (stream->state == SDW_STREAM_DEPREPARED) {
+		ret = 0;
+		goto state_err;
+	}
+
 	if (stream->state != SDW_STREAM_PREPARED &&
 	    stream->state != SDW_STREAM_DISABLED) {
 		pr_err("%s: %s: inconsistent state state %d\n",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 42+ messages in thread

* Re: [PATCH 00/19] soundwire: stream: cleanup of 'stream' support
  2022-01-26  1:16 ` Bard Liao
@ 2022-02-11  6:48   ` Vinod Koul
  -1 siblings, 0 replies; 42+ messages in thread
From: Vinod Koul @ 2022-02-11  6:48 UTC (permalink / raw)
  To: Bard Liao
  Cc: alsa-devel, linux-kernel, gregkh, srinivas.kandagatla,
	pierre-louis.bossart, sanyog.r.kale, bard.liao

On 26-01-22, 09:16, Bard Liao wrote:
> This series revisits the SoundWire 'sdw_stream' support to split allocation
> and configuration steps. This is necessary if for example the routines are
> called multiple times from the hw_params stage. This also helps with better
> error handling.

Patch 13 added a trailing empty line to file, I have fixed that up while
applying...

-- 
~Vinod

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 00/19] soundwire: stream: cleanup of 'stream' support
@ 2022-02-11  6:48   ` Vinod Koul
  0 siblings, 0 replies; 42+ messages in thread
From: Vinod Koul @ 2022-02-11  6:48 UTC (permalink / raw)
  To: Bard Liao
  Cc: alsa-devel, gregkh, linux-kernel, pierre-louis.bossart,
	srinivas.kandagatla, sanyog.r.kale, bard.liao

On 26-01-22, 09:16, Bard Liao wrote:
> This series revisits the SoundWire 'sdw_stream' support to split allocation
> and configuration steps. This is necessary if for example the routines are
> called multiple times from the hw_params stage. This also helps with better
> error handling.

Patch 13 added a trailing empty line to file, I have fixed that up while
applying...

-- 
~Vinod

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2022-02-11  6:49 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26  1:16 [PATCH 00/19] soundwire: stream: cleanup of 'stream' support Bard Liao
2022-01-26  1:16 ` Bard Liao
2022-01-26  1:16 ` [PATCH 01/19] soundwire: stream: remove unused parameter in sdw_stream_add_slave Bard Liao
2022-01-26  1:16   ` Bard Liao
2022-01-26  1:16 ` [PATCH 02/19] soundwire: stream: add slave runtime to list earlier Bard Liao
2022-01-26  1:16   ` Bard Liao
2022-01-26  1:16 ` [PATCH 03/19] soundwire: stream: simplify check on port range Bard Liao
2022-01-26  1:16   ` Bard Liao
2022-01-26  1:17 ` [PATCH 04/19] soundwire: stream: add alloc/config/free helpers for ports Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 05/19] soundwire: stream: split port allocation and configuration loops Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 06/19] soundwire: stream: split alloc and config in two functions Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 07/19] soundwire: stream: add 'slave' prefix for port range checks Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 08/19] soundwire: stream: group sdw_port and sdw_master/slave_port functions Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 09/19] soundwire: stream: simplify sdw_alloc_master_rt() Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 10/19] soundwire: stream: split sdw_alloc_master_rt() in alloc and config Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 11/19] soundwire: stream: move sdw_alloc_slave_rt() before 'master' helpers Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 12/19] soundwire: stream: split sdw_alloc_slave_rt() in alloc and config Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 13/19] soundwire: stream: group sdw_stream_ functions Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 14/19] soundwire: stream: rename and move master/slave_rt_free routines Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 15/19] soundwire: stream: move list addition to sdw_slave_alloc_rt() Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 16/19] soundwire: stream: separate alloc and config within sdw_stream_add_xxx() Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 17/19] soundwire: stream: introduce sdw_slave_rt_find() helper Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 18/19] soundwire: stream: sdw_stream_add_ functions can be called multiple times Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-01-26  1:17 ` [PATCH 19/19] soundwire: stream: make enable/disable/deprepare idempotent Bard Liao
2022-01-26  1:17   ` Bard Liao
2022-02-11  6:48 ` [PATCH 00/19] soundwire: stream: cleanup of 'stream' support Vinod Koul
2022-02-11  6:48   ` Vinod Koul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.