| Message ID | 20220506090517.5935-3-pali@kernel.org |
|---|---|
| State | Accepted |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | arm: mvebu: turris_omnia: Fix hangup in debug UART | expand |
On 06.05.22 11:05, Pali Rohár wrote: > Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE > needs to be done very early, prior calling any function which may touch > internal registers, like debug_uart_init(). > > So do it earlier in arch_very_early_init() instead of arch_cpu_init(). > > Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom > and bootrom requires internal registers at (old) expected location. > > Signed-off-by: Pali Rohár <pali@kernel.org> > --- > arch/arm/mach-mvebu/Kconfig | 1 + > arch/arm/mach-mvebu/Makefile | 1 + > arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- > arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ > 4 files changed, 29 insertions(+), 31 deletions(-) > create mode 100644 arch/arm/mach-mvebu/lowlevel.S > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > index a3f273f4f949..98b13d1e336d 100644 > --- a/arch/arm/mach-mvebu/Kconfig > +++ b/arch/arm/mach-mvebu/Kconfig > @@ -16,6 +16,7 @@ config ARMADA_32BIT > select SUPPORT_SPL > select TRANSLATION_OFFSET > select SPL_SYS_NO_VECTOR_TABLE if SPL > + select ARCH_VERY_EARLY_INIT > > config ARMADA_64BIT > bool > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile > index 1b451889d242..8bd2246325ca 100644 > --- a/arch/arm/mach-mvebu/Makefile > +++ b/arch/arm/mach-mvebu/Makefile > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD > > obj-y = cpu.o > obj-y += dram.o > +obj-y += lowlevel.o > obj-$(CONFIG_DM_RESET) += system-controller.o > ifndef CONFIG_SPL_BUILD > obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c > index 1e893777b292..173d95a760a3 100644 > --- a/arch/arm/mach-mvebu/cpu.c > +++ b/arch/arm/mach-mvebu/cpu.c > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) > } > } > > -void mmu_disable(void) > -{ > - asm volatile( > - "mrc p15, 0, r0, c1, c0, 0\n" > - "bic r0, #1\n" > - "mcr p15, 0, r0, c1, c0, 0\n"); > -} > - > #ifdef CONFIG_ARCH_CPU_INIT > -static void set_cbar(u32 addr) > -{ > - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); > -} > - > #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) > #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) & 0xF) << 2)) > #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) | \ > @@ -476,24 +463,6 @@ int arch_cpu_init(void) > struct pl310_regs *const pl310 = > (struct pl310_regs *)CONFIG_SYS_PL310_BASE; > > - /* > - * Only with disabled MMU its possible to switch the base > - * register address on Armada 38x. Without this the SDRAM > - * located at >= 0x4000.0000 is also not accessible, as its > - * still locked to cache. > - */ > - mmu_disable(); > - > - /* Linux expects the internal registers to be at 0xf1000000 */ > - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); > - set_cbar(SOC_REGS_PHY_BASE + 0xC000); > - > - /* > - * From this stage on, the SoC detection is working. As we have > - * configured the internal register base to the value used > - * in the macros / defines in the U-Boot header (soc.h). > - */ > - > if (mvebu_soc_family() == MVEBU_SOC_A38X) { > /* > * To fully release / unlock this area from cache, we need > diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S > new file mode 100644 > index 000000000000..2491310eb0c1 > --- /dev/null > +++ b/arch/arm/mach-mvebu/lowlevel.S > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#include <config.h> > +#include <linux/linkage.h> > + > +ENTRY(arch_very_early_init) > +#ifdef CONFIG_ARMADA_38X > + /* > + * Only with disabled MMU its possible to switch the base > + * register address on Armada 38x. Without this the SDRAM > + * located at >= 0x4000.0000 is also not accessible, as its > + * still locked to cache. > + */ > + mrc p15, 0, r0, c1, c0, 0 > + bic r0, #1 > + mcr p15, 0, r0, c1, c0, 0 > +#endif You are disabling the MMU here only for CONFIG_ARMADA_38X. In the non-asm version removed above, it's disabled in all cases in arch_cpu_init(). Is this correct? I might be missing something - it's been quite some time, since I worked on this code. Thanks, Stefan > + > + /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */ > + ldr r0, =SOC_REGS_PHY_BASE > + ldr r1, =INTREG_BASE_ADDR_REG > + str r0, [r1] > + add r0, r0, #0xC000 > + mcr p15, 4, r0, c15, c0 > + > + bx lr > +ENDPROC(arch_very_early_init) Viele Grüße, Stefan Roese
On Friday 06 May 2022 14:04:53 Stefan Roese wrote: > On 06.05.22 11:05, Pali Rohár wrote: > > Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE > > needs to be done very early, prior calling any function which may touch > > internal registers, like debug_uart_init(). > > > > So do it earlier in arch_very_early_init() instead of arch_cpu_init(). > > > > Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom > > and bootrom requires internal registers at (old) expected location. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > --- > > arch/arm/mach-mvebu/Kconfig | 1 + > > arch/arm/mach-mvebu/Makefile | 1 + > > arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- > > arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ > > 4 files changed, 29 insertions(+), 31 deletions(-) > > create mode 100644 arch/arm/mach-mvebu/lowlevel.S > > > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > > index a3f273f4f949..98b13d1e336d 100644 > > --- a/arch/arm/mach-mvebu/Kconfig > > +++ b/arch/arm/mach-mvebu/Kconfig > > @@ -16,6 +16,7 @@ config ARMADA_32BIT > > select SUPPORT_SPL > > select TRANSLATION_OFFSET > > select SPL_SYS_NO_VECTOR_TABLE if SPL > > + select ARCH_VERY_EARLY_INIT > > config ARMADA_64BIT > > bool > > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile > > index 1b451889d242..8bd2246325ca 100644 > > --- a/arch/arm/mach-mvebu/Makefile > > +++ b/arch/arm/mach-mvebu/Makefile > > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD > > obj-y = cpu.o > > obj-y += dram.o > > +obj-y += lowlevel.o > > obj-$(CONFIG_DM_RESET) += system-controller.o > > ifndef CONFIG_SPL_BUILD > > obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o > > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c > > index 1e893777b292..173d95a760a3 100644 > > --- a/arch/arm/mach-mvebu/cpu.c > > +++ b/arch/arm/mach-mvebu/cpu.c > > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) > > } > > } > > -void mmu_disable(void) > > -{ > > - asm volatile( > > - "mrc p15, 0, r0, c1, c0, 0\n" > > - "bic r0, #1\n" > > - "mcr p15, 0, r0, c1, c0, 0\n"); > > -} > > - > > #ifdef CONFIG_ARCH_CPU_INIT > > -static void set_cbar(u32 addr) > > -{ > > - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); > > -} > > - > > #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) > > #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) & 0xF) << 2)) > > #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) | \ > > @@ -476,24 +463,6 @@ int arch_cpu_init(void) > > struct pl310_regs *const pl310 = > > (struct pl310_regs *)CONFIG_SYS_PL310_BASE; > > - /* > > - * Only with disabled MMU its possible to switch the base > > - * register address on Armada 38x. Without this the SDRAM > > - * located at >= 0x4000.0000 is also not accessible, as its > > - * still locked to cache. > > - */ > > - mmu_disable(); > > - > > - /* Linux expects the internal registers to be at 0xf1000000 */ > > - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); > > - set_cbar(SOC_REGS_PHY_BASE + 0xC000); > > - > > - /* > > - * From this stage on, the SoC detection is working. As we have > > - * configured the internal register base to the value used > > - * in the macros / defines in the U-Boot header (soc.h). > > - */ > > - > > if (mvebu_soc_family() == MVEBU_SOC_A38X) { > > /* > > * To fully release / unlock this area from cache, we need > > diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S > > new file mode 100644 > > index 000000000000..2491310eb0c1 > > --- /dev/null > > +++ b/arch/arm/mach-mvebu/lowlevel.S > > @@ -0,0 +1,27 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > + > > +#include <config.h> > > +#include <linux/linkage.h> > > + > > +ENTRY(arch_very_early_init) > > +#ifdef CONFIG_ARMADA_38X > > + /* > > + * Only with disabled MMU its possible to switch the base > > + * register address on Armada 38x. Without this the SDRAM > > + * located at >= 0x4000.0000 is also not accessible, as its > > + * still locked to cache. > > + */ > > + mrc p15, 0, r0, c1, c0, 0 > > + bic r0, #1 > > + mcr p15, 0, r0, c1, c0, 0 > > +#endif > > You are disabling the MMU here only for CONFIG_ARMADA_38X. In the > non-asm version removed above, it's disabled in all cases in > arch_cpu_init(). Is this correct? I might be missing something - it's > been quite some time, since I worked on this code. I'm not sure. I was in impression from that comment that disabling MMU is needed only for A38x. But runtime detection of the board was not possible until registers are relocated to the new location, so code disabling MMU was called always. So, this change should be tested on some AXP, A370 or A375 board to verify that everything is working correctly. I tested it only on A385 Turris Omnia. > Thanks, > Stefan > > > + > > + /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */ > > + ldr r0, =SOC_REGS_PHY_BASE > > + ldr r1, =INTREG_BASE_ADDR_REG > > + str r0, [r1] > > + add r0, r0, #0xC000 > > + mcr p15, 4, r0, c15, c0 > > + > > + bx lr > > +ENDPROC(arch_very_early_init) > > Viele Grüße, > Stefan Roese > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On 06.05.22 14:09, Pali Rohár wrote: > On Friday 06 May 2022 14:04:53 Stefan Roese wrote: >> On 06.05.22 11:05, Pali Rohár wrote: >>> Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE >>> needs to be done very early, prior calling any function which may touch >>> internal registers, like debug_uart_init(). >>> >>> So do it earlier in arch_very_early_init() instead of arch_cpu_init(). >>> >>> Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom >>> and bootrom requires internal registers at (old) expected location. >>> >>> Signed-off-by: Pali Rohár <pali@kernel.org> >>> --- >>> arch/arm/mach-mvebu/Kconfig | 1 + >>> arch/arm/mach-mvebu/Makefile | 1 + >>> arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- >>> arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ >>> 4 files changed, 29 insertions(+), 31 deletions(-) >>> create mode 100644 arch/arm/mach-mvebu/lowlevel.S >>> >>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig >>> index a3f273f4f949..98b13d1e336d 100644 >>> --- a/arch/arm/mach-mvebu/Kconfig >>> +++ b/arch/arm/mach-mvebu/Kconfig >>> @@ -16,6 +16,7 @@ config ARMADA_32BIT >>> select SUPPORT_SPL >>> select TRANSLATION_OFFSET >>> select SPL_SYS_NO_VECTOR_TABLE if SPL >>> + select ARCH_VERY_EARLY_INIT >>> config ARMADA_64BIT >>> bool >>> diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile >>> index 1b451889d242..8bd2246325ca 100644 >>> --- a/arch/arm/mach-mvebu/Makefile >>> +++ b/arch/arm/mach-mvebu/Makefile >>> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD >>> obj-y = cpu.o >>> obj-y += dram.o >>> +obj-y += lowlevel.o >>> obj-$(CONFIG_DM_RESET) += system-controller.o >>> ifndef CONFIG_SPL_BUILD >>> obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o >>> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c >>> index 1e893777b292..173d95a760a3 100644 >>> --- a/arch/arm/mach-mvebu/cpu.c >>> +++ b/arch/arm/mach-mvebu/cpu.c >>> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) >>> } >>> } >>> -void mmu_disable(void) >>> -{ >>> - asm volatile( >>> - "mrc p15, 0, r0, c1, c0, 0\n" >>> - "bic r0, #1\n" >>> - "mcr p15, 0, r0, c1, c0, 0\n"); >>> -} >>> - >>> #ifdef CONFIG_ARCH_CPU_INIT >>> -static void set_cbar(u32 addr) >>> -{ >>> - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); >>> -} >>> - >>> #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) >>> #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) & 0xF) << 2)) >>> #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) | \ >>> @@ -476,24 +463,6 @@ int arch_cpu_init(void) >>> struct pl310_regs *const pl310 = >>> (struct pl310_regs *)CONFIG_SYS_PL310_BASE; >>> - /* >>> - * Only with disabled MMU its possible to switch the base >>> - * register address on Armada 38x. Without this the SDRAM >>> - * located at >= 0x4000.0000 is also not accessible, as its >>> - * still locked to cache. >>> - */ >>> - mmu_disable(); >>> - >>> - /* Linux expects the internal registers to be at 0xf1000000 */ >>> - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); >>> - set_cbar(SOC_REGS_PHY_BASE + 0xC000); >>> - >>> - /* >>> - * From this stage on, the SoC detection is working. As we have >>> - * configured the internal register base to the value used >>> - * in the macros / defines in the U-Boot header (soc.h). >>> - */ >>> - >>> if (mvebu_soc_family() == MVEBU_SOC_A38X) { >>> /* >>> * To fully release / unlock this area from cache, we need >>> diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S >>> new file mode 100644 >>> index 000000000000..2491310eb0c1 >>> --- /dev/null >>> +++ b/arch/arm/mach-mvebu/lowlevel.S >>> @@ -0,0 +1,27 @@ >>> +/* SPDX-License-Identifier: GPL-2.0+ */ >>> + >>> +#include <config.h> >>> +#include <linux/linkage.h> >>> + >>> +ENTRY(arch_very_early_init) >>> +#ifdef CONFIG_ARMADA_38X >>> + /* >>> + * Only with disabled MMU its possible to switch the base >>> + * register address on Armada 38x. Without this the SDRAM >>> + * located at >= 0x4000.0000 is also not accessible, as its >>> + * still locked to cache. >>> + */ >>> + mrc p15, 0, r0, c1, c0, 0 >>> + bic r0, #1 >>> + mcr p15, 0, r0, c1, c0, 0 >>> +#endif >> >> You are disabling the MMU here only for CONFIG_ARMADA_38X. In the >> non-asm version removed above, it's disabled in all cases in >> arch_cpu_init(). Is this correct? I might be missing something - it's >> been quite some time, since I worked on this code. > > I'm not sure. I was in impression from that comment that disabling MMU > is needed only for A38x. But runtime detection of the board was not > possible until registers are relocated to the new location, so code > disabling MMU was called always. > > So, this change should be tested on some AXP, A370 or A375 board to > verify that everything is working correctly. Okay, understood. I can do some testing on AXP. Thanks, Stefan > I tested it only on A385 Turris Omnia. > >> Thanks, >> Stefan >> >>> + >>> + /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */ >>> + ldr r0, =SOC_REGS_PHY_BASE >>> + ldr r1, =INTREG_BASE_ADDR_REG >>> + str r0, [r1] >>> + add r0, r0, #0xC000 >>> + mcr p15, 4, r0, c15, c0 >>> + >>> + bx lr >>> +ENDPROC(arch_very_early_init) >> >> Viele Grüße, >> Stefan Roese >> >> -- >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de Viele Grüße, Stefan Roese
On 06.05.22 14:16, Stefan Roese wrote: > On 06.05.22 14:09, Pali Rohár wrote: >> On Friday 06 May 2022 14:04:53 Stefan Roese wrote: >>> On 06.05.22 11:05, Pali Rohár wrote: >>>> Moving of internal registers from INTREG_BASE_ADDR_REG to >>>> SOC_REGS_PHY_BASE >>>> needs to be done very early, prior calling any function which may touch >>>> internal registers, like debug_uart_init(). >>>> >>>> So do it earlier in arch_very_early_init() instead of arch_cpu_init(). >>>> >>>> Movement is done in proper U-Boot, not in SPL. SPL may return to >>>> bootrom >>>> and bootrom requires internal registers at (old) expected location. >>>> >>>> Signed-off-by: Pali Rohár <pali@kernel.org> >>>> --- >>>> arch/arm/mach-mvebu/Kconfig | 1 + >>>> arch/arm/mach-mvebu/Makefile | 1 + >>>> arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- >>>> arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ >>>> 4 files changed, 29 insertions(+), 31 deletions(-) >>>> create mode 100644 arch/arm/mach-mvebu/lowlevel.S >>>> >>>> diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig >>>> index a3f273f4f949..98b13d1e336d 100644 >>>> --- a/arch/arm/mach-mvebu/Kconfig >>>> +++ b/arch/arm/mach-mvebu/Kconfig >>>> @@ -16,6 +16,7 @@ config ARMADA_32BIT >>>> select SUPPORT_SPL >>>> select TRANSLATION_OFFSET >>>> select SPL_SYS_NO_VECTOR_TABLE if SPL >>>> + select ARCH_VERY_EARLY_INIT >>>> config ARMADA_64BIT >>>> bool >>>> diff --git a/arch/arm/mach-mvebu/Makefile >>>> b/arch/arm/mach-mvebu/Makefile >>>> index 1b451889d242..8bd2246325ca 100644 >>>> --- a/arch/arm/mach-mvebu/Makefile >>>> +++ b/arch/arm/mach-mvebu/Makefile >>>> @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD >>>> obj-y = cpu.o >>>> obj-y += dram.o >>>> +obj-y += lowlevel.o >>>> obj-$(CONFIG_DM_RESET) += system-controller.o >>>> ifndef CONFIG_SPL_BUILD >>>> obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o >>>> diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c >>>> index 1e893777b292..173d95a760a3 100644 >>>> --- a/arch/arm/mach-mvebu/cpu.c >>>> +++ b/arch/arm/mach-mvebu/cpu.c >>>> @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) >>>> } >>>> } >>>> -void mmu_disable(void) >>>> -{ >>>> - asm volatile( >>>> - "mrc p15, 0, r0, c1, c0, 0\n" >>>> - "bic r0, #1\n" >>>> - "mcr p15, 0, r0, c1, c0, 0\n"); >>>> -} >>>> - >>>> #ifdef CONFIG_ARCH_CPU_INIT >>>> -static void set_cbar(u32 addr) >>>> -{ >>>> - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); >>>> -} >>>> - >>>> #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) >>>> #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) >>>> & 0xF) << 2)) >>>> #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) >>>> | \ >>>> @@ -476,24 +463,6 @@ int arch_cpu_init(void) >>>> struct pl310_regs *const pl310 = >>>> (struct pl310_regs *)CONFIG_SYS_PL310_BASE; >>>> - /* >>>> - * Only with disabled MMU its possible to switch the base >>>> - * register address on Armada 38x. Without this the SDRAM >>>> - * located at >= 0x4000.0000 is also not accessible, as its >>>> - * still locked to cache. >>>> - */ >>>> - mmu_disable(); >>>> - >>>> - /* Linux expects the internal registers to be at 0xf1000000 */ >>>> - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); >>>> - set_cbar(SOC_REGS_PHY_BASE + 0xC000); >>>> - >>>> - /* >>>> - * From this stage on, the SoC detection is working. As we have >>>> - * configured the internal register base to the value used >>>> - * in the macros / defines in the U-Boot header (soc.h). >>>> - */ >>>> - >>>> if (mvebu_soc_family() == MVEBU_SOC_A38X) { >>>> /* >>>> * To fully release / unlock this area from cache, we need >>>> diff --git a/arch/arm/mach-mvebu/lowlevel.S >>>> b/arch/arm/mach-mvebu/lowlevel.S >>>> new file mode 100644 >>>> index 000000000000..2491310eb0c1 >>>> --- /dev/null >>>> +++ b/arch/arm/mach-mvebu/lowlevel.S >>>> @@ -0,0 +1,27 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0+ */ >>>> + >>>> +#include <config.h> >>>> +#include <linux/linkage.h> >>>> + >>>> +ENTRY(arch_very_early_init) >>>> +#ifdef CONFIG_ARMADA_38X >>>> + /* >>>> + * Only with disabled MMU its possible to switch the base >>>> + * register address on Armada 38x. Without this the SDRAM >>>> + * located at >= 0x4000.0000 is also not accessible, as its >>>> + * still locked to cache. >>>> + */ >>>> + mrc p15, 0, r0, c1, c0, 0 >>>> + bic r0, #1 >>>> + mcr p15, 0, r0, c1, c0, 0 >>>> +#endif >>> >>> You are disabling the MMU here only for CONFIG_ARMADA_38X. In the >>> non-asm version removed above, it's disabled in all cases in >>> arch_cpu_init(). Is this correct? I might be missing something - it's >>> been quite some time, since I worked on this code. >> >> I'm not sure. I was in impression from that comment that disabling MMU >> is needed only for A38x. But runtime detection of the board was not >> possible until registers are relocated to the new location, so code >> disabling MMU was called always. >> >> So, this change should be tested on some AXP, A370 or A375 board to >> verify that everything is working correctly. > > Okay, understood. I can do some testing on AXP. Some quick tests on theadorable_debug (AXP) have shown, that it makes no difference, if the MMU is disabled or not AFAICT. While doing this I noticed though, that kwboot UART booting only worked in roughly 1 out of 2 cases. With no progress after this line: Sending boot message. Please reboot the target...\ IIRC, this worked more reliable a few weeks ago. Any idea, what might have caused this regression - if I am correct here? Thanks, Stefan > Thanks, > Stefan > >> I tested it only on A385 Turris Omnia. >> >>> Thanks, >>> Stefan >>> >>>> + >>>> + /* Move internal registers from INTREG_BASE_ADDR_REG to >>>> SOC_REGS_PHY_BASE */ >>>> + ldr r0, =SOC_REGS_PHY_BASE >>>> + ldr r1, =INTREG_BASE_ADDR_REG >>>> + str r0, [r1] >>>> + add r0, r0, #0xC000 >>>> + mcr p15, 4, r0, c15, c0 >>>> + >>>> + bx lr >>>> +ENDPROC(arch_very_early_init) >>> >>> Viele Grüße, >>> Stefan Roese >>> >>> -- >>> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >>> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de > > Viele Grüße, > Stefan Roese > Viele Grüße, Stefan Roese
On Friday 06 May 2022 14:35:55 Stefan Roese wrote: > On 06.05.22 14:16, Stefan Roese wrote: > > On 06.05.22 14:09, Pali Rohár wrote: > > > On Friday 06 May 2022 14:04:53 Stefan Roese wrote: > > > > On 06.05.22 11:05, Pali Rohár wrote: > > > > > Moving of internal registers from INTREG_BASE_ADDR_REG to > > > > > SOC_REGS_PHY_BASE > > > > > needs to be done very early, prior calling any function which may touch > > > > > internal registers, like debug_uart_init(). > > > > > > > > > > So do it earlier in arch_very_early_init() instead of arch_cpu_init(). > > > > > > > > > > Movement is done in proper U-Boot, not in SPL. SPL may > > > > > return to bootrom > > > > > and bootrom requires internal registers at (old) expected location. > > > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > --- > > > > > arch/arm/mach-mvebu/Kconfig | 1 + > > > > > arch/arm/mach-mvebu/Makefile | 1 + > > > > > arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- > > > > > arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ > > > > > 4 files changed, 29 insertions(+), 31 deletions(-) > > > > > create mode 100644 arch/arm/mach-mvebu/lowlevel.S > > > > > > > > > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > > > > > index a3f273f4f949..98b13d1e336d 100644 > > > > > --- a/arch/arm/mach-mvebu/Kconfig > > > > > +++ b/arch/arm/mach-mvebu/Kconfig > > > > > @@ -16,6 +16,7 @@ config ARMADA_32BIT > > > > > select SUPPORT_SPL > > > > > select TRANSLATION_OFFSET > > > > > select SPL_SYS_NO_VECTOR_TABLE if SPL > > > > > + select ARCH_VERY_EARLY_INIT > > > > > config ARMADA_64BIT > > > > > bool > > > > > diff --git a/arch/arm/mach-mvebu/Makefile > > > > > b/arch/arm/mach-mvebu/Makefile > > > > > index 1b451889d242..8bd2246325ca 100644 > > > > > --- a/arch/arm/mach-mvebu/Makefile > > > > > +++ b/arch/arm/mach-mvebu/Makefile > > > > > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD > > > > > obj-y = cpu.o > > > > > obj-y += dram.o > > > > > +obj-y += lowlevel.o > > > > > obj-$(CONFIG_DM_RESET) += system-controller.o > > > > > ifndef CONFIG_SPL_BUILD > > > > > obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o > > > > > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c > > > > > index 1e893777b292..173d95a760a3 100644 > > > > > --- a/arch/arm/mach-mvebu/cpu.c > > > > > +++ b/arch/arm/mach-mvebu/cpu.c > > > > > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) > > > > > } > > > > > } > > > > > -void mmu_disable(void) > > > > > -{ > > > > > - asm volatile( > > > > > - "mrc p15, 0, r0, c1, c0, 0\n" > > > > > - "bic r0, #1\n" > > > > > - "mcr p15, 0, r0, c1, c0, 0\n"); > > > > > -} > > > > > - > > > > > #ifdef CONFIG_ARCH_CPU_INIT > > > > > -static void set_cbar(u32 addr) > > > > > -{ > > > > > - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); > > > > > -} > > > > > - > > > > > #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) > > > > > #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | > > > > > (((reg) & 0xF) << 2)) > > > > > #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | > > > > > BIT(11) | \ > > > > > @@ -476,24 +463,6 @@ int arch_cpu_init(void) > > > > > struct pl310_regs *const pl310 = > > > > > (struct pl310_regs *)CONFIG_SYS_PL310_BASE; > > > > > - /* > > > > > - * Only with disabled MMU its possible to switch the base > > > > > - * register address on Armada 38x. Without this the SDRAM > > > > > - * located at >= 0x4000.0000 is also not accessible, as its > > > > > - * still locked to cache. > > > > > - */ > > > > > - mmu_disable(); > > > > > - > > > > > - /* Linux expects the internal registers to be at 0xf1000000 */ > > > > > - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); > > > > > - set_cbar(SOC_REGS_PHY_BASE + 0xC000); > > > > > - > > > > > - /* > > > > > - * From this stage on, the SoC detection is working. As we have > > > > > - * configured the internal register base to the value used > > > > > - * in the macros / defines in the U-Boot header (soc.h). > > > > > - */ > > > > > - > > > > > if (mvebu_soc_family() == MVEBU_SOC_A38X) { > > > > > /* > > > > > * To fully release / unlock this area from cache, we need > > > > > diff --git a/arch/arm/mach-mvebu/lowlevel.S > > > > > b/arch/arm/mach-mvebu/lowlevel.S > > > > > new file mode 100644 > > > > > index 000000000000..2491310eb0c1 > > > > > --- /dev/null > > > > > +++ b/arch/arm/mach-mvebu/lowlevel.S > > > > > @@ -0,0 +1,27 @@ > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > > > > + > > > > > +#include <config.h> > > > > > +#include <linux/linkage.h> > > > > > + > > > > > +ENTRY(arch_very_early_init) > > > > > +#ifdef CONFIG_ARMADA_38X > > > > > + /* > > > > > + * Only with disabled MMU its possible to switch the base > > > > > + * register address on Armada 38x. Without this the SDRAM > > > > > + * located at >= 0x4000.0000 is also not accessible, as its > > > > > + * still locked to cache. > > > > > + */ > > > > > + mrc p15, 0, r0, c1, c0, 0 > > > > > + bic r0, #1 > > > > > + mcr p15, 0, r0, c1, c0, 0 > > > > > +#endif > > > > > > > > You are disabling the MMU here only for CONFIG_ARMADA_38X. In the > > > > non-asm version removed above, it's disabled in all cases in > > > > arch_cpu_init(). Is this correct? I might be missing something - it's > > > > been quite some time, since I worked on this code. > > > > > > I'm not sure. I was in impression from that comment that disabling MMU > > > is needed only for A38x. But runtime detection of the board was not > > > possible until registers are relocated to the new location, so code > > > disabling MMU was called always. > > > > > > So, this change should be tested on some AXP, A370 or A375 board to > > > verify that everything is working correctly. > > > > Okay, understood. I can do some testing on AXP. > > Some quick tests on theadorable_debug (AXP) have shown, that it makes > no difference, if the MMU is disabled or not AFAICT. Ok! > While doing this I noticed though, that kwboot UART booting only worked > in roughly 1 out of 2 cases. With no progress after this line: > > Sending boot message. Please reboot the target...\ > > IIRC, this worked more reliable a few weeks ago. Any idea, what might > have caused this regression - if I am correct here? There were changes in handling of bootrom boot message. There is option -s which can change default timeout option. And plus there is option -a which sets this timeout option to AXP value. Default value is 50 and for AXP default value is 1000. Maybe it is needed to tune AXP value... Try to call with -s option between 50 and 1000... > Thanks, > Stefan > > > Thanks, > > Stefan > > > > > I tested it only on A385 Turris Omnia. > > > > > > > Thanks, > > > > Stefan > > > > > > > > > + > > > > > + /* Move internal registers from INTREG_BASE_ADDR_REG to > > > > > SOC_REGS_PHY_BASE */ > > > > > + ldr r0, =SOC_REGS_PHY_BASE > > > > > + ldr r1, =INTREG_BASE_ADDR_REG > > > > > + str r0, [r1] > > > > > + add r0, r0, #0xC000 > > > > > + mcr p15, 4, r0, c15, c0 > > > > > + > > > > > + bx lr > > > > > +ENDPROC(arch_very_early_init) > > > > > > > > Viele Grüße, > > > > Stefan Roese > > > > > > > > -- > > > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > > > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > > > > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de > > > > Viele Grüße, > > Stefan Roese > > > > Viele Grüße, > Stefan Roese > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de
On 06.05.22 11:05, Pali Rohár wrote: > Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE > needs to be done very early, prior calling any function which may touch > internal registers, like debug_uart_init(). > > So do it earlier in arch_very_early_init() instead of arch_cpu_init(). > > Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom > and bootrom requires internal registers at (old) expected location. > > Signed-off-by: Pali Rohár <pali@kernel.org> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > arch/arm/mach-mvebu/Kconfig | 1 + > arch/arm/mach-mvebu/Makefile | 1 + > arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- > arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ > 4 files changed, 29 insertions(+), 31 deletions(-) > create mode 100644 arch/arm/mach-mvebu/lowlevel.S > > diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig > index a3f273f4f949..98b13d1e336d 100644 > --- a/arch/arm/mach-mvebu/Kconfig > +++ b/arch/arm/mach-mvebu/Kconfig > @@ -16,6 +16,7 @@ config ARMADA_32BIT > select SUPPORT_SPL > select TRANSLATION_OFFSET > select SPL_SYS_NO_VECTOR_TABLE if SPL > + select ARCH_VERY_EARLY_INIT > > config ARMADA_64BIT > bool > diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile > index 1b451889d242..8bd2246325ca 100644 > --- a/arch/arm/mach-mvebu/Makefile > +++ b/arch/arm/mach-mvebu/Makefile > @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD > > obj-y = cpu.o > obj-y += dram.o > +obj-y += lowlevel.o > obj-$(CONFIG_DM_RESET) += system-controller.o > ifndef CONFIG_SPL_BUILD > obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o > diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c > index 1e893777b292..173d95a760a3 100644 > --- a/arch/arm/mach-mvebu/cpu.c > +++ b/arch/arm/mach-mvebu/cpu.c > @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) > } > } > > -void mmu_disable(void) > -{ > - asm volatile( > - "mrc p15, 0, r0, c1, c0, 0\n" > - "bic r0, #1\n" > - "mcr p15, 0, r0, c1, c0, 0\n"); > -} > - > #ifdef CONFIG_ARCH_CPU_INIT > -static void set_cbar(u32 addr) > -{ > - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); > -} > - > #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) > #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) & 0xF) << 2)) > #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) | \ > @@ -476,24 +463,6 @@ int arch_cpu_init(void) > struct pl310_regs *const pl310 = > (struct pl310_regs *)CONFIG_SYS_PL310_BASE; > > - /* > - * Only with disabled MMU its possible to switch the base > - * register address on Armada 38x. Without this the SDRAM > - * located at >= 0x4000.0000 is also not accessible, as its > - * still locked to cache. > - */ > - mmu_disable(); > - > - /* Linux expects the internal registers to be at 0xf1000000 */ > - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); > - set_cbar(SOC_REGS_PHY_BASE + 0xC000); > - > - /* > - * From this stage on, the SoC detection is working. As we have > - * configured the internal register base to the value used > - * in the macros / defines in the U-Boot header (soc.h). > - */ > - > if (mvebu_soc_family() == MVEBU_SOC_A38X) { > /* > * To fully release / unlock this area from cache, we need > diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S > new file mode 100644 > index 000000000000..2491310eb0c1 > --- /dev/null > +++ b/arch/arm/mach-mvebu/lowlevel.S > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#include <config.h> > +#include <linux/linkage.h> > + > +ENTRY(arch_very_early_init) > +#ifdef CONFIG_ARMADA_38X > + /* > + * Only with disabled MMU its possible to switch the base > + * register address on Armada 38x. Without this the SDRAM > + * located at >= 0x4000.0000 is also not accessible, as its > + * still locked to cache. > + */ > + mrc p15, 0, r0, c1, c0, 0 > + bic r0, #1 > + mcr p15, 0, r0, c1, c0, 0 > +#endif > + > + /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */ > + ldr r0, =SOC_REGS_PHY_BASE > + ldr r1, =INTREG_BASE_ADDR_REG > + str r0, [r1] > + add r0, r0, #0xC000 > + mcr p15, 4, r0, c15, c0 > + > + bx lr > +ENDPROC(arch_very_early_init) Viele Grüße, Stefan Roese
On Friday 06 May 2022 14:44:48 Pali Rohár wrote: > On Friday 06 May 2022 14:35:55 Stefan Roese wrote: > > While doing this I noticed though, that kwboot UART booting only worked > > in roughly 1 out of 2 cases. With no progress after this line: > > > > Sending boot message. Please reboot the target...\ > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might > > have caused this regression - if I am correct here? > > There were changes in handling of bootrom boot message. There is option > -s which can change default timeout option. And plus there is option -a > which sets this timeout option to AXP value. > > Default value is 50 and for AXP default value is 1000. > > Maybe it is needed to tune AXP value... Try to call with -s option > between 50 and 1000... Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board?
Hi Pali, On 01.06.22 12:27, Pali Rohár wrote: > On Friday 06 May 2022 14:44:48 Pali Rohár wrote: >> On Friday 06 May 2022 14:35:55 Stefan Roese wrote: >>> While doing this I noticed though, that kwboot UART booting only worked >>> in roughly 1 out of 2 cases. With no progress after this line: >>> >>> Sending boot message. Please reboot the target...\ >>> >>> IIRC, this worked more reliable a few weeks ago. Any idea, what might >>> have caused this regression - if I am correct here? >> >> There were changes in handling of bootrom boot message. There is option >> -s which can change default timeout option. And plus there is option -a >> which sets this timeout option to AXP value. >> >> Default value is 50 and for AXP default value is 1000. >> >> Maybe it is needed to tune AXP value... Try to call with -s option >> between 50 and 1000... > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board? Thanks for getting back on this. I totally forgot about it. '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked 5 out of 5 times. So I'll stick with this version I think. Thanks, Stefan
On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote: > Hi Pali, > > On 01.06.22 12:27, Pali Rohár wrote: > > On Friday 06 May 2022 14:44:48 Pali Rohár wrote: > > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote: > > > > While doing this I noticed though, that kwboot UART booting only worked > > > > in roughly 1 out of 2 cases. With no progress after this line: > > > > > > > > Sending boot message. Please reboot the target...\ > > > > > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might > > > > have caused this regression - if I am correct here? > > > > > > There were changes in handling of bootrom boot message. There is option > > > -s which can change default timeout option. And plus there is option -a > > > which sets this timeout option to AXP value. > > > > > > Default value is 50 and for AXP default value is 1000. > > > > > > Maybe it is needed to tune AXP value... Try to call with -s option > > > between 50 and 1000... > > > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board? > > Thanks for getting back on this. I totally forgot about it. > > '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked > 5 out of 5 times. So I'll stick with this version I think. > > Thanks, > Stefan Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option would work better.
On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote: > On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote: > > Hi Pali, > > > > On 01.06.22 12:27, Pali Rohár wrote: > > > On Friday 06 May 2022 14:44:48 Pali Rohár wrote: > > > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote: > > > > > While doing this I noticed though, that kwboot UART booting only worked > > > > > in roughly 1 out of 2 cases. With no progress after this line: > > > > > > > > > > Sending boot message. Please reboot the target...\ > > > > > > > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might > > > > > have caused this regression - if I am correct here? > > > > > > > > There were changes in handling of bootrom boot message. There is option > > > > -s which can change default timeout option. And plus there is option -a > > > > which sets this timeout option to AXP value. > > > > > > > > Default value is 50 and for AXP default value is 1000. > > > > > > > > Maybe it is needed to tune AXP value... Try to call with -s option > > > > between 50 and 1000... > > > > > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board? > > > > Thanks for getting back on this. I totally forgot about it. > > > > '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked > > 5 out of 5 times. So I'll stick with this version I think. > > > > Thanks, > > Stefan > > Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in > tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option > would work better. Hello! I would like to remind this issue. Could you look at it, so we do not have broken '-a' option in kwboot?
Hi Pali, On 16.08.22 20:41, Pali Rohár wrote: > On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote: >> On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote: >>> Hi Pali, >>> >>> On 01.06.22 12:27, Pali Rohár wrote: >>>> On Friday 06 May 2022 14:44:48 Pali Rohár wrote: >>>>> On Friday 06 May 2022 14:35:55 Stefan Roese wrote: >>>>>> While doing this I noticed though, that kwboot UART booting only worked >>>>>> in roughly 1 out of 2 cases. With no progress after this line: >>>>>> >>>>>> Sending boot message. Please reboot the target...\ >>>>>> >>>>>> IIRC, this worked more reliable a few weeks ago. Any idea, what might >>>>>> have caused this regression - if I am correct here? >>>>> >>>>> There were changes in handling of bootrom boot message. There is option >>>>> -s which can change default timeout option. And plus there is option -a >>>>> which sets this timeout option to AXP value. >>>>> >>>>> Default value is 50 and for AXP default value is 1000. >>>>> >>>>> Maybe it is needed to tune AXP value... Try to call with -s option >>>>> between 50 and 1000... >>>> >>>> Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board? >>> >>> Thanks for getting back on this. I totally forgot about it. >>> >>> '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked >>> 5 out of 5 times. So I'll stick with this version I think. >>> >>> Thanks, >>> Stefan >> >> Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in >> tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option >> would work better. > > Hello! I would like to remind this issue. Could you look at it, so we do > not have broken '-a' option in kwboot? Thanks for the reminder. I've checked again and "-a" is not working reliable on my AXP platform (any more?). Using "-s 10" seem to be much better. So we should either remove this -a option completely or change the define to 10 instead of 1000 IMHO. Thanks, Stefan
On Friday 19 August 2022 09:30:55 Stefan Roese wrote: > Hi Pali, > > On 16.08.22 20:41, Pali Rohár wrote: > > On Wednesday 01 June 2022 12:54:28 Pali Rohár wrote: > > > On Wednesday 01 June 2022 12:44:01 Stefan Roese wrote: > > > > Hi Pali, > > > > > > > > On 01.06.22 12:27, Pali Rohár wrote: > > > > > On Friday 06 May 2022 14:44:48 Pali Rohár wrote: > > > > > > On Friday 06 May 2022 14:35:55 Stefan Roese wrote: > > > > > > > While doing this I noticed though, that kwboot UART booting only worked > > > > > > > in roughly 1 out of 2 cases. With no progress after this line: > > > > > > > > > > > > > > Sending boot message. Please reboot the target...\ > > > > > > > > > > > > > > IIRC, this worked more reliable a few weeks ago. Any idea, what might > > > > > > > have caused this regression - if I am correct here? > > > > > > > > > > > > There were changes in handling of bootrom boot message. There is option > > > > > > -s which can change default timeout option. And plus there is option -a > > > > > > which sets this timeout option to AXP value. > > > > > > > > > > > > Default value is 50 and for AXP default value is 1000. > > > > > > > > > > > > Maybe it is needed to tune AXP value... Try to call with -s option > > > > > > between 50 and 1000... > > > > > > > > > > Hello Stefan! Have you tried to tune kwboot's -s or -a option for AXP board? > > > > > > > > Thanks for getting back on this. I totally forgot about it. > > > > > > > > '-a' makes it actually worse. Zero success with 5 tries. '-s 10' worked > > > > 5 out of 5 times. So I'll stick with this version I think. > > > > > > > > Thanks, > > > > Stefan > > > > > > Ok! So I think that you could change KWBOOT_MSG_RSP_TIMEO_AXP value in > > > tools/kwboot.c to 10 if it works fine for your AXP board. So '-a' option > > > would work better. > > > > Hello! I would like to remind this issue. Could you look at it, so we do > > not have broken '-a' option in kwboot? > > Thanks for the reminder. I've checked again and "-a" is not working > reliable on my AXP platform (any more?). Using "-s 10" seem to be much > better. > > So we should either remove this -a option completely or change the > define to 10 instead of 1000 IMHO. > > Thanks, > Stefan Yes, please check also if kwboot is reliable without any -a or -s option on AXP platform. If it works, then remove/deprecate -a option. If it does not work and requires specific -s 10 option, then change KWBOOT_MSG_RSP_TIMEO_AXP macro to 10.
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index a3f273f4f949..98b13d1e336d 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -16,6 +16,7 @@ config ARMADA_32BIT select SUPPORT_SPL select TRANSLATION_OFFSET select SPL_SYS_NO_VECTOR_TABLE if SPL + select ARCH_VERY_EARLY_INIT config ARMADA_64BIT bool diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index 1b451889d242..8bd2246325ca 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -21,6 +21,7 @@ else # CONFIG_ARCH_KIRKWOOD obj-y = cpu.o obj-y += dram.o +obj-y += lowlevel.o obj-$(CONFIG_DM_RESET) += system-controller.o ifndef CONFIG_SPL_BUILD obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index 1e893777b292..173d95a760a3 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -413,20 +413,7 @@ static void update_sdram_window_sizes(void) } } -void mmu_disable(void) -{ - asm volatile( - "mrc p15, 0, r0, c1, c0, 0\n" - "bic r0, #1\n" - "mcr p15, 0, r0, c1, c0, 0\n"); -} - #ifdef CONFIG_ARCH_CPU_INIT -static void set_cbar(u32 addr) -{ - asm("mcr p15, 4, %0, c15, c0" : : "r" (addr)); -} - #define MV_USB_PHY_BASE (MVEBU_AXP_USB_BASE + 0x800) #define MV_USB_PHY_PLL_REG(reg) (MV_USB_PHY_BASE | (((reg) & 0xF) << 2)) #define MV_USB_X3_BASE(addr) (MVEBU_AXP_USB_BASE | BIT(11) | \ @@ -476,24 +463,6 @@ int arch_cpu_init(void) struct pl310_regs *const pl310 = (struct pl310_regs *)CONFIG_SYS_PL310_BASE; - /* - * Only with disabled MMU its possible to switch the base - * register address on Armada 38x. Without this the SDRAM - * located at >= 0x4000.0000 is also not accessible, as its - * still locked to cache. - */ - mmu_disable(); - - /* Linux expects the internal registers to be at 0xf1000000 */ - writel(SOC_REGS_PHY_BASE, INTREG_BASE_ADDR_REG); - set_cbar(SOC_REGS_PHY_BASE + 0xC000); - - /* - * From this stage on, the SoC detection is working. As we have - * configured the internal register base to the value used - * in the macros / defines in the U-Boot header (soc.h). - */ - if (mvebu_soc_family() == MVEBU_SOC_A38X) { /* * To fully release / unlock this area from cache, we need diff --git a/arch/arm/mach-mvebu/lowlevel.S b/arch/arm/mach-mvebu/lowlevel.S new file mode 100644 index 000000000000..2491310eb0c1 --- /dev/null +++ b/arch/arm/mach-mvebu/lowlevel.S @@ -0,0 +1,27 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#include <config.h> +#include <linux/linkage.h> + +ENTRY(arch_very_early_init) +#ifdef CONFIG_ARMADA_38X + /* + * Only with disabled MMU its possible to switch the base + * register address on Armada 38x. Without this the SDRAM + * located at >= 0x4000.0000 is also not accessible, as its + * still locked to cache. + */ + mrc p15, 0, r0, c1, c0, 0 + bic r0, #1 + mcr p15, 0, r0, c1, c0, 0 +#endif + + /* Move internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE */ + ldr r0, =SOC_REGS_PHY_BASE + ldr r1, =INTREG_BASE_ADDR_REG + str r0, [r1] + add r0, r0, #0xC000 + mcr p15, 4, r0, c15, c0 + + bx lr +ENDPROC(arch_very_early_init)
Moving of internal registers from INTREG_BASE_ADDR_REG to SOC_REGS_PHY_BASE needs to be done very early, prior calling any function which may touch internal registers, like debug_uart_init(). So do it earlier in arch_very_early_init() instead of arch_cpu_init(). Movement is done in proper U-Boot, not in SPL. SPL may return to bootrom and bootrom requires internal registers at (old) expected location. Signed-off-by: Pali Rohár <pali@kernel.org> --- arch/arm/mach-mvebu/Kconfig | 1 + arch/arm/mach-mvebu/Makefile | 1 + arch/arm/mach-mvebu/cpu.c | 31 ------------------------------- arch/arm/mach-mvebu/lowlevel.S | 27 +++++++++++++++++++++++++++ 4 files changed, 29 insertions(+), 31 deletions(-) create mode 100644 arch/arm/mach-mvebu/lowlevel.S