On 3/31/22 6:19 PM, Andre Przywara wrote: > On Thu, 17 Mar 2022 22:54:03 -0500 > Samuel Holland <sam...@sholland.org> wrote: > > Hi Samuel, > >> When a pinctrl driver is available, it will take care of setting up >> these pins. However, for now this code is still needed in SPL. >> >> Reviewed-by: Simon Glass <s...@chromium.org> >> Signed-off-by: Samuel Holland <sam...@sholland.org> >> --- >> >> (no changes since v1) >> >> arch/arm/mach-sunxi/board.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c >> index 0071de19ff..32e2477ae7 100644 >> --- a/arch/arm/mach-sunxi/board.c >> +++ b/arch/arm/mach-sunxi/board.c >> @@ -79,6 +79,7 @@ ulong board_get_usable_ram_top(ulong total_size) >> static int gpio_init(void) >> { >> __maybe_unused uint val; >> +#if !CONFIG_IS_ENABLED(PINCTRL) > > So first this looks somewhat redundant, since the whole function (and > its caller) is already wrapped in #ifdef CONFIG_SPL_BUILD. Unless you > plan to have DM_PINCTRL for the RISC-V SPL?
Yes, I do. But that is not too relevant anyway, because this code is in arch/arm. So I am okay with dropping the patch. Regards, Samuel > But more importantly this function is already a nested #ifdef > nightmare. I experimented with: > if (CONFIG_IS_ENABLED(PINCTRL)) > return 0; > and the toolchain garbage collection did the rest. This naive > version breaks H6, AFAICS, but with a bit of refactoring this could be > solved. But this is probably something for an independent cleanup patch. > > So: if you agree to the SPL == !PINCTRL redundancy, please drop this > patch. If you want to keep the extra check, please at least add a > comment after the #endif, and I will look at cleaning this up > separately. > > Cheers, > Andre > > >> #if CONFIG_CONS_INDEX == 1 && defined(CONFIG_UART0_PORT_F) >> #if defined(CONFIG_MACH_SUN4I) || \ >> defined(CONFIG_MACH_SUN7I) || \ >> @@ -166,6 +167,7 @@ static int gpio_init(void) >> #else >> #error Unsupported console port number. Please fix pin mux settings in >> board.c >> #endif >> +#endif >> >> #ifdef CONFIG_SUN50I_GEN_H6 >> /* Update PIO power bias configuration by copy hardware detected value >> */ >