| Message ID | 20230216033659.3877684-11-trini@konsulko.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [01/13] common/Kconfig: Reword text for BOARD_TYPES | expand |
On 2/16/23 04:36, Tom Rini wrote: > On this platform spl_board_init is a call to arch_misc_init which is a > no-op, so drop the CONFIG options. > > Cc: Marek Vasut <marex@denx.de> > Cc: Olaf Mandel <o.mandel@menlosystems.com> btw. put those under --- next time, that way they don't end up in commit message. > Signed-off-by: Tom Rini <trini@konsulko.com> If CAAM is enabled, ARCH_MISC_INIT brings up the CAAM , and this is needed in SPL for U-Boot authentication using HABv4 . I believe that is why Verdin spl.c calls it.
On Thu, Feb 16, 2023 at 03:04:35PM +0100, Marek Vasut wrote: > On 2/16/23 04:36, Tom Rini wrote: > > On this platform spl_board_init is a call to arch_misc_init which is a > > no-op, so drop the CONFIG options. > > > > Cc: Marek Vasut <marex@denx.de> > > Cc: Olaf Mandel <o.mandel@menlosystems.com> > > btw. put those under --- next time, that way they don't end up in commit > message. Er, did the kernel change expected behavior here? > > Signed-off-by: Tom Rini <trini@konsulko.com> > > If CAAM is enabled, ARCH_MISC_INIT brings up the CAAM , and this is needed > in SPL for U-Boot authentication using HABv4 . I believe that is why Verdin > spl.c calls it. Then I'll put doing a follow-up on SPL_BOARD_INIT (which is another option that shouldn't be directly asked, but select'd when used) for that case. It fails to build in this series because CAAM isn't enabled so there's no arch_misc_init.
On 2/16/23 15:13, Tom Rini wrote: > On Thu, Feb 16, 2023 at 03:04:35PM +0100, Marek Vasut wrote: >> On 2/16/23 04:36, Tom Rini wrote: >>> On this platform spl_board_init is a call to arch_misc_init which is a >>> no-op, so drop the CONFIG options. >>> >>> Cc: Marek Vasut <marex@denx.de> >>> Cc: Olaf Mandel <o.mandel@menlosystems.com> >> >> btw. put those under --- next time, that way they don't end up in commit >> message. > > Er, did the kernel change expected behavior here? Er ... wasn't that the case for like a year now ? >>> Signed-off-by: Tom Rini <trini@konsulko.com> >> >> If CAAM is enabled, ARCH_MISC_INIT brings up the CAAM , and this is needed >> in SPL for U-Boot authentication using HABv4 . I believe that is why Verdin >> spl.c calls it. > > Then I'll put doing a follow-up on SPL_BOARD_INIT (which is another > option that shouldn't be directly asked, but select'd when used) for > that case. It fails to build in this series because CAAM isn't enabled > so there's no arch_misc_init. Just call the arch_board_init unconditionally, the CAAM inside of it is already conditional, so the compiler should inline the result if CAAM is disabled.
On Thu, Feb 16, 2023 at 03:21:43PM +0100, Marek Vasut wrote: > On 2/16/23 15:13, Tom Rini wrote: > > On Thu, Feb 16, 2023 at 03:04:35PM +0100, Marek Vasut wrote: > > > On 2/16/23 04:36, Tom Rini wrote: > > > > On this platform spl_board_init is a call to arch_misc_init which is a > > > > no-op, so drop the CONFIG options. > > > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > Cc: Olaf Mandel <o.mandel@menlosystems.com> > > > > > > btw. put those under --- next time, that way they don't end up in commit > > > message. > > > > Er, did the kernel change expected behavior here? > > Er ... wasn't that the case for like a year now ? Is that a yes then? > > > > Signed-off-by: Tom Rini <trini@konsulko.com> > > > > > > If CAAM is enabled, ARCH_MISC_INIT brings up the CAAM , and this is needed > > > in SPL for U-Boot authentication using HABv4 . I believe that is why Verdin > > > spl.c calls it. > > > > Then I'll put doing a follow-up on SPL_BOARD_INIT (which is another > > option that shouldn't be directly asked, but select'd when used) for > > that case. It fails to build in this series because CAAM isn't enabled > > so there's no arch_misc_init. > > Just call the arch_board_init unconditionally, the CAAM inside of it is > already conditional, so the compiler should inline the result if CAAM is > disabled. It doesn't, and only maybe does with LTO. But we also shouldn't be enabling unused hooks. It sounds like imx8m should follow the other platforms that have an spl_board_init under arch/ ?
On 2/16/23 15:41, Tom Rini wrote: > On Thu, Feb 16, 2023 at 03:21:43PM +0100, Marek Vasut wrote: >> On 2/16/23 15:13, Tom Rini wrote: >>> On Thu, Feb 16, 2023 at 03:04:35PM +0100, Marek Vasut wrote: >>>> On 2/16/23 04:36, Tom Rini wrote: >>>>> On this platform spl_board_init is a call to arch_misc_init which is a >>>>> no-op, so drop the CONFIG options. >>>>> >>>>> Cc: Marek Vasut <marex@denx.de> >>>>> Cc: Olaf Mandel <o.mandel@menlosystems.com> >>>> >>>> btw. put those under --- next time, that way they don't end up in commit >>>> message. >>> >>> Er, did the kernel change expected behavior here? >> >> Er ... wasn't that the case for like a year now ? > > Is that a yes then? I got repeated flak for sticking a wall of Cc: into the commit message recently, so I guess that's a yes . >>>>> Signed-off-by: Tom Rini <trini@konsulko.com> >>>> >>>> If CAAM is enabled, ARCH_MISC_INIT brings up the CAAM , and this is needed >>>> in SPL for U-Boot authentication using HABv4 . I believe that is why Verdin >>>> spl.c calls it. >>> >>> Then I'll put doing a follow-up on SPL_BOARD_INIT (which is another >>> option that shouldn't be directly asked, but select'd when used) for >>> that case. It fails to build in this series because CAAM isn't enabled >>> so there's no arch_misc_init. >> >> Just call the arch_board_init unconditionally, the CAAM inside of it is >> already conditional, so the compiler should inline the result if CAAM is >> disabled. > > It doesn't, and only maybe does with LTO. But we also shouldn't be > enabling unused hooks. It sounds like imx8m should follow the other > platforms that have an spl_board_init under arch/ ? spl_BOARD_init shouldn't be in arch in the first place, but I think what needs to be done here in the long run is, set DM_FLAG_PROBE_AFTER_BIND on CAAM in SPL if CAAM is enabled. That can be done somewhere in arch/arm/mach-imx/imx8m early boot code. And then let DM bring the CAAM up. I think that's the way to go with cleaning up the CAAM and spl_board_init part, without breaking support for HABv4.
diff --git a/configs/imx8mm-mx8menlo_defconfig b/configs/imx8mm-mx8menlo_defconfig index f1e48bba9653..7ca327676171 100644 --- a/configs/imx8mm-mx8menlo_defconfig +++ b/configs/imx8mm-mx8menlo_defconfig @@ -40,7 +40,6 @@ CONFIG_BOARD_LATE_INIT=y CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x910000 CONFIG_SPL_BSS_MAX_SIZE=0x2000 -CONFIG_SPL_BOARD_INIT=y # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set CONFIG_SPL_STACK=0x920000 CONFIG_SYS_SPL_MALLOC=y
On this platform spl_board_init is a call to arch_misc_init which is a no-op, so drop the CONFIG options. Cc: Marek Vasut <marex@denx.de> Cc: Olaf Mandel <o.mandel@menlosystems.com> Signed-off-by: Tom Rini <trini@konsulko.com> --- configs/imx8mm-mx8menlo_defconfig | 1 - 1 file changed, 1 deletion(-)