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
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
