All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig
@ 2022-03-14  8:26 Soeren Moch
  2022-03-14  8:26 ` [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL Soeren Moch
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Soeren Moch @ 2022-03-14  8:26 UTC (permalink / raw)
  To: Stefano Babic; +Cc: Soeren Moch, Tom Rini, Fabio Estevam, Simon Glass, u-boot

This saves about 12 kBytes image size and helps to stay within the
size limit.

Suggested-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Soeren Moch <smoch@web.de>
---
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: u-boot@lists.denx.de

Unfortunately we already are at the size limit for U-Boot v2022.04-rc3.
Maybe Tom can pick up this patch directly, otherwise it's also fine for
me to take this through the imx tree.
---
 configs/tbs2910_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index e1278f2e70..8a33160564 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -14,6 +14,7 @@ CONFIG_DEFAULT_DEVICE_TREE="imx6q-tbs2910"
 CONFIG_PRE_CON_BUF_ADDR=0x7c000000
 CONFIG_CMD_HDMIDETECT=y
 CONFIG_AHCI=y
+CONFIG_LTO=y
 CONFIG_SUPPORT_RAW_INITRD=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_BOOTCOMMAND=y
--
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14  8:26 [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Soeren Moch
@ 2022-03-14  8:26 ` Soeren Moch
  2022-03-14 18:24   ` Simon Glass
  2022-03-14 18:14 ` [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Tom Rini
  2022-03-14 18:24 ` Simon Glass
  2 siblings, 1 reply; 16+ messages in thread
From: Soeren Moch @ 2022-03-14  8:26 UTC (permalink / raw)
  To: Stefano Babic; +Cc: Soeren Moch, Fabio Estevam, Tom Rini, Simon Glass, u-boot

... 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@web.de>
---
Cc: Stefano Babic <sbabic@denx.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: u-boot@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.
---
 configs/tbs2910_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index 8a33160564..79454053b0 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -81,6 +81,8 @@ CONFIG_PINCTRL=y
 CONFIG_PINCTRL_IMX6=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
+CONFIG_DM_SERIAL=y
+CONFIG_SERIAL_RX_BUFFER=y
 CONFIG_MXC_UART=y
 CONFIG_DM_THERMAL=y
 CONFIG_IMX_THERMAL=y
--
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig
  2022-03-14  8:26 [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Soeren Moch
  2022-03-14  8:26 ` [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL Soeren Moch
@ 2022-03-14 18:14 ` Tom Rini
  2022-03-14 18:24 ` Simon Glass
  2 siblings, 0 replies; 16+ messages in thread
From: Tom Rini @ 2022-03-14 18:14 UTC (permalink / raw)
  To: Soeren Moch; +Cc: Stefano Babic, Fabio Estevam, Simon Glass, u-boot

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

On Mon, Mar 14, 2022 at 09:26:25AM +0100, Soeren Moch wrote:

> This saves about 12 kBytes image size and helps to stay within the
> size limit.
> 
> Suggested-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Soeren Moch <smoch@web.de>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig
  2022-03-14  8:26 [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Soeren Moch
  2022-03-14  8:26 ` [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL Soeren Moch
  2022-03-14 18:14 ` [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Tom Rini
@ 2022-03-14 18:24 ` Simon Glass
  2 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2022-03-14 18:24 UTC (permalink / raw)
  To: Soeren Moch; +Cc: Stefano Babic, Tom Rini, Fabio Estevam, U-Boot Mailing List

On Mon, 14 Mar 2022 at 02:26, Soeren Moch <smoch@web.de> wrote:
>
> This saves about 12 kBytes image size and helps to stay within the
> size limit.
>
> Suggested-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Soeren Moch <smoch@web.de>
> ---
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: u-boot@lists.denx.de
>
> Unfortunately we already are at the size limit for U-Boot v2022.04-rc3.
> Maybe Tom can pick up this patch directly, otherwise it's also fine for
> me to take this through the imx tree.
> ---
>  configs/tbs2910_defconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14  8:26 ` [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL Soeren Moch
@ 2022-03-14 18:24   ` Simon Glass
  2022-03-14 18:28     ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2022-03-14 18:24 UTC (permalink / raw)
  To: Soeren Moch; +Cc: Stefano Babic, Fabio Estevam, Tom Rini, U-Boot Mailing List

Hi Soeren,

On Mon, 14 Mar 2022 at 02:26, Soeren Moch <smoch@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@web.de>
> ---
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: u-boot@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?

Another recent way (in -next) is to use events to monitor the
EVT_DM_PRE_PROBE event for the serial driver.

> ---
>  configs/tbs2910_defconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
> index 8a33160564..79454053b0 100644
> --- a/configs/tbs2910_defconfig
> +++ b/configs/tbs2910_defconfig
> @@ -81,6 +81,8 @@ CONFIG_PINCTRL=y
>  CONFIG_PINCTRL_IMX6=y
>  CONFIG_DM_RTC=y
>  CONFIG_RTC_DS1307=y
> +CONFIG_DM_SERIAL=y
> +CONFIG_SERIAL_RX_BUFFER=y
>  CONFIG_MXC_UART=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_IMX_THERMAL=y
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 18:24   ` Simon Glass
@ 2022-03-14 18:28     ` Tom Rini
  2022-03-14 19:22       ` Soeren Moch
  0 siblings, 1 reply; 16+ messages in thread
From: Tom Rini @ 2022-03-14 18:28 UTC (permalink / raw)
  To: Simon Glass
  Cc: Soeren Moch, Stefano Babic, Fabio Estevam, U-Boot Mailing List

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

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@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@web.de>
> > ---
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Fabio Estevam <festevam@gmail.com>
> > Cc: Tom Rini <trini@konsulko.com>
> > Cc: Simon Glass <sjg@chromium.org>
> > Cc: u-boot@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?
> 
> Another recent way (in -next) is to use events to monitor the
> EVT_DM_PRE_PROBE event for the serial driver.

It's just the same thing every single imx platform is doing.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 18:28     ` Tom Rini
@ 2022-03-14 19:22       ` Soeren Moch
  2022-03-14 19:34         ` Tom Rini
  2022-03-14 19:37         ` Simon Glass
  0 siblings, 2 replies; 16+ messages in thread
From: Soeren Moch @ 2022-03-14 19:22 UTC (permalink / raw)
  To: Tom Rini, Simon Glass; +Cc: Stefano Babic, Fabio Estevam, U-Boot Mailing List


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@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@web.de>
>>> ---
>>> Cc: Stefano Babic <sbabic@denx.de>
>>> Cc: Fabio Estevam <festevam@gmail.com>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: Simon Glass <sjg@chromium.org>
>>> Cc: u-boot@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.

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

Thanks,
Soeren

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 19:22       ` Soeren Moch
@ 2022-03-14 19:34         ` Tom Rini
  2022-03-14 19:37         ` Simon Glass
  1 sibling, 0 replies; 16+ messages in thread
From: Tom Rini @ 2022-03-14 19:34 UTC (permalink / raw)
  To: Soeren Moch
  Cc: Simon Glass, Stefano Babic, Fabio Estevam, U-Boot Mailing List

[-- Attachment #1: Type: text/plain, Size: 3086 bytes --]

On Mon, Mar 14, 2022 at 08:22:10PM +0100, Soeren Moch 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@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@web.de>
> > > > ---
> > > > Cc: Stefano Babic <sbabic@denx.de>
> > > > Cc: Fabio Estevam <festevam@gmail.com>
> > > > Cc: Tom Rini <trini@konsulko.com>
> > > > Cc: Simon Glass <sjg@chromium.org>
> > > > Cc: u-boot@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.
> 
> 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.
> > > 
> > > 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?

My point, largely to Simon, sorry, is that this isn't a "tbs2910"
specific problem, but rather something that's not solved for any of the
imx platforms as they do some form of what you note above.  I suspect
it's not solved for anything else either and other platforms are doing
some number of what you said above (or relying on something else already
having done the pinmux).

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 19:22       ` Soeren Moch
  2022-03-14 19:34         ` Tom Rini
@ 2022-03-14 19:37         ` Simon Glass
  2022-03-14 21:51           ` Sören Moch
  1 sibling, 1 reply; 16+ messages in thread
From: Simon Glass @ 2022-03-14 19:37 UTC (permalink / raw)
  To: Soeren Moch; +Cc: Tom Rini, Stefano Babic, Fabio Estevam, U-Boot Mailing List

Hi Soeren,

On Mon, 14 Mar 2022 at 13:22, Soeren Moch <smoch@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@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@web.de>
> >>> ---
> >>> Cc: Stefano Babic <sbabic@denx.de>
> >>> Cc: Fabio Estevam <festevam@gmail.com>
> >>> Cc: Tom Rini <trini@konsulko.com>
> >>> Cc: Simon Glass <sjg@chromium.org>
> >>> Cc: u-boot@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.

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

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

You could look at firefly-rk3288 (or bob/coral/jerry) which I believe
is done fully with driver model.

Perhaps Tom has a better handle on the problem.

Regards,
Simon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 19:37         ` Simon Glass
@ 2022-03-14 21:51           ` Sören Moch
  2022-03-14 22:20             ` Simon Glass
  0 siblings, 1 reply; 16+ messages in thread
From: Sören Moch @ 2022-03-14 21:51 UTC (permalink / raw)
  To: Simon Glass; +Cc: Tom Rini, Stefano Babic, Fabio Estevam, U-Boot Mailing List

Hi Simon,

On 14.03.22 20:37, Simon Glass wrote:
> Hi Soeren,
>
> On Mon, 14 Mar 2022 at 13:22, Soeren Moch <smoch@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@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@web.de>
>>>>> ---
>>>>> Cc: Stefano Babic <sbabic@denx.de>
>>>>> Cc: Fabio Estevam <festevam@gmail.com>
>>>>> Cc: Tom Rini <trini@konsulko.com>
>>>>> Cc: Simon Glass <sjg@chromium.org>
>>>>> Cc: u-boot@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.
>> 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.
>>>> 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.

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.

Regards,
Soeren
>
> Regards,
> Simon


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 21:51           ` Sören Moch
@ 2022-03-14 22:20             ` Simon Glass
  2022-03-14 22:38               ` Tom Rini
  0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2022-03-14 22:20 UTC (permalink / raw)
  To: Sören Moch
  Cc: Tom Rini, Stefano Babic, Fabio Estevam, U-Boot Mailing List

Hi Sören,

On Mon, 14 Mar 2022 at 15:51, Sören Moch <smoch@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@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@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@web.de>
> >>>>> ---
> >>>>> Cc: Stefano Babic <sbabic@denx.de>
> >>>>> Cc: Fabio Estevam <festevam@gmail.com>
> >>>>> Cc: Tom Rini <trini@konsulko.com>
> >>>>> Cc: Simon Glass <sjg@chromium.org>
> >>>>> Cc: u-boot@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?

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@2100000 { /* AIPS2 */
+        aips2: aips-bus@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.

Regards,
Simon

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 22:20             ` Simon Glass
@ 2022-03-14 22:38               ` Tom Rini
  2022-03-14 22:46                 ` Simon Glass
  2022-03-14 23:16                 ` Fabio Estevam
  0 siblings, 2 replies; 16+ messages in thread
From: Tom Rini @ 2022-03-14 22:38 UTC (permalink / raw)
  To: Simon Glass
  Cc: Sören Moch, Stefano Babic, Fabio Estevam, U-Boot Mailing List

[-- Attachment #1: Type: text/plain, Size: 9333 bytes --]

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@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@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@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@web.de>
> > >>>>> ---
> > >>>>> Cc: Stefano Babic <sbabic@denx.de>
> > >>>>> Cc: Fabio Estevam <festevam@gmail.com>
> > >>>>> Cc: Tom Rini <trini@konsulko.com>
> > >>>>> Cc: Simon Glass <sjg@chromium.org>
> > >>>>> Cc: u-boot@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@2100000 { /* AIPS2 */
> +        aips2: aips-bus@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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 22:38               ` Tom Rini
@ 2022-03-14 22:46                 ` Simon Glass
  2022-03-14 23:16                 ` Fabio Estevam
  1 sibling, 0 replies; 16+ messages in thread
From: Simon Glass @ 2022-03-14 22:46 UTC (permalink / raw)
  To: Tom Rini
  Cc: Sören Moch, Stefano Babic, Fabio Estevam, U-Boot Mailing List

Hi Tom,

On Mon, 14 Mar 2022 at 16:38, Tom Rini <trini@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@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@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@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@web.de>
> > > >>>>> ---
> > > >>>>> Cc: Stefano Babic <sbabic@denx.de>
> > > >>>>> Cc: Fabio Estevam <festevam@gmail.com>
> > > >>>>> Cc: Tom Rini <trini@konsulko.com>
> > > >>>>> Cc: Simon Glass <sjg@chromium.org>
> > > >>>>> Cc: u-boot@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@2100000 { /* AIPS2 */
> > +        aips2: aips-bus@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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 22:38               ` Tom Rini
  2022-03-14 22:46                 ` Simon Glass
@ 2022-03-14 23:16                 ` Fabio Estevam
  2022-03-15 20:56                   ` Soeren Moch
  1 sibling, 1 reply; 16+ messages in thread
From: Fabio Estevam @ 2022-03-14 23:16 UTC (permalink / raw)
  To: Tom Rini; +Cc: Simon Glass, Sören Moch, Stefano Babic, U-Boot Mailing List

[-- Attachment #1: Type: text/plain, Size: 669 bytes --]

Hi Soeren,

On Mon, Mar 14, 2022 at 7:38 PM Tom Rini <trini@konsulko.com> wrote:

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

Does the attached patch work for you?

I tested a similar patch on warp7 and it works fine here and the early
U-Boot messages are shown.

[-- Attachment #2: 0001-tbs2910.patch --]
[-- Type: text/x-patch, Size: 3938 bytes --]

From a47c625f30333e45e3836ad797a47a3cc506ed94 Mon Sep 17 00:00:00 2001
From: Fabio Estevam <festevam@gmail.com>
Date: Mon, 14 Mar 2022 20:12:02 -0300
Subject: [PATCH] tbs2910: Convert to DM_SERIAL

Conversion to DM_SERIAL is mandatory.

Select DM_SERIAL and add a imx6q-tbs2910-u-boot.dtsi file
that describes the nodes that require dm-pre-reloc, which allows
the DM model to configure the UART pinctrl early.

Remove the now unneeded board UART initialization.

Signed-off-by: Fabio Estevam <festevam@gmail.com>
---
 arch/arm/dts/imx6q-tbs2910-u-boot.dtsi | 16 ++++++++++++++++
 arch/arm/dts/imx6qdl.dtsi              |  4 ++--
 board/tbs/tbs2910/tbs2910.c            | 26 --------------------------
 configs/tbs2910_defconfig              |  2 +-
 4 files changed, 19 insertions(+), 29 deletions(-)
 create mode 100644 arch/arm/dts/imx6q-tbs2910-u-boot.dtsi

diff --git a/arch/arm/dts/imx6q-tbs2910-u-boot.dtsi b/arch/arm/dts/imx6q-tbs2910-u-boot.dtsi
new file mode 100644
index 000000000000..4c164068b016
--- /dev/null
+++ b/arch/arm/dts/imx6q-tbs2910-u-boot.dtsi
@@ -0,0 +1,16 @@
+&aips1 {
+	u-boot,dm-pre-reloc;
+};
+
+
+&pinctrl_uart1 {
+	u-boot,dm-pre-reloc;
+};
+
+&soc {
+	u-boot,dm-pre-reloc;
+};
+
+&uart1 {
+	u-boot,dm-pre-reloc;
+};
diff --git a/arch/arm/dts/imx6qdl.dtsi b/arch/arm/dts/imx6qdl.dtsi
index efd89510d512..d89272039b28 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";
@@ -283,7 +283,7 @@
 			status = "disabled";
 		};
 
-		bus@2000000 { /* AIPS1 */
+		aips1: bus@2000000 { /* AIPS1 */
 			compatible = "fsl,aips-bus", "simple-bus";
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/board/tbs/tbs2910/tbs2910.c b/board/tbs/tbs2910/tbs2910.c
index faf73cc218c9..3a447ca8a93c 100644
--- a/board/tbs/tbs2910/tbs2910.c
+++ b/board/tbs/tbs2910/tbs2910.c
@@ -22,32 +22,12 @@
 #include <asm/arch/sys_proto.h>
 DECLARE_GLOBAL_DATA_PTR;
 
-#define UART_PAD_CTRL  (PAD_CTL_PUS_100K_UP |			\
-	PAD_CTL_SPEED_MED | PAD_CTL_DSE_40ohm |			\
-	PAD_CTL_SRE_FAST  | PAD_CTL_HYS)
-
-static iomux_v3_cfg_t const uart1_pads[] = {
-	MX6_PAD_CSI0_DAT10__UART1_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
-	MX6_PAD_CSI0_DAT11__UART1_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
-};
-
-static iomux_v3_cfg_t const uart2_pads[] = {
-	MX6_PAD_EIM_D26__UART2_TX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
-	MX6_PAD_EIM_D27__UART2_RX_DATA | MUX_PAD_CTRL(UART_PAD_CTRL),
-};
-
 int dram_init(void)
 {
 	gd->ram_size = 2048ul * 1024 * 1024;
 	return 0;
 }
 
-static void setup_iomux_uart(void)
-{
-	imx_iomux_v3_setup_multiple_pads(uart1_pads, ARRAY_SIZE(uart1_pads));
-	imx_iomux_v3_setup_multiple_pads(uart2_pads, ARRAY_SIZE(uart2_pads));
-}
-
 #ifdef CONFIG_FSL_ESDHC_IMX
 /* set environment device to boot device when booting from SD */
 int board_mmc_get_env_dev(int devno)
@@ -150,12 +130,6 @@ static void setup_display(void)
 }
 #endif /* CONFIG_VIDEO_IPUV3 */
 
-int board_early_init_f(void)
-{
-	setup_iomux_uart();
-	return 0;
-}
-
 #ifdef CONFIG_CMD_BMODE
 static const struct boot_mode board_boot_modes[] = {
 	/* 4 bit bus width */
diff --git a/configs/tbs2910_defconfig b/configs/tbs2910_defconfig
index 8a3316056408..63a778a45f0b 100644
--- a/configs/tbs2910_defconfig
+++ b/configs/tbs2910_defconfig
@@ -23,7 +23,6 @@ CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="echo PCI:; pci enum; pci 1; usb start; if hdmidet; then run set_con_hdmi; else run set_con_serial; fi"
 CONFIG_DEFAULT_FDT_FILE="imx6q-tbs2910.dtb"
 CONFIG_PRE_CONSOLE_BUFFER=y
-CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="Matrix U-Boot> "
 # CONFIG_CMD_BDI is not set
@@ -81,6 +80,7 @@ CONFIG_PINCTRL=y
 CONFIG_PINCTRL_IMX6=y
 CONFIG_DM_RTC=y
 CONFIG_RTC_DS1307=y
+CONFIG_DM_SERIAL=y
 CONFIG_MXC_UART=y
 CONFIG_DM_THERMAL=y
 CONFIG_IMX_THERMAL=y
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-14 23:16                 ` Fabio Estevam
@ 2022-03-15 20:56                   ` Soeren Moch
  2022-03-19 11:34                     ` Soeren Moch
  0 siblings, 1 reply; 16+ messages in thread
From: Soeren Moch @ 2022-03-15 20:56 UTC (permalink / raw)
  To: Fabio Estevam, Tom Rini; +Cc: Simon Glass, Stefano Babic, U-Boot Mailing List

Hi Fabio,

On 15.03.22 00:16, Fabio Estevam wrote:
> Hi Soeren,
>
> On Mon, Mar 14, 2022 at 7:38 PM Tom Rini <trini@konsulko.com> wrote:
>
>> 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.
> Does the attached patch work for you?
>
> I tested a similar patch on warp7 and it works fine here and the early
> U-Boot messages are shown.
This looks quite similar to my first attempt on this, but with
additional pre-reloc attributes on aips1, pinctrl_uart1, and soc.
Quite promising, probably exactly the right solution that I was looking for.

I promise to test this still this week, but may not be able to do so
before the weekend.

Thanks for working on this,
Soeren

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL
  2022-03-15 20:56                   ` Soeren Moch
@ 2022-03-19 11:34                     ` Soeren Moch
  0 siblings, 0 replies; 16+ messages in thread
From: Soeren Moch @ 2022-03-19 11:34 UTC (permalink / raw)
  To: Fabio Estevam, Tom Rini; +Cc: Simon Glass, Stefano Babic, U-Boot Mailing List

Hi Fabio,

On 15.03.22 21:56, Soeren Moch wrote:
> Hi Fabio,
>
> On 15.03.22 00:16, Fabio Estevam wrote:
>> Hi Soeren,
>>
>> On Mon, Mar 14, 2022 at 7:38 PM Tom Rini <trini@konsulko.com> wrote:
>>
>>> 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.
>> Does the attached patch work for you?
>>
>> I tested a similar patch on warp7 and it works fine here and the early
>> U-Boot messages are shown.
> This looks quite similar to my first attempt on this, but with
> additional pre-reloc attributes on aips1, pinctrl_uart1, and soc.
> Quite promising, probably exactly the right solution that I was
> looking for.
>
> I promise to test this still this week, but may not be able to do so
> before the weekend.
Your patch works great, I already sent my Tested-by to patchwork.
So my patch should be dropped in favour of your one, but I saw that
my patch already is marked 'Superseded'.

Regards,
Soeren
>
> Thanks for working on this,
> Soeren


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-03-19 11:34 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-14  8:26 [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Soeren Moch
2022-03-14  8:26 ` [PATCH 2/2] board: tbs2910: Convert to DM_SERIAL Soeren Moch
2022-03-14 18:24   ` Simon Glass
2022-03-14 18:28     ` Tom Rini
2022-03-14 19:22       ` Soeren Moch
2022-03-14 19:34         ` Tom Rini
2022-03-14 19:37         ` Simon Glass
2022-03-14 21:51           ` Sören Moch
2022-03-14 22:20             ` Simon Glass
2022-03-14 22:38               ` Tom Rini
2022-03-14 22:46                 ` Simon Glass
2022-03-14 23:16                 ` Fabio Estevam
2022-03-15 20:56                   ` Soeren Moch
2022-03-19 11:34                     ` Soeren Moch
2022-03-14 18:14 ` [PATCH 1/2] board: tbs2910: Enable Link Time Optimizations in defconfig Tom Rini
2022-03-14 18:24 ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.