On 2/1/21 9:50 AM, Srinivas Kandagatla wrote:
On 29/01/2021 19:33, Pierre-Louis Bossart wrote:
On 1/29/21 11:32 AM, Srinivas Kandagatla wrote:
In the existing code every soundwire register read and register write are kinda blocked. Each of these are using a special command id that
what does 'kinda blocked' mean?
I meant read/writes are waiting for completion interrupt!
generates interrupt after it successfully finishes. This is really overhead, limiting and not really necessary unless we are doing something special.
We can simply read/write the fifo that should also give exactly what we need! This will also allow to read/write registers in interrupt context, which was not possible with the special command approach.
This is really unclear, sorry.
If read/writes are waiting for an interrupt, it becomes difficult to read or write to any registers from same interrupt handler!
Well, yes, you need to handle the complete() at a lower level than the code that initiates the transactions otherwise you self-deadlock.
IIRC in the Intel initial code, the complete was in the handler and the register IOs in the thread.
+ if (id != SWR_BROADCAST_CMD_ID) { + if (id < 14) + id += 1; + else + id = 0;
that is really odd. if id=13 (group2) then id becomes 14 (master address). A comment is really needed here.
This is magic value for each fifo read or write, so that we can verify that them by comparing with this magic value!
This has nothing to do with device number!
You should probably add a comment here then, or use a #define instead of the 14 which threw me off.
+ if (cmd_id == SWR_BROADCAST_CMD_ID) { + /* + * sleep for 10ms for MSM soundwire variant to allow broadcast + * command to complete.
that's also super-odd. There is nothing in SoundWire that makes any difference between a regular and a broadcast command. they all complete in the same time (a frame).
+ */ + ret = wait_for_completion_timeout(&swrm->broadcast, (2 * HZ/10));
is this 10ms really or dependent on CONFIG_HZ?
comment missed?
+ if (!ret) + ret = SDW_CMD_IGNORED; + else + ret = SDW_CMD_OK;
no CMD_FAILED support?
Qcom controllers does not provide that information if the command is ignored or failed by any means!
That was the behavior from the starting of this driver.
ah yes, now I remember this.