DM_SERIAL is broken for Kirkwood boards

Fri Jan 27 23:19:40 CET 2023

Hi Tom,

On Fri, Jan 27, 2023 at 2:06 PM Tom Rini <trini at konsulko.com> wrote:
>
> On Fri, Jan 27, 2023 at 01:56:34PM -0800, Tony Dinh wrote:
> > Hi Tom,
> >
> > On Fri, Jan 27, 2023 at 5:39 AM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Fri, Jan 27, 2023 at 01:31:06PM +0100, Stefan Roese wrote:
> > > > Hi Tony,
> > > >
> > > > On 1/27/23 07:13, Tony Dinh wrote:
> > > > > Hi all,
> > > > >
> > > > > On Thu, Jan 26, 2023 at 3:38 PM Tony Dinh <mibodhi at gmail.com> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > I ran some tests today (Pogo V4 and NSA310S boards) with the latest
> > > > > > master branch and saw the same behavior we've seen before. The boards
> > > > > > hung, and the serial console was silent after kwboot finished
> > > > > > transferring the u-boot image. I'm running with this DTSI patch below
> > > > > > (to enable dm-pre-reloc for uart0).
> > > > > >
> > > > > > If I deselected DM_SERIAL then both boards booted OK with kwboot.
> > > > > >
> > > > > > diff --git a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > > b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > > index 09ee76c2a2..6c5a991fde 100644
> > > > > > --- a/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > > +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> > > > > > @@ -317,3 +317,8 @@
> > > > > >   &pcie0 {
> > > > > >          status = "okay";
> > > > > >   };
> > > > > > +
> > > > > > +&uart0 {
> > > > > > +        u-boot,dm-pre-reloc;
> > > > > > +        status = "okay";
> > > > > > +};
> > > > > > diff --git a/arch/arm/dts/kirkwood-pogoplug-series-4.dts
> > > > > > b/arch/arm/dts/kirkwood-pogoplug-series-4.dts
> > > > > > index 5aa4669ae2..ef495d69f5 100644
> > > > > > --- a/arch/arm/dts/kirkwood-pogoplug-series-4.dts
> > > > > > +++ b/arch/arm/dts/kirkwood-pogoplug-series-4.dts
> > > > > > @@ -98,6 +98,7 @@
> > > > > >   };
> > > > > >
> > > > > >   &uart0 {
> > > > > > +       u-boot,dm-pre-reloc;
> > > > > >          status = "okay";
> > > > > >   };
> > > > > >
> > > > > > @Michael, it would be great if you could try with your Buffalo board,
> > > > > > and see if you will experience the same behavior.
> > > > >
> > > > > Looks like this commit was the indirect cause of the problem.
> > > > >
> > > > > Convert CONFIG_SYS_NS16550_MEM32 et al to Kconfig
> > > > > https://github.com/u-boot/u-boot/commit/9591b63531fa5a34698ee7bb3800af6c4ea6ba2f
> > > > >
> > > > > -CONFIG_SYS_NS16550=y
> > > > > +CONFIG_SYS_NS16550_SERIAL=y
> > > > > +CONFIG_SYS_NS16550_REG_SIZE=-4
> > > >
> > > > Thanks for digging into this. Could you perhaps prepare a patch like
> > > > this:
> > > >
> > > > diff --git a/arch/arm/mach-kirkwood/Kconfig b/arch/arm/mach-kirkwood/Kconfig
> > > > index 45cc9326368c..438f86922310 100644
> > > > --- a/arch/arm/mach-kirkwood/Kconfig
> > > > +++ b/arch/arm/mach-kirkwood/Kconfig
> > > > @@ -15,6 +15,7 @@ config SHEEVA_88SV131
> > > >  config KIRKWOOD_COMMON
> > > >         bool
> > > >         select DM_SERIAL
> > > > +       select SYS_NS16550_SERIAL
> > > >
> > > >  config HAS_CUSTOM_SYS_INIT_SP_ADDR
> > > >          bool "Use a custom location for the initial stack pointer address"
> > > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> > > > index bb5083201b38..7b575295746b 100644
> > > > --- a/drivers/serial/Kconfig
> > > > +++ b/drivers/serial/Kconfig
> > > > @@ -773,7 +773,7 @@ config SYS_NS16550_REG_SIZE
> > > >         int "ns16550 register width and endianness"
> > > >         depends on SYS_NS16550_SERIAL || SPL_SYS_NS16550_SERIAL
> > > >         range -4 4
> > > > -       default -4 if ARCH_OMAP2PLUS || ARCH_SUNXI
> > > > +       default -4 if ARCH_OMAP2PLUS || ARCH_SUNXI || ARCH_KIRKWOOD
> > > >         default 1
> > > >         help
> > > >           Indicates register width and also endianness. If positive,
> > > > big-endian
> > > >
> > > > Does this work for you?
> > >
> > > Wait, when do you need serial and what are you select'ing for, exactly?
> > > The ns16550 driver stuff is indeed a bit of a mess, option wise, but
> > > SYS_NS16550 and SYS_NS16550_SERIAL are not the same. Are you having
> > > problems in a stage with, or without DM_SERIAL enabled?
> >
> > The problem was described in the initial email above.
> >
> > "I ran some tests today (Pogo V4 and NSA310S boards) with the latest
> > master branch and saw the same behavior we've seen before. The boards
> > hung, and the serial console was silent after kwboot finished
> > transferring the u-boot image. I'm running with this DTSI patch below
> > (to enable dm-pre-reloc for uart0).
> > If I deselected DM_SERIAL then both boards booted OK with kwboot."
>
> Yes, but there's SPL_DM_SERIAL and DM_SERIAL, and I don't recall these
> platforms well, sorry. So is the problem in full U-Boot, or SPL?

It's in full U-Boot. These Kirkwood boards don't use SPL.

>
> > So the problem is seen with DM_SERIAL enabled, and occurs immediately
> > when u-boot starts.
> >
> > If I understand correctly, it looks like we just need to select
> > SYS_NS16550 then the driver can be found by DM serial uclass. And that
> > worked!
>
> Right, CONFIG_SYS_NS16550=y sounds reasonable.
>
> > Stefan patch proposes that we make SYS_NS16550_SERIAL available for
> > ARCH_KIRKWOOD, but I think there is more change is needed with this
> > approach because:
> >
> > drivers/serial/Kconfig
> > config SYS_NS16550_SERIAL
> >         bool "NS16550 UART or compatible legacy driver"
> >         depends on !DM_SERIAL
> >         select SYS_NS16550
>
> Yes, I don't think SYS_NS16550_SERIAL is the right solution, which gets
> back to wondering about SPL or not. There are some platforms which do
> DM_SERIAL and !SPL_DM_SERIAL (for varying levels of valid and good
> reasons).

Since SPL does not come into play, I think we are OK with just
SYS_NS16550. However, Stefan patch also covers the
SYS_NS16550_REG_SIZE (ie. register width and endianness). Looks like
it is necessary to do that for ARCH_KIRKWOOD, too?

Thanks,
Tony

>
> --
> Tom