[PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
Simon Glass
sjg at chromium.org
Mon Mar 14 23:46:44 CET 2022
Hi Tom,
On Mon, 14 Mar 2022 at 16:38, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Mar 14, 2022 at 04:20:43PM -0600, Simon Glass wrote:
> > Hi Sören,
> >
> > On Mon, 14 Mar 2022 at 15:51, Sören Moch <smoch at web.de> wrote:
> > >
> > > Hi Simon,
> > >
> > > On 14.03.22 20:37, Simon Glass wrote:
> > > > Hi Soeren,
> > > >
> > > > On Mon, 14 Mar 2022 at 13:22, Soeren Moch <smoch at web.de> wrote:
> > > >>
> > > >> On 14.03.22 19:28, Tom Rini wrote:
> > > >>> On Mon, Mar 14, 2022 at 12:24:36PM -0600, Simon Glass wrote:
> > > >>>> Hi Soeren,
> > > >>>>
> > > >>>> On Mon, 14 Mar 2022 at 02:26, Soeren Moch <smoch at web.de> wrote:
> > > >>>>> ... to get rid of the build warning.
> > > >>>>> Unfortunately we still need the board specific serial pin init code.
> > > >>>>> Otherwise the first boot messages over the serial console are lost.
> > > >>>>>
> > > >>>>> Signed-off-by: Soeren Moch <smoch at web.de>
> > > >>>>> ---
> > > >>>>> Cc: Stefano Babic <sbabic at denx.de>
> > > >>>>> Cc: Fabio Estevam <festevam at gmail.com>
> > > >>>>> Cc: Tom Rini <trini at konsulko.com>
> > > >>>>> Cc: Simon Glass <sjg at chromium.org>
> > > >>>>> Cc: u-boot at lists.denx.de
> > > >>>>>
> > > >>>>> The whole purpose of DM is somewhat defeated when we still need board
> > > >>>>> specific initializations. Any ideas how we can get all boot messages
> > > >>>>> without board specific inits? 'u-boot,dm-pre-reloc;' in the uart device
> > > >>>>> tree node did not help.
> > > >>>> You can put that in your serial driver, perhaps? Or in the initial SoC
> > > >>>> init code?
> > > >> Why should I do so? The whole point of DM is initializing devices from
> > > >> DT. And when I wish to do so pre-relocation, it is advertised in DM to
> > > >> add 'u-boot,dm-pre-reloc;' for this purpose. I tried, it did not work.
> > > >> And this is nothing closely related to the serial driver itself, I just
> > > >> want the pin setup running pre-relocation and not as late as it is
> > > >> running now under DM_SERIAL.
> > > > If you have a pinctrl driver it will be used. I don't really
> > > > understand your problem.
> > > My problem is that pin initializations come too late (just before the
> > > "Core" boot message).
> > > Apparently I have a pinctrl driver, otherwise the pin init would not be
> > > done at all, I guess.
> >
> > Who knows, why don't you check?
> >
> > > >> I also do not want to run this pin setup twice (first in board or SoC
> > > >> code and again by DM_SERIAL later). Maybe I miss something obvious, but
> > > >> duplication of the setup code cannot be a proper solution.
> > > > Well the pinctrl will be triggered before relocation and after, if
> > > > enabled. We could solve that but have not tried.
> > > My problem is not runtime, if initialization is done twice from the same
> > > code this is probably OK. In my setup pins are _not_ initialized before
> > > relocation, when not done in board_early_init_f() "by hand", which I
> > > would like to avoid since this results in code duplication.
> > > Do I need to enable the before-relocation part somewhere? When yes, how
> > > exactly? 'u-boot,dm-pre-reloc;' in the uart DT node (as documented) did
> > > not work.
> >
> > You need your driver to be bound before relocation (so needs the tag
> > as you say). The infrastructure is all there and works on other
> > boards. It is strange that you don't use SPL, though. How do you init
> > the DRAM?
>
> Probably through the DCD script that CONFIG_IMX_CONFIG sets.
>
> > You could enable the debug UART as a starting point, if you don't have
> > JTAG debugging, since that will allow you to figure out why your
> > pinctrl driver is not being run.
> >
> > In the unlikely event that it helps, see the diff below that was
> > enough to get the serial going on an mx6 board in SPL about 2 years
> > ago (so the tags should work the same for U-Boot proper before
> > relocation).
> >
> > If the error checking is working correctly and people have not just
> > make drivers return 0 when something goes wrong, you can normally
> > figure out which driver is missing.
> >
> > new file mode 100644
> > index 00000000000..b83881780c3
> > --- /dev/null
> > +++ b/arch/arm/dts/imx6q-snappermx6-u-boot.dtsi
> > @@ -0,0 +1,22 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2020 Designa Electronics Ltd
> > + */
> > +
> > +/ {
> > + chosen {
> > + stdout-path = &uart5;
> > + };
> > +};
> > +
> > +&aips2 {
> > + u-boot,dm-pre-reloc;
> > +};
> > +
> > +&soc {
> > + u-boot,dm-pre-reloc;
> > +};
> > +
> > +&uart5 {
> > + u-boot,dm-pre-reloc;
> > +};
> > diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi
> > index e4daf150881..33e636b2d31 100644
> > --- a/arch/arm/dts/imx6qdl.dtsi
> > +++ b/arch/arm/dts/imx6qdl.dtsi
> > @@ -139,7 +139,7 @@
> > interrupts = <0 94 IRQ_TYPE_LEVEL_HIGH>;
> > };
> >
> > - soc {
> > + soc: soc {
> > #address-cells = <1>;
> > #size-cells = <1>;
> > compatible = "simple-bus";
> > @@ -913,7 +913,7 @@
> > };
> > };
> >
> > - aips-bus at 2100000 { /* AIPS2 */
> > + aips2: aips-bus at 2100000 { /* AIPS2 */
> > compatible = "fsl,aips-bus", "simple-bus";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> >
> > > >>>> Another recent way (in -next) is to use events to monitor the
> > > >>>> EVT_DM_PRE_PROBE event for the serial driver.
> > > >> I can monitor the probe event, OK. But how can this solve my problem?
> > > >> Again, maybe I miss something obvious, please tell me when I do so.
> > > >>> It's just the same thing every single imx platform is doing.
> > > >>>
> > > >> Sorry, I don't understand what you mean here. The reference platform for
> > > >> my board is mx6sabresd. This is not converted to DM_SERIAL yet. Most (?)
> > > >> imx boards use SPL, pin setup is different there.
> > > >> I looked into imx boards with DM_SERIAL. They either removed the
> > > >> board-specific setup code (which results in missing early boot messages:
> > > >> u-boot version, board name, DDR size, ...) or they are playing tricks in
> > > >> SPL (not the clean and easy solution that DM promises). Maybe I missed a
> > > >> better reference for the DM_SERIAL conversion without SPL. Can you point
> > > >> me to such board?
> > > > If you want to use pinctrl in SPL, you can do all of this cleanly. If
> > > > you have code-size constraints, then you may want to do something like
> > > > rockchip, where only specific peripherals are supported in pinctrl in
> > > > SPL.
> > > I do not use SPL, only U-Boot proper.
> > > >
> > > > You could look at firefly-rk3288 (or bob/coral/jerry) which I believe
> > > > is done fully with driver model.
> > > I want a proper solution without SPL, see above.
> > > > Perhaps Tom has a better handle on the problem.
> > > "Great." You are forcing everyone in DM conversions with deadlines. This
> > > conversion does not work as documented. When asked for help you only
> > > provide answers to different questions than what was asked. And you
> > > conclude with "create your own solution or ask someone else", at least
> > > this is as I understand this.
> >
> > Your expectations are way out of whack. U-Boot has supported driver
> > model for serial for 8 years. U-Boot relies on people digging in and
> > figuring out their own problems. I have converted more than a dozen
> > boards to driver model (I am not actually sure how many) but I am just
> > one person and there are over a thousand boards.
> >
> > >
> > > All this while DM conversions only bring additional work for maintainers
> > > of existing boards, DM conversions always come with increased code size,
> > > and only in the best case everything works like before.
> > >
> > > On the other hand you complain about slow conversions and maintainers
> > > that wait for the very end of the deadline. Do you see the relation?
> > >
> > >
> > > So I ask you again, Simon. How is this DM_SERIAL conversion supposed to
> > > be done properly? In general and especially for imx boards without SPL?
> > > Or is all this as incomplete as it looks like? In this case the
> > > conversion probably will again last until the end of the real deadline,
> > > about 3 years from now. And it will be done as in this patch (with your
> > > Reviewed-by blessing), papering over the fact that all the old code is
> > > still active, only the build warning is silenced. Exactly what we want
> > > to avoid, bigger code not better code. I hope we can clean this up in a
> > > follow-up patch.
> >
> > I would suggest that instead of complaining and accusing people of
> > things, you would be better to sit down and take a hard look at it. It
> > is not that difficult to figure out, if you have a debug UART or JTAG.
> > It looks like 50% of iMX6 boards enable DM_SERIAL so it cannot be
> > impossible. There really is nothing magical about driver model. It is
> > just a model for how drivers fit together. It still runs the same
> > code, just organised in a somewhat more rational way, at the cost of
> > some extra complexity and code size.
>
> Again, DM_SERIAL can be enabled on the board as he's already shown by
> setting two options, which silences the warning, increases the size and
> doesn't make anything better. That's likely what the other boards are
> doing, or they're just having some console output be missing. What
> would need to be set where, so that serial output isn't missing, without
> having to manually call setup_iomux_uart() like so many imx platforms
> are doing.
Probably missing soc/clock node as per my patch above. But it needs
investigation.
Regards,
Simon
More information about the U-Boot
mailing list