Re: [alsa-devel] [PATCH v4 07/13] soundwire: Add stream configuration APIs

+static int _sdw_prepare_stream(struct sdw_stream_runtime *stream) +{

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;
• struct sdw_master_prop *prop = NULL;
• struct sdw_bus_params params;
• int ret;
• prop = &bus->prop;
• memcpy(&params, &bus->params, sizeof(params));
• /* TODO: Support Asynchronous mode */
• if ((prop->max_freq % stream->params.rate) != 0) {

• dev_err(bus->dev, "Async mode not supported");
  

• return -EINVAL;
  

• }
• /* Increment cumulative bus bandwidth */
• bus->params.bandwidth += m_rt->stream->params.rate *

• m_rt->ch_count * m_rt->stream->params.bps;
  

This also does not work for device-to-device communication, see e.g. the earlier documentation.

ch_count: Number of channels handled by the Master for + * this stream, can be zero.

should it be m_rt->stream->params->ch_count?

+static int _sdw_deprepare_stream(struct sdw_stream_runtime *stream) +{

• struct sdw_master_runtime *m_rt = stream->m_rt;
• struct sdw_bus *bus = m_rt->bus;
• int ret = 0;
• /* De-prepare port(s) */
• ret = sdw_prep_deprep_ports(m_rt, false);
• if (ret < 0) {

• dev_err(bus->dev, "De-prepare port(s) failed: %d", ret);
  

• return ret;
  

• }
• bus->params.bandwidth -= m_rt->stream->params.rate *

• m_rt->ch_count * m_rt->stream->params.bps;
  

And same here, the ch_count can be zero.

• if (!bus->params.bandwidth) {

• bus->params.row = 0;
  

• bus->params.col = 0;
  

• goto exit;
  

What is the intent with this test+goto? Shouldn't you program the parameters if you want to change the frame shape?

• }
• /* Program params */
• ret = sdw_program_params(bus);
• if (ret < 0) {

• dev_err(bus->dev, "Program params failed: %d", ret);
  

• return ret;
  

• }
• return do_bank_switch(stream);

+exit:

• stream->state = SDW_STREAM_DEPREPARED;
• return ret;

+}