Messages in this thread | | | Subject | Re: [RFC PATCH 3/3] decompress/keepalive.h: add config option for toggling a set of bits | From | Rasmus Villemoes <> | Date | Thu, 24 Oct 2019 15:45:20 +0200 |
| |
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
| |