diff mbox series

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

Message ID e617f766-083b-0cd2-690a-987fad82aaea@denx.de
State Deferred
Delegated to: Tom Rini
Headers show
Series dm, serial: problem with using ns16550 driver before relocation on mpc83xx | expand

Commit Message

Heiko Schocher Feb. 5, 2020, 9:04 a.m. UTC 
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@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:


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?

bye,
Heiko

Comments

Simon Glass Feb. 5, 2020, 5:59 p.m. UTC | #1 
Hi Heiko,

On Wed, 5 Feb 2020 at 02:04, Heiko Schocher <hs@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@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@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.

Regards,
Simon
Heiko Schocher Feb. 6, 2020, 5:19 a.m. UTC | #2 
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@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@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@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?

bye,
Heiko
[1] https://gitlab.denx.de/u-boot/u-boot/blob/master/arch/powerpc/dts/gdsys/gazerbeam-uboot.dtsi#L238
Simon Glass Feb. 6, 2020, 5:46 p.m. UTC | #3 
Hi Heiko,

On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs@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@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@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@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
Heiko Schocher Feb. 7, 2020, 5:53 a.m. UTC | #4 
Hi Simon,

removed Dirk from cc and added Mario Six

@Mario: Dirk is maintainer of the gazerbeam board:

https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2

but EMail get not delivered to his EMail address ... so I added
you to cc ... may you have a gazerbeam board? May you can try,
if current U-Boot mainline works (in special serial console) on it?

thanks!

Am 06.02.2020 um 18:46 schrieb Simon Glass:
> Hi Heiko,
> 
> On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs@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@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@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@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.

Ok, I try to find out more, thanks!

bye,
Heiko
Simon Glass Feb. 7, 2020, 5:37 p.m. UTC | #5 
Hi Heiko,

On Thu, 6 Feb 2020 at 22:53, Heiko Schocher <hs@denx.de> wrote:
>
> Hi Simon,
>
> removed Dirk from cc and added Mario Six
>
> @Mario: Dirk is maintainer of the gazerbeam board:
>
> https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2
>
> but EMail get not delivered to his EMail address ... so I added
> you to cc ... may you have a gazerbeam board? May you can try,
> if current U-Boot mainline works (in special serial console) on it?

BTW I don't have one of these. I can't even find it on the internet!

Regards,
Simon


>
> thanks!
>
> Am 06.02.2020 um 18:46 schrieb Simon Glass:
> > Hi Heiko,
> >
> > On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs@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@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@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@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.
>
> Ok, I try to find out more, thanks!
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de
Mario Six Feb. 10, 2020, 6:16 a.m. UTC | #6 
Hi Heiko,

On Fri, Feb 7, 2020 at 6:53 AM Heiko Schocher <hs@denx.de> wrote:
>
> Hi Simon,
>
> removed Dirk from cc and added Mario Six
>
> @Mario: Dirk is maintainer of the gazerbeam board:
>
> https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2
>
> but EMail get not delivered to his EMail address ... so I added
> you to cc ... may you have a gazerbeam board? May you can try,
> if current U-Boot mainline works (in special serial console) on it?
>
Dirk no longer works as gdsys (which is also the reason why I'm much less
active on the mailing list than I used to be).

I have a gazerbeam board. I'll try to get some time to test mainline on it some
time this week.

> thanks!
>
Regards,
Mario

> Am 06.02.2020 um 18:46 schrieb Simon Glass:
> > Hi Heiko,
> >
> > On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs@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@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@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@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.
>
> Ok, I try to find out more, thanks!
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de
Heiko Schocher Feb. 10, 2020, 7:24 a.m. UTC | #7 
Hello Mario,

Am 10.02.2020 um 07:16 schrieb Mario Six:
> Hi Heiko,
> 
> On Fri, Feb 7, 2020 at 6:53 AM Heiko Schocher <hs@denx.de> wrote:
>>
>> Hi Simon,
>>
>> removed Dirk from cc and added Mario Six
>>
>> @Mario: Dirk is maintainer of the gazerbeam board:
>>
>> https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2
>>
>> but EMail get not delivered to his EMail address ... so I added
>> you to cc ... may you have a gazerbeam board? May you can try,
>> if current U-Boot mainline works (in special serial console) on it?
>>
> Dirk no longer works as gdsys (which is also the reason why I'm much less
> active on the mailing list than I used to be).

Ah, ok! Thanks for the info ... may you can add overtake the
maintainership for the gazerbeam board?

> I have a gazerbeam board. I'll try to get some time to test mainline on it some
> time this week.

Many Thanks for your time!

bye,
Heiko
> 
>> thanks!
>>
> Regards,
> Mario
> 
>> Am 06.02.2020 um 18:46 schrieb Simon Glass:
>>> Hi Heiko,
>>>
>>> On Wed, 5 Feb 2020 at 22:19, Heiko Schocher <hs@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@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@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@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.
>>
>> Ok, I try to find out more, thanks!
>>
>> bye,
>> Heiko
>> --
>> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs@denx.de
>
Heiko Schocher Feb. 12, 2020, 6:36 a.m. UTC | #8 
Hello Mario, Simon,

Am 10.02.2020 um 07:16 schrieb Mario Six:
> Hi Heiko,
> 
> On Fri, Feb 7, 2020 at 6:53 AM Heiko Schocher <hs@denx.de> wrote:
>>
>> Hi Simon,
>>
>> removed Dirk from cc and added Mario Six
>>
>> @Mario: Dirk is maintainer of the gazerbeam board:
>>
>> https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2
>>
>> but EMail get not delivered to his EMail address ... so I added
>> you to cc ... may you have a gazerbeam board? May you can try,
>> if current U-Boot mainline works (in special serial console) on it?
>>
> Dirk no longer works as gdsys (which is also the reason why I'm much less
> active on the mailing list than I used to be).
> 
> I have a gazerbeam board. I'll try to get some time to test mainline on it some
> time this week.

The problem is gone, no need for my fix. I made a cleanbuild after
I rebased with mainline this morning (without my "fix"), and all works
fine.

Sorry for the inconvenience!

bye,
Heiko
Simon Glass Feb. 12, 2020, 5:14 p.m. UTC | #9 
Hi Heiko,

On Tue, 11 Feb 2020 at 23:37, Heiko Schocher <hs@denx.de> wrote:
>
> Hello Mario, Simon,
>
> Am 10.02.2020 um 07:16 schrieb Mario Six:
> > Hi Heiko,
> >
> > On Fri, Feb 7, 2020 at 6:53 AM Heiko Schocher <hs@denx.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> removed Dirk from cc and added Mario Six
> >>
> >> @Mario: Dirk is maintainer of the gazerbeam board:
> >>
> >> https://gitlab.denx.de/u-boot/u-boot/blob/master/board/gdsys/mpc8308/MAINTAINERS#L2
> >>
> >> but EMail get not delivered to his EMail address ... so I added
> >> you to cc ... may you have a gazerbeam board? May you can try,
> >> if current U-Boot mainline works (in special serial console) on it?
> >>
> > Dirk no longer works as gdsys (which is also the reason why I'm much less
> > active on the mailing list than I used to be).
> >
> > I have a gazerbeam board. I'll try to get some time to test mainline on it some
> > time this week.
>
> The problem is gone, no need for my fix. I made a cleanbuild after
> I rebased with mainline this morning (without my "fix"), and all works
> fine.
>
> Sorry for the inconvenience!

Oh strange, but good news!

Regards,
Simon
diff mbox series

Patch

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@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
  };