[PATCH 00/19] soundwire: stream: cleanup of 'stream' support
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(-)
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",
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;
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;
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;
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;
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;
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;
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 *
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,
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);
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 *
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;
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); +
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);
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)
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); -
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); } }
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;
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",
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...
participants (2)
-
Bard Liao
-
Vinod Koul