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

[Sanyog Kale <sanyog.r.kale@intel.com>]

On Sat, Apr 21, 2018 at 09:43:15PM +0530, Vinod Koul wrote:
> On Sat, Apr 21, 2018 at 06:56:26AM -0700, Pierre-Louis Bossart wrote:
> > 
> > >+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.
> 
> Same as above..
> 
> > 
> > >+
> > >+	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?
> 
> hmmm that seems correct point to me, but then we are going idle and should
> ideally power down. Let me check again if that is the reason.
>

We do not want to program parameters or change frame shape in case of bandwidth
required is 0 on bus (ie. no stream running). The row and col values are reset
to 0 and will be re-computed again in prepare_stream of next stream based
on stream requirement.

> > 
> > >+	}
> > >+
> > >+	/* 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;
> > >+}
> 
> -- 
> ~Vinod

--