From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: dma/contiguous: Fix broken build Date: Tue, 31 Mar 2026 17:07:44 +1000 Message-ID: In-Reply-To: <20260330-dma-build-fix-v2-1-26d844575815@kernel.org> References: <20260330-dma-build-fix-v2-1-26d844575815@kernel.org> <20260330-dma-build-fix-v2-1-26d844575815@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 **Commit message**: The description is clear about the problem and rationale. One sentence is grammatically broken: > In the case where the default CMA region is not defined in the DT though used to be the case covered by the now removed dma_heap_cma_register_heap() in dma_contiguous_reserve(). This should read something like: "In the case where the default CMA region is not defined in the DT, this used to be the case covered by..." **Forward declaration and stub** (`kernel/dma/contiguous.c`): ```c +#ifdef CONFIG_OF_RESERVED_MEM +static int rmem_cma_insert_area(struct cma *cma); +#else +static inline int rmem_cma_insert_area(struct cma *cma) +{ + return 0; +} +#endif ``` This is the correct pattern for conditionally-compiled functions in the kernel. The `#else` stub returning 0 means that when `CONFIG_OF_RESERVED_MEM` is disabled, the default CMA area silently won't be queued for legacy-name heap creation. This seems acceptable since: - The "default_cma_region" name is still created via `dev_get_cma_area()` in the CMA heap driver. - The legacy name is primarily relevant for DT-based systems, which would have `CONFIG_OF_RESERVED_MEM` enabled. One minor concern: the forward declaration lacks `__init`. If the actual definition of `rmem_cma_insert_area()` (from commit 3a236f6a5cf2) is marked `__init`, there would be a section mismatch warning. I can't verify this since the function definition isn't in the drm-next tree I have access to, but it's worth checking. **Call site replacement** (`kernel/dma/contiguous.c`): ```c - ret = dma_heap_cma_register_heap(dma_contiguous_default_area); + ret = rmem_cma_insert_area(dma_contiguous_default_area); if (ret) - pr_warn("Couldn't register default CMA heap."); + pr_warn("Couldn't queue default CMA region for heap creation."); ``` Clean replacement. The updated warning message is more accurate for the new semantics. **Header removal** (`include/linux/dma-buf/heaps/cma.h`): The header only contained the `dma_heap_cma_register_heap()` declaration. With the function gone, removing the entire header and its includes from `cma_heap.c` and `contiguous.c` is correct cleanup. **Verdict**: The patch looks correct as a build fix. The only actionable item is the grammatical issue in the commit message and verifying the `__init` annotation on the actual `rmem_cma_insert_area()` definition. --- Generated by Claude Code Patch Reviewer