DM_SERIAL is broken for Kirkwood boards

[Tom Rini]

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?

> 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).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230127/b9ad8528/attachment.sig>