| Message ID | 20191017114906.30302-4-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | watchdog servicing during decompression | expand |
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
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
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)"
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(+)