DM_SERIAL is broken for Kirkwood boards
Tom Rini
trini at konsulko.com
Fri Jan 27 23:53:28 CET 2023
On Fri, Jan 27, 2023 at 02:19:40PM -0800, Tony Dinh wrote:
> 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.
Thanks.
> > > 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?
You don't need SYS_NS16550_REG_SIZE with DM_SERIAL. It's also true that
the commit you initially reference broke a number of platforms in
different and unique ways and it's only in the last week or so that I
pushed the patch such that no, really, DM_SERIAL does not try and set
SYS_NS16550_REG_SIZE or do silly things.
--
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/57f489b4/attachment.sig>
More information about the U-Boot
mailing list