From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: dma: contiguous: Turn heap registration logic around
Date: Wed, 01 Apr 2026 07:50:36 +1000
Message-ID: <review-patch1-20260331-dma-buf-heaps-as-modules-v4-1-e18fda504419@kernel.org>
In-Reply-To: <20260331-dma-buf-heaps-as-modules-v4-1-e18fda504419@kernel.org>
References: <20260331-dma-buf-heaps-as-modules-v4-0-e18fda504419@kernel.org> <20260331-dma-buf-heaps-as-modules-v4-1-e18fda504419@kernel.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is the core architectural change. It moves the CMA area tracking array from `cma_heap.c` into `contiguous.c` and provides `dma_contiguous_get_area_by_idx()` as an iterator for the heap to use.

**Potential duplicate default CMA area issue:**

In `rmem_cma_setup()`, the patch inserts **every** CMA area (including one marked `linux,cma-default`) into `dma_contiguous_areas[]` via `dma_contiguous_insert_area()`. Then in `dma_contiguous_reserve()`, if `!dma_contiguous_default_area`, it creates a default area and **also** inserts it.

On the consumer side in `add_cma_heaps()`:

```c
if (default_cma) {
    ret = __add_cma_heap(default_cma, DEFAULT_CMA_NAME);
    ...
}

for (i = 0; (cma = dma_contiguous_get_area_by_idx(i)) != NULL; i++) {
    ret = __add_cma_heap(cma, cma_get_name(cma));
```

When the default CMA area comes from the DT (`rmem_cma_setup` sets `dma_contiguous_default_area` AND inserts it), the loop will encounter it again. `__add_cma_heap` will attempt to create a second heap for the same CMA region. This was **already** the case in the old code (the original `dma_areas[]` had the same issue), so this patch doesn't introduce a regression -- but it would be good to add a check in the loop to skip the default area:

```c
for (i = 0; (cma = dma_contiguous_get_area_by_idx(i)) != NULL; i++) {
    if (cma == default_cma)
        continue;
```

Wait -- looking more carefully at the old code, the original `add_cma_heaps()` also iterated `dma_areas[]` which included the default area registered by `rmem_cma_setup()`. So this is pre-existing behavior. The `dma_heap_add()` call presumably handles or rejects duplicates. Still worth a comment.

The comment block added in `dma_contiguous_reserve()` is well-written and explains the non-DT case clearly.

**Minor:** `dma_contiguous_areas` is not `__initdata` unlike the old `dma_areas` in `cma_heap.c`. This is correct since it's now accessed at module load time (not just `__init`), but worth noting that this adds a small permanent memory cost for the array (up to `MAX_CMA_AREAS` pointers).

No other issues.

---
Generated by Claude Code Patch Reviewer