On Mon, Apr 16, 2018 at 06:15:11PM -0500, Pierre-Louis Bossart wrote:
+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_slave_runtime *s_rt = NULL;
- s_rt = kzalloc(sizeof(*s_rt), GFP_KERNEL);
- if (!s_rt)
return NULL;remove extra line
ok
+static void sdw_release_slave_stream(struct sdw_slave *slave,
struct sdw_stream_runtime *stream)+{
- struct sdw_slave_runtime *s_rt, *_s_rt;
- struct sdw_master_runtime *m_rt = stream->m_rt;
- /* Retrieve Slave runtime handle */
- list_for_each_entry_safe(s_rt, _s_rt,
&m_rt->slave_rt_list, m_rt_node) {if (s_rt->slave == slave) {list_del(&s_rt->m_rt_node);kfree(s_rt);return;}- }
+}
add kernel doc style for sdw_release_master_stream(), with same note that it's called with bus_lock held?
done
+static void sdw_release_master_stream(struct sdw_stream_runtime *stream) +{
- struct sdw_master_runtime *m_rt = stream->m_rt;
- struct sdw_slave_runtime *s_rt, *_s_rt;
- list_for_each_entry_safe(s_rt, _s_rt,
&m_rt->slave_rt_list, m_rt_node)sdw_release_slave_stream(s_rt->slave, stream);So if the release_master_stream is called first it'll call sdw_release_slave_stream(), so sdw_stream_remove_slave() will in effect do nothing?
I guess it's the dual of what happens for allocation - where the master_rt might be allocated by the first slave_add but might be worth a comment or two so that people understand the intent and don't believe it's a design issue.
Yes makes sense, added now
+static int sdw_config_stream(struct device *dev,
struct sdw_stream_runtime *stream,struct sdw_stream_config *stream_config, bool is_slave)+{
- /*
* Update the stream rate, channel and bps based on data* source. For more than one data source (multilink),* match the rate, bps, stream type and increment number of channels.*/- if ((stream->params.rate) &&
(stream->params.rate != stream_config->frame_rate)) {dev_err(dev, "rate not matching, stream:%s", stream->name);return -EINVAL;- }
- if ((stream->params.bps) &&
(stream->params.bps != stream_config->bps)) {dev_err(dev, "bps not matching, stream:%s", stream->name);return -EINVAL;- }
- stream->type = stream_config->type;
- stream->params.rate = stream_config->frame_rate;
- stream->params.bps = stream_config->bps;
- if (is_slave)
stream->params.ch_count += stream_config->ch_count;I just realized this will not work for device-to-device communication. You have to count all TX ports or all RX ports, not the ports for Slaves.
Correct, will update the check when we add support for device-to-device communication