All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sh: boot: Remove sh5 cache handling
@ 2024-04-24 11:54 Geert Uytterhoeven
  2024-04-29  7:46 ` John Paul Adrian Glaubitz
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2024-04-24 11:54 UTC (permalink / raw
  To: Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
	Arnd Bergmann
  Cc: linux-sh, Geert Uytterhoeven

Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
remove the sh5 cache handling.

Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 arch/sh/boot/compressed/Makefile |  2 +-
 arch/sh/boot/compressed/cache.c  | 13 -------------
 arch/sh/boot/compressed/misc.c   |  7 -------
 3 files changed, 1 insertion(+), 21 deletions(-)
 delete mode 100644 arch/sh/boot/compressed/cache.c

diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
index 6c6c791a1d0630e2..54efed53c8918eef 100644
--- a/arch/sh/boot/compressed/Makefile
+++ b/arch/sh/boot/compressed/Makefile
@@ -5,7 +5,7 @@
 # create a compressed vmlinux image from the original vmlinux
 #
 
-OBJECTS := head_32.o misc.o cache.o piggy.o \
+OBJECTS := head_32.o misc.o piggy.o \
            ashiftrt.o ashldi3.o ashrsi3.o ashlsi3.o lshrsi3.o
 
 targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 \
diff --git a/arch/sh/boot/compressed/cache.c b/arch/sh/boot/compressed/cache.c
deleted file mode 100644
index 31e04ff4841ed084..0000000000000000
--- a/arch/sh/boot/compressed/cache.c
+++ /dev/null
@@ -1,13 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-int cache_control(unsigned int command)
-{
-	volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
-	int i;
-
-	for (i = 0; i < (32 * 1024); i += 32) {
-		(void)*p;
-		p += (32 / sizeof(int));
-	}
-
-	return 0;
-}
diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
index ca05c99a3d5b488d..195367d40031f9e9 100644
--- a/arch/sh/boot/compressed/misc.c
+++ b/arch/sh/boot/compressed/misc.c
@@ -26,11 +26,6 @@
 #undef memcpy
 #define memzero(s, n)     memset ((s), 0, (n))
 
-/* cache.c */
-#define CACHE_ENABLE      0
-#define CACHE_DISABLE     1
-int cache_control(unsigned int command);
-
 extern char input_data[];
 extern int input_len;
 static unsigned char *output;
@@ -139,8 +134,6 @@ void decompress_kernel(void)
 	free_mem_end_ptr = free_mem_ptr + HEAP_SIZE;
 
 	puts("Uncompressing Linux... ");
-	cache_control(CACHE_ENABLE);
 	__decompress(input_data, input_len, NULL, NULL, output, 0, NULL, error);
-	cache_control(CACHE_DISABLE);
 	puts("Ok, booting the kernel.\n");
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-24 11:54 [PATCH] sh: boot: Remove sh5 cache handling Geert Uytterhoeven
@ 2024-04-29  7:46 ` John Paul Adrian Glaubitz
  2024-04-29  7:49   ` Geert Uytterhoeven
  2024-05-01  9:26 ` John Paul Adrian Glaubitz
  2024-05-02 10:32 ` John Paul Adrian Glaubitz
  2 siblings, 1 reply; 11+ messages in thread
From: John Paul Adrian Glaubitz @ 2024-04-29  7:46 UTC (permalink / raw
  To: Geert Uytterhoeven, Yoshinori Sato, Rich Felker, Arnd Bergmann; +Cc: linux-sh

Hi Geert,

On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> remove the sh5 cache handling.
> 
> Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/sh/boot/compressed/Makefile |  2 +-
>  arch/sh/boot/compressed/cache.c  | 13 -------------
>  arch/sh/boot/compressed/misc.c   |  7 -------
>  3 files changed, 1 insertion(+), 21 deletions(-)
>  delete mode 100644 arch/sh/boot/compressed/cache.c
> 
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index 6c6c791a1d0630e2..54efed53c8918eef 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -5,7 +5,7 @@
>  # create a compressed vmlinux image from the original vmlinux
>  #
>  
> -OBJECTS := head_32.o misc.o cache.o piggy.o \
> +OBJECTS := head_32.o misc.o piggy.o \
>             ashiftrt.o ashldi3.o ashrsi3.o ashlsi3.o lshrsi3.o
>  
>  targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 \
> diff --git a/arch/sh/boot/compressed/cache.c b/arch/sh/boot/compressed/cache.c
> deleted file mode 100644
> index 31e04ff4841ed084..0000000000000000
> --- a/arch/sh/boot/compressed/cache.c
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -int cache_control(unsigned int command)
> -{
> -	volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> -	int i;
> -
> -	for (i = 0; i < (32 * 1024); i += 32) {
> -		(void)*p;
> -		p += (32 / sizeof(int));
> -	}
> -
> -	return 0;
> -}
> diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
> index ca05c99a3d5b488d..195367d40031f9e9 100644
> --- a/arch/sh/boot/compressed/misc.c
> +++ b/arch/sh/boot/compressed/misc.c
> @@ -26,11 +26,6 @@
>  #undef memcpy
>  #define memzero(s, n)     memset ((s), 0, (n))
>  
> -/* cache.c */
> -#define CACHE_ENABLE      0
> -#define CACHE_DISABLE     1
> -int cache_control(unsigned int command);
> -
>  extern char input_data[];
>  extern int input_len;
>  static unsigned char *output;
> @@ -139,8 +134,6 @@ void decompress_kernel(void)
>  	free_mem_end_ptr = free_mem_ptr + HEAP_SIZE;
>  
>  	puts("Uncompressing Linux... ");
> -	cache_control(CACHE_ENABLE);
>  	__decompress(input_data, input_len, NULL, NULL, output, 0, NULL, error);
> -	cache_control(CACHE_DISABLE);
>  	puts("Ok, booting the kernel.\n");
>  }

Interesting, looking at boot/compressed/cache.c, it seems that the whole code
is actually a no-op and does nothing but increasing a pointer. So I agree we
should just delete it.

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  7:46 ` John Paul Adrian Glaubitz
@ 2024-04-29  7:49   ` Geert Uytterhoeven
  2024-04-29  7:52     ` John Paul Adrian Glaubitz
  0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2024-04-29  7:49 UTC (permalink / raw
  To: John Paul Adrian Glaubitz
  Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, linux-sh

Hi Adrian,

On Mon, Apr 29, 2024 at 9:46 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> > Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> > remove the sh5 cache handling.
> >
> > Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/arch/sh/boot/compressed/cache.c
> > +++ /dev/null
> > @@ -1,13 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -int cache_control(unsigned int command)
> > -{
> > -     volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> > -     int i;
> > -
> > -     for (i = 0; i < (32 * 1024); i += 32) {
> > -             (void)*p;
> > -             p += (32 / sizeof(int));
> > -     }
> > -
> > -     return 0;
> > -}

> Interesting, looking at boot/compressed/cache.c, it seems that the whole code
> is actually a no-op and does nothing but increasing a pointer. So I agree we
> should just delete it.

It is not a no-op: it also reads from memory, to load new data in
the cache, and evicting the old data.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  7:49   ` Geert Uytterhoeven
@ 2024-04-29  7:52     ` John Paul Adrian Glaubitz
  2024-04-29  8:06       ` Geert Uytterhoeven
  0 siblings, 1 reply; 11+ messages in thread
From: John Paul Adrian Glaubitz @ 2024-04-29  7:52 UTC (permalink / raw
  To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, linux-sh

On Mon, 2024-04-29 at 09:49 +0200, Geert Uytterhoeven wrote:
> Hi Adrian,
> 
> On Mon, Apr 29, 2024 at 9:46 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> > > Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> > > remove the sh5 cache handling.
> > > 
> > > Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > --- a/arch/sh/boot/compressed/cache.c
> > > +++ /dev/null
> > > @@ -1,13 +0,0 @@
> > > -// SPDX-License-Identifier: GPL-2.0
> > > -int cache_control(unsigned int command)
> > > -{
> > > -     volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> > > -     int i;
> > > -
> > > -     for (i = 0; i < (32 * 1024); i += 32) {
> > > -             (void)*p;
> > > -             p += (32 / sizeof(int));
> > > -     }
> > > -
> > > -     return 0;
> > > -}
> 
> > Interesting, looking at boot/compressed/cache.c, it seems that the whole code
> > is actually a no-op and does nothing but increasing a pointer. So I agree we
> > should just delete it.
> 
> It is not a no-op: it also reads from memory, to load new data in
> the cache, and evicting the old data.

Yeah, I actually came to this conclusion right after sending my reply. However, the
command parameter is never used.

Don't have the 32-bit SH CPUs any caches? The code itself is unconditionally executed,
it seems.

Adrian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  7:52     ` John Paul Adrian Glaubitz
@ 2024-04-29  8:06       ` Geert Uytterhoeven
  2024-04-29  8:22         ` John Paul Adrian Glaubitz
  2024-05-02 13:50         ` Yoshinori Sato
  0 siblings, 2 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2024-04-29  8:06 UTC (permalink / raw
  To: John Paul Adrian Glaubitz
  Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, linux-sh

Hi Adrian,

On Mon, Apr 29, 2024 at 9:52 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Mon, 2024-04-29 at 09:49 +0200, Geert Uytterhoeven wrote:
> > On Mon, Apr 29, 2024 at 9:46 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > > On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> > > > Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> > > > remove the sh5 cache handling.
> > > >
> > > > Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >
> > > > --- a/arch/sh/boot/compressed/cache.c
> > > > +++ /dev/null
> > > > @@ -1,13 +0,0 @@
> > > > -// SPDX-License-Identifier: GPL-2.0
> > > > -int cache_control(unsigned int command)
> > > > -{
> > > > -     volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> > > > -     int i;
> > > > -
> > > > -     for (i = 0; i < (32 * 1024); i += 32) {
> > > > -             (void)*p;
> > > > -             p += (32 / sizeof(int));
> > > > -     }
> > > > -
> > > > -     return 0;
> > > > -}
> >
> > > Interesting, looking at boot/compressed/cache.c, it seems that the whole code
> > > is actually a no-op and does nothing but increasing a pointer. So I agree we
> > > should just delete it.
> >
> > It is not a no-op: it also reads from memory, to load new data in
> > the cache, and evicting the old data.
>
> Yeah, I actually came to this conclusion right after sending my reply. However, the
> command parameter is never used.
>
> Don't have the 32-bit SH CPUs any caches? The code itself is unconditionally executed,
> it seems.

They do. E.g. SH7751 has 8+8 KiB of L1 cache.
But e.g. sh7724 has 32+32KiB L1 cache, and 256 KiB of unified L2 cache.
SH772[34] have l2_cache_init() to enable the L2 cache, so probably they
boot with L2 disabled, and we are fine.

Unfortunately I don't have access to a SH772[34] system.
Sato-san: can you confirm?
Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  8:06       ` Geert Uytterhoeven
@ 2024-04-29  8:22         ` John Paul Adrian Glaubitz
  2024-04-29  8:29           ` Geert Uytterhoeven
  2024-05-02 13:50         ` Yoshinori Sato
  1 sibling, 1 reply; 11+ messages in thread
From: John Paul Adrian Glaubitz @ 2024-04-29  8:22 UTC (permalink / raw
  To: Geert Uytterhoeven; +Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, linux-sh

Hi Geert,

On Mon, 2024-04-29 at 10:06 +0200, Geert Uytterhoeven wrote:
> On Mon, Apr 29, 2024 at 9:52 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > 
> > Don't have the 32-bit SH CPUs any caches? The code itself is unconditionally executed,
> > it seems.
> 
> They do. E.g. SH7751 has 8+8 KiB of L1 cache.
> But e.g. sh7724 has 32+32KiB L1 cache, and 256 KiB of unified L2 cache.
> SH772[34] have l2_cache_init() to enable the L2 cache, so probably they
> boot with L2 disabled, and we are fine.

Understood. But what exactly was the job of cache_control() when all it
does was flushing the cache area? The cache is not enabled or disabled
here, is it?

Also, I was wondering whether this could be related to the boot lockups
that I am seeing on my SH-7786LCR which stops after printing "Uncompressing
Linux...".

I will test later this week whether your patch actually fixes this issue.

> Unfortunately I don't have access to a SH772[34] system.
> Sato-san: can you confirm?
> Thanks!

Yoshinari's input would indeed be valuable here.

Adrian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  8:22         ` John Paul Adrian Glaubitz
@ 2024-04-29  8:29           ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2024-04-29  8:29 UTC (permalink / raw
  To: John Paul Adrian Glaubitz
  Cc: Yoshinori Sato, Rich Felker, Arnd Bergmann, linux-sh

Hi Adrian,

On Mon, Apr 29, 2024 at 10:22 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Mon, 2024-04-29 at 10:06 +0200, Geert Uytterhoeven wrote:
> > On Mon, Apr 29, 2024 at 9:52 AM John Paul Adrian Glaubitz
> > <glaubitz@physik.fu-berlin.de> wrote:
> > >
> > > Don't have the 32-bit SH CPUs any caches? The code itself is unconditionally executed,
> > > it seems.
> >
> > They do. E.g. SH7751 has 8+8 KiB of L1 cache.
> > But e.g. sh7724 has 32+32KiB L1 cache, and 256 KiB of unified L2 cache.
> > SH772[34] have l2_cache_init() to enable the L2 cache, so probably they
> > boot with L2 disabled, and we are fine.
>
> Understood. But what exactly was the job of cache_control() when all it
> does was flushing the cache area? The cache is not enabled or disabled

I assume flushing the cache was needed to push all code written to RAM.

> here, is it?

No, it is not enabled or disabled. Probably that was planned for later,
cfr. the command parameter.

> Also, I was wondering whether this could be related to the boot lockups
> that I am seeing on my SH-7786LCR which stops after printing "Uncompressing
> Linux...".

I doubt it, unless reading from 0x80000000 causes a lock-up...


> I will test later this week whether your patch actually fixes this issue.

... in which case my patch will help ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-24 11:54 [PATCH] sh: boot: Remove sh5 cache handling Geert Uytterhoeven
  2024-04-29  7:46 ` John Paul Adrian Glaubitz
@ 2024-05-01  9:26 ` John Paul Adrian Glaubitz
  2024-05-02  7:25   ` Geert Uytterhoeven
  2024-05-02 10:32 ` John Paul Adrian Glaubitz
  2 siblings, 1 reply; 11+ messages in thread
From: John Paul Adrian Glaubitz @ 2024-05-01  9:26 UTC (permalink / raw
  To: Geert Uytterhoeven, Yoshinori Sato, Rich Felker, Arnd Bergmann; +Cc: linux-sh

Hi Geert,

On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> remove the sh5 cache handling.
> 
> Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/sh/boot/compressed/Makefile |  2 +-
>  arch/sh/boot/compressed/cache.c  | 13 -------------
>  arch/sh/boot/compressed/misc.c   |  7 -------
>  3 files changed, 1 insertion(+), 21 deletions(-)
>  delete mode 100644 arch/sh/boot/compressed/cache.c
> 
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index 6c6c791a1d0630e2..54efed53c8918eef 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -5,7 +5,7 @@
>  # create a compressed vmlinux image from the original vmlinux
>  #
>  
> -OBJECTS := head_32.o misc.o cache.o piggy.o \
> +OBJECTS := head_32.o misc.o piggy.o \
>             ashiftrt.o ashldi3.o ashrsi3.o ashlsi3.o lshrsi3.o
>  
>  targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 \
> diff --git a/arch/sh/boot/compressed/cache.c b/arch/sh/boot/compressed/cache.c
> deleted file mode 100644
> index 31e04ff4841ed084..0000000000000000
> --- a/arch/sh/boot/compressed/cache.c
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -int cache_control(unsigned int command)
> -{
> -	volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> -	int i;
> -
> -	for (i = 0; i < (32 * 1024); i += 32) {
> -		(void)*p;
> -		p += (32 / sizeof(int));
> -	}
> -
> -	return 0;
> -}
> diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
> index ca05c99a3d5b488d..195367d40031f9e9 100644
> --- a/arch/sh/boot/compressed/misc.c
> +++ b/arch/sh/boot/compressed/misc.c
> @@ -26,11 +26,6 @@
>  #undef memcpy
>  #define memzero(s, n)     memset ((s), 0, (n))
>  
> -/* cache.c */
> -#define CACHE_ENABLE      0
> -#define CACHE_DISABLE     1
> -int cache_control(unsigned int command);
> -
>  extern char input_data[];
>  extern int input_len;
>  static unsigned char *output;
> @@ -139,8 +134,6 @@ void decompress_kernel(void)
>  	free_mem_end_ptr = free_mem_ptr + HEAP_SIZE;
>  
>  	puts("Uncompressing Linux... ");
> -	cache_control(CACHE_ENABLE);
>  	__decompress(input_data, input_len, NULL, NULL, output, 0, NULL, error);
> -	cache_control(CACHE_DISABLE);
>  	puts("Ok, booting the kernel.\n");
>  }

Could you maybe send a version of this patch which will apply on top
of your first series "sh: Fix missing prototypes"?

I'm asking because this patch conflicts with patch #8 from the series
which still fixes some missing protoype warnings for the following
functions, so we can't drop patch #8:

- void arch_ftrace_ops_list_func(void);
- void decompress_kernel(void);
- void ftrace_stub(void);

Adrian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-05-01  9:26 ` John Paul Adrian Glaubitz
@ 2024-05-02  7:25   ` Geert Uytterhoeven
  0 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2024-05-02  7:25 UTC (permalink / raw
  To: John Paul Adrian Glaubitz
  Cc: Geert Uytterhoeven, Yoshinori Sato, Rich Felker, Arnd Bergmann,
	linux-sh

Hi Adrian,

On Wed, May 1, 2024 at 11:26 AM John Paul Adrian Glaubitz
<glaubitz@physik.fu-berlin.de> wrote:
> On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> > Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> > remove the sh5 cache handling.
> >
> > Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

>
> Could you maybe send a version of this patch which will apply on top
> of your first series "sh: Fix missing prototypes"?
>
> I'm asking because this patch conflicts with patch #8 from the series
> which still fixes some missing protoype warnings for the following
> functions, so we can't drop patch #8:
>
> - void arch_ftrace_ops_list_func(void);
> - void decompress_kernel(void);
> - void ftrace_stub(void);

As Sato-san objected against v1 of patch #8, I did send a v2 of patch #8
https://lore.kernel.org/all/b7ea770a3bf26fb2a5f59f4bb83072b2526f7134.1713959841.git.geert+renesas@glider.be/
which applies on top of this one.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-24 11:54 [PATCH] sh: boot: Remove sh5 cache handling Geert Uytterhoeven
  2024-04-29  7:46 ` John Paul Adrian Glaubitz
  2024-05-01  9:26 ` John Paul Adrian Glaubitz
@ 2024-05-02 10:32 ` John Paul Adrian Glaubitz
  2 siblings, 0 replies; 11+ messages in thread
From: John Paul Adrian Glaubitz @ 2024-05-02 10:32 UTC (permalink / raw
  To: Geert Uytterhoeven, Yoshinori Sato, Rich Felker, Arnd Bergmann; +Cc: linux-sh

On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> remove the sh5 cache handling.
> 
> Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/sh/boot/compressed/Makefile |  2 +-
>  arch/sh/boot/compressed/cache.c  | 13 -------------
>  arch/sh/boot/compressed/misc.c   |  7 -------
>  3 files changed, 1 insertion(+), 21 deletions(-)
>  delete mode 100644 arch/sh/boot/compressed/cache.c
> 
> diff --git a/arch/sh/boot/compressed/Makefile b/arch/sh/boot/compressed/Makefile
> index 6c6c791a1d0630e2..54efed53c8918eef 100644
> --- a/arch/sh/boot/compressed/Makefile
> +++ b/arch/sh/boot/compressed/Makefile
> @@ -5,7 +5,7 @@
>  # create a compressed vmlinux image from the original vmlinux
>  #
>  
> -OBJECTS := head_32.o misc.o cache.o piggy.o \
> +OBJECTS := head_32.o misc.o piggy.o \
>             ashiftrt.o ashldi3.o ashrsi3.o ashlsi3.o lshrsi3.o
>  
>  targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 \
> diff --git a/arch/sh/boot/compressed/cache.c b/arch/sh/boot/compressed/cache.c
> deleted file mode 100644
> index 31e04ff4841ed084..0000000000000000
> --- a/arch/sh/boot/compressed/cache.c
> +++ /dev/null
> @@ -1,13 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0
> -int cache_control(unsigned int command)
> -{
> -	volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> -	int i;
> -
> -	for (i = 0; i < (32 * 1024); i += 32) {
> -		(void)*p;
> -		p += (32 / sizeof(int));
> -	}
> -
> -	return 0;
> -}
> diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
> index ca05c99a3d5b488d..195367d40031f9e9 100644
> --- a/arch/sh/boot/compressed/misc.c
> +++ b/arch/sh/boot/compressed/misc.c
> @@ -26,11 +26,6 @@
>  #undef memcpy
>  #define memzero(s, n)     memset ((s), 0, (n))
>  
> -/* cache.c */
> -#define CACHE_ENABLE      0
> -#define CACHE_DISABLE     1
> -int cache_control(unsigned int command);
> -
>  extern char input_data[];
>  extern int input_len;
>  static unsigned char *output;
> @@ -139,8 +134,6 @@ void decompress_kernel(void)
>  	free_mem_end_ptr = free_mem_ptr + HEAP_SIZE;
>  
>  	puts("Uncompressing Linux... ");
> -	cache_control(CACHE_ENABLE);
>  	__decompress(input_data, input_len, NULL, NULL, output, 0, NULL, error);
> -	cache_control(CACHE_DISABLE);
>  	puts("Ok, booting the kernel.\n");
>  }

Applied to my sh-linux tree in the for-next branch.

Thanks,
Adrian

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] sh: boot: Remove sh5 cache handling
  2024-04-29  8:06       ` Geert Uytterhoeven
  2024-04-29  8:22         ` John Paul Adrian Glaubitz
@ 2024-05-02 13:50         ` Yoshinori Sato
  1 sibling, 0 replies; 11+ messages in thread
From: Yoshinori Sato @ 2024-05-02 13:50 UTC (permalink / raw
  To: Geert Uytterhoeven
  Cc: John Paul Adrian Glaubitz, Rich Felker, Arnd Bergmann, linux-sh

On Mon, 29 Apr 2024 17:06:44 +0900,
Geert Uytterhoeven wrote:
> 
> Hi Adrian,
> 
> On Mon, Apr 29, 2024 at 9:52 AM John Paul Adrian Glaubitz
> <glaubitz@physik.fu-berlin.de> wrote:
> > On Mon, 2024-04-29 at 09:49 +0200, Geert Uytterhoeven wrote:
> > > On Mon, Apr 29, 2024 at 9:46 AM John Paul Adrian Glaubitz
> > > <glaubitz@physik.fu-berlin.de> wrote:
> > > > On Wed, 2024-04-24 at 13:54 +0200, Geert Uytterhoeven wrote:
> > > > > Commit 37744feebc086908 ("sh: remove sh5 support") in v5.8 forgot to
> > > > > remove the sh5 cache handling.
> > > > >
> > > > > Suggested-by: Yoshinori Sato <ysato@users.sourceforge.jp>
> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > >
> > > > > --- a/arch/sh/boot/compressed/cache.c
> > > > > +++ /dev/null
> > > > > @@ -1,13 +0,0 @@
> > > > > -// SPDX-License-Identifier: GPL-2.0
> > > > > -int cache_control(unsigned int command)
> > > > > -{
> > > > > -     volatile unsigned int *p = (volatile unsigned int *) 0x80000000;
> > > > > -     int i;
> > > > > -
> > > > > -     for (i = 0; i < (32 * 1024); i += 32) {
> > > > > -             (void)*p;
> > > > > -             p += (32 / sizeof(int));
> > > > > -     }
> > > > > -
> > > > > -     return 0;
> > > > > -}
> > >
> > > > Interesting, looking at boot/compressed/cache.c, it seems that the whole code
> > > > is actually a no-op and does nothing but increasing a pointer. So I agree we
> > > > should just delete it.
> > >
> > > It is not a no-op: it also reads from memory, to load new data in
> > > the cache, and evicting the old data.
> >
> > Yeah, I actually came to this conclusion right after sending my reply. However, the
> > command parameter is never used.
> >
> > Don't have the 32-bit SH CPUs any caches? The code itself is unconditionally executed,
> > it seems.
> 
> They do. E.g. SH7751 has 8+8 KiB of L1 cache.
> But e.g. sh7724 has 32+32KiB L1 cache, and 256 KiB of unified L2 cache.
> SH772[34] have l2_cache_init() to enable the L2 cache, so probably they
> boot with L2 disabled, and we are fine.
> 
> Unfortunately I don't have access to a SH772[34] system.
> Sato-san: can you confirm?
> Thanks!

32-bit SH requires different cache control for each SoC.
It's difficult to put general purpose cache control code here.

The location where the zImage is expanded is not in the instruction cache,
so there is no problem even if it is not explicitly flushed after expansion.
boot/compressed/cache.c also has no meaning now.

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> 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

-- 
Yosinori Sato

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-05-02 14:00 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-24 11:54 [PATCH] sh: boot: Remove sh5 cache handling Geert Uytterhoeven
2024-04-29  7:46 ` John Paul Adrian Glaubitz
2024-04-29  7:49   ` Geert Uytterhoeven
2024-04-29  7:52     ` John Paul Adrian Glaubitz
2024-04-29  8:06       ` Geert Uytterhoeven
2024-04-29  8:22         ` John Paul Adrian Glaubitz
2024-04-29  8:29           ` Geert Uytterhoeven
2024-05-02 13:50         ` Yoshinori Sato
2024-05-01  9:26 ` John Paul Adrian Glaubitz
2024-05-02  7:25   ` Geert Uytterhoeven
2024-05-02 10:32 ` John Paul Adrian Glaubitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.