Claude review: misc: fastrpc: Add cache maintenance for non-coherent platforms

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

Patch Review

**BUG (Critical): Direction mismatch in `fastrpc_inv_args()`**

```c
if (ctx->olaps[i].mstart) {
    dma_buf_begin_cpu_access(map->buf, DMA_FROM_DEVICE);
    dma_buf_end_cpu_access(map->buf, DMA_TO_DEVICE);
}
```

The `begin_cpu_access` uses `DMA_FROM_DEVICE` (invalidate — correct for output buffers being read by CPU), but `end_cpu_access` uses `DMA_TO_DEVICE` instead of `DMA_FROM_DEVICE`. The direction passed to `end_cpu_access` must match the direction used in `begin_cpu_access` — these are paired calls bracketing a CPU access with a specific direction. Using `DMA_TO_DEVICE` in the end call means the exporter receives a mismatched direction, which can result in the wrong cache operation (a clean instead of an invalidate) or no operation at all, depending on the exporter implementation. This should be:

```c
dma_buf_begin_cpu_access(map->buf, DMA_FROM_DEVICE);
dma_buf_end_cpu_access(map->buf, DMA_FROM_DEVICE);
```

**Issue (Medium): Questionable cache maintenance pattern**

Both `fastrpc_flush_args()` and `fastrpc_inv_args()` immediately pair `begin_cpu_access` + `end_cpu_access` with no actual CPU access in between:

```c
dma_buf_begin_cpu_access(map->buf, DMA_TO_DEVICE);
dma_buf_end_cpu_access(map->buf, DMA_TO_DEVICE);
```

The dma-buf API documentation states: *"Only when cpu access is bracketed by both calls is it guaranteed to be coherent with other DMA access."* The intended usage is: `begin` → perform CPU reads/writes → `end`. Here the CPU access (the copy_from_user into the buffer) has *already happened* before `fastrpc_flush_args()` is called, so the bracket is empty. While this may work on some exporters as a side-effect (the `end_cpu_access` callback might do a cache clean), it's relying on implementation details rather than the documented contract. The correct approach would be to call `dma_buf_begin_cpu_access` *before* the CPU writes to the buffer, and `dma_buf_end_cpu_access` after. Similarly for the invalidation path — `begin_cpu_access(DMA_FROM_DEVICE)` should be called before `fastrpc_put_args` reads the output data, and `end_cpu_access` after.

**Issue (Medium): Return values of dma_buf_{begin,end}_cpu_access are ignored**

Both `dma_buf_begin_cpu_access()` and `dma_buf_end_cpu_access()` return `int` error codes. Failures are silently ignored:

```c
dma_buf_begin_cpu_access(map->buf, DMA_TO_DEVICE);
dma_buf_end_cpu_access(map->buf, DMA_TO_DEVICE);
```

At minimum, `begin_cpu_access` failures should be checked and, if failing, the `end_cpu_access` call skipped (and preferably the error propagated upward, since `fastrpc_get_args` already returns int).

**Issue (Minor): `uintptr_t` cast on kernel virtual address in `fastrpc_inv_args`**

```c
if (((uintptr_t)rpra & PAGE_MASK) ==
    ((uintptr_t)rpra[raix].buf.pv & PAGE_MASK))
```

`rpra` is a kernel pointer, `rpra[raix].buf.pv` is a `u64` from the remote arg union. Casting a kernel pointer to `uintptr_t` and masking it works but is a bit fragile — would be clearer to compare against the buffer's known DMA address or page frame, and it's worth a comment explaining that `buf.pv` here holds a kernel virtual address within the same allocation as `rpra`.

**Observation: `coherent` field not propagated to duplicate sessions**

The `coherent` field is set per session in `fastrpc_cb_probe()`:

```c
sess->coherent = of_property_read_bool(dev->of_node, "dma-coherent");
```

The duplicate sessions created in the loop below are copied via `memcpy(dup_sess, sess, sizeof(*dup_sess))`, so they *do* inherit the `coherent` flag. This is correct.

**Nit: Filtering logic in `fastrpc_flush_args` could be simplified**

```c
if (raix + 1 > inbufs)
    continue;
```

This is equivalent to `raix >= inbufs` which would be more idiomatic. Similarly in `fastrpc_inv_args`: `raix + 1 <= inbufs` is `raix < inbufs`. While functionally correct, the `+1` pattern is slightly confusing.

**Summary of required changes:**
1. Fix `dma_buf_end_cpu_access` direction to `DMA_FROM_DEVICE` in `fastrpc_inv_args()` — this is a correctness bug.
2. Check return values of `dma_buf_begin_cpu_access()`.
3. Reconsider the placement of begin/end brackets to properly surround the actual CPU access rather than being empty pairs.

---
Generated by Claude Code Patch Reviewer