Claude review: dma-buf: heaps: Add Coherent heap to dmabuf heaps

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

Patch Review

This is the main heap implementation.

**Issue: `begin_cpu_access` / `end_cpu_access` syncing for coherent memory**

```c
static int coherent_heap_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
						  enum dma_data_direction direction)
{
	...
	if (buffer->vmap_cnt)
		invalidate_kernel_vmap_range(buffer->vaddr, buffer->len);

	list_for_each_entry(a, &buffer->attachments, list) {
		if (!a->mapped)
			continue;
		dma_sync_sgtable_for_cpu(a->dev, &a->table, direction);
	}
```

For *coherent* memory, the CPU and device see the same data without cache maintenance. The `dma_sync_sgtable_for_cpu/device()` calls and `invalidate_kernel_vmap_range`/`flush_kernel_vmap_range` are unnecessary for coherent allocations and add overhead. The CMA heap does this because CMA memory isn't inherently coherent, but coherent reserved-memory is, by definition, coherent. These sync operations should be documented as to why they're present (perhaps for non-cache-coherent architectures?) or reconsidered.

**Issue: `virt_to_page()` on `dma_alloc_coherent()` return value**

```c
	for (pg = 0; pg < pagecount; pg++)
		buffer->pages[pg] = virt_to_page((char *)buffer->alloc_vaddr +
						 (pg * PAGE_SIZE));
```

`dma_alloc_coherent()` returns a virtual address, but on some architectures (notably ARM with `no-map` regions), this might return an ioremap'd address rather than a regular kernel linear map address. `virt_to_page()` on such addresses is **undefined behavior** and will produce garbage `struct page *` pointers. This is a significant concern on ARM platforms where these coherent reserved-memory regions are commonly used with `no-map`. The pages obtained this way are then used in `sg_alloc_table_from_pages()` during attach, which would corrupt the scatterlist.

Consider using `pfn_to_page(PHYS_PFN(buffer->dma_addr + pg * PAGE_SIZE))` or directly computing pages from the physical address of the rmem region instead.

**Issue: `coherent_heap_map_dma_buf` returns `-ENOMEM` for all errors**

```c
	ret = dma_map_sgtable(attachment->dev, table, direction, 0);
	if (ret)
		return ERR_PTR(-ENOMEM);
```

Should return `ERR_PTR(ret)` to preserve the actual error code.

**Issue: Memory leak of `coh_heap->name` on registration failure**

In `__coherent_heap_register()`:
```c
destroy_heap:
	dma_heap_destroy(coh_heap->heap);
	coh_heap->heap = NULL;
free_name:
	kfree(coh_heap->name);
```

But `dma_heap_destroy()` → `put_device()` → `dma_heap_dev_release()` will `kfree(heap->name)`, which is the **same pointer** as `coh_heap->name` (since `exp_info.name = coh_heap->name` and `dma_heap_create` stores it). So `free_name` does a double-free. Either `dma_heap_dev_release()` should not free the name (see patch 2 issue), or this `kfree(coh_heap->name)` should be removed.

**Issue: No `module_exit` — intentional but no suppression of warning**

The commit message says the heap can't unload. When built as a module (patch 6), the lack of `module_exit` will generate a compiler warning. Consider adding a comment or `__module_exit` stub.

**Minor: Missing newline in pr_warn**

```c
		pr_warn("Failed to add coherent heap %s",
			rmem->name ? rmem->name : "unknown");
```

Missing `\n` at the end of the format string.

---

---
Generated by Claude Code Patch Reviewer