[RFC,3/3] decompress/keepalive.h: add config option for toggling a set of bits
diff mbox series

Message ID 20191017114906.30302-4-linux@rasmusvillemoes.dk
State New
Headers show
Series
  • watchdog servicing during decompression
Related show

Commit Message

Rasmus Villemoes Oct. 17, 2019, 11:49 a.m. UTC
It's quite common to have an external watchdog which is serviced via
flipping a GPIO, with the value of that GPIO being settable directly
in a memory-mapped register. So add kconfig options defining the
physical address of such a register as well as a mask to have the
decompressor periodically xor into that register.

If and when other decompress_keepalive methods are added, this can be
made into a "choice DECOMPRESS_KEEPALIVE".

Since only LZ4 is wired up currently, this is "depends on KERNEL_LZ4"
for now. Also, prevent this option from being shown to the average
user.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/decompress/keepalive.h |  8 +++++++
 init/Kconfig                         | 33 ++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

Comments

Linus Walleij Oct. 24, 2019, 12:17 p.m. UTC | #1
On Thu, Oct 17, 2019 at 1:49 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:

> +config DECOMPRESS_KEEPALIVE_TOGGLE_REG
> +       hex "Address of register to modify while decompressing"
> +       help
> +         Set this to a physical address of a 32-bit memory word to
> +         modify while decompressing.
> +
> +config DECOMPRESS_KEEPALIVE_TOGGLE_MASK
> +       hex "Bit mask to toggle while decompressing"
> +       help
> +         The register selected above will periodically be xor'ed with
> +         this value during decompression.

I would not allow users to store these vital hex values in their
defconfig and other unsafe places. Instead follow the pattern from
arch/arm/Kconfig.debug for storing the DEBUG_UART_PHYS:

config DEBUG_UART_PHYS
        hex "Physical base address of debug UART"
        default 0x01c20000 if DEBUG_DAVINCI_DMx_UART0
        default 0x01c28000 if DEBUG_SUNXI_UART0
        default 0x01c28400 if DEBUG_SUNXI_UART1
....
i.e. make sure to provide the right default values. We probably
need at least one example for others to follow.

Maybe this is your plan, I don't know, wanted to point it out
anyways.

Yours,
Linus Walleij
Rasmus Villemoes Oct. 24, 2019, 1:45 p.m. UTC | #2
On 24/10/2019 14.17, Linus Walleij wrote:
> On Thu, Oct 17, 2019 at 1:49 PM Rasmus Villemoes
> <linux@rasmusvillemoes.dk> wrote:
> 
>> +config DECOMPRESS_KEEPALIVE_TOGGLE_REG
>> +       hex "Address of register to modify while decompressing"
>> +       help
>> +         Set this to a physical address of a 32-bit memory word to
>> +         modify while decompressing.
>> +
>> +config DECOMPRESS_KEEPALIVE_TOGGLE_MASK
>> +       hex "Bit mask to toggle while decompressing"
>> +       help
>> +         The register selected above will periodically be xor'ed with
>> +         this value during decompression.
> 
> I would not allow users to store these vital hex values in their
> defconfig and other unsafe places. Instead follow the pattern from
> arch/arm/Kconfig.debug for storing the DEBUG_UART_PHYS:
> 
> config DEBUG_UART_PHYS
>         hex "Physical base address of debug UART"
>         default 0x01c20000 if DEBUG_DAVINCI_DMx_UART0
>         default 0x01c28000 if DEBUG_SUNXI_UART0
>         default 0x01c28400 if DEBUG_SUNXI_UART1
> ....
> i.e. make sure to provide the right default values. We probably
> need at least one example for others to follow.
>
> Maybe this is your plan, I don't know, wanted to point it out
> anyways.

The thing is, there is no proper default value for the use cases I have
in mind: Custom hardware based on some SOC, where the designer has wired
on an external gpio-triggered watchdog. That could be gpio 25 of gpio
bank 0, or gpio 2 of gpio bank 3, or ... so I don't see how there could
possibly be any sane default value - the kernel certainly shouldn't grow
a config option for every single custom board out there.

That's why this is different from the previously existing
arch_decomp_wdog - that was (AFAICT) about feeding the SOC's builtin
watchdog.

I realize this is rather specific, and the current implementation for
example won't work if the gpio value cannot be toggled in such a simple
way (perhaps there are separate "set" and "clear" registers or whatnot)
- but as I said, it is sufficient for the many different cases I've seen
so far (and something like my patches have been used for years on those
boards).

An alternative is to simply provide a complete implementation of
decompress_keepalive() (or arch_decomp_wdog if we want to keep that
name) in an external .o/.c/.S file, and do something like

OBJS += $(CONFIG_DECOMP_WDOG:"%"=%)

in arch/foo/boot/compressed/Makefile. Then the physical address etc. do
not get written in Kconfig, and it should work for all cases, including
the ones that need to write 0x55, 0xaa, 0x12 in order to some
SOC-specific register.

Rasmus

Patch
diff mbox series

diff --git a/include/linux/decompress/keepalive.h b/include/linux/decompress/keepalive.h
index 39caa7693624..c62e49bee7cf 100644
--- a/include/linux/decompress/keepalive.h
+++ b/include/linux/decompress/keepalive.h
@@ -5,6 +5,14 @@ 
 
 #ifdef PREBOOT
 
+#ifdef CONFIG_DECOMPRESS_KEEPALIVE_TOGGLE
+#define decompress_keepalive() do {					\
+		long addr = CONFIG_DECOMPRESS_KEEPALIVE_TOGGLE_REG;	\
+		volatile unsigned *reg = (volatile unsigned *)addr;	\
+		*reg ^= CONFIG_DECOMPRESS_KEEPALIVE_TOGGLE_MASK;	\
+} while (0)
+#endif
+
 #endif
 
 #ifndef decompress_keepalive
diff --git a/init/Kconfig b/init/Kconfig
index b4daad2bac23..8a894d9fdd77 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -289,6 +289,39 @@  config KERNEL_UNCOMPRESSED
 
 endchoice
 
+config DECOMPRESS_KEEPALIVE_TOGGLE
+	depends on KERNEL_LZ4
+	depends on EXPERT
+	bool "Toggle some bits while decompressing"
+	help
+	  Some embedded boards have an always-running hardware
+	  watchdog with a timeout short enough that the board is reset
+	  during decompression, thus preventing the board from ever
+	  booting.
+
+	  Enable this to toggle certain bits in a memory register
+	  while decompressing the kernel. This can for example be used
+	  in the common case of an external watchdog serviced via a
+	  memory-mapped GPIO.
+
+	  Say N unless you know you need this.
+
+if DECOMPRESS_KEEPALIVE_TOGGLE
+
+config DECOMPRESS_KEEPALIVE_TOGGLE_REG
+	hex "Address of register to modify while decompressing"
+	help
+	  Set this to a physical address of a 32-bit memory word to
+	  modify while decompressing.
+
+config DECOMPRESS_KEEPALIVE_TOGGLE_MASK
+	hex "Bit mask to toggle while decompressing"
+	help
+	  The register selected above will periodically be xor'ed with
+	  this value during decompression.
+
+endif
+
 config DEFAULT_HOSTNAME
 	string "Default hostname"
 	default "(none)"