Claude review: misc: fastrpc: Add polling mode support for fastRPC driver

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Status: Needs attention on a few points.**

**Issue 1 (medium): Race between polling and glink callback on `is_work_done`**

The `is_work_done` flag is read in `poll_for_remote_response()` (from the invoking thread context) and written in `fastrpc_rpmsg_callback()` (from the rpmsg callback context) without any synchronization:

```c
// In poll_for_remote_response():
ret = read_poll_timeout_atomic(fastrpc_read_poll_addr, val,
                               (val == FASTRPC_POLL_RESPONSE) || ctx->is_work_done, ...);
```

```c
// In fastrpc_rpmsg_callback():
ctx->retval = rsp->retval;
ctx->is_work_done = true;
complete(&ctx->work);
```

While `is_work_done` is a `bool` and the race is "benign" in the sense that both paths converge to the same outcome (work is done), the `ctx->retval` assignment in the callback could race with the polling path reading `ctx->retval = 0`. If the callback sets `is_work_done = true` and `ctx->retval` to an error, but the polling path already saw `is_work_done = true` and set `ctx->retval = 0`, the actual error return value from DSP would be lost. Consider using `WRITE_ONCE`/`READ_ONCE` for `is_work_done` at minimum, and ordering the `retval` write before the flag write with a barrier.

**Issue 2 (minor): `read_poll_timeout_atomic` holds preemption disabled for up to 10ms**

```c
#define FASTRPC_POLL_MAX_TIMEOUT_US (10000)
```

`read_poll_timeout_atomic` uses `udelay` internally, meaning it runs with preemption disabled for up to 10ms. This is at the upper edge of what's acceptable for atomic context. This is a design tradeoff (the whole point is avoiding scheduling latency), but it's worth noting in the commit message or comments that this is intentional and the timeout is bounded.

**Issue 3 (minor): `-EIO` mapping for timeout is unusual**

```c
if (ret == -ETIMEDOUT)
    ret = -EIO;
```

When the poll times out, the code remaps `-ETIMEDOUT` to `-EIO` but then falls through to `fastrpc_wait_for_response()` which will wait for the glink interrupt normally. The error code from `poll_for_remote_response()` is never actually used since `fastrpc_wait_for_completion()` returns the result of `fastrpc_wait_for_response()` on poll failure:

```c
if (ctx->is_polled) {
    err = poll_for_remote_response(ctx);
    if (!err)
        return 0;
    ctx->is_polled = false;
}
return fastrpc_wait_for_response(ctx, kernel);
```

The `-EIO` mapping is dead code — `err` is overwritten by `fastrpc_wait_for_response()`. The `ret` variable and its mapping can be simplified, or just removed entirely since any non-zero return from `poll_for_remote_response()` simply triggers the fallback.

**Issue 4 (minor): `__maybe_unused` on `fastrpc_poll_supported_machines`**

```c
static const struct of_device_id fastrpc_poll_supported_machines[] __maybe_unused = {
```

This `__maybe_unused` is there to suppress warnings when `CONFIG_OF` is disabled (since `of_machine_get_match()` returns `NULL` as a static inline and the array would be unreferenced). This is correct but worth confirming that the whole polling feature should be available even without `CONFIG_OF` — in that case `poll_mode_supported` for the exception-list platforms would silently be false, which is fine.

**Issue 5 (nit): UAPI extensibility of `fastrpc_ioctl_set_option`**

The new UAPI struct:
```c
struct fastrpc_ioctl_set_option {
    __u32 request_id;
    __u32 value;
    __s32 reserved[6];
};
```

The ioctl is defined as `_IOWR` but nothing is written back to userspace. `_IOW` would be more appropriate unless there's a plan to return data in the reserved fields.

**Issue 6 (nit): `dma_rmb()` placement in polling path**

```c
static u32 fastrpc_read_poll_addr(struct fastrpc_invoke_ctx *ctx)
{
    dma_rmb();
    return READ_ONCE(*ctx->poll_addr);
}
```

The `dma_rmb()` is placed *before* the read, which ensures previous DMA reads are ordered before this one. However, the more typical pattern is to place the barrier *after* reading the flag and *before* reading the data that depends on it. Here, when the poll succeeds, the caller in `fastrpc_internal_invoke()` proceeds to `fastrpc_put_args()` which reads output buffers — the existing `dma_rmb()` at line 1241 in the original code handles that. But note that in the polling path, this `dma_rmb()` was removed from `fastrpc_internal_invoke()` (it's still there in the original but the patch replaces the wait code). Let me check... Actually the `dma_rmb()` at the original line 1241 comes *after* the wait and *before* `fastrpc_put_args()`, and it's not touched by this patch, so it's still present. The barrier in `fastrpc_read_poll_addr()` ensures each poll iteration sees fresh DMA data, which is correct.

Overall, this is a solid v12 series. The main concern is the lack of ordering guarantees around `is_work_done`/`retval` between the polling thread and the rpmsg callback, and the dead `-EIO` mapping code. The rest is in good shape.

---
Generated by Claude Code Patch Reviewer