On 18-06-18, 15:20, Shreyas NC wrote:
On Thu, Jun 14, 2018 at 01:34:13PM -0500, Pierre-Louis Bossart wrote:
diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c index 0e8a2eb5..771c963 100644 --- a/drivers/soundwire/stream.c +++ b/drivers/soundwire/stream.c @@ -638,6 +638,8 @@ static int sdw_bank_switch(struct sdw_bus *bus) if (!wr_msg) return -ENOMEM;
- bus->defer_msg.msg = wr_msg;
- wbuf = kzalloc(sizeof(*wbuf), GFP_KERNEL); if (!wbuf) { ret = -ENOMEM;
@@ -658,17 +660,23 @@ static int sdw_bank_switch(struct sdw_bus *bus) SDW_MSG_FLAG_WRITE, wbuf); wr_msg->ssp_sync = true;
- ret = sdw_transfer(bus, wr_msg);
- if (bus->multi_link)
ret = sdw_transfer_defer(bus, wr_msg, &bus->defer_msg);- else
ret = sdw_transfer(bus, wr_msg);Re-reading this v3, I wonder if there is a small logic flaw here or an intended behavior.
the bus->multi_link field is a hardware capability, it provides information on whether the hardware can support synchronized bank switches across multiple Master interfaces. This is independent of the streams being handled, it's a bus property - not a stream one.
Pierre,
You are right that multi-link is a hw capability BUT it is also a property of a stream. I can envision machines where bus may have the capability but the system configuration may have links being not clubbed in one board and made a multi-link stream in another board :)
However the default/typical case is a stream handled by a single master, so when the hardware is capable of handling multiple masters this does not mean you should use the deferred transfer, the regular case would work just fine for a regular stream. If your SSP rate is faster than the common sync signal it might even be beneficial not to use the deferred transfer.
in other words, should the code be written with something like if (bus->multi_link && master_rt_count > 1) with master_rt_count being determined based on the stream properties.
Yes that makes sense to me
Or was the intent that you always rely on the deferred mechanism, even if the stream is not handled by multiple masters? In that case, do we even need the regular bank switch?
After discussing with folks here, I understand that intent was to have an unified flow for both Multi and Single instance cases. Yes, we can always use the sdw_transfer_defer() and remove the references to bus->multi_link possibly. So, I will fix this up and post v4.
Shreyas,
That sounds like a good plan to me