On Wed, Sep 23, 2020 at 02:24:56PM +0200, Cezary Rojewski wrote:
> Implement IRQ handlers for immediate and delayed replies and
> notifications. Communication is synchronous and allows for serialization
> of maximum one message at a time.
>
> DSP may respond with ADSP_PENDING status for a request - known as
> delayed reply - and when situation occurs, framework keeps the lock and
> awaits upcoming response through IPCD channel which is handled in
> bottom-half. Immediate replies spawn no BH at all as their processing is
> very short.
...
> +static void catpt_dsp_send_tx(struct catpt_dev *cdev,
> + const struct catpt_ipc_msg *tx)
> +{
> + u32 header = tx->header | CATPT_IPCC_BUSY;
> + if (tx->size)
This check is not needed.
> + memcpy_toio(catpt_outbox_addr(cdev), tx->data, tx->size);
> + catpt_writel_shim(cdev, IPCC, header);
> +}
...
> + ret = ipc->rx.rsp.status;
> + if (reply) {
> + reply->header = ipc->rx.header;
So, even in case of error the header is still updated?
> + if (!ret && reply->data && reply->size)
> + memcpy(reply->data, ipc->rx.data, ipc->rx.size);
This I didn't get. You copy data by using source size?!
> + }
> +
> + return ret;
I guess the above piece may be refactored, but I don't know how until it is
clear why it's written like this.
...
> +static void catpt_dsp_copy_rx(struct catpt_dev *cdev, u32 header)
> +{
> + struct catpt_ipc *ipc = &cdev->ipc;
> +
> + ipc->rx.header = header;
> + if (ipc->rx.size && ipc->rx.rsp.status == CATPT_REPLY_SUCCESS) {
Check for size is redundant.
> + memcpy_fromio(ipc->rx.data, catpt_outbox_addr(cdev),
> + ipc->rx.size);
> + }
Can be done like
if (status != SUCCESS)
return;
memcpy_fromio(...);
> +}
--
With Best Regards,
Andy Shevchenko
