[alsa-devel] [PATCH 2/3] ASoC: SOF: imx: Add i.MX8 HW support

[Cezary Rojewski]

On 2019-08-19 17:21, Pierre-Louis Bossart wrote:
> On 8/17/19 10:21 AM, Cezary Rojewski wrote:
>> On 2019-08-15 17:44, Pierre-Louis Bossart wrote:
>>> From: Daniel Baluta <daniel.baluta at nxp.com>
>>>
>>> Add support for the audio DSP hardware found on NXP i.MX8 platform.
>>>
>>> Signed-off-by: Daniel Baluta <daniel.baluta at nxp.com>
>>> Signed-off-by: Pierre-Louis Bossart 
>>> <pierre-louis.bossart at linux.intel.com>
>>> +static void imx8_get_reply(struct snd_sof_dev *sdev)
>>> +{
>>> +    struct snd_sof_ipc_msg *msg = sdev->msg;
>>> +    struct sof_ipc_reply reply;
>>> +    int ret = 0;
>>> +
>>> +    if (!msg) {
>>> +        dev_warn(sdev->dev, "unexpected ipc interrupt\n");
>>> +        return;
>>> +    }
>>> +
>>> +    /* get reply */
>>> +    sof_mailbox_read(sdev, sdev->host_box.offset, &reply, 
>>> sizeof(reply));
>>> +
>>> +    if (reply.error < 0) {
>>> +        memcpy(msg->reply_data, &reply, sizeof(reply));
>>> +        ret = reply.error;
>>> +    } else {
>>> +        /* reply has correct size? */
>>> +        if (reply.hdr.size != msg->reply_size) {
>>> +            dev_err(sdev->dev, "error: reply expected %zu got %u 
>>> bytes\n",
>>> +                msg->reply_size, reply.hdr.size);
>>> +            ret = -EINVAL;
>>> +        }
>>> +
>>> +        /* read the message */
>>> +        if (msg->reply_size > 0)
>>> +            sof_mailbox_read(sdev, sdev->host_box.offset,
>>> +                     msg->reply_data, msg->reply_size);
>>> +    }
>>> +
>>> +    msg->reply_error = ret;
>>> +}
>>
>> Assuming reply.hdr.size is coming from HW IPC regs, msg object is 
>> representing application side - SW, kernel. If so, is msg->reply_size 
>> value an estimated size (which can be overestimated since exact size 
>> may be unknown by the host) -or- the exact size of incoming IPC reply?
>>
>> The estimated-case is usually permissive as long as assumed size is >= 
>> reply.hdr.size - dev_err needed. In the exact-case, it should be 
>> viewed as a requirement. If such "requirement" fails, is it valid to 
>> read mailbox regardless? Is this to extract some error-debug payload 
>> sent by FW?
>>
>> Just curious, please feel free to correct me here, Pierre.
> 
> I don't quite understand the question. There is no use of HW IPC 
> registers (as done in SKL) since this is not portable across hardware. 
> There *may* be information sent over IPC registers but that would have 
> to be done in platform-specific ways - as will be done for Intel to e.g. 
> avoid using memory windows that may not be powered.

TLDR:
Was wondering about sof_mailbox_read being invoked regardless of 
size-check outcome (dev_err reports size mismatch after all).

Given your answer, guess the error-payload may still be sent back so 
it's preferable to read the uplink despite the unexpected circumstances 
- size mismatch.