On 1/16/23 01:53, Mukunda,Vijendar wrote:
On 14/01/23 00:11, Pierre-Louis Bossart wrote:
- for (index = 0; index < 2; index++) {
if (response_buf[index] == -ETIMEDOUT) {
dev_err(ctrl->dev, "Program SCP cmd timeout\n");
timeout = 1;
} else if (!(response_buf[index] & AMD_SDW_MCP_RESP_ACK)) {
no_ack = 1;
if (response_buf[index] & AMD_SDW_MCP_RESP_NACK) {
nack = 1;
dev_err(ctrl->dev, "Program SCP NACK received\n");
}
this is a copy of the cadence_master.c code... With the error added that this is not for a controller but for a master...
Its manager instance only. Our immediate command and response mechanism allows sending commands over the link and get the response for every command immediately, unlike as mentioned in candence_master.c.
I don't get the reply. The Cadence IP also has the ability to get the response immediately. There's limited scope for creativity, the commands are defined in the spec and the responses as well.
As per our understanding in Intel code, responses are processed after sending all commands. In our case, we send the command and process the response immediately before invoking the next command.
The Cadence IP can queue a number of commands, I think 8 off the top of my head. But the response is provided immediately after each command.
Maybe the disconnect is that there's an ability to define a watermark on the response buffer, so that the software can decide to process the command responses in one shot.
}
- }
- if (timeout) {
dev_err_ratelimited(ctrl->dev,
"SCP_addrpage command timeout for Slave %d\n", msg->dev_num);
return SDW_CMD_TIMEOUT;
- }
- if (nack) {
dev_err_ratelimited(ctrl->dev,
"SCP_addrpage NACKed for Slave %d\n", msg->dev_num);
return SDW_CMD_FAIL;
- }
- if (no_ack) {
dev_dbg_ratelimited(ctrl->dev,
"SCP_addrpage ignored for Slave %d\n", msg->dev_num);
return SDW_CMD_IGNORED;
- }
- return SDW_CMD_OK;
this should probably become a helper since the response is really the same as in cadence_master.c
There's really room for optimization and reuse here.
not really needed. Please refer above comment as command/response mechanism differs from Intel's implementation.
how? there's a buffer of responses in both cases. please clarify.
Ours implementation is not interrupt driven like Intel. When we send command over the link, we will wait for command's response in polling method and process the response immediately before issuing the new command.
On the Intel side we use an interrupt to avoid polling, and in case of N commands the watermark will be set to N to reduce the overhead. That said, most users only use 1 command at a time, it's only recently that Cirrus Logic experimented with multiple commands to speed-up transfers.
Even if there are differences in the way the responses are processed, whether one-at-a-time or in a batch, the point remains that each command response can be individually analyzed and that could be using a helper - moving code from cadence_master.c into the bus layer.