diff mbox series

[RFC,u-boot-mvebu] arm: kirkwood: Move internal registers in arch_very_early_init() function

Message ID 20230311105701.19090-1-pali@kernel.org
State Accepted
Commit ae60fc6902c262de8f4168b1dada0e1fe0ec5692
Delegated to: Stefan Roese
Headers show
Series [RFC,u-boot-mvebu] arm: kirkwood: Move internal registers in arch_very_early_init() function | expand

Commit Message

Pali Rohár March 11, 2023, 10:57 a.m. UTC
Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
internal registers in arch_very_early_init() function") but for kirkwood.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Hello! Please test this RFC patch on more Kirkwood boards if there is
any issue with it.
---
 arch/arm/mach-kirkwood/Kconfig    |  2 ++
 arch/arm/mach-kirkwood/Makefile   |  1 +
 arch/arm/mach-kirkwood/cpu.c      |  3 ---
 arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
 4 files changed, 15 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/mach-kirkwood/lowlevel.S

Comments

Tony Dinh March 11, 2023, 11:47 p.m. UTC | #1
Hi Pali,

On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
>
> Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> internal registers in arch_very_early_init() function") but for kirkwood.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Hello! Please test this RFC patch on more Kirkwood boards if there is
> any issue with it.

I've run a regression test with this patch (Debug UART is not
enabled). And everything was OK. No change in behavior.

However, when I turned on Debug UART on the nsa310s (88F6702) board,
and ran with kwboot, it froze right away upon starting. Unrelated to
this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
perhaps sometime late December to present, but I did not have time to
track it down. Here is the last thread that I had Debug UART working
on the Pogo V4 (88F6192):
https://lists.denx.de/pipermail/u-boot/2022-December/502605.html

For reference, here is my local patch to configure the NSA310S, in
addition to this patch.

diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
index 76839e62dd..4bef35d576 100644
--- a/configs/nsa310s_defconfig
+++ b/configs/nsa310s_defconfig
@@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
 CONFIG_ENV_OFFSET=0xE0000
 CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
 CONFIG_SYS_PROMPT="NSA310s> "
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=250000000
 CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
 CONFIG_SYS_LOAD_ADDR=0x800000
+CONFIG_DEBUG_UART=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_PREBOOT=y
@@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
 CONFIG_PHY_MARVELL=y
 CONFIG_MVGBE=y
 CONFIG_MII=y
+CONFIG_DEBUG_UART_ANNOUNCE=y
 CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_UBIFS_SILENCE_MSG=y

diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
b/arch/arm/dts/kirkwood-nsa310s.dts
index 09ee76c2a2..2df45aa6da 100644
--- a/arch/arm/dts/kirkwood-nsa310s.dts
+++ b/arch/arm/dts/kirkwood-nsa310s.dts
@@ -22,6 +22,10 @@
                reg = <0x00000000 0x10000000>;
        };

+       aliases {
+               serial0 = &uart0;
+       };
+
        chosen {
                bootargs = "console=ttyS0,115200";
                stdout-path = &uart0;
@@ -317,3 +321,8 @@
 &pcie0 {
        status = "okay";
 };
+
+&uart0 {
+        status = "okay";
+        u-boot,dm-pre-reloc;
+};

If anybody is available to test this patch with Debug UART enabled, it
would be great. I'll wait a few days and if there is no suggestion, I
would do a bisect from Dec 19th.

Thanks,
Tony

> ---
>  arch/arm/mach-kirkwood/Kconfig    |  2 ++
>  arch/arm/mach-kirkwood/Makefile   |  1 +
>  arch/arm/mach-kirkwood/cpu.c      |  3 ---
>  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
>  4 files changed, 15 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
>
> diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> index c8a193dd4cdf..ba39e9ae416e 100644
> --- a/arch/arm/mach-kirkwood/Kconfig
> +++ b/arch/arm/mach-kirkwood/Kconfig
> @@ -5,9 +5,11 @@ config FEROCEON_88FR131
>
>  config KW88F6192
>         bool
> +       select ARCH_VERY_EARLY_INIT
>
>  config KW88F6281
>         bool
> +       select ARCH_VERY_EARLY_INIT
>
>  config SHEEVA_88SV131
>         bool
> diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> index 3b2eef8d5419..0fb5a2326f5f 100644
> --- a/arch/arm/mach-kirkwood/Makefile
> +++ b/arch/arm/mach-kirkwood/Makefile
> @@ -6,6 +6,7 @@
>
>  obj-y  = cpu.o
>  obj-y  += cache.o
> +obj-y  += lowlevel.o
>  obj-y  += mpp.o
>
>  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index df3e8f11782a..2b493b36c20d 100644
> --- a/arch/arm/mach-kirkwood/cpu.c
> +++ b/arch/arm/mach-kirkwood/cpu.c
> @@ -189,9 +189,6 @@ int arch_cpu_init(void)
>         struct kwcpu_registers *cpureg =
>                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
>
> -       /* Linux expects the internal registers to be at 0xf1000000 */
> -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> -
>         /* Enable and invalidate L2 cache in write through mode */
>         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
>         invalidate_l2_cache();
> diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> new file mode 100644
> index 000000000000..3b339f97f056
> --- /dev/null
> +++ b/arch/arm/mach-kirkwood/lowlevel.S
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(arch_very_early_init)
> +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> +       ldr     r0, =KW_REGS_PHY_BASE
> +       ldr     r1, =KW_OFFSET_REG
> +       str     r0, [r1]
> +       bx      lr
> +ENDPROC(arch_very_early_init)
> --
> 2.20.1
>
Pali Rohár March 12, 2023, 12:29 a.m. UTC | #2
On Saturday 11 March 2023 15:47:29 Tony Dinh wrote:
> Hi Pali,
> 
> On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> > internal registers in arch_very_early_init() function") but for kirkwood.
> >
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> > Hello! Please test this RFC patch on more Kirkwood boards if there is
> > any issue with it.
> 
> I've run a regression test with this patch (Debug UART is not
> enabled). And everything was OK. No change in behavior.

Ok! Thanks for testing.

> However, when I turned on Debug UART on the nsa310s (88F6702) board,
> and ran with kwboot, it froze right away upon starting. Unrelated to
> this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
> perhaps sometime late December to present, but I did not have time to
> track it down. Here is the last thread that I had Debug UART working
> on the Pogo V4 (88F6192):
> https://lists.denx.de/pipermail/u-boot/2022-December/502605.html
> 
> For reference, here is my local patch to configure the NSA310S, in
> addition to this patch.

I do not know what could broke it, but I see there two things which you
could change in your configuration.

1. Try to set UART shift register to 2. Not sure what is default but in
   kirkwood.dtsi file it is 2.

   CONFIG_DEBUG_UART_SHIFT=2

2. Recheck UART clock. In kw88f6281.h is defined that TCLK is either
   166666667 or 200000000. And it is configured by strapping pins. TCLK
   cannot be 250000000 on 6281 for sure. I do not know to which clock
   is connected UART base clock, but my guess is that it is TCLK.

> diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> index 76839e62dd..4bef35d576 100644
> --- a/configs/nsa310s_defconfig
> +++ b/configs/nsa310s_defconfig
> @@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
>  CONFIG_ENV_OFFSET=0xE0000
>  CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
>  CONFIG_SYS_PROMPT="NSA310s> "
> +CONFIG_DEBUG_UART_BASE=0xf1012000
> +CONFIG_DEBUG_UART_CLOCK=250000000
>  CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
>  CONFIG_SYS_LOAD_ADDR=0x800000
> +CONFIG_DEBUG_UART=y
>  CONFIG_DISTRO_DEFAULTS=y
>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_PREBOOT=y
> @@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
>  CONFIG_PHY_MARVELL=y
>  CONFIG_MVGBE=y
>  CONFIG_MII=y
> +CONFIG_DEBUG_UART_ANNOUNCE=y
>  CONFIG_USB=y
>  CONFIG_USB_EHCI_HCD=y
>  CONFIG_UBIFS_SILENCE_MSG=y
> 
> diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> b/arch/arm/dts/kirkwood-nsa310s.dts
> index 09ee76c2a2..2df45aa6da 100644
> --- a/arch/arm/dts/kirkwood-nsa310s.dts
> +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> @@ -22,6 +22,10 @@
>                 reg = <0x00000000 0x10000000>;
>         };
> 
> +       aliases {
> +               serial0 = &uart0;
> +       };
> +
>         chosen {
>                 bootargs = "console=ttyS0,115200";
>                 stdout-path = &uart0;
> @@ -317,3 +321,8 @@
>  &pcie0 {
>         status = "okay";
>  };
> +
> +&uart0 {
> +        status = "okay";
> +        u-boot,dm-pre-reloc;
> +};
> 
> If anybody is available to test this patch with Debug UART enabled, it
> would be great. I'll wait a few days and if there is no suggestion, I
> would do a bisect from Dec 19th.
> 
> Thanks,
> Tony
> 
> > ---
> >  arch/arm/mach-kirkwood/Kconfig    |  2 ++
> >  arch/arm/mach-kirkwood/Makefile   |  1 +
> >  arch/arm/mach-kirkwood/cpu.c      |  3 ---
> >  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
> >  4 files changed, 15 insertions(+), 3 deletions(-)
> >  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> >
> > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > index c8a193dd4cdf..ba39e9ae416e 100644
> > --- a/arch/arm/mach-kirkwood/Kconfig
> > +++ b/arch/arm/mach-kirkwood/Kconfig
> > @@ -5,9 +5,11 @@ config FEROCEON_88FR131
> >
> >  config KW88F6192
> >         bool
> > +       select ARCH_VERY_EARLY_INIT
> >
> >  config KW88F6281
> >         bool
> > +       select ARCH_VERY_EARLY_INIT
> >
> >  config SHEEVA_88SV131
> >         bool
> > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > index 3b2eef8d5419..0fb5a2326f5f 100644
> > --- a/arch/arm/mach-kirkwood/Makefile
> > +++ b/arch/arm/mach-kirkwood/Makefile
> > @@ -6,6 +6,7 @@
> >
> >  obj-y  = cpu.o
> >  obj-y  += cache.o
> > +obj-y  += lowlevel.o
> >  obj-y  += mpp.o
> >
> >  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> > index df3e8f11782a..2b493b36c20d 100644
> > --- a/arch/arm/mach-kirkwood/cpu.c
> > +++ b/arch/arm/mach-kirkwood/cpu.c
> > @@ -189,9 +189,6 @@ int arch_cpu_init(void)
> >         struct kwcpu_registers *cpureg =
> >                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
> >
> > -       /* Linux expects the internal registers to be at 0xf1000000 */
> > -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> > -
> >         /* Enable and invalidate L2 cache in write through mode */
> >         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
> >         invalidate_l2_cache();
> > diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> > new file mode 100644
> > index 000000000000..3b339f97f056
> > --- /dev/null
> > +++ b/arch/arm/mach-kirkwood/lowlevel.S
> > @@ -0,0 +1,12 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +
> > +#include <config.h>
> > +#include <linux/linkage.h>
> > +
> > +ENTRY(arch_very_early_init)
> > +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> > +       ldr     r0, =KW_REGS_PHY_BASE
> > +       ldr     r1, =KW_OFFSET_REG
> > +       str     r0, [r1]
> > +       bx      lr
> > +ENDPROC(arch_very_early_init)
> > --
> > 2.20.1
> >
Tony Dinh March 12, 2023, 9:30 p.m. UTC | #3
Hi Pali,

On Sat, Mar 11, 2023 at 4:29 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Saturday 11 March 2023 15:47:29 Tony Dinh wrote:
> > Hi Pali,
> >
> > On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> > > internal registers in arch_very_early_init() function") but for kirkwood.
> > >
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > > Hello! Please test this RFC patch on more Kirkwood boards if there is
> > > any issue with it.
> >
> > I've run a regression test with this patch (Debug UART is not
> > enabled). And everything was OK. No change in behavior.
>
> Ok! Thanks for testing.
>
> > However, when I turned on Debug UART on the nsa310s (88F6702) board,
> > and ran with kwboot, it froze right away upon starting. Unrelated to
> > this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
> > perhaps sometime late December to present, but I did not have time to
> > track it down. Here is the last thread that I had Debug UART working
> > on the Pogo V4 (88F6192):
> > https://lists.denx.de/pipermail/u-boot/2022-December/502605.html
> >
> > For reference, here is my local patch to configure the NSA310S, in
> > addition to this patch.
>
> I do not know what could broke it, but I see there two things which you
> could change in your configuration.
>
> 1. Try to set UART shift register to 2. Not sure what is default but in
>    kirkwood.dtsi file it is 2.
>
>    CONFIG_DEBUG_UART_SHIFT=2
>
> 2. Recheck UART clock. In kw88f6281.h is defined that TCLK is either
>    166666667 or 200000000. And it is configured by strapping pins. TCLK
>    cannot be 250000000 on 6281 for sure. I do not know to which clock
>    is connected UART base clock, but my guess is that it is TCLK.

Thanks for the suggestion. Indeed, the combination of
CONFIG_DEBUG_UART_CLOCK=166666667 and CONFIG_DEBUG_UART_SHIFT=2 works
as it should (88F6702/88F6192 TCLK=166666667).

Without setting CONFIG_DEBUG_UART_SHIFT in defconfig, it is default to 0.

CONFIG_DEBUG_UART_SHIFT=2  is the key here, because U-Boot does not
hang with CONFIG_DEBUG_UART_CLOCK=250000000. The board runs OK, but
the Debug UART announcement output is gibberish.

So I guess back on the Dec 19th build, perhaps I was just lucky that
some internal values were used for Kirkwood, but not CONFIG_xxx.

Thanks,
Tony

> > diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> > index 76839e62dd..4bef35d576 100644
> > --- a/configs/nsa310s_defconfig
> > +++ b/configs/nsa310s_defconfig
> > @@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
> >  CONFIG_ENV_OFFSET=0xE0000
> >  CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
> >  CONFIG_SYS_PROMPT="NSA310s> "
> > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > +CONFIG_DEBUG_UART_CLOCK=250000000
> >  CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
> >  CONFIG_SYS_LOAD_ADDR=0x800000
> > +CONFIG_DEBUG_UART=y
> >  CONFIG_DISTRO_DEFAULTS=y
> >  CONFIG_BOOTDELAY=3
> >  CONFIG_USE_PREBOOT=y
> > @@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
> >  CONFIG_PHY_MARVELL=y
> >  CONFIG_MVGBE=y
> >  CONFIG_MII=y
> > +CONFIG_DEBUG_UART_ANNOUNCE=y
> >  CONFIG_USB=y
> >  CONFIG_USB_EHCI_HCD=y
> >  CONFIG_UBIFS_SILENCE_MSG=y
> >
> > diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> > b/arch/arm/dts/kirkwood-nsa310s.dts
> > index 09ee76c2a2..2df45aa6da 100644
> > --- a/arch/arm/dts/kirkwood-nsa310s.dts
> > +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> > @@ -22,6 +22,10 @@
> >                 reg = <0x00000000 0x10000000>;
> >         };
> >
> > +       aliases {
> > +               serial0 = &uart0;
> > +       };
> > +
> >         chosen {
> >                 bootargs = "console=ttyS0,115200";
> >                 stdout-path = &uart0;
> > @@ -317,3 +321,8 @@
> >  &pcie0 {
> >         status = "okay";
> >  };
> > +
> > +&uart0 {
> > +        status = "okay";
> > +        u-boot,dm-pre-reloc;
> > +};
> >
> > If anybody is available to test this patch with Debug UART enabled, it
> > would be great. I'll wait a few days and if there is no suggestion, I
> > would do a bisect from Dec 19th.
> >
> > Thanks,
> > Tony
> >
> > > ---
> > >  arch/arm/mach-kirkwood/Kconfig    |  2 ++
> > >  arch/arm/mach-kirkwood/Makefile   |  1 +
> > >  arch/arm/mach-kirkwood/cpu.c      |  3 ---
> > >  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
> > >  4 files changed, 15 insertions(+), 3 deletions(-)
> > >  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> > >
> > > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > > index c8a193dd4cdf..ba39e9ae416e 100644
> > > --- a/arch/arm/mach-kirkwood/Kconfig
> > > +++ b/arch/arm/mach-kirkwood/Kconfig
> > > @@ -5,9 +5,11 @@ config FEROCEON_88FR131
> > >
> > >  config KW88F6192
> > >         bool
> > > +       select ARCH_VERY_EARLY_INIT
> > >
> > >  config KW88F6281
> > >         bool
> > > +       select ARCH_VERY_EARLY_INIT
> > >
> > >  config SHEEVA_88SV131
> > >         bool
> > > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > > index 3b2eef8d5419..0fb5a2326f5f 100644
> > > --- a/arch/arm/mach-kirkwood/Makefile
> > > +++ b/arch/arm/mach-kirkwood/Makefile
> > > @@ -6,6 +6,7 @@
> > >
> > >  obj-y  = cpu.o
> > >  obj-y  += cache.o
> > > +obj-y  += lowlevel.o
> > >  obj-y  += mpp.o
> > >
> > >  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> > > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> > > index df3e8f11782a..2b493b36c20d 100644
> > > --- a/arch/arm/mach-kirkwood/cpu.c
> > > +++ b/arch/arm/mach-kirkwood/cpu.c
> > > @@ -189,9 +189,6 @@ int arch_cpu_init(void)
> > >         struct kwcpu_registers *cpureg =
> > >                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
> > >
> > > -       /* Linux expects the internal registers to be at 0xf1000000 */
> > > -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> > > -
> > >         /* Enable and invalidate L2 cache in write through mode */
> > >         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
> > >         invalidate_l2_cache();
> > > diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> > > new file mode 100644
> > > index 000000000000..3b339f97f056
> > > --- /dev/null
> > > +++ b/arch/arm/mach-kirkwood/lowlevel.S
> > > @@ -0,0 +1,12 @@
> > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > +
> > > +#include <config.h>
> > > +#include <linux/linkage.h>
> > > +
> > > +ENTRY(arch_very_early_init)
> > > +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> > > +       ldr     r0, =KW_REGS_PHY_BASE
> > > +       ldr     r1, =KW_OFFSET_REG
> > > +       str     r0, [r1]
> > > +       bx      lr
> > > +ENDPROC(arch_very_early_init)
> > > --
> > > 2.20.1
> > >
Pali Rohár March 12, 2023, 9:38 p.m. UTC | #4
On Sunday 12 March 2023 14:30:29 Tony Dinh wrote:
> Hi Pali,
> 
> On Sat, Mar 11, 2023 at 4:29 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Saturday 11 March 2023 15:47:29 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> > > > internal registers in arch_very_early_init() function") but for kirkwood.
> > > >
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > ---
> > > > Hello! Please test this RFC patch on more Kirkwood boards if there is
> > > > any issue with it.
> > >
> > > I've run a regression test with this patch (Debug UART is not
> > > enabled). And everything was OK. No change in behavior.
> >
> > Ok! Thanks for testing.
> >
> > > However, when I turned on Debug UART on the nsa310s (88F6702) board,
> > > and ran with kwboot, it froze right away upon starting. Unrelated to
> > > this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
> > > perhaps sometime late December to present, but I did not have time to
> > > track it down. Here is the last thread that I had Debug UART working
> > > on the Pogo V4 (88F6192):
> > > https://lists.denx.de/pipermail/u-boot/2022-December/502605.html
> > >
> > > For reference, here is my local patch to configure the NSA310S, in
> > > addition to this patch.
> >
> > I do not know what could broke it, but I see there two things which you
> > could change in your configuration.
> >
> > 1. Try to set UART shift register to 2. Not sure what is default but in
> >    kirkwood.dtsi file it is 2.
> >
> >    CONFIG_DEBUG_UART_SHIFT=2
> >
> > 2. Recheck UART clock. In kw88f6281.h is defined that TCLK is either
> >    166666667 or 200000000. And it is configured by strapping pins. TCLK
> >    cannot be 250000000 on 6281 for sure. I do not know to which clock
> >    is connected UART base clock, but my guess is that it is TCLK.
> 
> Thanks for the suggestion. Indeed, the combination of
> CONFIG_DEBUG_UART_CLOCK=166666667 and CONFIG_DEBUG_UART_SHIFT=2 works
> as it should (88F6702/88F6192 TCLK=166666667).
> 
> Without setting CONFIG_DEBUG_UART_SHIFT in defconfig, it is default to 0.
> 
> CONFIG_DEBUG_UART_SHIFT=2  is the key here, because U-Boot does not
> hang with CONFIG_DEBUG_UART_CLOCK=250000000. The board runs OK, but
> the Debug UART announcement output is gibberish.

Perfect! So ideally send a patch with adding those options into
nsa310s_defconfig. I guess that CONFIG_DEBUG_UART_ANNOUNCE is not
needed.

> So I guess back on the Dec 19th build, perhaps I was just lucky that
> some internal values were used for Kirkwood, but not CONFIG_xxx.
> 
> Thanks,
> Tony
> 
> > > diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> > > index 76839e62dd..4bef35d576 100644
> > > --- a/configs/nsa310s_defconfig
> > > +++ b/configs/nsa310s_defconfig
> > > @@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
> > >  CONFIG_ENV_OFFSET=0xE0000
> > >  CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
> > >  CONFIG_SYS_PROMPT="NSA310s> "
> > > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > > +CONFIG_DEBUG_UART_CLOCK=250000000
> > >  CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
> > >  CONFIG_SYS_LOAD_ADDR=0x800000
> > > +CONFIG_DEBUG_UART=y
> > >  CONFIG_DISTRO_DEFAULTS=y
> > >  CONFIG_BOOTDELAY=3
> > >  CONFIG_USE_PREBOOT=y
> > > @@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
> > >  CONFIG_PHY_MARVELL=y
> > >  CONFIG_MVGBE=y
> > >  CONFIG_MII=y
> > > +CONFIG_DEBUG_UART_ANNOUNCE=y
> > >  CONFIG_USB=y
> > >  CONFIG_USB_EHCI_HCD=y
> > >  CONFIG_UBIFS_SILENCE_MSG=y
> > >
> > > diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> > > b/arch/arm/dts/kirkwood-nsa310s.dts
> > > index 09ee76c2a2..2df45aa6da 100644
> > > --- a/arch/arm/dts/kirkwood-nsa310s.dts
> > > +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> > > @@ -22,6 +22,10 @@
> > >                 reg = <0x00000000 0x10000000>;
> > >         };
> > >
> > > +       aliases {
> > > +               serial0 = &uart0;
> > > +       };
> > > +
> > >         chosen {
> > >                 bootargs = "console=ttyS0,115200";
> > >                 stdout-path = &uart0;
> > > @@ -317,3 +321,8 @@
> > >  &pcie0 {
> > >         status = "okay";
> > >  };
> > > +
> > > +&uart0 {
> > > +        status = "okay";
> > > +        u-boot,dm-pre-reloc;
> > > +};
> > >
> > > If anybody is available to test this patch with Debug UART enabled, it
> > > would be great. I'll wait a few days and if there is no suggestion, I
> > > would do a bisect from Dec 19th.
> > >
> > > Thanks,
> > > Tony
> > >
> > > > ---
> > > >  arch/arm/mach-kirkwood/Kconfig    |  2 ++
> > > >  arch/arm/mach-kirkwood/Makefile   |  1 +
> > > >  arch/arm/mach-kirkwood/cpu.c      |  3 ---
> > > >  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
> > > >  4 files changed, 15 insertions(+), 3 deletions(-)
> > > >  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> > > >
> > > > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > > > index c8a193dd4cdf..ba39e9ae416e 100644
> > > > --- a/arch/arm/mach-kirkwood/Kconfig
> > > > +++ b/arch/arm/mach-kirkwood/Kconfig
> > > > @@ -5,9 +5,11 @@ config FEROCEON_88FR131
> > > >
> > > >  config KW88F6192
> > > >         bool
> > > > +       select ARCH_VERY_EARLY_INIT
> > > >
> > > >  config KW88F6281
> > > >         bool
> > > > +       select ARCH_VERY_EARLY_INIT
> > > >
> > > >  config SHEEVA_88SV131
> > > >         bool
> > > > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > > > index 3b2eef8d5419..0fb5a2326f5f 100644
> > > > --- a/arch/arm/mach-kirkwood/Makefile
> > > > +++ b/arch/arm/mach-kirkwood/Makefile
> > > > @@ -6,6 +6,7 @@
> > > >
> > > >  obj-y  = cpu.o
> > > >  obj-y  += cache.o
> > > > +obj-y  += lowlevel.o
> > > >  obj-y  += mpp.o
> > > >
> > > >  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> > > > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> > > > index df3e8f11782a..2b493b36c20d 100644
> > > > --- a/arch/arm/mach-kirkwood/cpu.c
> > > > +++ b/arch/arm/mach-kirkwood/cpu.c
> > > > @@ -189,9 +189,6 @@ int arch_cpu_init(void)
> > > >         struct kwcpu_registers *cpureg =
> > > >                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
> > > >
> > > > -       /* Linux expects the internal registers to be at 0xf1000000 */
> > > > -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> > > > -
> > > >         /* Enable and invalidate L2 cache in write through mode */
> > > >         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
> > > >         invalidate_l2_cache();
> > > > diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> > > > new file mode 100644
> > > > index 000000000000..3b339f97f056
> > > > --- /dev/null
> > > > +++ b/arch/arm/mach-kirkwood/lowlevel.S
> > > > @@ -0,0 +1,12 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > +
> > > > +#include <config.h>
> > > > +#include <linux/linkage.h>
> > > > +
> > > > +ENTRY(arch_very_early_init)
> > > > +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> > > > +       ldr     r0, =KW_REGS_PHY_BASE
> > > > +       ldr     r1, =KW_OFFSET_REG
> > > > +       str     r0, [r1]
> > > > +       bx      lr
> > > > +ENDPROC(arch_very_early_init)
> > > > --
> > > > 2.20.1
> > > >
Tony Dinh March 13, 2023, 12:12 a.m. UTC | #5
Hi Pali,

On Sun, Mar 12, 2023 at 2:38 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Sunday 12 March 2023 14:30:29 Tony Dinh wrote:
> > Hi Pali,
> >
> > On Sat, Mar 11, 2023 at 4:29 PM Pali Rohár <pali@kernel.org> wrote:
> > >
> > > On Saturday 11 March 2023 15:47:29 Tony Dinh wrote:
> > > > Hi Pali,
> > > >
> > > > On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
> > > > >
> > > > > Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> > > > > internal registers in arch_very_early_init() function") but for kirkwood.
> > > > >
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > ---
> > > > > Hello! Please test this RFC patch on more Kirkwood boards if there is
> > > > > any issue with it.
> > > >
> > > > I've run a regression test with this patch (Debug UART is not
> > > > enabled). And everything was OK. No change in behavior.
> > >
> > > Ok! Thanks for testing.
> > >
> > > > However, when I turned on Debug UART on the nsa310s (88F6702) board,
> > > > and ran with kwboot, it froze right away upon starting. Unrelated to
> > > > this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
> > > > perhaps sometime late December to present, but I did not have time to
> > > > track it down. Here is the last thread that I had Debug UART working
> > > > on the Pogo V4 (88F6192):
> > > > https://lists.denx.de/pipermail/u-boot/2022-December/502605.html
> > > >
> > > > For reference, here is my local patch to configure the NSA310S, in
> > > > addition to this patch.
> > >
> > > I do not know what could broke it, but I see there two things which you
> > > could change in your configuration.
> > >
> > > 1. Try to set UART shift register to 2. Not sure what is default but in
> > >    kirkwood.dtsi file it is 2.
> > >
> > >    CONFIG_DEBUG_UART_SHIFT=2
> > >
> > > 2. Recheck UART clock. In kw88f6281.h is defined that TCLK is either
> > >    166666667 or 200000000. And it is configured by strapping pins. TCLK
> > >    cannot be 250000000 on 6281 for sure. I do not know to which clock
> > >    is connected UART base clock, but my guess is that it is TCLK.
> >
> > Thanks for the suggestion. Indeed, the combination of
> > CONFIG_DEBUG_UART_CLOCK=166666667 and CONFIG_DEBUG_UART_SHIFT=2 works
> > as it should (88F6702/88F6192 TCLK=166666667).
> >
> > Without setting CONFIG_DEBUG_UART_SHIFT in defconfig, it is default to 0.
> >
> > CONFIG_DEBUG_UART_SHIFT=2  is the key here, because U-Boot does not
> > hang with CONFIG_DEBUG_UART_CLOCK=250000000. The board runs OK, but
> > the Debug UART announcement output is gibberish.
>
> Perfect! So ideally send a patch with adding those options into
> nsa310s_defconfig. I guess that CONFIG_DEBUG_UART_ANNOUNCE is not
> needed.

Sure, I will do that. In the meantime, I ran some more tests and here
are some interesting results (thanks to Debug UART working now).

1. I've confirmed that, at the moment, we need your patch to see
DEBUG_UART early output for Kirkwood. Without it, with  DEBUG_UART
configured correctly, I don't see any output on this board. It will
run fine but behaving like there is no Debug UART enabled.

2. There is no need for the u-boot,dm-pre-reloc tag on Kirkwood
boards. The serial-uclass function serial_check_stdout() binds it
anyway ff the console is not marked to be bound before relocation:
https://github.com/u-boot/u-boot/blob/master/drivers/serial/serial-uclass.c#L67

3. The previous problem we saw with the Pogo V4 (Kirwood 6192) DM
serial might be related to all this. Hopefully with your patch I can
track it down further.

Thanks,
Tony

>
> > So I guess back on the Dec 19th build, perhaps I was just lucky that
> > some internal values were used for Kirkwood, but not CONFIG_xxx.
> >
> > Thanks,
> > Tony
> >
> > > > diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> > > > index 76839e62dd..4bef35d576 100644
> > > > --- a/configs/nsa310s_defconfig
> > > > +++ b/configs/nsa310s_defconfig
> > > > @@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
> > > >  CONFIG_ENV_OFFSET=0xE0000
> > > >  CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
> > > >  CONFIG_SYS_PROMPT="NSA310s> "
> > > > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > > > +CONFIG_DEBUG_UART_CLOCK=250000000
> > > >  CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
> > > >  CONFIG_SYS_LOAD_ADDR=0x800000
> > > > +CONFIG_DEBUG_UART=y
> > > >  CONFIG_DISTRO_DEFAULTS=y
> > > >  CONFIG_BOOTDELAY=3
> > > >  CONFIG_USE_PREBOOT=y
> > > > @@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
> > > >  CONFIG_PHY_MARVELL=y
> > > >  CONFIG_MVGBE=y
> > > >  CONFIG_MII=y
> > > > +CONFIG_DEBUG_UART_ANNOUNCE=y
> > > >  CONFIG_USB=y
> > > >  CONFIG_USB_EHCI_HCD=y
> > > >  CONFIG_UBIFS_SILENCE_MSG=y
> > > >
> > > > diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > index 09ee76c2a2..2df45aa6da 100644
> > > > --- a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > @@ -22,6 +22,10 @@
> > > >                 reg = <0x00000000 0x10000000>;
> > > >         };
> > > >
> > > > +       aliases {
> > > > +               serial0 = &uart0;
> > > > +       };
> > > > +
> > > >         chosen {
> > > >                 bootargs = "console=ttyS0,115200";
> > > >                 stdout-path = &uart0;
> > > > @@ -317,3 +321,8 @@
> > > >  &pcie0 {
> > > >         status = "okay";
> > > >  };
> > > > +
> > > > +&uart0 {
> > > > +        status = "okay";
> > > > +        u-boot,dm-pre-reloc;
> > > > +};
> > > >
> > > > If anybody is available to test this patch with Debug UART enabled, it
> > > > would be great. I'll wait a few days and if there is no suggestion, I
> > > > would do a bisect from Dec 19th.
> > > >
> > > > Thanks,
> > > > Tony
> > > >
> > > > > ---
> > > > >  arch/arm/mach-kirkwood/Kconfig    |  2 ++
> > > > >  arch/arm/mach-kirkwood/Makefile   |  1 +
> > > > >  arch/arm/mach-kirkwood/cpu.c      |  3 ---
> > > > >  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
> > > > >  4 files changed, 15 insertions(+), 3 deletions(-)
> > > > >  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> > > > >
> > > > > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > > > > index c8a193dd4cdf..ba39e9ae416e 100644
> > > > > --- a/arch/arm/mach-kirkwood/Kconfig
> > > > > +++ b/arch/arm/mach-kirkwood/Kconfig
> > > > > @@ -5,9 +5,11 @@ config FEROCEON_88FR131
> > > > >
> > > > >  config KW88F6192
> > > > >         bool
> > > > > +       select ARCH_VERY_EARLY_INIT
> > > > >
> > > > >  config KW88F6281
> > > > >         bool
> > > > > +       select ARCH_VERY_EARLY_INIT
> > > > >
> > > > >  config SHEEVA_88SV131
> > > > >         bool
> > > > > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > > > > index 3b2eef8d5419..0fb5a2326f5f 100644
> > > > > --- a/arch/arm/mach-kirkwood/Makefile
> > > > > +++ b/arch/arm/mach-kirkwood/Makefile
> > > > > @@ -6,6 +6,7 @@
> > > > >
> > > > >  obj-y  = cpu.o
> > > > >  obj-y  += cache.o
> > > > > +obj-y  += lowlevel.o
> > > > >  obj-y  += mpp.o
> > > > >
> > > > >  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> > > > > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> > > > > index df3e8f11782a..2b493b36c20d 100644
> > > > > --- a/arch/arm/mach-kirkwood/cpu.c
> > > > > +++ b/arch/arm/mach-kirkwood/cpu.c
> > > > > @@ -189,9 +189,6 @@ int arch_cpu_init(void)
> > > > >         struct kwcpu_registers *cpureg =
> > > > >                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
> > > > >
> > > > > -       /* Linux expects the internal registers to be at 0xf1000000 */
> > > > > -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> > > > > -
> > > > >         /* Enable and invalidate L2 cache in write through mode */
> > > > >         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
> > > > >         invalidate_l2_cache();
> > > > > diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> > > > > new file mode 100644
> > > > > index 000000000000..3b339f97f056
> > > > > --- /dev/null
> > > > > +++ b/arch/arm/mach-kirkwood/lowlevel.S
> > > > > @@ -0,0 +1,12 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +
> > > > > +#include <config.h>
> > > > > +#include <linux/linkage.h>
> > > > > +
> > > > > +ENTRY(arch_very_early_init)
> > > > > +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> > > > > +       ldr     r0, =KW_REGS_PHY_BASE
> > > > > +       ldr     r1, =KW_OFFSET_REG
> > > > > +       str     r0, [r1]
> > > > > +       bx      lr
> > > > > +ENDPROC(arch_very_early_init)
> > > > > --
> > > > > 2.20.1
> > > > >
Tony Dinh March 14, 2023, 6:02 a.m. UTC | #6
On Sun, Mar 12, 2023 at 5:12 PM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Pali,
>
> On Sun, Mar 12, 2023 at 2:38 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Sunday 12 March 2023 14:30:29 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > On Sat, Mar 11, 2023 at 4:29 PM Pali Rohár <pali@kernel.org> wrote:
> > > >
> > > > On Saturday 11 March 2023 15:47:29 Tony Dinh wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Sat, Mar 11, 2023 at 2:57 AM Pali Rohár <pali@kernel.org> wrote:
> > > > > >
> > > > > > Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> > > > > > internal registers in arch_very_early_init() function") but for kirkwood.
> > > > > >
> > > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > > ---
> > > > > > Hello! Please test this RFC patch on more Kirkwood boards if there is
> > > > > > any issue with it.
> > > > >
> > > > > I've run a regression test with this patch (Debug UART is not
> > > > > enabled). And everything was OK. No change in behavior.
> > > >
> > > > Ok! Thanks for testing.
> > > >
> > > > > However, when I turned on Debug UART on the nsa310s (88F6702) board,
> > > > > and ran with kwboot, it froze right away upon starting. Unrelated to
> > > > > this patch, I believe DEBUG_UART has been broken for Kirkwood lately,
> > > > > perhaps sometime late December to present, but I did not have time to
> > > > > track it down. Here is the last thread that I had Debug UART working
> > > > > on the Pogo V4 (88F6192):
> > > > > https://lists.denx.de/pipermail/u-boot/2022-December/502605.html
> > > > >
> > > > > For reference, here is my local patch to configure the NSA310S, in
> > > > > addition to this patch.
> > > >
> > > > I do not know what could broke it, but I see there two things which you
> > > > could change in your configuration.
> > > >
> > > > 1. Try to set UART shift register to 2. Not sure what is default but in
> > > >    kirkwood.dtsi file it is 2.
> > > >
> > > >    CONFIG_DEBUG_UART_SHIFT=2
> > > >
> > > > 2. Recheck UART clock. In kw88f6281.h is defined that TCLK is either
> > > >    166666667 or 200000000. And it is configured by strapping pins. TCLK
> > > >    cannot be 250000000 on 6281 for sure. I do not know to which clock
> > > >    is connected UART base clock, but my guess is that it is TCLK.
> > >
> > > Thanks for the suggestion. Indeed, the combination of
> > > CONFIG_DEBUG_UART_CLOCK=166666667 and CONFIG_DEBUG_UART_SHIFT=2 works
> > > as it should (88F6702/88F6192 TCLK=166666667).
> > >
> > > Without setting CONFIG_DEBUG_UART_SHIFT in defconfig, it is default to 0.
> > >
> > > CONFIG_DEBUG_UART_SHIFT=2  is the key here, because U-Boot does not
> > > hang with CONFIG_DEBUG_UART_CLOCK=250000000. The board runs OK, but
> > > the Debug UART announcement output is gibberish.
> >
> > Perfect! So ideally send a patch with adding those options into
> > nsa310s_defconfig. I guess that CONFIG_DEBUG_UART_ANNOUNCE is not
> > needed.
>
> Sure, I will do that. In the meantime, I ran some more tests and here
> are some interesting results (thanks to Debug UART working now).
>
> 1. I've confirmed that, at the moment, we need your patch to see
> DEBUG_UART early output for Kirkwood. Without it, with  DEBUG_UART
> configured correctly, I don't see any output on this board. It will
> run fine but behaving like there is no Debug UART enabled.
>
> 2. There is no need for the u-boot,dm-pre-reloc tag on Kirkwood
> boards. The serial-uclass function serial_check_stdout() binds it
> anyway ff the console is not marked to be bound before relocation:
> https://github.com/u-boot/u-boot/blob/master/drivers/serial/serial-uclass.c#L67
>
> 3. The previous problem we saw with the Pogo V4 (Kirwood 6192) DM
> serial might be related to all this. Hopefully with your patch I can
> track it down further.
>
> Thanks,
> Tony
>
> >
> > > So I guess back on the Dec 19th build, perhaps I was just lucky that
> > > some internal values were used for Kirkwood, but not CONFIG_xxx.
> > >
> > > Thanks,
> > > Tony
> > >
> > > > > diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> > > > > index 76839e62dd..4bef35d576 100644
> > > > > --- a/configs/nsa310s_defconfig
> > > > > +++ b/configs/nsa310s_defconfig
> > > > > @@ -15,8 +15,11 @@ CONFIG_ENV_SIZE=0x20000
> > > > >  CONFIG_ENV_OFFSET=0xE0000
> > > > >  CONFIG_DEFAULT_DEVICE_TREE="kirkwood-nsa310s"
> > > > >  CONFIG_SYS_PROMPT="NSA310s> "
> > > > > +CONFIG_DEBUG_UART_BASE=0xf1012000
> > > > > +CONFIG_DEBUG_UART_CLOCK=250000000
> > > > >  CONFIG_IDENT_STRING="\nZyXEL NSA310S/320S 1/2-Bay Power Media Server"
> > > > >  CONFIG_SYS_LOAD_ADDR=0x800000
> > > > > +CONFIG_DEBUG_UART=y
> > > > >  CONFIG_DISTRO_DEFAULTS=y
> > > > >  CONFIG_BOOTDELAY=3
> > > > >  CONFIG_USE_PREBOOT=y
> > > > > @@ -50,6 +53,7 @@ CONFIG_MTD_RAW_NAND=y
> > > > >  CONFIG_PHY_MARVELL=y
> > > > >  CONFIG_MVGBE=y
> > > > >  CONFIG_MII=y
> > > > > +CONFIG_DEBUG_UART_ANNOUNCE=y
> > > > >  CONFIG_USB=y
> > > > >  CONFIG_USB_EHCI_HCD=y
> > > > >  CONFIG_UBIFS_SILENCE_MSG=y
> > > > >
> > > > > diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > index 09ee76c2a2..2df45aa6da 100644
> > > > > --- a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > @@ -22,6 +22,10 @@
> > > > >                 reg = <0x00000000 0x10000000>;
> > > > >         };
> > > > >
> > > > > +       aliases {
> > > > > +               serial0 = &uart0;
> > > > > +       };
> > > > > +
> > > > >         chosen {
> > > > >                 bootargs = "console=ttyS0,115200";
> > > > >                 stdout-path = &uart0;
> > > > > @@ -317,3 +321,8 @@
> > > > >  &pcie0 {
> > > > >         status = "okay";
> > > > >  };
> > > > > +
> > > > > +&uart0 {
> > > > > +        status = "okay";
> > > > > +        u-boot,dm-pre-reloc;
> > > > > +};
> > > > >
> > > > > If anybody is available to test this patch with Debug UART enabled, it
> > > > > would be great. I'll wait a few days and if there is no suggestion, I
> > > > > would do a bisect from Dec 19th.
> > > > >
> > > > > Thanks,
> > > > > Tony
> > > > >
> > > > > > ---
> > > > > >  arch/arm/mach-kirkwood/Kconfig    |  2 ++
> > > > > >  arch/arm/mach-kirkwood/Makefile   |  1 +
> > > > > >  arch/arm/mach-kirkwood/cpu.c      |  3 ---
> > > > > >  arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
> > > > > >  4 files changed, 15 insertions(+), 3 deletions(-)
> > > > > >  create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> > > > > >
> > > > > > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > > > > > index c8a193dd4cdf..ba39e9ae416e 100644
> > > > > > --- a/arch/arm/mach-kirkwood/Kconfig
> > > > > > +++ b/arch/arm/mach-kirkwood/Kconfig
> > > > > > @@ -5,9 +5,11 @@ config FEROCEON_88FR131
> > > > > >
> > > > > >  config KW88F6192
> > > > > >         bool
> > > > > > +       select ARCH_VERY_EARLY_INIT
> > > > > >
> > > > > >  config KW88F6281
> > > > > >         bool
> > > > > > +       select ARCH_VERY_EARLY_INIT
> > > > > >
> > > > > >  config SHEEVA_88SV131
> > > > > >         bool
> > > > > > diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> > > > > > index 3b2eef8d5419..0fb5a2326f5f 100644
> > > > > > --- a/arch/arm/mach-kirkwood/Makefile
> > > > > > +++ b/arch/arm/mach-kirkwood/Makefile
> > > > > > @@ -6,6 +6,7 @@
> > > > > >
> > > > > >  obj-y  = cpu.o
> > > > > >  obj-y  += cache.o
> > > > > > +obj-y  += lowlevel.o
> > > > > >  obj-y  += mpp.o
> > > > > >
> > > > > >  # cpu.o and cache.o contain CP15 instructions which cannot be run in
> > > > > > diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> > > > > > index df3e8f11782a..2b493b36c20d 100644
> > > > > > --- a/arch/arm/mach-kirkwood/cpu.c
> > > > > > +++ b/arch/arm/mach-kirkwood/cpu.c
> > > > > > @@ -189,9 +189,6 @@ int arch_cpu_init(void)
> > > > > >         struct kwcpu_registers *cpureg =
> > > > > >                 (struct kwcpu_registers *)KW_CPU_REG_BASE;
> > > > > >
> > > > > > -       /* Linux expects the internal registers to be at 0xf1000000 */
> > > > > > -       writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> > > > > > -
> > > > > >         /* Enable and invalidate L2 cache in write through mode */
> > > > > >         writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
> > > > > >         invalidate_l2_cache();
> > > > > > diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> > > > > > new file mode 100644
> > > > > > index 000000000000..3b339f97f056
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm/mach-kirkwood/lowlevel.S
> > > > > > @@ -0,0 +1,12 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > > +
> > > > > > +#include <config.h>
> > > > > > +#include <linux/linkage.h>
> > > > > > +
> > > > > > +ENTRY(arch_very_early_init)
> > > > > > +       /* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> > > > > > +       ldr     r0, =KW_REGS_PHY_BASE
> > > > > > +       ldr     r1, =KW_OFFSET_REG
> > > > > > +       str     r0, [r1]
> > > > > > +       bx      lr
> > > > > > +ENDPROC(arch_very_early_init)
> > > > > > --
> > > > > > 2.20.1
> > > > > >

All checked out OK on these Kirkwood boards:

Zyxel NSA310S (88F6702)
Cloudengine  Pogo V4 (88F6192)
Seagate Goflex Home (88F6281)

Tested-by: Tony Dinh <mibodhi@gmail.com>

Thanks,
Tony
Stefan Roese March 16, 2023, 9:04 a.m. UTC | #7
On 3/11/23 11:57, Pali Rohár wrote:
> Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> internal registers in arch_very_early_init() function") but for kirkwood.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> Hello! Please test this RFC patch on more Kirkwood boards if there is
> any issue with it.
> ---
>   arch/arm/mach-kirkwood/Kconfig    |  2 ++
>   arch/arm/mach-kirkwood/Makefile   |  1 +
>   arch/arm/mach-kirkwood/cpu.c      |  3 ---
>   arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
>   4 files changed, 15 insertions(+), 3 deletions(-)
>   create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> 
> diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> index c8a193dd4cdf..ba39e9ae416e 100644
> --- a/arch/arm/mach-kirkwood/Kconfig
> +++ b/arch/arm/mach-kirkwood/Kconfig
> @@ -5,9 +5,11 @@ config FEROCEON_88FR131
>   
>   config KW88F6192
>   	bool
> +	select ARCH_VERY_EARLY_INIT
>   
>   config KW88F6281
>   	bool
> +	select ARCH_VERY_EARLY_INIT
>   
>   config SHEEVA_88SV131
>   	bool
> diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> index 3b2eef8d5419..0fb5a2326f5f 100644
> --- a/arch/arm/mach-kirkwood/Makefile
> +++ b/arch/arm/mach-kirkwood/Makefile
> @@ -6,6 +6,7 @@
>   
>   obj-y	= cpu.o
>   obj-y	+= cache.o
> +obj-y	+= lowlevel.o
>   obj-y	+= mpp.o
>   
>   # cpu.o and cache.o contain CP15 instructions which cannot be run in
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index df3e8f11782a..2b493b36c20d 100644
> --- a/arch/arm/mach-kirkwood/cpu.c
> +++ b/arch/arm/mach-kirkwood/cpu.c
> @@ -189,9 +189,6 @@ int arch_cpu_init(void)
>   	struct kwcpu_registers *cpureg =
>   		(struct kwcpu_registers *)KW_CPU_REG_BASE;
>   
> -	/* Linux expects the internal registers to be at 0xf1000000 */
> -	writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> -
>   	/* Enable and invalidate L2 cache in write through mode */
>   	writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
>   	invalidate_l2_cache();
> diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> new file mode 100644
> index 000000000000..3b339f97f056
> --- /dev/null
> +++ b/arch/arm/mach-kirkwood/lowlevel.S
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(arch_very_early_init)
> +	/* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> +	ldr	r0, =KW_REGS_PHY_BASE
> +	ldr	r1, =KW_OFFSET_REG
> +	str	r0, [r1]
> +	bx	lr
> +ENDPROC(arch_very_early_init)

Viele Grüße,
Stefan Roese
Stefan Roese March 24, 2023, 2:42 p.m. UTC | #8
On 3/11/23 11:57, Pali Rohár wrote:
> Same change as was done for mvebu in commit 5bb2c550b11e ("arm: mvebu: Move
> internal registers in arch_very_early_init() function") but for kirkwood.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot-marvell/next

Thanks,
Stefan

> ---
> Hello! Please test this RFC patch on more Kirkwood boards if there is
> any issue with it.
> ---
>   arch/arm/mach-kirkwood/Kconfig    |  2 ++
>   arch/arm/mach-kirkwood/Makefile   |  1 +
>   arch/arm/mach-kirkwood/cpu.c      |  3 ---
>   arch/arm/mach-kirkwood/lowlevel.S | 12 ++++++++++++
>   4 files changed, 15 insertions(+), 3 deletions(-)
>   create mode 100644 arch/arm/mach-kirkwood/lowlevel.S
> 
> diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> index c8a193dd4cdf..ba39e9ae416e 100644
> --- a/arch/arm/mach-kirkwood/Kconfig
> +++ b/arch/arm/mach-kirkwood/Kconfig
> @@ -5,9 +5,11 @@ config FEROCEON_88FR131
>   
>   config KW88F6192
>   	bool
> +	select ARCH_VERY_EARLY_INIT
>   
>   config KW88F6281
>   	bool
> +	select ARCH_VERY_EARLY_INIT
>   
>   config SHEEVA_88SV131
>   	bool
> diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
> index 3b2eef8d5419..0fb5a2326f5f 100644
> --- a/arch/arm/mach-kirkwood/Makefile
> +++ b/arch/arm/mach-kirkwood/Makefile
> @@ -6,6 +6,7 @@
>   
>   obj-y	= cpu.o
>   obj-y	+= cache.o
> +obj-y	+= lowlevel.o
>   obj-y	+= mpp.o
>   
>   # cpu.o and cache.o contain CP15 instructions which cannot be run in
> diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
> index df3e8f11782a..2b493b36c20d 100644
> --- a/arch/arm/mach-kirkwood/cpu.c
> +++ b/arch/arm/mach-kirkwood/cpu.c
> @@ -189,9 +189,6 @@ int arch_cpu_init(void)
>   	struct kwcpu_registers *cpureg =
>   		(struct kwcpu_registers *)KW_CPU_REG_BASE;
>   
> -	/* Linux expects the internal registers to be at 0xf1000000 */
> -	writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
> -
>   	/* Enable and invalidate L2 cache in write through mode */
>   	writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
>   	invalidate_l2_cache();
> diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
> new file mode 100644
> index 000000000000..3b339f97f056
> --- /dev/null
> +++ b/arch/arm/mach-kirkwood/lowlevel.S
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#include <config.h>
> +#include <linux/linkage.h>
> +
> +ENTRY(arch_very_early_init)
> +	/* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
> +	ldr	r0, =KW_REGS_PHY_BASE
> +	ldr	r1, =KW_OFFSET_REG
> +	str	r0, [r1]
> +	bx	lr
> +ENDPROC(arch_very_early_init)

Viele Grüße,
Stefan Roese
diff mbox series

Patch

diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
index c8a193dd4cdf..ba39e9ae416e 100644
--- a/arch/arm/mach-kirkwood/Kconfig
+++ b/arch/arm/mach-kirkwood/Kconfig
@@ -5,9 +5,11 @@  config FEROCEON_88FR131
 
 config KW88F6192
 	bool
+	select ARCH_VERY_EARLY_INIT
 
 config KW88F6281
 	bool
+	select ARCH_VERY_EARLY_INIT
 
 config SHEEVA_88SV131
 	bool
diff --git a/arch/arm/mach-kirkwood/Makefile b/arch/arm/mach-kirkwood/Makefile
index 3b2eef8d5419..0fb5a2326f5f 100644
--- a/arch/arm/mach-kirkwood/Makefile
+++ b/arch/arm/mach-kirkwood/Makefile
@@ -6,6 +6,7 @@ 
 
 obj-y	= cpu.o
 obj-y	+= cache.o
+obj-y	+= lowlevel.o
 obj-y	+= mpp.o
 
 # cpu.o and cache.o contain CP15 instructions which cannot be run in
diff --git a/arch/arm/mach-kirkwood/cpu.c b/arch/arm/mach-kirkwood/cpu.c
index df3e8f11782a..2b493b36c20d 100644
--- a/arch/arm/mach-kirkwood/cpu.c
+++ b/arch/arm/mach-kirkwood/cpu.c
@@ -189,9 +189,6 @@  int arch_cpu_init(void)
 	struct kwcpu_registers *cpureg =
 		(struct kwcpu_registers *)KW_CPU_REG_BASE;
 
-	/* Linux expects the internal registers to be at 0xf1000000 */
-	writel(KW_REGS_PHY_BASE, KW_OFFSET_REG);
-
 	/* Enable and invalidate L2 cache in write through mode */
 	writel(readl(&cpureg->l2_cfg) | 0x18, &cpureg->l2_cfg);
 	invalidate_l2_cache();
diff --git a/arch/arm/mach-kirkwood/lowlevel.S b/arch/arm/mach-kirkwood/lowlevel.S
new file mode 100644
index 000000000000..3b339f97f056
--- /dev/null
+++ b/arch/arm/mach-kirkwood/lowlevel.S
@@ -0,0 +1,12 @@ 
+/* SPDX-License-Identifier: GPL-2.0+ */
+
+#include <config.h>
+#include <linux/linkage.h>
+
+ENTRY(arch_very_early_init)
+	/* Move internal registers from KW_OFFSET_REG to KW_REGS_PHY_BASE */
+	ldr	r0, =KW_REGS_PHY_BASE
+	ldr	r1, =KW_OFFSET_REG
+	str	r0, [r1]
+	bx	lr
+ENDPROC(arch_very_early_init)