Messages in this thread | | | Date | Mon, 3 Jul 2023 15:23:36 +0200 | From | Petr Tesařík <> | Subject | Re: arch/superh: Suspicious coherent DMA allocations for CEU video buffers |
| |
Hi Jacopo,
On Mon, 3 Jul 2023 00:32:54 +0200 Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:
> Hi Petr, > > On Thu, Jun 29, 2023 at 03:11:24PM +0200, Petr Tesařík wrote: > > Hi Jacopo, > > > > I've just noticed a few calls to dma_declare_coherent_memory() which > > look suspicious to me, all authored by you: > > > > - arch/sh/boards/mach-ap325rxa/setup.c > > - arch/sh/boards/mach-ecovec24/setup.c > > - arch/sh/boards/mach-kfr2r09/setup.c > > - arch/sh/boards/mach-migor/setup.c > > - arch/sh/boards/mach-se/7724/setup.c > > > > In these files, the last argument to dma_declare_coherent_memory() > > looks like the last address to be used, but the expected value should > > be the size of the reserved region, e.g.: > > > > dma_declare_coherent_memory(&ap325rxa_ceu_device.dev, > > ceu_dma_membase, ceu_dma_membase, > > ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1); > > > > Do you (or anyone else on Cc) agree that this is a braino, and all these > > boards should actually use CEU_BUFFER_MEMORY_SIZE as the size of their > > DMA pools rather than membase + CEU_BUFFER_MEMORY_SIZE - 1? > > > > Thanks for spotting this.. I tried to track down where this comes from > digging out the CEU and the platform file from 4.16, but it seems this > simply is a brain fart from my side. > > I presume this is very much dead code, as the commit message of > 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera > driver") says: "Compile tested only." and nobody has ever reported > bugs. > > Feel free to send patches and cc me, as long as this code is around > it's nice to have it correct at least.
Thank you for confirmation. I'll do that.
> Thank you! > j > > PS: if you ask me, as nobody clearly run this code in the last 5 > years, I would simply drop all these platform files. But that's > maintainers' call.
Since the beginning of the coherent region is declared correctly, it is possible that it just happens to work, merely unreliably, frustrating users with occasional freezes/crashes which may not be easily linked with the CEU camera.
OTOH it is much more likely that this code is simply dead.
Petr T
| |