* [PATCH v2] dma/contiguous: Fix broken build @ 2026-03-30 15:40 ` Maxime Ripard 2026-03-30 17:49 ` Marek Szyprowski ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Maxime Ripard @ 2026-03-30 15:40 UTC (permalink / raw) To: Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier, Christian König, Marek Szyprowski, Robin Murphy, Albert Esteve Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu, Mark Brown, Maxime Ripard Commit 3a236f6a5cf2 ("dma: contiguous: Turn heap registration logic around") didn't remove one last call to dma_heap_cma_register_heap() that it removed, thus breaking the build. That last call is in dma_contiguous_reserve(), to handle the registration of the default CMA region heap instance. The default CMA region instance is already somewhat handled by retrieving it through the dev_get_cma_area() call in the CMA heap driver. However, since commit 854acbe75ff4 ("dma-buf: heaps: Give default CMA heap a fixed name"), we will create two heap instances for the CMA default region. The first one is always called "default_cma_region", and is the one handled by the call to dev_get_cma_area() mentioned earlier. The second one is the name it used to have prior to that last commit for backward compatibility. In the case where the default CMA region is defined in the DT, then that region is registered through rmem_cma_setup() and that region is added to the list of CMA regions to create a CMA heap instance for. 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(). If we only remove the call to dma_heap_cma_register_heap(), then the legacy name of the CMA heap will not be registered anymore. We thus need to replace that call with a call to rmem_cma_insert_area() to make sure we queue this instance, if created, to create a heap instance. Once that call to dma_heap_cma_register_heap() replaced, we can also remove the now unused function definition, its now empty header, and all includes of this header. Fixes: 3a236f6a5cf2 ("dma: contiguous: Turn heap registration logic around") Reported-by: Mark Brown <broonie@kernel.org> Closes: https://lore.kernel.org/linux-next/acbjaDJ1a-YQC64d@sirena.co.uk/ Signed-off-by: Maxime Ripard <mripard@kernel.org> --- Changes in v2: - Fix creation of the CMA heap instance with the legacy name when not declared in the DT. - Link to v1: https://lore.kernel.org/r/20260330-dma-build-fix-v1-1-748b64f0d8af@kernel.org --- drivers/dma-buf/heaps/cma_heap.c | 1 - include/linux/dma-buf/heaps/cma.h | 16 ---------------- kernel/dma/contiguous.c | 14 +++++++++++--- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 7216a14262b04bb6130ddf26b7d009f7d15b03fd..9a8b36bc929f6daa483a0139a2919d95127e0d23 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -12,11 +12,10 @@ #define pr_fmt(fmt) "cma_heap: " fmt #include <linux/cma.h> #include <linux/dma-buf.h> -#include <linux/dma-buf/heaps/cma.h> #include <linux/dma-heap.h> #include <linux/dma-map-ops.h> #include <linux/err.h> #include <linux/highmem.h> #include <linux/io.h> diff --git a/include/linux/dma-buf/heaps/cma.h b/include/linux/dma-buf/heaps/cma.h deleted file mode 100644 index e751479e21e703e24a5f799b4a7fc8bd0df3c1c4..0000000000000000000000000000000000000000 --- a/include/linux/dma-buf/heaps/cma.h +++ /dev/null @@ -1,16 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#ifndef DMA_BUF_HEAP_CMA_H_ -#define DMA_BUF_HEAP_CMA_H_ - -struct cma; - -#ifdef CONFIG_DMABUF_HEAPS_CMA -int dma_heap_cma_register_heap(struct cma *cma); -#else -static inline int dma_heap_cma_register_heap(struct cma *cma) -{ - return 0; -} -#endif // CONFIG_DMABUF_HEAPS_CMA - -#endif // DMA_BUF_HEAP_CMA_H_ diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c index ad50512d71d3088a73e4b1ac02d6e6122374888e..d5d15983060c5c54744d6a63f2b591e1a3455b86 100644 --- a/kernel/dma/contiguous.c +++ b/kernel/dma/contiguous.c @@ -40,11 +40,10 @@ #include <asm/page.h> #include <linux/memblock.h> #include <linux/err.h> #include <linux/sizes.h> -#include <linux/dma-buf/heaps/cma.h> #include <linux/dma-map-ops.h> #include <linux/cma.h> #include <linux/nospec.h> #ifdef CONFIG_CMA_SIZE_MBYTES @@ -217,10 +216,19 @@ static void __init dma_numa_cma_reserve(void) static inline void __init dma_numa_cma_reserve(void) { } #endif +#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 + /** * dma_contiguous_reserve() - reserve area(s) for contiguous memory handling * @limit: End address of the reserved memory (optional, 0 for any). * * This function reserves memory from early allocator. It should be @@ -271,13 +279,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit) &dma_contiguous_default_area, fixed); if (ret) return; - 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."); } } void __weak dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) --- base-commit: 6c683d5b1903a14e362c9f1628ce9fe61eac35e7 change-id: 20260330-dma-build-fix-706a4feb0e0f Best regards, -- Maxime Ripard <mripard@kernel.org> ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] dma/contiguous: Fix broken build 2026-03-30 15:40 ` [PATCH v2] dma/contiguous: Fix broken build Maxime Ripard @ 2026-03-30 17:49 ` Marek Szyprowski 2026-03-31 7:07 ` Claude review: " Claude Code Review Bot 2026-03-31 7:07 ` Claude Code Review Bot 2 siblings, 0 replies; 4+ messages in thread From: Marek Szyprowski @ 2026-03-30 17:49 UTC (permalink / raw) To: Maxime Ripard, Sumit Semwal, Benjamin Gaignard, Brian Starkey, John Stultz, T.J. Mercier, Christian König, Robin Murphy, Albert Esteve Cc: linux-kernel, linux-media, dri-devel, linaro-mm-sig, iommu, Mark Brown On 30.03.2026 17:40, Maxime Ripard wrote: > Commit 3a236f6a5cf2 ("dma: contiguous: Turn heap registration logic > around") didn't remove one last call to dma_heap_cma_register_heap() > that it removed, thus breaking the build. > > That last call is in dma_contiguous_reserve(), to handle the > registration of the default CMA region heap instance. > > The default CMA region instance is already somewhat handled by > retrieving it through the dev_get_cma_area() call in the CMA heap > driver. However, since commit 854acbe75ff4 ("dma-buf: heaps: Give > default CMA heap a fixed name"), we will create two heap instances for > the CMA default region. > > The first one is always called "default_cma_region", and is the one > handled by the call to dev_get_cma_area() mentioned earlier. The second > one is the name it used to have prior to that last commit for backward > compatibility. > > In the case where the default CMA region is defined in the DT, then that > region is registered through rmem_cma_setup() and that region is added > to the list of CMA regions to create a CMA heap instance for. > > 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(). If we only > remove the call to dma_heap_cma_register_heap(), then the legacy name of > the CMA heap will not be registered anymore. We thus need to replace > that call with a call to rmem_cma_insert_area() to make sure we queue > this instance, if created, to create a heap instance. > > Once that call to dma_heap_cma_register_heap() replaced, we can also > remove the now unused function definition, its now empty header, and all > includes of this header. > > Fixes: 3a236f6a5cf2 ("dma: contiguous: Turn heap registration logic around") > Reported-by: Mark Brown <broonie@kernel.org> > Closes: https://lore.kernel.org/linux-next/acbjaDJ1a-YQC64d@sirena.co.uk/ > Signed-off-by: Maxime Ripard <mripard@kernel.org> > --- > Changes in v2: > - Fix creation of the CMA heap instance with the legacy name when not > declared in the DT. > - Link to v1: https://lore.kernel.org/r/20260330-dma-build-fix-v1-1-748b64f0d8af@kernel.org > --- > drivers/dma-buf/heaps/cma_heap.c | 1 - > include/linux/dma-buf/heaps/cma.h | 16 ---------------- > kernel/dma/contiguous.c | 14 +++++++++++--- > 3 files changed, 11 insertions(+), 20 deletions(-) > > diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c > index 7216a14262b04bb6130ddf26b7d009f7d15b03fd..9a8b36bc929f6daa483a0139a2919d95127e0d23 100644 > --- a/drivers/dma-buf/heaps/cma_heap.c > +++ b/drivers/dma-buf/heaps/cma_heap.c > @@ -12,11 +12,10 @@ > > #define pr_fmt(fmt) "cma_heap: " fmt > > #include <linux/cma.h> > #include <linux/dma-buf.h> > -#include <linux/dma-buf/heaps/cma.h> > #include <linux/dma-heap.h> > #include <linux/dma-map-ops.h> > #include <linux/err.h> > #include <linux/highmem.h> > #include <linux/io.h> > diff --git a/include/linux/dma-buf/heaps/cma.h b/include/linux/dma-buf/heaps/cma.h > deleted file mode 100644 > index e751479e21e703e24a5f799b4a7fc8bd0df3c1c4..0000000000000000000000000000000000000000 > --- a/include/linux/dma-buf/heaps/cma.h > +++ /dev/null > @@ -1,16 +0,0 @@ > -/* SPDX-License-Identifier: GPL-2.0 */ > -#ifndef DMA_BUF_HEAP_CMA_H_ > -#define DMA_BUF_HEAP_CMA_H_ > - > -struct cma; > - > -#ifdef CONFIG_DMABUF_HEAPS_CMA > -int dma_heap_cma_register_heap(struct cma *cma); > -#else > -static inline int dma_heap_cma_register_heap(struct cma *cma) > -{ > - return 0; > -} > -#endif // CONFIG_DMABUF_HEAPS_CMA > - > -#endif // DMA_BUF_HEAP_CMA_H_ > diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c > index ad50512d71d3088a73e4b1ac02d6e6122374888e..d5d15983060c5c54744d6a63f2b591e1a3455b86 100644 > --- a/kernel/dma/contiguous.c > +++ b/kernel/dma/contiguous.c > @@ -40,11 +40,10 @@ > #include <asm/page.h> > > #include <linux/memblock.h> > #include <linux/err.h> > #include <linux/sizes.h> > -#include <linux/dma-buf/heaps/cma.h> > #include <linux/dma-map-ops.h> > #include <linux/cma.h> > #include <linux/nospec.h> > > #ifdef CONFIG_CMA_SIZE_MBYTES > @@ -217,10 +216,19 @@ static void __init dma_numa_cma_reserve(void) > static inline void __init dma_numa_cma_reserve(void) > { > } > #endif > > +#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 > + > /** > * dma_contiguous_reserve() - reserve area(s) for contiguous memory handling > * @limit: End address of the reserved memory (optional, 0 for any). > * > * This function reserves memory from early allocator. It should be > @@ -271,13 +279,13 @@ void __init dma_contiguous_reserve(phys_addr_t limit) > &dma_contiguous_default_area, > fixed); > if (ret) > return; > > - 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."); > } > } > > void __weak > dma_contiguous_early_fixup(phys_addr_t base, unsigned long size) > > --- Huh, this is still not the correct fix to restore the old behavior. When !CONFIG_OF_RESERVED_MEM the default cma area should be still registered as the default cma dma-buf heap. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: dma/contiguous: Fix broken build 2026-03-30 15:40 ` [PATCH v2] dma/contiguous: Fix broken build Maxime Ripard 2026-03-30 17:49 ` Marek Szyprowski @ 2026-03-31 7:07 ` Claude Code Review Bot 2026-03-31 7:07 ` Claude Code Review Bot 2 siblings, 0 replies; 4+ messages in thread From: Claude Code Review Bot @ 2026-03-31 7:07 UTC (permalink / raw) To: dri-devel-reviews Overall Series Review Subject: dma/contiguous: Fix broken build Author: Maxime Ripard <mripard@kernel.org> Patches: 2 Reviewed: 2026-03-31T17:07:44.040479 --- This is a single-patch build fix from Maxime Ripard addressing a broken build caused by commit 3a236f6a5cf2 ("dma: contiguous: Turn heap registration logic around") which removed the definition of `dma_heap_cma_register_heap()` but left behind a call site in `dma_contiguous_reserve()`. The fix is logically sound: it replaces the dangling call with `rmem_cma_insert_area()` (introduced by the same broken commit), removes the now-empty header `linux/dma-buf/heaps/cma.h`, and cleans up all includes of it. The approach of queueing the CMA area via `rmem_cma_insert_area()` instead of directly registering the heap aligns with the new registration architecture that 3a236f6a5cf2 intended. There are a couple of minor observations but nothing blocking. --- Generated by Claude Code Patch Reviewer ^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: dma/contiguous: Fix broken build 2026-03-30 15:40 ` [PATCH v2] dma/contiguous: Fix broken build Maxime Ripard 2026-03-30 17:49 ` Marek Szyprowski 2026-03-31 7:07 ` Claude review: " Claude Code Review Bot @ 2026-03-31 7:07 ` Claude Code Review Bot 2 siblings, 0 replies; 4+ messages in thread From: Claude Code Review Bot @ 2026-03-31 7:07 UTC (permalink / raw) To: dri-devel-reviews 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-31 7:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20260330154021eucas1p1ddf347acd21646c96fda4b266ddfc03b@eucas1p1.samsung.com>
2026-03-30 15:40 ` [PATCH v2] dma/contiguous: Fix broken build Maxime Ripard
2026-03-30 17:49 ` Marek Szyprowski
2026-03-31 7:07 ` Claude review: " Claude Code Review Bot
2026-03-31 7:07 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox