dm, serial: problem with using ns16550 driver before relocation on mpc83xx

[Simon Glass]

Hi Heiko,

On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs at denx.de> wrote:
>
> Hello Simon,
>
> Am 05.02.2020 um 18:59 schrieb Simon Glass:
> > Hi Heiko,
> >
> > On Wed, 5 Feb 2020 at 02:04, Heiko Schocher <hs at denx.de> wrote:
> >>
> >> Hello Bin, Simon,
> >>
> >> I just porting the mpc83xx based kmcoge5ne board support DTS and got
> >> problems using the serial ns16550 driver.
> >>
> >> I need the serial driver before rolcation, so I enabled
> >> "u-boot,dm-pre-reloc;" as usual in the device tree, but board does not
> >> boot ...
> >>
> >> I found the commit:
> >>
> >> commit 4687919684e0e4390b9fc20d1809ecaa9dc3cb81
> >> Author: Bin Meng <bmeng.cn at gmail.com>
> >> Date:   Wed Oct 24 06:36:36 2018 -0700
> >>
> >>       serial: Remove DM_FLAG_PRE_RELOC flag in various drivers
> >>
> >> which added to the ns16550 serial driver:
> >>
> >> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> >> index 04b604fa2c..1e6fc6c668 100644
> >> --- a/drivers/serial/ns16550.c
> >> +++ b/drivers/serial/ns16550.c
> >> @@ -487,7 +487,9 @@ U_BOOT_DRIVER(ns16550_serial) = {
> >>           .priv_auto_alloc_size = sizeof(struct NS16550),
> >>           .probe = ns16550_serial_probe,
> >>           .ops    = &ns16550_serial_ops,
> >> +#if !CONFIG_IS_ENABLED(OF_CONTROL)
> >>           .flags  = DM_FLAG_PRE_RELOC,
> >> +#endif
> >>    };
> >>    #endif
> >>    #endif /* SERIAL_PRESENT */
> >>
> >> So, as OF_CONTROL is defined for me, the flag "u-boot,dm-pre-reloc" seems
> >> not working anymore ...
> >>
> >> Adding this back:
> >>
> >> hs at xmglap:u-boot-secu  [20200205-temp] $ git diff
> >> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> >> index 9851663dc5..386ca9cffa 100644
> >> --- a/drivers/serial/ns16550.c
> >> +++ b/drivers/serial/ns16550.c
> >> @@ -528,7 +528,7 @@ U_BOOT_DRIVER(ns16550_serial) = {
> >>           .priv_auto_alloc_size = sizeof(struct NS16550),
> >>           .probe = ns16550_serial_probe,
> >>           .ops    = &ns16550_serial_ops,
> >> -#if !CONFIG_IS_ENABLED(OF_CONTROL)
> >> +#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
> >>           .flags  = DM_FLAG_PRE_RELOC,
> >>    #endif
> >>    };
> >>
> >> and board boots fine with the flag "u-boot,dm-pre-reloc" in DTS ...
> >>
> >> May I do something wrong here? I found in mainline for example
> >> the "arch/powerpc/dts/gdsys/gazerbeam-uboot.dtsi" board, which
> >> has the exactly same dts settings than I have now.
> >>
> >> @Dirk: Can you check, if this board boots with current mainline?
> >>
> >> Shouldn;t be the logic, that in case OF_CONTROL is enabled and if
> >> flag "u-boot,dm-pre-reloc" is set in DTS for the device, the device
> >> should be bound before relocation, and we do not need to check, if
> >> the driver sets DM_FLAG_PRE_RELOC ?
> >>
> >> But may I miss here something ...
> >>
> >> Any hints?
> >
> > +Tom Rini
> >
> > I found I needed this for rpi.
> >
> > http://patchwork.ozlabs.org/patch/1202913/
> >
> > But I still haven't gone back to figure out why Tom doesn't.
>
> Hmm... I have added the "u-boot,dm-pre-reloc;" to the uart node.
>
> Like it is for the gazerbeam board, see [1]
>
> It works if "DM_FLAG_PRE_RELOC" is set the driver in flags... no
> need for a gpio node before relocation like it is inabove patch.
>
> I wonder if we need DM_FLAG_PRE_RELOC at all in a driver and
> OF_CONTROL case. Shouldn't it be enough if the DTB node for the
> driver contains the "u-boot,dm-pre-reloc;" property?

Well in the rpi case it is the pinctrl that needs that property. I
think you should dig into exactly what is going wrong on the board you
have. Then it should be possible to see what is missing and add it.

Regards,
Simon