Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 3 Jul 2023 15:23:36 +0200
From:   Petr =?UTF-8?B?VGVzYcWZw61r?= <petr@tesarici.cz>
To:     Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Cc:     Yoshinori Sato <ysato@users.sourceforge.jp>,
        Rich Felker <dalias@libc.org>,
        John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>,
        "open list:SUPERH" <linux-sh@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: arch/superh: Suspicious coherent DMA allocations for CEU video
 buffers
Message-ID: <20230703152336.43c5d129@meshulam.tesarici.cz>
In-Reply-To: <6hbvbciyyx7wcuwzhgaczqearmu2mu3h2u447mljbp24emtjaz@ciel5ezmehfq>
References: <20230629151124.34e72817@meshulam.tesarici.cz>
        <6hbvbciyyx7wcuwzhgaczqearmu2mu3h2u447mljbp24emtjaz@ciel5ezmehfq>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Jacopo,

On Mon, 3 Jul 2023 00:32:54 +0200
Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote:

> Hi Petr,
>=20
> On Thu, Jun 29, 2023 at 03:11:24PM +0200, Petr Tesa=C5=99=C3=ADk 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?
> > =20
>=20
> 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.
>=20
> 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.
>=20
> 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
>=20
> 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