From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/gem-dma: Add flag for bidirectional mapping of non-coherent GEM DMA buffers Date: Fri, 27 Feb 2026 12:10:07 +1000 Message-ID: In-Reply-To: <20260226-neutron-v1-1-46eccb3bb50a@nxp.com> References: <20260226-neutron-v1-0-46eccb3bb50a@nxp.com> <20260226-neutron-v1-1-46eccb3bb50a@nxp.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review The approach works but adding a second boolean for what is essentially a direction parameter is inelegant. Consider replacing the pair (`map_noncoherent` + `map_bidirectional`) with a single `enum dma_data_direction` field, or at minimum a `dma_data_direction map_dir` that drivers set directly. This would be cleaner and more extensible. That said, the current code is functionally correct. The `drm_gem_dma_mmap()` path for noncoherent uses `dma_mmap_pages()` which doesn't take a direction, so no changes needed there. One concern: `map_bidirectional` is only meaningful when `map_noncoherent` is true, but nothing enforces this. If a driver sets `map_bidirectional = true` without `map_noncoherent = true`, the flag is silently ignored. The doc comment says "valid only if map_noncoherent flag is set" but there's no runtime check. A `WARN_ON` or at least asserting the invariant would be safer. --- Generated by Claude Code Patch Reviewer