2023-07-24 12:16:31

by Petr Tesarik

[permalink] [raw]
Subject: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

From: Petr Tesarik <[email protected]>

In all these cases, the last argument to dma_declare_coherent_memory() is
the buffer end address, but the expected value should be the size of the
reserved region.

Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
Signed-off-by: Petr Tesarik <[email protected]>
---
arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
arch/sh/boards/mach-ecovec24/setup.c | 6 ++----
arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
arch/sh/boards/mach-migor/setup.c | 2 +-
arch/sh/boards/mach-se/7724/setup.c | 6 ++----
5 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
index 151792162152..645cccf3da88 100644
--- a/arch/sh/boards/mach-ap325rxa/setup.c
+++ b/arch/sh/boards/mach-ap325rxa/setup.c
@@ -531,7 +531,7 @@ static int __init ap325rxa_devices_setup(void)
device_initialize(&ap325rxa_ceu_device.dev);
dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
ceu_dma_membase, ceu_dma_membase,
- ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);

platform_device_add(&ap325rxa_ceu_device);

diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 674da7ebd8b7..7ec03d4a4edf 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -1454,15 +1454,13 @@ static int __init arch_setup(void)
device_initialize(&ecovec_ceu_devices[0]->dev);
dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
ceu0_dma_membase, ceu0_dma_membase,
- ceu0_dma_membase +
- CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);
platform_device_add(ecovec_ceu_devices[0]);

device_initialize(&ecovec_ceu_devices[1]->dev);
dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
ceu1_dma_membase, ceu1_dma_membase,
- ceu1_dma_membase +
- CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);
platform_device_add(ecovec_ceu_devices[1]);

gpiod_add_lookup_table(&cn12_power_gpiod_table);
diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
index 20f4db778ed6..c6d556dfbbbe 100644
--- a/arch/sh/boards/mach-kfr2r09/setup.c
+++ b/arch/sh/boards/mach-kfr2r09/setup.c
@@ -603,7 +603,7 @@ static int __init kfr2r09_devices_setup(void)
device_initialize(&kfr2r09_ceu_device.dev);
dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
ceu_dma_membase, ceu_dma_membase,
- ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);

platform_device_add(&kfr2r09_ceu_device);

diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
index f60061283c48..773ee767d0c4 100644
--- a/arch/sh/boards/mach-migor/setup.c
+++ b/arch/sh/boards/mach-migor/setup.c
@@ -604,7 +604,7 @@ static int __init migor_devices_setup(void)
device_initialize(&migor_ceu_device.dev);
dma_declare_coherent_memory(&migor_ceu_device.dev,
ceu_dma_membase, ceu_dma_membase,
- ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);

platform_device_add(&migor_ceu_device);

diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
index b60a2626e18b..6495f9354065 100644
--- a/arch/sh/boards/mach-se/7724/setup.c
+++ b/arch/sh/boards/mach-se/7724/setup.c
@@ -940,15 +940,13 @@ static int __init devices_setup(void)
device_initialize(&ms7724se_ceu_devices[0]->dev);
dma_declare_coherent_memory(&ms7724se_ceu_devices[0]->dev,
ceu0_dma_membase, ceu0_dma_membase,
- ceu0_dma_membase +
- CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);
platform_device_add(ms7724se_ceu_devices[0]);

device_initialize(&ms7724se_ceu_devices[1]->dev);
dma_declare_coherent_memory(&ms7724se_ceu_devices[1]->dev,
ceu1_dma_membase, ceu1_dma_membase,
- ceu1_dma_membase +
- CEU_BUFFER_MEMORY_SIZE - 1);
+ CEU_BUFFER_MEMORY_SIZE);
platform_device_add(ms7724se_ceu_devices[1]);

return platform_add_devices(ms7724se_devices,
--
2.25.1



2023-07-24 13:28:33

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, Jul 24, 2023 at 2:15 PM Petr Tesarik
<[email protected]> wrote:
> From: Petr Tesarik <[email protected]>
>
> In all these cases, the last argument to dma_declare_coherent_memory() is
> the buffer end address, but the expected value should be the size of the
> reserved region.
>
> Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> Signed-off-by: Petr Tesarik <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2023-07-24 13:35:47

by Jacopo Mondi

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

Hi Petr,

On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <[email protected]>
>
> In all these cases, the last argument to dma_declare_coherent_memory() is
> the buffer end address, but the expected value should be the size of the
> reserved region.
>
> Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")

Ups, seems like I have introduced all of these! thanks for fixing
Reviewed-by: Jacopo Mondi <[email protected]>

Thanks
j

> Signed-off-by: Petr Tesarik <[email protected]>
> ---
> arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
> arch/sh/boards/mach-ecovec24/setup.c | 6 ++----
> arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
> arch/sh/boards/mach-migor/setup.c | 2 +-
> arch/sh/boards/mach-se/7724/setup.c | 6 ++----
> 5 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
> index 151792162152..645cccf3da88 100644
> --- a/arch/sh/boards/mach-ap325rxa/setup.c
> +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> @@ -531,7 +531,7 @@ static int __init ap325rxa_devices_setup(void)
> device_initialize(&ap325rxa_ceu_device.dev);
> dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&ap325rxa_ceu_device);
>
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> index 674da7ebd8b7..7ec03d4a4edf 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -1454,15 +1454,13 @@ static int __init arch_setup(void)
> device_initialize(&ecovec_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[0]);
>
> device_initialize(&ecovec_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[1]);
>
> gpiod_add_lookup_table(&cn12_power_gpiod_table);
> diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
> index 20f4db778ed6..c6d556dfbbbe 100644
> --- a/arch/sh/boards/mach-kfr2r09/setup.c
> +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> @@ -603,7 +603,7 @@ static int __init kfr2r09_devices_setup(void)
> device_initialize(&kfr2r09_ceu_device.dev);
> dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&kfr2r09_ceu_device);
>
> diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> index f60061283c48..773ee767d0c4 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -604,7 +604,7 @@ static int __init migor_devices_setup(void)
> device_initialize(&migor_ceu_device.dev);
> dma_declare_coherent_memory(&migor_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&migor_ceu_device);
>
> diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
> index b60a2626e18b..6495f9354065 100644
> --- a/arch/sh/boards/mach-se/7724/setup.c
> +++ b/arch/sh/boards/mach-se/7724/setup.c
> @@ -940,15 +940,13 @@ static int __init devices_setup(void)
> device_initialize(&ms7724se_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[0]);
>
> device_initialize(&ms7724se_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[1]);
>
> return platform_add_devices(ms7724se_devices,
> --
> 2.25.1
>

2023-07-24 15:43:56

by Petr Tesařík

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, 24 Jul 2023 15:22:50 +0200
Jacopo Mondi <[email protected]> wrote:

> Hi Petr,
>
> On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> > From: Petr Tesarik <[email protected]>
> >
> > In all these cases, the last argument to dma_declare_coherent_memory() is
> > the buffer end address, but the expected value should be the size of the
> > reserved region.
> >
> > Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> > Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> > Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> > Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> > Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
>
> Ups, seems like I have introduced all of these! thanks for fixing

No problem. The same code was obviously copied to all boards that have
a renesas-ceu camera. ;-)

Petr T

> Reviewed-by: Jacopo Mondi <[email protected]>
>
> Thanks
> j
>
> > Signed-off-by: Petr Tesarik <[email protected]>
> > ---
> > arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
> > arch/sh/boards/mach-ecovec24/setup.c | 6 ++----
> > arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
> > arch/sh/boards/mach-migor/setup.c | 2 +-
> > arch/sh/boards/mach-se/7724/setup.c | 6 ++----
> > 5 files changed, 7 insertions(+), 11 deletions(-)
> >
> > diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
> > index 151792162152..645cccf3da88 100644
> > --- a/arch/sh/boards/mach-ap325rxa/setup.c
> > +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> > @@ -531,7 +531,7 @@ static int __init ap325rxa_devices_setup(void)
> > device_initialize(&ap325rxa_ceu_device.dev);
> > dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
> > ceu_dma_membase, ceu_dma_membase,
> > - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> >
> > platform_device_add(&ap325rxa_ceu_device);
> >
> > diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> > index 674da7ebd8b7..7ec03d4a4edf 100644
> > --- a/arch/sh/boards/mach-ecovec24/setup.c
> > +++ b/arch/sh/boards/mach-ecovec24/setup.c
> > @@ -1454,15 +1454,13 @@ static int __init arch_setup(void)
> > device_initialize(&ecovec_ceu_devices[0]->dev);
> > dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
> > ceu0_dma_membase, ceu0_dma_membase,
> > - ceu0_dma_membase +
> > - CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> > platform_device_add(ecovec_ceu_devices[0]);
> >
> > device_initialize(&ecovec_ceu_devices[1]->dev);
> > dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
> > ceu1_dma_membase, ceu1_dma_membase,
> > - ceu1_dma_membase +
> > - CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> > platform_device_add(ecovec_ceu_devices[1]);
> >
> > gpiod_add_lookup_table(&cn12_power_gpiod_table);
> > diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
> > index 20f4db778ed6..c6d556dfbbbe 100644
> > --- a/arch/sh/boards/mach-kfr2r09/setup.c
> > +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> > @@ -603,7 +603,7 @@ static int __init kfr2r09_devices_setup(void)
> > device_initialize(&kfr2r09_ceu_device.dev);
> > dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
> > ceu_dma_membase, ceu_dma_membase,
> > - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> >
> > platform_device_add(&kfr2r09_ceu_device);
> >
> > diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> > index f60061283c48..773ee767d0c4 100644
> > --- a/arch/sh/boards/mach-migor/setup.c
> > +++ b/arch/sh/boards/mach-migor/setup.c
> > @@ -604,7 +604,7 @@ static int __init migor_devices_setup(void)
> > device_initialize(&migor_ceu_device.dev);
> > dma_declare_coherent_memory(&migor_ceu_device.dev,
> > ceu_dma_membase, ceu_dma_membase,
> > - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> >
> > platform_device_add(&migor_ceu_device);
> >
> > diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
> > index b60a2626e18b..6495f9354065 100644
> > --- a/arch/sh/boards/mach-se/7724/setup.c
> > +++ b/arch/sh/boards/mach-se/7724/setup.c
> > @@ -940,15 +940,13 @@ static int __init devices_setup(void)
> > device_initialize(&ms7724se_ceu_devices[0]->dev);
> > dma_declare_coherent_memory(&ms7724se_ceu_devices[0]->dev,
> > ceu0_dma_membase, ceu0_dma_membase,
> > - ceu0_dma_membase +
> > - CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> > platform_device_add(ms7724se_ceu_devices[0]);
> >
> > device_initialize(&ms7724se_ceu_devices[1]->dev);
> > dma_declare_coherent_memory(&ms7724se_ceu_devices[1]->dev,
> > ceu1_dma_membase, ceu1_dma_membase,
> > - ceu1_dma_membase +
> > - CEU_BUFFER_MEMORY_SIZE - 1);
> > + CEU_BUFFER_MEMORY_SIZE);
> > platform_device_add(ms7724se_ceu_devices[1]);
> >
> > return platform_add_devices(ms7724se_devices,
> > --
> > 2.25.1
> >


Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

Hi Laurent!

On Mon, 2023-07-24 at 20:12 +0300, Laurent Pinchart wrote:
> Thank you for the patch.
>
> On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> > From: Petr Tesarik <[email protected]>
> >
> > In all these cases, the last argument to dma_declare_coherent_memory() is
> > the buffer end address, but the expected value should be the size of the
> > reserved region.
> >
> > Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> > Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> > Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> > Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> > Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> > Signed-off-by: Petr Tesarik <[email protected]>
>
> Reviewed-by: Laurent Pinchart <[email protected]>
>
> But given that nobody noticed for 5 years, maybe we should drop
> renesas-ceu support from those boards ? Or drop the boards completely ?

arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
going to convert the architecture to using Device Trees which should reduce the maintenance
burden anyways.

Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2023-07-24 17:54:38

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, Jul 24, 2023 at 07:29:27PM +0200, John Paul Adrian Glaubitz wrote:
> On Mon, 2023-07-24 at 20:12 +0300, Laurent Pinchart wrote:
> > On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> > > From: Petr Tesarik <[email protected]>
> > >
> > > In all these cases, the last argument to dma_declare_coherent_memory() is
> > > the buffer end address, but the expected value should be the size of the
> > > reserved region.
> > >
> > > Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> > > Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> > > Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> > > Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> > > Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> > > Signed-off-by: Petr Tesarik <[email protected]>
> >
> > Reviewed-by: Laurent Pinchart <[email protected]>
> >
> > But given that nobody noticed for 5 years, maybe we should drop
> > renesas-ceu support from those boards ? Or drop the boards completely ?
>
> arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> going to convert the architecture to using Device Trees which should reduce the maintenance
> burden anyways.

Keeping the architecture is fine for newer systems, but is anyone really
maintaining the Renesas SH board ?

--
Regards,

Laurent Pinchart

2023-07-24 17:59:38

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

Hi Petr,

Thank you for the patch.

On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> From: Petr Tesarik <[email protected]>
>
> In all these cases, the last argument to dma_declare_coherent_memory() is
> the buffer end address, but the expected value should be the size of the
> reserved region.
>
> Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> Signed-off-by: Petr Tesarik <[email protected]>

Reviewed-by: Laurent Pinchart <[email protected]>

But given that nobody noticed for 5 years, maybe we should drop
renesas-ceu support from those boards ? Or drop the boards completely ?

> ---
> arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
> arch/sh/boards/mach-ecovec24/setup.c | 6 ++----
> arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
> arch/sh/boards/mach-migor/setup.c | 2 +-
> arch/sh/boards/mach-se/7724/setup.c | 6 ++----
> 5 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
> index 151792162152..645cccf3da88 100644
> --- a/arch/sh/boards/mach-ap325rxa/setup.c
> +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> @@ -531,7 +531,7 @@ static int __init ap325rxa_devices_setup(void)
> device_initialize(&ap325rxa_ceu_device.dev);
> dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&ap325rxa_ceu_device);
>
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> index 674da7ebd8b7..7ec03d4a4edf 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -1454,15 +1454,13 @@ static int __init arch_setup(void)
> device_initialize(&ecovec_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[0]);
>
> device_initialize(&ecovec_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[1]);
>
> gpiod_add_lookup_table(&cn12_power_gpiod_table);
> diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
> index 20f4db778ed6..c6d556dfbbbe 100644
> --- a/arch/sh/boards/mach-kfr2r09/setup.c
> +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> @@ -603,7 +603,7 @@ static int __init kfr2r09_devices_setup(void)
> device_initialize(&kfr2r09_ceu_device.dev);
> dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&kfr2r09_ceu_device);
>
> diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> index f60061283c48..773ee767d0c4 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -604,7 +604,7 @@ static int __init migor_devices_setup(void)
> device_initialize(&migor_ceu_device.dev);
> dma_declare_coherent_memory(&migor_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&migor_ceu_device);
>
> diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
> index b60a2626e18b..6495f9354065 100644
> --- a/arch/sh/boards/mach-se/7724/setup.c
> +++ b/arch/sh/boards/mach-se/7724/setup.c
> @@ -940,15 +940,13 @@ static int __init devices_setup(void)
> device_initialize(&ms7724se_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[0]);
>
> device_initialize(&ms7724se_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[1]);
>
> return platform_add_devices(ms7724se_devices,

--
Regards,

Laurent Pinchart

Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, 2023-07-24 at 20:43 +0300, Laurent Pinchart wrote:
> > arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> > going to convert the architecture to using Device Trees which should reduce the maintenance
> > burden anyways.
>
> Keeping the architecture is fine for newer systems, but is anyone really
> maintaining the Renesas SH board ?

I own Renesas evaluation boards, including SH7785LCR-based and SH7724-based boards.

Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2023-07-25 06:12:05

by Petr Tesařík

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, 24 Jul 2023 20:43:31 +0300
Laurent Pinchart <[email protected]> wrote:

> On Mon, Jul 24, 2023 at 07:29:27PM +0200, John Paul Adrian Glaubitz wrote:
> > On Mon, 2023-07-24 at 20:12 +0300, Laurent Pinchart wrote:
> > > On Mon, Jul 24, 2023 at 02:07:42PM +0200, Petr Tesarik wrote:
> > > > From: Petr Tesarik <[email protected]>
> > > >
> > > > In all these cases, the last argument to dma_declare_coherent_memory() is
> > > > the buffer end address, but the expected value should be the size of the
> > > > reserved region.
> > > >
> > > > Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> > > > Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> > > > Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> > > > Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> > > > Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> > > > Signed-off-by: Petr Tesarik <[email protected]>
> > >
> > > Reviewed-by: Laurent Pinchart <[email protected]>
> > >
> > > But given that nobody noticed for 5 years, maybe we should drop
> > > renesas-ceu support from those boards ? Or drop the boards completely ?
> >
> > arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> > going to convert the architecture to using Device Trees which should reduce the maintenance
> > burden anyways.
>
> Keeping the architecture is fine for newer systems, but is anyone really
> maintaining the Renesas SH board ?

FWIW I don't have any opinion here. I found these calls as a side
effect of making an overview of DMA-capable devices (in preparation for
moving all DMA-related members away from struct device). I don't have
any of those boards myself.

Petr T

2023-07-25 11:44:41

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Tue, Jul 25, 2023 at 07:50:56AM +0200, John Paul Adrian Glaubitz wrote:
> On Mon, 2023-07-24 at 20:43 +0300, Laurent Pinchart wrote:
> > > arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> > > going to convert the architecture to using Device Trees which should reduce the maintenance
> > > burden anyways.
> >
> > Keeping the architecture is fine for newer systems, but is anyone really
> > maintaining the Renesas SH board ?
>
> I own Renesas evaluation boards, including SH7785LCR-based and
> SH7724-based boards.

Will you have time to port them to DT, or would you rather focus on
J-core systems ? Do those boards still boot a mainline kernel ?

Dropping Renesas SH board files doesn't preclude anyone from moving them
to DT, all the information will remain in the git history. Unless you
plan to move to DT in a reasonably near future, I think dropping support
for the CEU at least, if not the whole board files, could be a good
option.

--
Regards,

Laurent Pinchart

Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Tue, 2023-07-25 at 14:09 +0300, Laurent Pinchart wrote:
> On Tue, Jul 25, 2023 at 07:50:56AM +0200, John Paul Adrian Glaubitz wrote:
> > On Mon, 2023-07-24 at 20:43 +0300, Laurent Pinchart wrote:
> > > > arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> > > > going to convert the architecture to using Device Trees which should reduce the maintenance
> > > > burden anyways.
> > >
> > > Keeping the architecture is fine for newer systems, but is anyone really
> > > maintaining the Renesas SH board ?
> >
> > I own Renesas evaluation boards, including SH7785LCR-based and
> > SH7724-based boards.
>
> Will you have time to port them to DT, or would you rather focus on
> J-core systems ? Do those boards still boot a mainline kernel ?
>
> Dropping Renesas SH board files doesn't preclude anyone from moving them
> to DT, all the information will remain in the git history. Unless you
> plan to move to DT in a reasonably near future, I think dropping support
> for the CEU at least, if not the whole board files, could be a good
> option.

I'm not sure why you are trying to convince me to kill off support for SuperH
boards. I have just stepped up maintenance of arch/sh to keep SuperH hardware
supported in the kernel because I have been a maintainer of Debian's SuperH
port for several years now.

There is also a small community of SuperH enthusiasts now hacking on the kernel
which is coming together for discussion in #linux-sh on libera IRC.

Thanks,
Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913

2023-07-25 12:45:31

by D. Jeff Dionne

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Jul 25, 2023, at 20:09, Laurent Pinchart <[email protected]> wrote:

> Will you have time to port them to DT, or would you rather focus on
> J-core systems ? Do those boards still boot a mainline kernel ?
>
> Dropping Renesas SH board files doesn't preclude anyone from moving them
> to DT, all the information will remain in the git history.

J-Core developers and engineers are not in favor of gratuitously dropping Hitachi/Renesas ports. That’s our history.

J.

2023-07-25 13:08:43

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Tue, Jul 25, 2023 at 01:38:25PM +0200, John Paul Adrian Glaubitz wrote:
> On Tue, 2023-07-25 at 14:09 +0300, Laurent Pinchart wrote:
> > On Tue, Jul 25, 2023 at 07:50:56AM +0200, John Paul Adrian Glaubitz wrote:
> > > On Mon, 2023-07-24 at 20:43 +0300, Laurent Pinchart wrote:
> > > > > arch/sh is being maintained again, so it's save to keep these boards. At some point, we're
> > > > > going to convert the architecture to using Device Trees which should reduce the maintenance
> > > > > burden anyways.
> > > >
> > > > Keeping the architecture is fine for newer systems, but is anyone really
> > > > maintaining the Renesas SH board ?
> > >
> > > I own Renesas evaluation boards, including SH7785LCR-based and
> > > SH7724-based boards.
> >
> > Will you have time to port them to DT, or would you rather focus on
> > J-core systems ? Do those boards still boot a mainline kernel ?
> >
> > Dropping Renesas SH board files doesn't preclude anyone from moving them
> > to DT, all the information will remain in the git history. Unless you
> > plan to move to DT in a reasonably near future, I think dropping support
> > for the CEU at least, if not the whole board files, could be a good
> > option.
>
> I'm not sure why you are trying to convince me to kill off support for SuperH
> boards. I have just stepped up maintenance of arch/sh to keep SuperH hardware
> supported in the kernel because I have been a maintainer of Debian's SuperH
> port for several years now.

If you're willing to maintain the SuperH support, that's nice :-) I'm
not concerned about the arch side, but I'd like to drop the non-DT
support in corresponding drivers in DRM and V4L2.

> There is also a small community of SuperH enthusiasts now hacking on the kernel
> which is coming together for discussion in #linux-sh on libera IRC.

--
Regards,

Laurent Pinchart

Subject: Re: [PATCH v1] sh: boards: fix CEU buffer size passed to dma_declare_coherent_memory()

On Mon, 2023-07-24 at 14:07 +0200, Petr Tesarik wrote:
> From: Petr Tesarik <[email protected]>
>
> In all these cases, the last argument to dma_declare_coherent_memory() is
> the buffer end address, but the expected value should be the size of the
> reserved region.
>
> Fixes: 39fb993038e1 ("media: arch: sh: ap325rxa: Use new renesas-ceu camera driver")
> Fixes: c2f9b05fd5c1 ("media: arch: sh: ecovec: Use new renesas-ceu camera driver")
> Fixes: f3590dc32974 ("media: arch: sh: kfr2r09: Use new renesas-ceu camera driver")
> Fixes: 186c446f4b84 ("media: arch: sh: migor: Use new renesas-ceu camera driver")
> Fixes: 1a3c230b4151 ("media: arch: sh: ms7724se: Use new renesas-ceu camera driver")
> Signed-off-by: Petr Tesarik <[email protected]>
> ---
> arch/sh/boards/mach-ap325rxa/setup.c | 2 +-
> arch/sh/boards/mach-ecovec24/setup.c | 6 ++----
> arch/sh/boards/mach-kfr2r09/setup.c | 2 +-
> arch/sh/boards/mach-migor/setup.c | 2 +-
> arch/sh/boards/mach-se/7724/setup.c | 6 ++----
> 5 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/arch/sh/boards/mach-ap325rxa/setup.c b/arch/sh/boards/mach-ap325rxa/setup.c
> index 151792162152..645cccf3da88 100644
> --- a/arch/sh/boards/mach-ap325rxa/setup.c
> +++ b/arch/sh/boards/mach-ap325rxa/setup.c
> @@ -531,7 +531,7 @@ static int __init ap325rxa_devices_setup(void)
> device_initialize(&ap325rxa_ceu_device.dev);
> dma_declare_coherent_memory(&ap325rxa_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&ap325rxa_ceu_device);
>
> diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
> index 674da7ebd8b7..7ec03d4a4edf 100644
> --- a/arch/sh/boards/mach-ecovec24/setup.c
> +++ b/arch/sh/boards/mach-ecovec24/setup.c
> @@ -1454,15 +1454,13 @@ static int __init arch_setup(void)
> device_initialize(&ecovec_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[0]);
>
> device_initialize(&ecovec_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ecovec_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ecovec_ceu_devices[1]);
>
> gpiod_add_lookup_table(&cn12_power_gpiod_table);
> diff --git a/arch/sh/boards/mach-kfr2r09/setup.c b/arch/sh/boards/mach-kfr2r09/setup.c
> index 20f4db778ed6..c6d556dfbbbe 100644
> --- a/arch/sh/boards/mach-kfr2r09/setup.c
> +++ b/arch/sh/boards/mach-kfr2r09/setup.c
> @@ -603,7 +603,7 @@ static int __init kfr2r09_devices_setup(void)
> device_initialize(&kfr2r09_ceu_device.dev);
> dma_declare_coherent_memory(&kfr2r09_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&kfr2r09_ceu_device);
>
> diff --git a/arch/sh/boards/mach-migor/setup.c b/arch/sh/boards/mach-migor/setup.c
> index f60061283c48..773ee767d0c4 100644
> --- a/arch/sh/boards/mach-migor/setup.c
> +++ b/arch/sh/boards/mach-migor/setup.c
> @@ -604,7 +604,7 @@ static int __init migor_devices_setup(void)
> device_initialize(&migor_ceu_device.dev);
> dma_declare_coherent_memory(&migor_ceu_device.dev,
> ceu_dma_membase, ceu_dma_membase,
> - ceu_dma_membase + CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
>
> platform_device_add(&migor_ceu_device);
>
> diff --git a/arch/sh/boards/mach-se/7724/setup.c b/arch/sh/boards/mach-se/7724/setup.c
> index b60a2626e18b..6495f9354065 100644
> --- a/arch/sh/boards/mach-se/7724/setup.c
> +++ b/arch/sh/boards/mach-se/7724/setup.c
> @@ -940,15 +940,13 @@ static int __init devices_setup(void)
> device_initialize(&ms7724se_ceu_devices[0]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[0]->dev,
> ceu0_dma_membase, ceu0_dma_membase,
> - ceu0_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[0]);
>
> device_initialize(&ms7724se_ceu_devices[1]->dev);
> dma_declare_coherent_memory(&ms7724se_ceu_devices[1]->dev,
> ceu1_dma_membase, ceu1_dma_membase,
> - ceu1_dma_membase +
> - CEU_BUFFER_MEMORY_SIZE - 1);
> + CEU_BUFFER_MEMORY_SIZE);
> platform_device_add(ms7724se_ceu_devices[1]);
>
> return platform_add_devices(ms7724se_devices,

Reviewed-by: John Paul Adrian Glaubitz <[email protected]>

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913