Re: [PATCH v3 05/35] efi: Create a 64-bit app
On 10/26/21 5:30 AM, Simon Glass wrote:
Most modern platforms use 64-bit EFI so it is useful to have a U-Boot app
that runs under that. Add a (non-functional) build for this.
Note that --whole-archive causes the gcc 9.2 linker to crash, so disable
this for now. Once this is resolved, things should work.
For now, avoid mentioning the documentation for the 64-bit app, since it
does not work.
Signed-off-by: Simon Glass
---
Changes in v3:
- Default to 256MB of RAM for U-Boot instead of 32MB
Changes in v2:
- Add MAINTAINERS entry
- Add a work-around to avoid a toolchain crash
MAINTAINERS | 3 ++
Makefile | 8 +++-
arch/x86/cpu/u-boot-64.lds| 2 +
arch/x86/cpu/x86_64/Makefile | 4 ++
arch/x86/cpu/x86_64/cpu.c | 17
arch/x86/cpu/x86_64/misc.c| 25
board/efi/Kconfig | 15 ++-
board/efi/efi-x86_app/Kconfig | 2 +-
board/efi/efi-x86_app/MAINTAINERS | 11 +-
..._app_defconfig => efi-x86_app32_defconfig} | 2 +-
configs/efi-x86_app64_defconfig | 39 +++
doc/develop/uefi/u-boot_on_efi.rst| 6 +--
12 files changed, 106 insertions(+), 28 deletions(-)
create mode 100644 arch/x86/cpu/x86_64/misc.c
rename configs/{efi-x86_app_defconfig => efi-x86_app32_defconfig} (97%)
create mode 100644 configs/efi-x86_app64_defconfig
diff --git a/MAINTAINERS b/MAINTAINERS
index a46ef70d3cd..ca015e3f3e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -698,6 +698,9 @@ M: Simon Glass
M:Heinrich Schuchardt
S:Maintained
W:https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
+F: board/efi/efi-x86_app
+F: configs/efi-x86_app*
+F: doc/develop/uefi/u-boot_on_efi.rst
F:lib/efi/efi_app.c
F:scripts/build-efi.sh
diff --git a/Makefile b/Makefile
index fd2b328e021..1f043c380a5 100644
--- a/Makefile
+++ b/Makefile
@@ -1753,12 +1753,16 @@ quiet_cmd_u-boot__ ?= LTO $@
-Wl,-Map,u-boot.map;
\
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
else
+# Note: Linking efi-x86_app64 causes a segfault in the linker at present
+# when using x86_64-linux-gnu-ld.bfd
+# For now, disable --whole-archive which makes things link, although not
+# correctly
quiet_cmd_u-boot__ ?= LD $@
cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@
\
-T u-boot.lds $(u-boot-init)
\
- --whole-archive
\
+ $(if $(CONFIG_EFI_APP_64BIT),,--whole-archive)
\
$(u-boot-main)
\
- --no-whole-archive
\
+ $(if $(CONFIG_EFI_APP_64BIT),,--no-whole-archive)
\
$(PLATFORM_LIBS) -Map u-boot.map;
\
$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
endif
diff --git a/arch/x86/cpu/u-boot-64.lds b/arch/x86/cpu/u-boot-64.lds
index ee0812aefbc..92a30c2a387 100644
--- a/arch/x86/cpu/u-boot-64.lds
+++ b/arch/x86/cpu/u-boot-64.lds
@@ -15,7 +15,9 @@ SECTIONS
/DISCARD/ : { *(.u_boot_list_2_cmd_*) }
#endif
+#ifdef CONFIG_SYS_TEXT_BASE
. = CONFIG_SYS_TEXT_BASE; /* Location of bootcode in flash */
+#endif
__text_start = .;
.text.start : { *(.text.start); }
diff --git a/arch/x86/cpu/x86_64/Makefile b/arch/x86/cpu/x86_64/Makefile
index 400f0ffe397..e929563b2c1 100644
--- a/arch/x86/cpu/x86_64/Makefile
+++ b/arch/x86/cpu/x86_64/Makefile
@@ -4,3 +4,7 @@
#
obj-y += cpu.o interrupts.o setjmp.o
+
+ifndef CONFIG_EFI
+obj-y += misc.o
+endif
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index 90a766c3c57..8f72c9951a1 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -49,23 +49,6 @@ int x86_mp_init(void)
return 0;
}
-int misc_init_r(void)
-{
- return 0;
-}
-
-#ifndef CONFIG_SYS_COREBOOT
-int checkcpu(void)
-{
- return 0;
-}
-
-int print_cpuinfo(void)
-{
- return 0;
-}
-#endif
-
int x86_cpu_reinit_f(void)
{
return 0;
diff --git a/arch/x86/cpu/x86_64/misc.c b/arch/x86/cpu/x86_64/misc.c
new file mode 100644
index 000..02587ff0c50
--- /dev/null
+++ b/arch/x86/cpu/x86_64/misc.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2016 Google, Inc
+ * Written by Simon Glass
+ */
+
+#include
+#include
+
+int misc_init_r(void)
+{
+ return 0;
+}
+
+#ifndef CONFIG_SYS_COREBOOT
+int checkcpu(void)
+{
+ return 0;
+}
+
+int print_cpuinfo(void)
+{
+ return 0;
+}
+#endif
diff --git
Re: [PATCH v3 03/35] efi: Drop the OF_EMBED warning for EFI
On 10/26/21 5:30 AM, Simon Glass wrote: For the EFI app, we must embed the devicetree in the ELF file since that is the only thing that is run by UEFI. Drop the warning to avoid confusion. Signed-off-by: Simon Glass Reviewed-by: Heinrich Schuchardt --- (no changes since v2) Changes in v2: - Add new patch to drop the OF_EMBED warning for EFI Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5194e4dc782..fd2b328e021 100644 --- a/Makefile +++ b/Makefile @@ -1091,7 +1091,7 @@ endif ifeq ($(CONFIG_DEPRECATED),y) $(warning "You have deprecated configuration options enabled in your .config! Please check your configuration.") endif -ifeq ($(CONFIG_OF_EMBED),y) +ifeq ($(CONFIG_OF_EMBED)$(CONFIG_EFI_APP),y) @echo >&2 "= WARNING ==" @echo >&2 "CONFIG_OF_EMBED is enabled. This option should only" @echo >&2 "be used for debugging purposes. Please use"
Re: [PATCH v3 02/35] efi: Enable DM_ETH for the app
On 10/26/21 5:30 AM, Simon Glass wrote: There is no need to avoid driver model for networking. Drop this. Signed-off-by: Simon Glass This seems to be an unrelated problem in building efi-x86_app_defconfig: LD u-boot ld.bfd: arch/x86/lib/bios_asm.o: warning: relocation against `idt_ptr' in read-only section `.text' ld.bfd: warning: creating DT_TEXTREL in a shared object I have been using binutils 2.35.2-2 gcc 4:10.2.1-1 as provided with Debian Bulleyes. Reviewed-by: Heinrich Schuchardt --- (no changes since v2) Changes in v2: - Add new patch to enable DM_ETH for the app configs/efi-x86_app_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/efi-x86_app_defconfig b/configs/efi-x86_app_defconfig index e9ee66250cf..3f7c9ba9008 100644 --- a/configs/efi-x86_app_defconfig +++ b/configs/efi-x86_app_defconfig @@ -32,7 +32,6 @@ CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_REGMAP=y CONFIG_SYSCON=y -# CONFIG_DM_ETH is not set # CONFIG_REGEX is not set # CONFIG_GZIP is not set CONFIG_EFI=y
Re: [PATCH v3 01/35] efi: Add a script for building and testing U-Boot on UEFI
On 10/26/21 5:30 AM, Simon Glass wrote:
It is quite complicated to run U-Boot on QEMU since we have four
different builds and they must use different versions of qemu and the
UEFI binaries.
Add a script to help.
Signed-off-by: Simon Glass
It seems that you missed some of my review comments:
* The script does not work:
$ scripts/build-efi.sh -wsPr
Packaging efi-x86_app32
[sudo] password for user:
cp: cannot stat '/tmp/b//efi-x86_app32/u-boot-app.efi': No such file or
directory
* Do not use fixed directories in /tmp but use 'mktemp -d' to create a
unique directory.
Best regards
Heinrich
---
Changes in v3:
- Fix 'complicating' typo
- s/qemu/QEMU/
Changes in v2:
- Add MAINTAINERS entry
- Add support for creating a partition table with a filesystem inside
- Add support for running qemu with just a serial console (no display)
MAINTAINERS| 1 +
doc/develop/uefi/u-boot_on_efi.rst | 62 +
scripts/build-efi.sh | 193 +
3 files changed, 256 insertions(+)
create mode 100755 scripts/build-efi.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 5069f188065..a46ef70d3cd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -699,6 +699,7 @@ M: Heinrich Schuchardt
S:Maintained
W:https://u-boot.readthedocs.io/en/latest/develop/uefi/u-boot_on_efi.html
F:lib/efi/efi_app.c
+F: scripts/build-efi.sh
EFI PAYLOAD
M:Heinrich Schuchardt
diff --git a/doc/develop/uefi/u-boot_on_efi.rst
b/doc/develop/uefi/u-boot_on_efi.rst
index c9a41bc919f..4b2a733076d 100644
--- a/doc/develop/uefi/u-boot_on_efi.rst
+++ b/doc/develop/uefi/u-boot_on_efi.rst
@@ -96,6 +96,11 @@ that EFI does not support booting a 64-bit application from
a 32-bit
EFI (or vice versa). Also it will often fail to print an error message if
you get this wrong.
+You may find the script `scripts/build-efi.sh` helpful for building and testing
+U-Boot on UEFI on QEMU. It also includes links to UEFI binaries dating from
+2021.
+
+See `Example run`_ for an example run.
Inner workings
--
@@ -191,6 +196,63 @@ of code is built this way (see the extra- line in
lib/efi/Makefile).
Everything else is built as a normal U-Boot, so is always 32-bit on x86 at
present.
+Example run
+---
+
+This shows running with serial enabled (see `include/configs/efi-x86_app.h`)::
+
+ $ scripts/build-efi.sh -wsPr
+ Packaging efi-x86_app32
+ Running qemu-system-i386
+
+ BdsDxe: failed to load Boot0001 "UEFI QEMU HARDDISK QM5 " from
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0): Not Found
+ BdsDxe: loading Boot0002 "EFI Internal Shell" from
Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(7C04A583-9E3E-4F1C-AD65-E05268D0B4D1)
+ BdsDxe: starting Boot0002 "EFI Internal Shell" from
Fv(7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1)/FvFile(7C04A583-9E3E-4F1C-AD65-E05268D0B4D1)
+
+ UEFI Interactive Shell v2.2
+ EDK II
+ UEFI v2.70 (EDK II, 0x0001)
+ Mapping table
+ FS0: Alias(s):HD0a65535a1:;BLK1:
+
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)/HD(1,GPT,0FFD5E61-3B0C-4326-8049-BDCDC910AF72,0x800,0xB000)
+BLK0: Alias(s):
+ PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)
+
+ Press ESC in 5 seconds to skip startup.nsh or any other key to continue.
+ Shell> fs0:u-boot-app.efi
+ U-Boot EFI App (using allocated RAM address 47d4000) key=8d4, image=06a6f610
+ starting
+
+
+ U-Boot 2022.01-rc4 (Sep 19 2021 - 14:03:20 -0600)
+
+ CPU: x86, vendor Intel, device 663h
+ DRAM: 32 MiB
+0: efi_media_0 PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)
+1:
PciRoot(0x0)/Pci(0x3,0x0)/Sata(0x0,0x,0x0)/HD(1,GPT,0FFD5E61-3B0C-4326-8049-BDCDC910AF72,0x800,0xB000)
+ Loading Environment from nowhere... OK
+ Model: EFI x86 Application
+ Hit any key to stop autoboot: 0
+
+ Partition Map for EFI device 0 -- Partition Type: EFI
+
+ PartStart LBA End LBAName
+ Attributes
+ Type GUID
+ Partition GUID
+ 1 0x0800 0xb7ff "boot"
+ attrs: 0x
+ type: ebd0a0a2-b9e5-4433-87c0-68b6b72699c7
+ guid: 0ffd5e61-3b0c-4326-8049-bdcdc910af72
+ 19 startup.nsh
+ 528384 u-boot-app.efi
+ 10181 NvVars
+
+ 3 file(s), 0 dir(s)
+
+ => QEMU: Terminated
+
+
Future work
---
This work could be extended in a number of ways:
diff --git a/scripts/build-efi.sh b/scripts/build-efi.sh
new file mode 100755
index 000..ce761840097
--- /dev/null
+++ b/scripts/build-efi.sh
@@ -0,0 +1,193 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Script to build an EFI thing suitable for booting with QEMU, possibly running
+# it also.
+
+# This just an example. It assumes that
+
+# - you build U-Boot in ${ubdir}/ where is the U-Boot board config
+# - /mnt/x is a directory used for mounting
+# - you have access to the 'pure UEFI' builds for QEMU
+#
Re: [PATCH 1/1] video: fix positioning in TrueType console
On 10/27/21 9:52 PM, Anatolij Gustschin wrote: Hi Heinrich, On Wed, 27 Oct 2021 20:10:36 +0200 Heinrich Schuchardt xypron.g...@gmx.de wrote: Dear Anatolij, somehow you lost track of this patch since March: https://patchwork.ozlabs.org/project/uboot/patch/20210203161254.970-1-xypron.g...@gmx.de/ Could you, please, review it and consider it for merging. I've applied the patches for build testing and see sandbox test errors: https://source.denx.de/u-boot/custodians/u-boot-video/-/jobs/342128 Could you please fix them? Thanks! Dear Anatolij, The sandbox test relies on hashing the screen contents and these hash changes. The true test here is running 'u-boot -T -l' and checking the output. I have sent you a version 2 with adjusted hash values. Gitlab CI is happy now: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/9633 Best regards Heinrich -- Anatolij
[PATCH v2 1/1] video: fix positioning in TrueType console
With the patch accurate positioning is possible for mono-typed fonts:
Fix the return value of console_truetype_putc_xy(). The current position
is passed as parameter x. Some part of x represents a fractional pixel.
The return value represents how much the character position must be
advanced. This should only comprise the width of the current character and
not the preexisting fractional pixel position.
Characters are not square. As all characters of a mono-type font we can
take the width of any character. 'W' as one of the widest ANSI characters
provides also a good value for variable width fonts.
The character width must be a float.
Signed-off-by: Heinrich Schuchardt
---
v2:
Adjust hash values in tests
---
drivers/video/console_truetype.c | 27 ---
include/video_console.h | 2 +-
test/dm/video.c | 6 +++---
3 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c
index d9ad52cce0..6fe238eab3 100644
--- a/drivers/video/console_truetype.c
+++ b/drivers/video/console_truetype.c
@@ -209,7 +209,7 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
int width_frac, linenum;
struct pos_info *pos;
u8 *bits, *data;
- int advance;
+ int advance, kern_adv;
void *start, *line, *sync_start, *sync_end;
int row, ret;
int bg_r, bg_g, bg_b;
@@ -224,8 +224,11 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
* this character */
xpos = frac(VID_TO_PIXEL((double)x));
if (vc_priv->last_ch) {
- xpos += priv->scale * stbtt_GetCodepointKernAdvance(font,
- vc_priv->last_ch, ch);
+ kern_adv = stbtt_GetCodepointKernAdvance(font, vc_priv->last_ch,
+ch);
+ xpos += priv->scale * kern_adv;
+ } else {
+ kern_adv = 0;
}
/*
@@ -236,8 +239,8 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
*/
x_shift = xpos - (double)tt_floor(xpos);
xpos += advance * priv->scale;
- width_frac = (int)VID_TO_POS(xpos);
- if (x + width_frac >= vc_priv->xsize_frac)
+ width_frac = VID_TO_POS(priv->scale * (kern_adv + advance));
+ if (x + (int)VID_TO_POS(xpos) >= vc_priv->xsize_frac)
return -EAGAIN;
/* Write the current cursor position into history */
@@ -585,20 +588,21 @@ static int console_truetype_probe(struct udevice *dev)
struct udevice *vid_dev = dev->parent;
struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
stbtt_fontinfo *font = >font;
- int ascent;
+ int advance, ascent, lsb;
debug("%s: start\n", __func__);
+
if (vid_priv->font_size)
priv->font_size = vid_priv->font_size;
else
priv->font_size = CONFIG_CONSOLE_TRUETYPE_SIZE;
+
priv->font_data = console_truetype_find_font();
if (!priv->font_data) {
debug("%s: Could not find any fonts\n", __func__);
return -EBFONT;
}
- vc_priv->x_charsize = priv->font_size;
vc_priv->y_charsize = priv->font_size;
vc_priv->xstart_frac = VID_TO_POS(2);
vc_priv->cols = vid_priv->xsize / priv->font_size;
@@ -612,6 +616,15 @@ static int console_truetype_probe(struct udevice *dev)
/* Pre-calculate some things we will need regularly */
priv->scale = stbtt_ScaleForPixelHeight(font, priv->font_size);
+
+ /* Assuming that 'W' is the widest character */
+ stbtt_GetCodepointHMetrics(font, 'W', , );
+ advance += stbtt_GetCodepointKernAdvance(font, 'W', 'W');
+ vc_priv->cols =
+ (int)VID_TO_POS(vid_priv->xsize - 2) /
+ (int)VID_TO_POS(advance * priv->scale);
+ vc_priv->x_charsize = advance * priv->scale;
+
stbtt_GetFontVMetrics(font, , 0, 0);
priv->baseline = (int)(ascent * priv->scale);
debug("%s: ready\n", __func__);
diff --git a/include/video_console.h b/include/video_console.h
index 06b798ef10..6ec9b4c5b0 100644
--- a/include/video_console.h
+++ b/include/video_console.h
@@ -68,7 +68,7 @@ struct vidconsole_priv {
int ycur;
int rows;
int cols;
- int x_charsize;
+ double x_charsize;
int y_charsize;
int tab_width_frac;
int xsize_frac;
diff --git a/test/dm/video.c b/test/dm/video.c
index 1d29b2d61c..c0ad83521a 100644
--- a/test/dm/video.c
+++ b/test/dm/video.c
@@ -344,7 +344,7 @@ static int dm_test_video_truetype(struct unit_test_state
*uts)
ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, ));
ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, ));
vidconsole_put_string(con, test_string);
-
[BUG] Spurious error in Gitlab CI for sandbox_spl test.py
Hello Tom, we have some trouble with Gitlab CI: In https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/342220 sandbox: + sandbox_spl +/usr/bin/ld: /tmp/u-boot-spl.Z4D5y0.ltrans0.ltrans.o: in function `dm_setup_inst.lto_priv.0': +drivers/core/root.c:134: undefined reference to `_u_boot_list_2_udevice_2_root' +/usr/bin/ld: /tmp/u-boot-spl.Z4D5y0.ltrans0.ltrans.o:(.u_boot_list_2_uclass_2_irq+0x10): undefined reference to `_u_boot_list_2_udevice_2_irq_sbox' +/usr/bin/ld: /tmp/u-boot-spl.Z4D5y0.ltrans0.ltrans.o:(.u_boot_list_2_uclass_2_irq+0x18): undefined reference to `_u_boot_list_2_udevice_2_irq_sbox' Repeating the job showed no error: https://source.denx.de/u-boot/custodians/u-boot-efi/-/jobs/342252 Best regards Heinrich
Re: [PATCH 1/3] net: dsa: Use true instead of 1 in the set_promisc() call
On Wed, Oct 27, 2021 at 5:19 AM Bin Meng wrote: > > On Sun, Oct 17, 2021 at 2:26 AM Ramon Fried wrote: > > > > On Wed, Sep 29, 2021 at 4:32 PM Vladimir Oltean > > wrote: > > > > > > On Wed, Sep 29, 2021 at 01:50:44PM +0800, Bin Meng wrote: > > > > set_promisc() call accepts the parameter of a bool type. Make it > > > > clear by using true instead of 1. > > > > > > > > Signed-off-by: Bin Meng > > > > --- > > > > > > > > net/dsa-uclass.c | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c > > > > index 694664d81b..dcefec03f4 100644 > > > > --- a/net/dsa-uclass.c > > > > +++ b/net/dsa-uclass.c > > > > @@ -282,7 +282,7 @@ static int dsa_port_probe(struct udevice *pdev) > > > > struct eth_ops *eth_ops = eth_get_ops(master); > > > > > > > > if (eth_ops->set_promisc) > > > > - eth_ops->set_promisc(master, 1); > > > > + eth_ops->set_promisc(master, true); > > > > > > > > return 0; > > > > } > > > > -- > > > > 2.25.1 > > > > > > > > > > Reviewed-by: Vladimir Oltean > > Reviewed-by: Ramon Fried > > Ping for apply? I'll get to the patches today. thanks for waking me up :)
[PATCH 5/5] arm: mvebu: Espressobin: Use new API for setting default env at runtime
From: Marek Behún
ESPRESSObin's board code uses an ad-hoc solution for ensuring that
ethaddrs are not overwritten by `env default -a` command and that the
`fdtfile` is set to correct value when `env default -a` is called.
This ad-hoc solution is overwriting the default_environment[] buffer in
board_late_init().
Since now we have a specific API for overwriting default environment,
convert this ad-hoc code to this new API.
Since the default_environment[] buffer is not overwritten anymore by any
board, remove support for read-write default_environment[].
Signed-off-by: Marek Behún
---
board/Marvell/mvebu_armada-37xx/board.c | 120
configs/mvebu_espressobin-88f3720_defconfig | 1 -
include/configs/mvebu_armada-37xx.h | 17 +--
include/env_default.h | 2 -
include/env_internal.h | 4 -
5 files changed, 75 insertions(+), 69 deletions(-)
diff --git a/board/Marvell/mvebu_armada-37xx/board.c
b/board/Marvell/mvebu_armada-37xx/board.c
index d7b6ecafbf..5464482423 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -84,43 +84,16 @@ int board_init(void)
return 0;
}
-#ifdef CONFIG_BOARD_LATE_INIT
-int board_late_init(void)
+static bool ram_is_ddr4(void)
{
- char *ptr = _environment[0];
- struct udevice *dev;
- struct mmc *mmc_dev;
- bool ddr4, emmc;
- const char *mac;
- char eth[10];
- int i;
-
- if (!of_machine_is_compatible("globalscale,espressobin"))
- return 0;
-
- /* Find free buffer in default_environment[] for new variables */
- while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
- ptr += 2;
-
- /*
-* Ensure that 'env default -a' does not erase permanent MAC addresses
-* stored in env variables: $ethaddr, $eth1addr, $eth2addr and $eth3addr
-*/
-
- mac = env_get("ethaddr");
- if (mac && strlen(mac) <= 17)
- ptr += sprintf(ptr, "ethaddr=%s", mac) + 1;
-
- for (i = 1; i <= 3; i++) {
- sprintf(eth, "eth%daddr", i);
- mac = env_get(eth);
- if (mac && strlen(mac) <= 17)
- ptr += sprintf(ptr, "%s=%s", eth, mac) + 1;
- }
-
- /* If the memory controller has been configured for DDR4, we're running
on v7 */
- ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >>
A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
+ return ((readl(A3700_CH0_MC_CTRL2_REG) >>
A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
& A3700_MC_CTRL2_SDRAM_TYPE_MASK) ==
A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
+}
+
+static bool has_emmc(void)
+{
+ struct mmc *mmc_dev;
+ bool emmc;
/* eMMC is mmc dev num 1 */
mmc_dev = find_mmc_device(1);
@@ -128,24 +101,79 @@ int board_late_init(void)
/* if eMMC is not present then remove it from DM */
if (!emmc && mmc_dev) {
+ struct udevice *dev;
dev = mmc_dev->dev;
device_remove(dev, DM_REMOVE_NORMAL);
device_unbind(dev);
}
- /* Ensure that 'env default -a' set correct value to $fdtfile */
- if (ddr4 && emmc)
- strcpy(ptr,
"fdtfile=marvell/armada-3720-espressobin-v7-emmc.dtb");
- else if (ddr4)
- strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-v7.dtb");
- else if (emmc)
- strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-emmc.dtb");
- else
- strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin.dtb");
+ return emmc;
+}
- return 0;
+const char *board_special_default_env(unsigned i, const char **name)
+{
+ static unsigned nvars;
+ static bool built;
+ static struct {
+ const char *name;
+ char *value;
+ } vars[5];
+
+ if (!of_machine_is_compatible("globalscale,espressobin"))
+ return NULL;
+
+ /*
+* We can access ethNaddr variables only when environment is valid.
+* We can access mmc only if relocated (initr_env() is called after
+* initr_mmc(), so at this point mmc device is present.
+*/
+ if (gd->env_valid != ENV_VALID || !(gd->flags & GD_FLG_RELOC))
+ return NULL;
+
+ if (!built) {
+ const char *names[4] = { "ethaddr", "eth1addr", "eth2addr",
+"eth3addr" };
+ bool ddr4, emmc;
+
+ for (i = 0; i < 4; ++i) {
+ const char *mac;
+
+ mac = env_get(names[i]);
+ if (mac && strlen(mac) <= 17) {
+ vars[nvars].name = names[i];
+ vars[nvars].value = strdup(mac);
+ ++nvars;
+ }
+ }
+
+ /*
+* If the memory controller has been configured
[PATCH 4/5] env: Add support for board specific special default environment
From: Marek Behún
The default_environment[] buffer is built at compile time, but sometimes
it makes sense for some default environment variables to be determined
at runtime, for example:
- one board code may support different boards, and needs that
fdtfile, board, board_name
are set appropriately when command
env default -a
is executed
- some boards may want to prohibit the
env default -a
command to remove device MAC addresses stored in
ethaddr, ethNaddr.
This is the case for the ESPRESSObin board code, for example, where
currently the board_late_init() function rewrites the default
environment array to achieve this.
Add a new board specific function,
const char *board_special_default_env(unsigned i, const char **name);
which returns the value of i-th board special default environemnt
variable, while storing it's name to *name.
Add default weak implementation of this functions returning NULL.
Add code to default environemnt handlers in env/common.c, which iterate
these special board default environment variables and get it's values in
precedence to values in the default_environment[] buffer.
Signed-off-by: Marek Behún
---
env/common.c | 99 ++--
1 file changed, 96 insertions(+), 3 deletions(-)
diff --git a/env/common.c b/env/common.c
index 208e2adaa0..d157ca562a 100644
--- a/env/common.c
+++ b/env/common.c
@@ -148,6 +148,41 @@ char *from_env(const char *envvar)
return ret;
}
+__weak const char *board_special_default_env(unsigned i, const char **name)
+{
+ return NULL;
+}
+
+static int board_special_default_env_get(const char *var, char *buf,
+unsigned len)
+{
+ int i;
+
+ /*
+* Iterate all board special default env variables, and if one
+* exists with the requested name, return it.
+*/
+ for (i = 0; ; ++i) {
+ const char *name, *value;
+
+ value = board_special_default_env(i, );
+ if (!value)
+ break;
+
+ if (!strcmp(var, name)) {
+ unsigned res = strlen(value);
+
+ memcpy(buf, value, min(len, res + 1));
+ if (len <= res)
+ buf[len - 1] = '\0';
+
+ return res;
+ }
+ }
+
+ return -1;
+}
+
static int env_get_from_linear(const char *env, const char *name, char *buf,
unsigned len)
{
@@ -157,6 +192,17 @@ static int env_get_from_linear(const char *env, const char
*name, char *buf,
if (name == NULL || *name == '\0')
return -1;
+ if (env == default_environment) {
+ int res = board_special_default_env_get(name, buf, len);
+
+ /*
+* Board special default envs take precedence over the
+* default_environment[] array.
+*/
+ if (res >= 0)
+ return res;
+ }
+
name_len = strlen(name);
for (p = env; *p != '\0'; p = end + 1) {
@@ -248,6 +294,41 @@ char *env_get_default(const char *name)
return NULL;
}
+static int import_board_special_default_envs(bool all, int nvars,
+char * const vars[], int flags)
+{
+ int i;
+
+ for (i = 0; ; ++i) {
+ const char *name, *value;
+ struct env_entry e, *ep;
+
+ value = board_special_default_env(i, );
+ if (!value)
+ break;
+
+ if (!all) {
+ int j;
+
+ /* If name is not in vars, skip */
+
+ for (j = 0; j < nvars; ++j)
+ if (!strcmp(name, vars[j]))
+ break;
+ if (j == nvars)
+ continue;
+ }
+
+ e.key = name;
+ e.data = (char *)value;
+
+ if (!hsearch_r(e, ENV_ENTER, , _htab, flags))
+ return -1;
+ }
+
+ return 0;
+}
+
void env_set_default(const char *s, int flags)
{
if (s) {
@@ -270,6 +351,12 @@ void env_set_default(const char *s, int flags)
return;
}
+ if (import_board_special_default_envs(true, 0, NULL, flags) < 0) {
+ pr_err("## Error: Board special default environment import
failed: errno = %d\n",
+ errno);
+ return;
+ }
+
gd->flags |= GD_FLG_ENV_READY;
gd->flags |= GD_FLG_ENV_DEFAULT;
}
@@ -278,14 +365,20 @@ void env_set_default(const char *s, int flags)
/* [re]set individual variables to their value in the default environment */
int env_set_default_vars(int nvars, char * const vars[], int flags)
{
+ int res;
+
/*
*
[PATCH 3/5] env: Simplify env_get_default()
From: Marek Behún
Instead of pretending that we don't have environment to force searching
default environment in env_get_default(), get the data from the
default_environment[] buffer directly.
Signed-off-by: Marek Behún
---
env/common.c | 45 -
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/env/common.c b/env/common.c
index 757c5f9ecd..208e2adaa0 100644
--- a/env/common.c
+++ b/env/common.c
@@ -148,12 +148,10 @@ char *from_env(const char *envvar)
return ret;
}
-/*
- * Look up variable from environment for restricted C runtime env.
- */
-int env_get_f(const char *name, char *buf, unsigned len)
+static int env_get_from_linear(const char *env, const char *name, char *buf,
+ unsigned len)
{
- const char *env, *p, *end;
+ const char *p, *end;
size_t name_len;
if (name == NULL || *name == '\0')
@@ -161,11 +159,6 @@ int env_get_f(const char *name, char *buf, unsigned len)
name_len = strlen(name);
- if (gd->env_valid == ENV_INVALID)
- env = default_environment;
- else
- env = (const char *)gd->env_addr;
-
for (p = env; *p != '\0'; p = end + 1) {
const char *value;
unsigned res;
@@ -193,6 +186,21 @@ int env_get_f(const char *name, char *buf, unsigned len)
return -1;
}
+/*
+ * Look up variable from environment for restricted C runtime env.
+ */
+int env_get_f(const char *name, char *buf, unsigned len)
+{
+ const char *env;
+
+ if (gd->env_valid == ENV_INVALID)
+ env = default_environment;
+ else
+ env = (const char *)gd->env_addr;
+
+ return env_get_from_linear(env, name, buf, len);
+}
+
/**
* Decode the integer value of an environment variable and return it.
*
@@ -232,17 +240,12 @@ int env_get_yesno(const char *var)
*/
char *env_get_default(const char *name)
{
- char *ret_val;
- unsigned long really_valid = gd->env_valid;
- unsigned long real_gd_flags = gd->flags;
-
- /* Pretend that the image is bad. */
- gd->flags &= ~GD_FLG_ENV_READY;
- gd->env_valid = ENV_INVALID;
- ret_val = env_get(name);
- gd->env_valid = really_valid;
- gd->flags = real_gd_flags;
- return ret_val;
+ if (env_get_from_linear(default_environment, name,
+ (char *)(gd->env_buf),
+ sizeof(gd->env_buf)) >= 0)
+ return (char *)(gd->env_buf);
+
+ return NULL;
}
void env_set_default(const char *s, int flags)
--
2.32.0
[PATCH 1/5] env: Don't set ready flag if import failed in env_set_default()
From: Marek Behún
Do not set GD_FLG_ENV_READY nor GD_FLG_ENV_DEFAULT if failed importing
in env_set_default().
Signed-off-by: Marek Behún
---
env/common.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/env/common.c b/env/common.c
index 99729ca002..2aa23545ba 100644
--- a/env/common.c
+++ b/env/common.c
@@ -261,9 +261,11 @@ void env_set_default(const char *s, int flags)
flags |= H_DEFAULT;
if (himport_r(_htab, default_environment,
sizeof(default_environment), '\0', flags, 0,
- 0, NULL) == 0)
+ 0, NULL) == 0) {
pr_err("## Error: Environment import failed: errno = %d\n",
errno);
+ return;
+ }
gd->flags |= GD_FLG_ENV_READY;
gd->flags |= GD_FLG_ENV_DEFAULT;
--
2.32.0
[PATCH 2/5] env: Fix env_get() when returning empty string using env_get_f()
From: Marek Behún The env_get_f() function returns -1 on failure. Returning 0 means that the variable exists, and is empty string. Signed-off-by: Marek Behún --- env/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/env/common.c b/env/common.c index 2aa23545ba..757c5f9ecd 100644 --- a/env/common.c +++ b/env/common.c @@ -125,7 +125,7 @@ char *env_get(const char *name) } /* restricted capabilities before import */ - if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) > 0) + if (env_get_f(name, (char *)(gd->env_buf), sizeof(gd->env_buf)) >= 0) return (char *)(gd->env_buf); return NULL; -- 2.32.0
[PATCH 0/5] Board specific runtime determined default env
From: Marek Behún
Hello Simon, Stefan, Pali,
this series adds support for board specific runtime determined
default environment variables.
IMPORTANT: This series depends on the series
http://patchwork.ozlabs.org/project/uboot/list/?series=268452
Currently the
env default [-a]
command uses the default_environment[] buffer to get the default
env variables.
Sometimes it makes sense to have runtime determined default env
settings.
For example the ESPRESSObin board has 4 variants
([ddr3 vs ddr4] x [emmc vs sd]), and each uses different device tree.
Thus the `fdtfile` env variable has different values for these 4
variants. (We can't set this variable via env_set() in some board init
function, because then the user would be unable to overwrite it.)
In order for the command
env default fdtfile
to work as the user would expect, we need to support overwriting default
environment in runtime.
Pali solved this for ESPRESSObin by declaring the default_environment[]
buffer read-write, instead of read-only, and adding ad-hoc code into
board_late_init() that writes into the default_environment[] buffer.
This ad-hoc code works, but it would be better to have a generic API
for this, since there are other boards which could benefit.
The first 3 patches in this series fix and simplify code in
env/common.c.
The 4th patch adds support for board specific runtime determined
default environment in such a way that if a board code defines function
const char *board_special_default_env(unsigned i, const char **name);
The default weak implementation of this function is trivial and just
returns NULL.
This function is to be defined in board code, and when defined, it must
return the value of the i-th runtime determined default env variable,
while storing its name into *name.
For example:
const char *board_special_default_env(unsigned i, const char **name)
{
switch (i) {
case 0:
*name = "board";
return is_ddr4() ? "my_board_ddr4" : "my_board";
case 1:
*name = "fdtfile";
return is_ddr4() ? "my-board-ddr4.dtb" : "my-board.dtb";
default:
return NULL;
}
The last patch (NOT TESTED) converts the ESPRESSObin ad-hoc code to
use this API.
Marek
Marek Behún (5):
env: Don't set ready flag if import failed in env_set_default()
env: Fix env_get() when returning empty string using env_get_f()
env: Simplify env_get_default()
env: Add support for board specific special default environment
arm: mvebu: Espressobin: Use new API for setting default env at
runtime
board/Marvell/mvebu_armada-37xx/board.c | 120 ++--
configs/mvebu_espressobin-88f3720_defconfig | 1 -
env/common.c| 150
include/configs/mvebu_armada-37xx.h | 17 +--
include/env_default.h | 2 -
include/env_internal.h | 4 -
6 files changed, 199 insertions(+), 95 deletions(-)
--
2.32.0
Re: kirkwood stack size
Hi Marek, Ah. That makes sense! I don't have the km_kirkwood_xxx boards. Thanks, Tony On Wed, Oct 27, 2021 at 6:28 PM Marek Behún wrote: > > On Wed, 27 Oct 2021 16:06:58 -0700 > Tony Dinh wrote: > > > Hi Marek, > > > > In reference to: > > https://lists.denx.de/pipermail/u-boot/2021-October/465221.html > > > > I have several Kirkwood boards, and currently support these boards in > > mainline: Sheevaplug, Zyxel NSA310S, GoFlex Home, Dockstar. I also > > have a few out-of-tree Kirkwood u-boots that have not been submitted > > for mainline yet. > > > > I did not see the stack size problem, or any other problem. And I'm > > running the 2021.07 version. > > > > Regards, > > Tony > > Hi Tony, > > thank you for your reply. > > I should have mentioned that this concerns only the following > defconfigs: > km_kirkwood_128m16_defconfig > km_kirkwood_defconfig > km_kirkwood_pci_defconfig > since only these store environment in EEPROM. > > Marek
Re: kirkwood stack size
On Wed, 27 Oct 2021 16:06:58 -0700 Tony Dinh wrote: > Hi Marek, > > In reference to: > https://lists.denx.de/pipermail/u-boot/2021-October/465221.html > > I have several Kirkwood boards, and currently support these boards in > mainline: Sheevaplug, Zyxel NSA310S, GoFlex Home, Dockstar. I also > have a few out-of-tree Kirkwood u-boots that have not been submitted > for mainline yet. > > I did not see the stack size problem, or any other problem. And I'm > running the 2021.07 version. > > Regards, > Tony Hi Tony, thank you for your reply. I should have mentioned that this concerns only the following defconfigs: km_kirkwood_128m16_defconfig km_kirkwood_defconfig km_kirkwood_pci_defconfig since only these store environment in EEPROM. Marek
Re: [PATCH 2/2] cmd: sbi: show SBI implementation version
> From: Heinrich Schuchardt > Sent: Monday, October 25, 2021 9:10 PM > To: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) > > Cc: u-boot@lists.denx.de; Heinrich Schuchardt > > Subject: [PATCH 2/2] cmd: sbi: show SBI implementation version > > Let the sbi command show the SBI implementation version > > Signed-off-by: Heinrich Schuchardt > --- > cmd/riscv/sbi.c | 26 ++ > 1 file changed, 18 insertions(+), 8 deletions(-) Reviewed-by: Rick Chen
Re: [PATCH 1/2] riscv: function to retrieve SBI implementation version
> From: Heinrich Schuchardt > Sent: Monday, October 25, 2021 9:10 PM > To: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) > > Cc: u-boot@lists.denx.de; Heinrich Schuchardt > > Subject: [PATCH 1/2] riscv: function to retrieve SBI implementation version > > Provide function sbi_get_impl_version() to retrieve the SBI implementation > version. > > Signed-off-by: Heinrich Schuchardt > --- > arch/riscv/include/asm/sbi.h | 1 + > arch/riscv/lib/sbi.c | 19 +++ > 2 files changed, 20 insertions(+) Reviewed-by: Rick Chen
Re: kirkwood stack size
Hi Marek, In reference to: https://lists.denx.de/pipermail/u-boot/2021-October/465221.html I have several Kirkwood boards, and currently support these boards in mainline: Sheevaplug, Zyxel NSA310S, GoFlex Home, Dockstar. I also have a few out-of-tree Kirkwood u-boots that have not been submitted for mainline yet. I did not see the stack size problem, or any other problem. And I'm running the 2021.07 version. Regards, Tony
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
> From: Simon Glass > Date: Wed, 27 Oct 2021 12:23:21 -0600 > > Hi François, > > On Wed, 27 Oct 2021 at 09:14, François Ozog wrote: > > > > > > > > On Wed, 27 Oct 2021 at 16:08, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 00:07, François Ozog > >> wrote: > >> > > >> > Hi Simon > >> > > >> > Position unchanged on this series: adding fake dts for boards that > >> > generate their device tree in the dts directory is not good. If you have > >> > them in documentation the it is acceptable. > >> > >> I think we are going to have to disagree on this one. I actually used > >> the qemu one in testing/development recently. We have to have a way to > >> develop in-tree with U-Boot. It does not impinge on any of your use > >> cases, so far as I know. > > > > I am not the only one in disagreement... You just saw Alex Bénée from Qemu > > saying the same thing. > > I understand the advanced debug/development scenario you mention. > > But locating the DT files in the dts directory is mis-leading the > > contributors to think that they need to compile the DT for the targeted > > platforms. > > For your advanced scenario, you can still have the dts in the documentation > > area, or whatever directory (except dts). compile it and supply to U-Boot. > > We have this situation with rpi 1, 2 and 3 and I don't believe anyone > has noticed. U-Boot handles the build automatically. If you turn off > OF_BOARD, it will use the U-Boot devicetree always so you know what is > going on. Until. The Raspberry Pi foundation releases a new firmware that configures the hardware differently such that the addresses in the U-Boot devicetree are wrong and nothing works anymore. Can't speak for the rpi 1, but this has happened in the past for the rpi 2 and 3 as well as more recently on the rpi 4. > We can add a message to U-Boot indicating where the devicetree came > from, perhaps? That might be useful given everything that is going on. > > As in this case, quite often in these discussions I struggle to > understand what is behind the objection. Is it that your customers are > demanding that devicetrees become private, secret data, not included > in open-source projects? Or is it just the strange case of QEMU that > is informing your thinking? I know of at least one project where the > first-stage bootloader produces a devicetree and no one has the source > for it. I believe TF-A was created for licensing reasons...so can you > be a bit clearer about what the problem actually is? If a board is > in-tree in U-Boot I would like it to have a devicetree there, at least > until we have a better option. At the very least, it MUST be > discoverable and it must be possible to undertake U-Boot development > easily without a lot of messing around. How many people are there out there that work on U-Boot that don't have a Linux source tree checked out? Even I have several of those lying around on my development systems and I am an OpenBSD developer ;).
[PATCH] Fix MMC numbering issue for Raspberry Pi 3
From: Mike Karels Using mmc.dtbo from rpi-firmware to switch the controller for the SD card slot from sdhci to sdhost causes the numbering to change; the SD card is then not recognized at boot. Add to the range checked. --- include/configs/rpi.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/configs/rpi.h b/include/configs/rpi.h index 55768a46da..4c5c1ac31f 100644 --- a/include/configs/rpi.h +++ b/include/configs/rpi.h @@ -137,7 +137,8 @@ #if CONFIG_IS_ENABLED(CMD_MMC) #define BOOT_TARGET_MMC(func) \ func(MMC, mmc, 0) \ - func(MMC, mmc, 1) + func(MMC, mmc, 1) \ + func(MMC, mmc, 2) #else #define BOOT_TARGET_MMC(func) #endif -- 2.32.0
kirkwood stack size
Hello Stefan, do you have some ARM Kirkwood board? I am working on some env patches, because the code in the env/ directory is a pain in the ass of old relics, and Kirkwood is the only platform using the eeprom environment driver. I would like to ask about Kirkwood stack size. In arch/arm/mach-kirkwood/include/mach/config.h it says /* Kirkwood has 2k of Security SRAM, use it for SP */ #define CONFIG_SYS_INIT_SP_ADDR 0xC8012000 So if I understand correctly, Kirkwood has only 2k of init stack. But the env_eeprom_load() method in env/eeprom.c allocates a CONFIG_ENV_SIZE bytes long array for environment, and CONFIG_ENV_SIZE is 2k. Is the init stack changed to another stack before environment is loaded? I couldn't find code which would do that. As far as I am able to determine, this 2k long init stack is used, and this means that the code in env/eeprom.c is wrong, since it allocates below stack space. Maybe no one used newer U-Boot on Kirkwood for a long time and this was not noticed? Should we remove support for Kirkwood? Marek
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
Hi Tom Le mer. 27 oct. 2021 à 21:06, Tom Rini a écrit : > On Wed, Oct 27, 2021 at 06:02:19PM +0200, François Ozog wrote: > > Hi Mark, > > > > On Wed, 27 Oct 2021 at 17:10, Mark Kettenis > wrote: > > > > > > From: François Ozog > > > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > > > > > Hi, > > > > > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > > > Hi all, > > > > > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini > wrote: > > > > > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini > > > wrote: > > > > > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass > wrote: > > > > > > > > > > Hi François, > > > > > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass < > > > s...@chromium.org> > > > > > a écrit : > > > > > > > > > > >> > > > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > > > >> > > > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini < > > > tr...@konsulko.com> > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng > > > wrote: > > > > > > > > > > >> > > Hi Simon, > > > > > > > > > > >> > > > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > > > s...@chromium.org> wrote: > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped > OF_PRIOR_STAGE > > > and > > > > > OF_HOSTFILE so > > > > > > > > > > >> > > > there are only three ways to obtain a > devicetree: > > > > > > > > > > >> > > > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the > > > devicetree > > > > > is built and > > > > > > > > > > >> > > > appended to U-Boot > > > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > > > devicetree is embedded in > > > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its > own > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > The last one is currently set up so that no > > > devicetree > > > > > is needed at all > > > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, > but > > > > > some don't. Some > > > > > > > > > > >> > > > don't even provide instructions on how to boot > on > > > the > > > > > board. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > The problems with this approach are documented > at > > > [1]. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct > from > > > > > OF_SEPARATE. Any board > > > > > > > > > > >> > > > can obtain its devicetree at runtime, even it is > > > has a > > > > > devicetree built > > > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a > > > second-stage > > > > > bootloader and its > > > > > > > > > > >> > > > caller may have a better idea about the hardware > > > > > available in the machine. > > > > > > > > > > >> > > > This is the case with a few QEMU boards, for > > > example. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a > > > 'choice'. It > > > > > should be an > > > > > > > > > > >> > > > option, available with either OF_SEPARATE or > > > OF_EMBED. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > This series makes this change, adding various > > > missing > > > > > devicetree files > > > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > > > >> > > > > > > > > > > > > >> > > Adding device trees that are never used sounds > like a > > > > > hack to me. > > > > > > > > > > >> > > > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on > the > > > fly > > > > > based on > > > > > > > > > > >> > > command line parameters, and the device tree you > put > > > in > > > > > this series > > > > > > > > > > >> > > has various hardcoded values which > normally > > > do > > > > > not show up > > > > > > > > > > >> > > in hand-written dts files. > > > > > > > > > > >> > > > > > > > > > > > > >> > > I am not sure I understand the whole point of > this. > > > > > > > > > > >> > > > > > > > > > > > >> > I am also confused and do not like the idea of > adding > > > > > device trees for > > > > > > > > > > >> > platforms that are capable of and can / do have a > device > > > > > tree to give us > > > > > > > > > > >> > at run time. > > > > > > > > > > >> > > > > > > > > > > >> (I'll just reply to this one email, since the same > points > > > > > applies to > > > > > > > > > > >> all replies I think) > > > > > > > > > > >> > > >
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
> From: Simon Glass > Date: Wed, 27 Oct 2021 09:24:25 -0600 > > Hi Mark, > > On Wed, 27 Oct 2021 at 09:11, Mark Kettenis wrote: > > > > > From: François Ozog > > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > > > In my view U-Boot shall be able to leverage device tree format > > > (source and binary) to store its own data. When you say "the" > > > DT, I always think this is "the" DT that is passed to OS and in > > > "that" DT, there should be no U-Boot entries. > > > > Why not? As long as the device tree validates, it is perfectly fine > > to have additional nodes and properties present. The propertiesand > > nodes will be simply ignored by the OS. > > > > OpenBSD will print: > > > > "binman" not configured > > > > for the binman node that some of the U-Boot board targets now have, > > but it doesn't really make a difference. If there is a proper binding > > for that node, I could simply filter it out. Or we have U-Boot filter > > it out before the DT gets passed along like Tom suggests. > > Just on that point, I believe the binman falls into the same bucket > that Tom is talking about here, in that it should be a standard > binding. Ideally I would like this to become a standard format so that > anything in firmware can use it to find stuff. I believe it is a good > and extensible way to describe the structure of firmware across all > projects. Oh, I agree that it is a reasonable thing to have a description of the structure of the firmware in the device tree. > Does "not configured" mean that it did not find the compatible string? > We could add one of those, for now, perhaps. "not configured" just means that no device driver attached to the node. Usually that is because we don't have a device driver for the device corresponding to the node yet. But in the case of the "binman" node it doesn't really make sense for a device driver to attach. In such a case we tend to filter out the node such that the "not configured" line isn't printed. That can be done either by name or by compatible string. So an "official" binding would help here and it should either use a standardized name (that shouldn't be used for other purposes then) or it should use defined a compatible string. Anyway, this is not really critical. I just brought it up to illustrate that such nodes are mostly harmless.
Re: [PATCH v5 26/26] fdt: Don't call board_fdt_blob_setup() without OF_BOARD
On Wed, Oct 27, 2021 at 11:33:53PM +0200, François Ozog wrote:
> Hi Tom
>
> Le mer. 27 oct. 2021 à 21:12, Tom Rini a écrit :
>
> > On Wed, Oct 27, 2021 at 10:17:05AM +0300, Ilias Apalodimas wrote:
> > > Hi Simon,
> > >
> > > On Tue, 26 Oct 2021 at 18:27, Simon Glass wrote:
> > > >
> > > > Hi Ilias,
> > > >
> > > > On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas
> > > > wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > As I said here [1], this is moving on an entirely different
> > direction I had
> > > > > in mind. I'd much prefer starting the discussions for a solution that
> > > > > allows us to scale.
> > > >
> > > > I am missing the point here. Is there something in the plans that I
> > > > don't know about?
> > >
> > > I have some ideas, but haven't found time to code it and send patches
> > yet.
> > >
> > > >
> > > > > FWIW I think the current code is still not clean for my taste.
> > Commit
> > > > > 3b595da441cf ("fdtdec: allow board to provide fdt for
> > CONFIG_OF_SEPARATE")
> > > > > allowed this function to be used regardless of the config options.
> > IMHO we
> > > > > should have 2 clear options:
> > > > > - U-Boot provides the DTB
> > > >
> > > > Supported with: OF_SEPARATE
> > > >
> > > > > - It's somehow passed over to U-Boot
> > > >
> > > > Supported with: OF_SEPARATE + OF_BOARD
> > >
> > > That's exactly what I don't personally like. In your example OF_BOARD
> > > means "U-Boot has a DTB and here's a way to override it". In my head
> >
> > In some ways this is just highlighting a long standing problem of
> > working with DTB. There's certainly been some places where splitting
> > the data from the driver has helped. But in some other places, my
> > goodness has it made the development cycle worse. Doing this on QEMU
> > probably is on one of the most annoying cases too since yes, you start
> > by dumping the dtb you would get, then iterating the dts outside of the
> > rest of the scope of your build. In the kernel you can "make dtbs" at
> > least and get things spit out. I'm just repeating over here that the
> > development cycle of working on device trees is at times not great.
>
> In general you do this only once, unless you use the virt machine and in
> that case there is no documentation about DT generated for each device
> added on the command line.
> I had lots of difficulties getting all UART possibilities: virtio <1.0,
> 1.0,1.1; mmio or pci.
> But once you get the command line, the dTB is fixed.
> Of course it would be better to get Qemu document DT generation of its
> devices.
In a lot of ways QEMU isn't even the real problem, unless it's applying
rand() to some parts of where devices end up, etc. The cycle there
isn't any worse than where the device tree resides in hardware. The
device tree development cycle is a pain. QEMU is actually not so bad in
this regard really since it's just dumpdtb and pass back the modified
one, it takes care of then ensuring it's in the correct spot. The part
where every blob must be in the right place is another part of that
horror show that binman and fconf both try and address.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v5 26/26] fdt: Don't call board_fdt_blob_setup() without OF_BOARD
Hi Tom
Le mer. 27 oct. 2021 à 21:12, Tom Rini a écrit :
> On Wed, Oct 27, 2021 at 10:17:05AM +0300, Ilias Apalodimas wrote:
> > Hi Simon,
> >
> > On Tue, 26 Oct 2021 at 18:27, Simon Glass wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas
> > > wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > As I said here [1], this is moving on an entirely different
> direction I had
> > > > in mind. I'd much prefer starting the discussions for a solution that
> > > > allows us to scale.
> > >
> > > I am missing the point here. Is there something in the plans that I
> > > don't know about?
> >
> > I have some ideas, but haven't found time to code it and send patches
> yet.
> >
> > >
> > > > FWIW I think the current code is still not clean for my taste.
> Commit
> > > > 3b595da441cf ("fdtdec: allow board to provide fdt for
> CONFIG_OF_SEPARATE")
> > > > allowed this function to be used regardless of the config options.
> IMHO we
> > > > should have 2 clear options:
> > > > - U-Boot provides the DTB
> > >
> > > Supported with: OF_SEPARATE
> > >
> > > > - It's somehow passed over to U-Boot
> > >
> > > Supported with: OF_SEPARATE + OF_BOARD
> >
> > That's exactly what I don't personally like. In your example OF_BOARD
> > means "U-Boot has a DTB and here's a way to override it". In my head
>
> In some ways this is just highlighting a long standing problem of
> working with DTB. There's certainly been some places where splitting
> the data from the driver has helped. But in some other places, my
> goodness has it made the development cycle worse. Doing this on QEMU
> probably is on one of the most annoying cases too since yes, you start
> by dumping the dtb you would get, then iterating the dts outside of the
> rest of the scope of your build. In the kernel you can "make dtbs" at
> least and get things spit out. I'm just repeating over here that the
> development cycle of working on device trees is at times not great.
In general you do this only once, unless you use the virt machine and in
that case there is no documentation about DT generated for each device
added on the command line.
I had lots of difficulties getting all UART possibilities: virtio <1.0,
1.0,1.1; mmio or pci.
But once you get the command line, the dTB is fixed.
Of course it would be better to get Qemu document DT generation of its
devices.
>
>
> --
> Tom
>
--
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog
Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements
On Wednesday 27 October 2021 17:27:41 Stefan Roese wrote: > Still I see no speed change. But this is a different story... Could you try to upload some file in u-boot via xmodem at different speeds? E.g. via loadx command: loadx 921600 and loadx 115200 And compare if xmodem transfer is in U-Boot faster or not. Ideally also try via kermit protocol at different speeds as kernel supports streaming. This could verify if issue is in kwboot or in USB-UART/kernel.
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 10:11:36PM +0200, Robert Marko wrote: > On Wed, Oct 27, 2021 at 10:07 PM Tom Rini wrote: > > > > On Wed, Oct 27, 2021 at 09:53:24PM +0200, Robert Marko wrote: > > > On Wed, Oct 27, 2021 at 9:48 PM Marek Behún wrote: > > > > > > > > On Wed, 27 Oct 2021 21:40:11 +0200 > > > > Robert Marko wrote: > > > > > > > > > To me, it also doesn't make sense to do it like that as what's the > > > > > purpose of having stuff that is completely unsupported in U-boot in > > > > > DTS? > > > > > > > > It makes sense because the device tree should compeltely describe the > > > > device. It doesn't matter if some of it's components don't have U-Boot > > > > drivers. Devicetree is independent on sw, and should be the same for > > > > Linux, BSD, and U-Boot. > > > > > > Trust me, I wish this was true, but I don't see it happening anytime soon > > > as bindings will always change but it is something to aspire to. > > > > > > > > > Also, take a look at other Armada 7k/8k devices, they are all doing it > > > > > the same way as I am. > > > > > > > > Some of them (most?) are, but that's just because we haven't synced > > > > them yet. > > > > > > > > But you don't need to do it now, IMO, if it would take too much work. > > > > > > The thing is that you cant just copy/paste the Linux DTS, it won't compile > > > as the SoC DTSI as well as CP DTSI are different. > > > Otherwise having a -u-boot.dtsi would work and I honestly don't have time > > > to sync those and then fix all of the regressions. > > > > The intention is that you copy/paste (well, cp) the DTS files in to > > U-Boot and then use -u-boot.dtsi to do whatever further is needed. > > I understand that, but SoC and CP DTSI-s are really out of sync with Linux, > it would require to pretty much override/delete half the DTS for -u-boot.dtsi > for it to work. > Otherwise, other 7k/8device would use -u-boot.dtsi as well. OK, so it needs to be done then and fixed up. The files weren't supposed to get out of sync to start with. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
Hi Tom Le mer. 27 oct. 2021 à 21:48, Tom Rini a écrit : > On Wed, Oct 27, 2021 at 05:38:28PM +0200, François Ozog wrote: > > Hi Simon, > > > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > > > > > Hi François, > > > > > > On Tue, 26 Oct 2021 at 09:57, François Ozog > > > wrote: > > > > > > > > > > > > > > > > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > > > >> > > > >> Hi François, > > > >> > > > >> On Tue, 26 Oct 2021 at 08:31, François Ozog < > francois.o...@linaro.org> > > > wrote: > > > >> > > > > >> > Hi Simon, > > > >> > > > > >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass > wrote: > > > >> >> > > > >> >> At present some of the ideas and techniques behind devicetree in > > > U-Boot > > > >> >> are assumed, implied or unsaid. Add some documentation to cover > how > > > >> >> devicetree is build, how it can be modified and the rules about > using > > > >> >> the various CONFIG_OF_... options. > > > >> >> > > > > > > [..] > > > > > > >> >> +Why not have two devicetrees? > > > >> >> +- > > > >> >> + > > > >> >> +Setting aside the argument for restricting U-Boot from having > its > > > own nodes and > > > >> >> +properties, another idea proposed is to have two devicetrees, > one > > > for the > > > >> >> +U-Boot-specific bits (here called `special`) and one for > everything > > > else (here > > > >> >> +called `linux`). > > > >> >> + > > > >> >> +On the positive side, it might quieten the discussion alluded > to in > > > the section > > > >> >> +above. But there are many negatives to consider and many open > > > questions to > > > >> >> +resolve. > > > >> >> + > > > >> >> +- **Bindings** - Presumably the special devicetree would have > its > > > own bindings. > > > >> >> + It would not be necessary to put a `u-boot,` prefix on > anything. > > > People coming > > > >> >> + across the devicetree source would wonder how it fits in with > the > > > Linux > > > >> >> + devicetree. > > > >> >> + > > > >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > > > devicetree. This > > > >> >> + would need to be expanded to support two trees. Features which > > > need to access > > > >> >> + both (such as a device driver which reads the special > devicetree > > > to get some > > > >> >> + configuration info) could become quite confusing to read and > > > write. > > > >> >> + > > > >> >> +- **Merging** - Can the two devicetree be merged if a platform > > > desires it? If > > > >> >> + so, how is this managed in tooling? Does it happen during the > > > build, in which > > > >> >> + case they are not really separate at all. Or does U-Boot merge > > > them at > > > >> >> + runtime, in which case this adds time and memory? > > > >> >> + > > > >> >> +- **Efficiency** - A second device tree adds more code and more > > > code paths. It > > > >> >> + requires that both be made available to the code in U-Boot, > e.g. > > > via a > > > >> >> + separate pointer or argument or API. Overall the separation > would > > > certainly > > > >> >> + not speed up U-Boot, nor decrease its size. > > > >> >> + > > > >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > > > pieces needed for > > > >> >> + U-Boot for a particular board. Would we use these same files > for > > > the special > > > >> >> + devicetree? > > > >> >> + > > > >> >> +- **Complexity** - Two devicetrees complicates the build system > > > since it must > > > >> >> + build and package them both. Errors must be reported in such a > > > way that it > > > >> >> + is obvious which one is failing. > > > >> >> + > > > >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > > > driver model > > > >> >> + are currently placed in the nodes they relate to. How would > these > > > tags > > > >> >> + reference a node that is in a separate devicetree? What extra > > > validation would > > > >> >> + be needed? > > > >> >> + > > > >> >> +- **Storage** - How would the two devicetrees be stored in the > > > image? At present > > > >> >> + we simply concatenate the U-Boot binary and the devicetree. We > > > could add the > > > >> >> + special devicetree before the Linux one, so two are > concatenated, > > > but it is > > > >> >> + not pretty. We could use binman to support more complex > > > arrangements, but only > > > >> >> + some boards use this at present, so it would be a big change. > > > >> >> + > > > >> >> +- **API** - How would another project provide two devicetree > files > > > to U-Boot at > > > >> >> + runtime? Presumably this would just be too painful. But if it > > > doesn't, it > > > >> >> + would be unable to configure run-time features of U-Boot > during > > > the boot. > > > >> >> + > > > >> >> +- **Confusion** - No other project has two devicetrees. U-Boot > > > would be in the > > > >> >> + unfortunate position of having to describe this fact to new > > > users, along with > > > >> >> + the (arguably contrived) reason for the arrangement. > > >
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 10:07 PM Tom Rini wrote: > > On Wed, Oct 27, 2021 at 09:53:24PM +0200, Robert Marko wrote: > > On Wed, Oct 27, 2021 at 9:48 PM Marek Behún wrote: > > > > > > On Wed, 27 Oct 2021 21:40:11 +0200 > > > Robert Marko wrote: > > > > > > > To me, it also doesn't make sense to do it like that as what's the > > > > purpose of having stuff that is completely unsupported in U-boot in > > > > DTS? > > > > > > It makes sense because the device tree should compeltely describe the > > > device. It doesn't matter if some of it's components don't have U-Boot > > > drivers. Devicetree is independent on sw, and should be the same for > > > Linux, BSD, and U-Boot. > > > > Trust me, I wish this was true, but I don't see it happening anytime soon > > as bindings will always change but it is something to aspire to. > > > > > > > Also, take a look at other Armada 7k/8k devices, they are all doing it > > > > the same way as I am. > > > > > > Some of them (most?) are, but that's just because we haven't synced > > > them yet. > > > > > > But you don't need to do it now, IMO, if it would take too much work. > > > > The thing is that you cant just copy/paste the Linux DTS, it won't compile > > as the SoC DTSI as well as CP DTSI are different. > > Otherwise having a -u-boot.dtsi would work and I honestly don't have time > > to sync those and then fix all of the regressions. > > The intention is that you copy/paste (well, cp) the DTS files in to > U-Boot and then use -u-boot.dtsi to do whatever further is needed. I understand that, but SoC and CP DTSI-s are really out of sync with Linux, it would require to pretty much override/delete half the DTS for -u-boot.dtsi for it to work. Otherwise, other 7k/8device would use -u-boot.dtsi as well. Regards, Robert > > -- > Tom -- Robert Marko Staff Embedded Linux Engineer Sartura Ltd. Lendavska ulica 16a 1 Zagreb, Croatia Email: robert.ma...@sartura.hr Web: www.sartura.hr
Re: [PATCH v4 1/4] tools: Separate image types which depend on OpenSSL
> > >> I also have been doing some packaging of u-boot for GNU Guix, where I > > >> suspect the stance wouldn't be as willing to accept such a compromise... > > >> > > >> So... I would *love* an option to be able to build a board-only config > > >> without any of the tools; > > > > > > Why is this a problem (see above)? Who is building board builds? It's > > > either the maintainer when creating the binary package, or a curious user, > > > right? And they can surely *use* OpenSSL during build time - if it's > > > needed by the board. > > > > Sure, if there is no actual openssl code embedded in the resulting > > binary with GPLv2 code, it shouldn't be a problem... > > > > > > It's a mess of an issue to tease out exactly what codepaths trigger and > > do not trigger the compatibility issues between openssl and GPL... > > > > > > Depending on openssl in a project with GPLv2-only code does seem at risk > > to introduce license compatibility issues without sufficient and > > constant review and dilligence, even if it is technically ok how it is > > done right now... > > There's still the long standing request to migrate the tooling to use a > different library, but it's apparently not been a large enough concern > of company with concerns to fund a developer of theirs to do the > migration. I feel like that might be one of the better, at least in > terms of license, fixes for this issue. > > And then maybe we do just need a way to say if you're building for > platform X then you must also have the crypto requirement resolved to > build mkimage. And conversely if you aren't building those platforms, > it's OK to not. Does the re-license [1] to Apache License 2.0 for openssl 3+ change this situation at all? For reference all the pieces of U-Boot that Fedora builds seem to build fine with openssl 3. [1] https://www.openssl.org/blog/blog/2021/09/07/OpenSSL3.Final/
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
Hi Simon Le mer. 27 oct. 2021 à 20:23, Simon Glass a écrit : > Hi François, > > On Wed, 27 Oct 2021 at 09:14, François Ozog > wrote: > > > > > > > > On Wed, 27 Oct 2021 at 16:08, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 00:07, François Ozog > wrote: > >> > > >> > Hi Simon > >> > > >> > Position unchanged on this series: adding fake dts for boards that > generate their device tree in the dts directory is not good. If you have > them in documentation the it is acceptable. > >> > >> I think we are going to have to disagree on this one. I actually used > >> the qemu one in testing/development recently. We have to have a way to > >> develop in-tree with U-Boot. It does not impinge on any of your use > >> cases, so far as I know. > > > > I am not the only one in disagreement... You just saw Alex Bénée from > Qemu saying the same thing. > > I understand the advanced debug/development scenario you mention. > > But locating the DT files in the dts directory is mis-leading the > contributors to think that they need to compile the DT for the targeted > platforms. > > For your advanced scenario, you can still have the dts in the > documentation area, or whatever directory (except dts). compile it and > supply to U-Boot. > > We have this situation with rpi 1, 2 and 3 and I don't believe anyone > has noticed. U-Boot handles the build automatically. If you turn off > OF_BOARD, it will use the U-Boot devicetree always so you know what is > going on. > > We can add a message to U-Boot indicating where the devicetree came > from, perhaps? That might be useful given everything that is going on. > > As in this case, quite often in these discussions I struggle to > understand what is behind the objection. Is it that your customers are > demanding that devicetrees become private, secret data, not included > in open-source projects? Or is it just the strange case of QEMU that > is informing your thinking? I know of at least one project where the > first-stage bootloader produces a devicetree and no one has the source > for it. I believe TF-A was created for licensing reasons...so can you > be a bit clearer about what the problem actually is? there are situations where U-Boot must have a dtb. Then those dTB sources are logically found in the dts directory. There are situations where U-Boot should not have a dtb. In that case there should be no element in the dts directory. Otherwise it creates confusion. Now as you point out, we need “playgrounds” to deal with some situation. So having examples in an ad-hoc directory, different from dts is fine. I proposed documentation but you may find a better place. In other words, dts shall host only dt source when U-Boot cannot do but make a dTB for a platform. As you have seen in different mail thread (firmware sustainability and OCP checklist) freedom is important to Linaro and there are no hidden Trojan horse for licensing. If a board is > in-tree in U-Boot I would like it to have a devicetree there, at least > until we have a better option. At the very least, it MUST be > discoverable and it must be possible to undertake U-Boot development > easily without a lot of messing around. You can if you keep two dts directories separate: dts for boards for which U-Boot must have one debug-dts for boards for which U-Boot get the DT at runtime but for which you want a playground for debug/easier development. > > >> > >> > >> But trying to do any driver / core work for a board where you don't > >> have the devicetree is currently not possible. The devicetree is a > >> core component and being unable to modify it is simply not practical. > >> We are talking here about an option that can be enabled or disabled. > >> In my case I am able to disable it locally and do my development work. > >> > >> > >> BTW I've got the bloblist handoff working with a devicetree inside it, > >> for qemu_arm. I need to try it on a real board to figure out what the > >> difference is. > >> > > That's great news and much needed for stabilizing the inbound ABI from > prior loader to U-Boot. Let's create another thread to discuss this > important topic. > >> > > My scenario is not all that advanced and I am using qemu_arm to test > the bloblist handoff stuff and include it in CI, with a suitable > devicetree and a binman node. > > Regards, > Simon > > >> > > >> > Cheers > >> > > >> > FF > >> > > >> > Le mar. 26 oct. 2021 à 02:24, Simon Glass a écrit > : > >> >> > >> >> With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so > >> >> there are only three ways to obtain a devicetree: > >> >> > >> >>- OF_SEPARATE - the normal way, where the devicetree is built and > >> >> appended to U-Boot > >> >>- OF_EMBED - for development purposes, the devicetree is embedded > in > >> >> the ELF file (also used for EFI) > >> >>- OF_BOARD - the board figures it out on its own > >> >> > >> >> The last one is currently set up so that no devicetree is needed at
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 09:53:24PM +0200, Robert Marko wrote: > On Wed, Oct 27, 2021 at 9:48 PM Marek Behún wrote: > > > > On Wed, 27 Oct 2021 21:40:11 +0200 > > Robert Marko wrote: > > > > > To me, it also doesn't make sense to do it like that as what's the > > > purpose of having stuff that is completely unsupported in U-boot in > > > DTS? > > > > It makes sense because the device tree should compeltely describe the > > device. It doesn't matter if some of it's components don't have U-Boot > > drivers. Devicetree is independent on sw, and should be the same for > > Linux, BSD, and U-Boot. > > Trust me, I wish this was true, but I don't see it happening anytime soon > as bindings will always change but it is something to aspire to. > > > > > Also, take a look at other Armada 7k/8k devices, they are all doing it > > > the same way as I am. > > > > Some of them (most?) are, but that's just because we haven't synced > > them yet. > > > > But you don't need to do it now, IMO, if it would take too much work. > > The thing is that you cant just copy/paste the Linux DTS, it won't compile > as the SoC DTSI as well as CP DTSI are different. > Otherwise having a -u-boot.dtsi would work and I honestly don't have time > to sync those and then fix all of the regressions. The intention is that you copy/paste (well, cp) the DTS files in to U-Boot and then use -u-boot.dtsi to do whatever further is needed. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 9:48 PM Marek Behún wrote: > > On Wed, 27 Oct 2021 21:40:11 +0200 > Robert Marko wrote: > > > To me, it also doesn't make sense to do it like that as what's the > > purpose of having stuff that is completely unsupported in U-boot in > > DTS? > > It makes sense because the device tree should compeltely describe the > device. It doesn't matter if some of it's components don't have U-Boot > drivers. Devicetree is independent on sw, and should be the same for > Linux, BSD, and U-Boot. Trust me, I wish this was true, but I don't see it happening anytime soon as bindings will always change but it is something to aspire to. > > > Also, take a look at other Armada 7k/8k devices, they are all doing it > > the same way as I am. > > Some of them (most?) are, but that's just because we haven't synced > them yet. > > But you don't need to do it now, IMO, if it would take too much work. The thing is that you cant just copy/paste the Linux DTS, it won't compile as the SoC DTSI as well as CP DTSI are different. Otherwise having a -u-boot.dtsi would work and I honestly don't have time to sync those and then fix all of the regressions. Regards, Robert > > Marek -- Robert Marko Staff Embedded Linux Engineer Sartura Ltd. Lendavska ulica 16a 1 Zagreb, Croatia Email: robert.ma...@sartura.hr Web: www.sartura.hr
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
Hi Ilias, On Wed, 27 Oct 2021 at 13:13, Ilias Apalodimas wrote: > > On Wed, 27 Oct 2021 at 21:33, Simon Glass wrote: > > > > Hi François, > > > > On Wed, 27 Oct 2021 at 09:38, François Ozog > > wrote: > > > > > > Hi Simon, > > > > > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > > >> > > >> Hi François, > > >> > > >> On Tue, 26 Oct 2021 at 09:57, François Ozog > > >> wrote: > > >> > > > >> > > > >> > > > >> > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > > >> >> > > >> >> Hi François, > > >> >> > > >> >> On Tue, 26 Oct 2021 at 08:31, François Ozog > > >> >> wrote: > > >> >> > > > >> >> > Hi Simon, > > >> >> > > > >> >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass wrote: > > >> >> >> > > >> >> >> At present some of the ideas and techniques behind devicetree in > > >> >> >> U-Boot > > >> >> >> are assumed, implied or unsaid. Add some documentation to cover how > > >> >> >> devicetree is build, how it can be modified and the rules about > > >> >> >> using > > >> >> >> the various CONFIG_OF_... options. > > >> >> >> > > >> > > >> [..] > > >> > > >> >> >> +Why not have two devicetrees? > > >> >> >> +- > > >> >> >> + > > >> >> >> +Setting aside the argument for restricting U-Boot from having its > > >> >> >> own nodes and > > >> >> >> +properties, another idea proposed is to have two devicetrees, one > > >> >> >> for the > > >> >> >> +U-Boot-specific bits (here called `special`) and one for > > >> >> >> everything else (here > > >> >> >> +called `linux`). > > >> >> >> + > > >> >> >> +On the positive side, it might quieten the discussion alluded to > > >> >> >> in the section > > >> >> >> +above. But there are many negatives to consider and many open > > >> >> >> questions to > > >> >> >> +resolve. > > >> >> >> + > > >> >> >> +- **Bindings** - Presumably the special devicetree would have its > > >> >> >> own bindings. > > >> >> >> + It would not be necessary to put a `u-boot,` prefix on > > >> >> >> anything. People coming > > >> >> >> + across the devicetree source would wonder how it fits in with > > >> >> >> the Linux > > >> >> >> + devicetree. > > >> >> >> + > > >> >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > > >> >> >> devicetree. This > > >> >> >> + would need to be expanded to support two trees. Features which > > >> >> >> need to access > > >> >> >> + both (such as a device driver which reads the special > > >> >> >> devicetree to get some > > >> >> >> + configuration info) could become quite confusing to read and > > >> >> >> write. > > >> >> >> + > > >> >> >> +- **Merging** - Can the two devicetree be merged if a platform > > >> >> >> desires it? If > > >> >> >> + so, how is this managed in tooling? Does it happen during the > > >> >> >> build, in which > > >> >> >> + case they are not really separate at all. Or does U-Boot merge > > >> >> >> them at > > >> >> >> + runtime, in which case this adds time and memory? > > >> >> >> + > > >> >> >> +- **Efficiency** - A second device tree adds more code and more > > >> >> >> code paths. It > > >> >> >> + requires that both be made available to the code in U-Boot, > > >> >> >> e.g. via a > > >> >> >> + separate pointer or argument or API. Overall the separation > > >> >> >> would certainly > > >> >> >> + not speed up U-Boot, nor decrease its size. > > >> >> >> + > > >> >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > > >> >> >> pieces needed for > > >> >> >> + U-Boot for a particular board. Would we use these same files > > >> >> >> for the special > > >> >> >> + devicetree? > > >> >> >> + > > >> >> >> +- **Complexity** - Two devicetrees complicates the build system > > >> >> >> since it must > > >> >> >> + build and package them both. Errors must be reported in such a > > >> >> >> way that it > > >> >> >> + is obvious which one is failing. > > >> >> >> + > > >> >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > > >> >> >> driver model > > >> >> >> + are currently placed in the nodes they relate to. How would > > >> >> >> these tags > > >> >> >> + reference a node that is in a separate devicetree? What extra > > >> >> >> validation would > > >> >> >> + be needed? > > >> >> >> + > > >> >> >> +- **Storage** - How would the two devicetrees be stored in the > > >> >> >> image? At present > > >> >> >> + we simply concatenate the U-Boot binary and the devicetree. We > > >> >> >> could add the > > >> >> >> + special devicetree before the Linux one, so two are > > >> >> >> concatenated, but it is > > >> >> >> + not pretty. We could use binman to support more complex > > >> >> >> arrangements, but only > > >> >> >> + some boards use this at present, so it would be a big change. > > >> >> >> + > > >> >> >> +- **API** - How would another project provide two devicetree > > >> >> >> files to U-Boot at > > >> >> >> + runtime? Presumably this would just be too painful. But if it > > >> >> >>
Re: [PATCH 1/1] video: fix positioning in TrueType console
Hi Heinrich, On Wed, 27 Oct 2021 20:10:36 +0200 Heinrich Schuchardt xypron.g...@gmx.de wrote: > Dear Anatolij, > > somehow you lost track of this patch since March: > > https://patchwork.ozlabs.org/project/uboot/patch/20210203161254.970-1-xypron.g...@gmx.de/ > > Could you, please, review it and consider it for merging. I've applied the patches for build testing and see sandbox test errors: https://source.denx.de/u-boot/custodians/u-boot-video/-/jobs/342128 Could you please fix them? Thanks! -- Anatolij
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, 27 Oct 2021 21:40:11 +0200 Robert Marko wrote: > To me, it also doesn't make sense to do it like that as what's the > purpose of having stuff that is completely unsupported in U-boot in > DTS? It makes sense because the device tree should compeltely describe the device. It doesn't matter if some of it's components don't have U-Boot drivers. Devicetree is independent on sw, and should be the same for Linux, BSD, and U-Boot. > Also, take a look at other Armada 7k/8k devices, they are all doing it > the same way as I am. Some of them (most?) are, but that's just because we haven't synced them yet. But you don't need to do it now, IMO, if it would take too much work. Marek
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
On Wed, Oct 27, 2021 at 05:38:28PM +0200, François Ozog wrote: > Hi Simon, > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > > > Hi François, > > > > On Tue, 26 Oct 2021 at 09:57, François Ozog > > wrote: > > > > > > > > > > > > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > > >> > > >> Hi François, > > >> > > >> On Tue, 26 Oct 2021 at 08:31, François Ozog > > wrote: > > >> > > > >> > Hi Simon, > > >> > > > >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass wrote: > > >> >> > > >> >> At present some of the ideas and techniques behind devicetree in > > U-Boot > > >> >> are assumed, implied or unsaid. Add some documentation to cover how > > >> >> devicetree is build, how it can be modified and the rules about using > > >> >> the various CONFIG_OF_... options. > > >> >> > > > > [..] > > > > >> >> +Why not have two devicetrees? > > >> >> +- > > >> >> + > > >> >> +Setting aside the argument for restricting U-Boot from having its > > own nodes and > > >> >> +properties, another idea proposed is to have two devicetrees, one > > for the > > >> >> +U-Boot-specific bits (here called `special`) and one for everything > > else (here > > >> >> +called `linux`). > > >> >> + > > >> >> +On the positive side, it might quieten the discussion alluded to in > > the section > > >> >> +above. But there are many negatives to consider and many open > > questions to > > >> >> +resolve. > > >> >> + > > >> >> +- **Bindings** - Presumably the special devicetree would have its > > own bindings. > > >> >> + It would not be necessary to put a `u-boot,` prefix on anything. > > People coming > > >> >> + across the devicetree source would wonder how it fits in with the > > Linux > > >> >> + devicetree. > > >> >> + > > >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > > devicetree. This > > >> >> + would need to be expanded to support two trees. Features which > > need to access > > >> >> + both (such as a device driver which reads the special devicetree > > to get some > > >> >> + configuration info) could become quite confusing to read and > > write. > > >> >> + > > >> >> +- **Merging** - Can the two devicetree be merged if a platform > > desires it? If > > >> >> + so, how is this managed in tooling? Does it happen during the > > build, in which > > >> >> + case they are not really separate at all. Or does U-Boot merge > > them at > > >> >> + runtime, in which case this adds time and memory? > > >> >> + > > >> >> +- **Efficiency** - A second device tree adds more code and more > > code paths. It > > >> >> + requires that both be made available to the code in U-Boot, e.g. > > via a > > >> >> + separate pointer or argument or API. Overall the separation would > > certainly > > >> >> + not speed up U-Boot, nor decrease its size. > > >> >> + > > >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > > pieces needed for > > >> >> + U-Boot for a particular board. Would we use these same files for > > the special > > >> >> + devicetree? > > >> >> + > > >> >> +- **Complexity** - Two devicetrees complicates the build system > > since it must > > >> >> + build and package them both. Errors must be reported in such a > > way that it > > >> >> + is obvious which one is failing. > > >> >> + > > >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > > driver model > > >> >> + are currently placed in the nodes they relate to. How would these > > tags > > >> >> + reference a node that is in a separate devicetree? What extra > > validation would > > >> >> + be needed? > > >> >> + > > >> >> +- **Storage** - How would the two devicetrees be stored in the > > image? At present > > >> >> + we simply concatenate the U-Boot binary and the devicetree. We > > could add the > > >> >> + special devicetree before the Linux one, so two are concatenated, > > but it is > > >> >> + not pretty. We could use binman to support more complex > > arrangements, but only > > >> >> + some boards use this at present, so it would be a big change. > > >> >> + > > >> >> +- **API** - How would another project provide two devicetree files > > to U-Boot at > > >> >> + runtime? Presumably this would just be too painful. But if it > > doesn't, it > > >> >> + would be unable to configure run-time features of U-Boot during > > the boot. > > >> >> + > > >> >> +- **Confusion** - No other project has two devicetrees. U-Boot > > would be in the > > >> >> + unfortunate position of having to describe this fact to new > > users, along with > > >> >> + the (arguably contrived) reason for the arrangement. > > >> >> + > > >> > > > >> > False: > > >> > 1) projects in trustedfirmware.org are built to have multiple FDT > > objects, some for "dynamic" configuration purposes. > > >> > > >> Can you provided a link and I can update this. > > > > > > > > https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html > > > Bindings: > > > for FCONF: > >
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
Hi Simon, Le mer. 27 oct. 2021 à 20:33, Simon Glass a écrit : > Hi François, > > On Wed, 27 Oct 2021 at 09:38, François Ozog > wrote: > > > > Hi Simon, > > > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 09:57, François Ozog > wrote: > >> > > >> > > >> > > >> > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > >> >> > >> >> Hi François, > >> >> > >> >> On Tue, 26 Oct 2021 at 08:31, François Ozog < > francois.o...@linaro.org> wrote: > >> >> > > >> >> > Hi Simon, > >> >> > > >> >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass > wrote: > >> >> >> > >> >> >> At present some of the ideas and techniques behind devicetree in > U-Boot > >> >> >> are assumed, implied or unsaid. Add some documentation to cover > how > >> >> >> devicetree is build, how it can be modified and the rules about > using > >> >> >> the various CONFIG_OF_... options. > >> >> >> > >> > >> [..] > >> > >> >> >> +Why not have two devicetrees? > >> >> >> +- > >> >> >> + > >> >> >> +Setting aside the argument for restricting U-Boot from having > its own nodes and > >> >> >> +properties, another idea proposed is to have two devicetrees, > one for the > >> >> >> +U-Boot-specific bits (here called `special`) and one for > everything else (here > >> >> >> +called `linux`). > >> >> >> + > >> >> >> +On the positive side, it might quieten the discussion alluded to > in the section > >> >> >> +above. But there are many negatives to consider and many open > questions to > >> >> >> +resolve. > >> >> >> + > >> >> >> +- **Bindings** - Presumably the special devicetree would have > its own bindings. > >> >> >> + It would not be necessary to put a `u-boot,` prefix on > anything. People coming > >> >> >> + across the devicetree source would wonder how it fits in with > the Linux > >> >> >> + devicetree. > >> >> >> + > >> >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > devicetree. This > >> >> >> + would need to be expanded to support two trees. Features which > need to access > >> >> >> + both (such as a device driver which reads the special > devicetree to get some > >> >> >> + configuration info) could become quite confusing to read and > write. > >> >> >> + > >> >> >> +- **Merging** - Can the two devicetree be merged if a platform > desires it? If > >> >> >> + so, how is this managed in tooling? Does it happen during the > build, in which > >> >> >> + case they are not really separate at all. Or does U-Boot merge > them at > >> >> >> + runtime, in which case this adds time and memory? > >> >> >> + > >> >> >> +- **Efficiency** - A second device tree adds more code and more > code paths. It > >> >> >> + requires that both be made available to the code in U-Boot, > e.g. via a > >> >> >> + separate pointer or argument or API. Overall the separation > would certainly > >> >> >> + not speed up U-Boot, nor decrease its size. > >> >> >> + > >> >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > pieces needed for > >> >> >> + U-Boot for a particular board. Would we use these same files > for the special > >> >> >> + devicetree? > >> >> >> + > >> >> >> +- **Complexity** - Two devicetrees complicates the build system > since it must > >> >> >> + build and package them both. Errors must be reported in such a > way that it > >> >> >> + is obvious which one is failing. > >> >> >> + > >> >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > driver model > >> >> >> + are currently placed in the nodes they relate to. How would > these tags > >> >> >> + reference a node that is in a separate devicetree? What extra > validation would > >> >> >> + be needed? > >> >> >> + > >> >> >> +- **Storage** - How would the two devicetrees be stored in the > image? At present > >> >> >> + we simply concatenate the U-Boot binary and the devicetree. We > could add the > >> >> >> + special devicetree before the Linux one, so two are > concatenated, but it is > >> >> >> + not pretty. We could use binman to support more complex > arrangements, but only > >> >> >> + some boards use this at present, so it would be a big change. > >> >> >> + > >> >> >> +- **API** - How would another project provide two devicetree > files to U-Boot at > >> >> >> + runtime? Presumably this would just be too painful. But if it > doesn't, it > >> >> >> + would be unable to configure run-time features of U-Boot > during the boot. > >> >> >> + > >> >> >> +- **Confusion** - No other project has two devicetrees. U-Boot > would be in the > >> >> >> + unfortunate position of having to describe this fact to new > users, along with > >> >> >> + the (arguably contrived) reason for the arrangement. > >> >> >> + > >> >> > > >> >> > False: > >> >> > 1) projects in trustedfirmware.org are built to have multiple FDT > objects, some for "dynamic" configuration purposes. > >> >> > >> >> Can you provided a link and I can update this. > >> > > >> > >
Re: [PATCH 05/16] arm: qemu: Add a devicetree file for qemu_arm64
There are QEMU targets that have fixed hardware which do rely on the firmware to have a fixed view of the hardware. sbsa-ref is one such board although there will be some versioning required for later levels. On Wed, 27 Oct 2021, 19:39 Tom Rini, wrote: > On Wed, Oct 27, 2021 at 12:34:26PM -0600, Simon Glass wrote: > > Hi all, > > > > On Wed, 27 Oct 2021 at 08:56, Tom Rini wrote: > > > > > > On Wed, Oct 27, 2021 at 03:44:08PM +0100, Alex Bennée wrote: > > > > > > > > François Ozog writes: > > > > > > > > > Hi Simon > > > > > > > > > > The only place I could agree with this file presence is in the > documentation directory, not in dts. It creates a mental picture for the > reader > > > > > an entirely bad mind scheme around Qemu and DT. > > > > > > > > > > And even in a documentation directory I would place a bug warning: > don’t use this with any kernel , Qemu generates a DT dynamically > > > > > based on cpu, memory and devices specified at the command line. > > > > > > > > Certainly for the arm, aarch64 and riscv "virt" machines you should > > > > always use the QEMU generated DTB. I'm not entirely clear what a > > > > qemu_arm and qemu_arm64 def targets are meant to be in this context. > > > > > > Agreed. We cannot include random device trees in U-Boot for devices > > > that generate their own at run time or otherwise have the source of > > > truth elsewhere. > > > > Until we have a way of bringing in the u-boot.dtsi that people in QEMU > > can agree on, I don't see an alternative. I will send a series for the > > bloblist handoff next week and I think you will all see what I mean. > > I think the alternative is that QEMU in U-Boot just can't be used for > certain features. Which is annoying in that it would be good to use it > to test certain feature, yes. It's generating a good and valid enough > dtb for Linux, so it should be good enough for us in general. > > -- > Tom >
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 8:32 PM Pali Rohár wrote:
>
> On Wednesday 27 October 2021 20:01:33 Robert Marko wrote:
> > Globalscale MOCHAbin is a Armada 7040 based development board.
> >
> > Specifications:
> > * Armada 7040 Quad core ARMv8 Cortex A-72 @ 1.4GHz
> > * 2 / 4 / 8 GB of DDR4 DRAM
> > * 16 GB eMMC
> > * 4MB SPI-NOR (Bootloader)
> > * 1x M.2-2280 B-key socket (for SSD expansion, SATA3 only)
> > * 1x M.2-2250 B-key socket (for modems, USB2.0 and I2C only)
> > * 1x Mini-PCIe 3.0 (x1, USB2.0 and I2C)
> > * 1x SATA 7+15 socket (SATA3)
> > * 1x 16-pin (2×8) MikroBus Connector
> > * 1x SIM card slot (Connected to the mini-PCIe and both M.2 slots)
> > * 2x USB3.0 Type-A ports via SMSC USB5434B hub
> > * Cortex 2x5 JTAG
> > * microUSB port for UART (PL2303GL/PL2303SA onboard)
> > * 1x 10G SFP+
> > * 1x 1G SFP (Connected to 88E1512 PHY)
> > * 1x 1G RJ45 with PoE PD (Connected to 88E1512 PHY)
> > * 4x 1G RJ45 ports via Topaz 88E6141 switch
> > * RTC with battery holder (SoC provided, requires CR2032 battery)
> > * 1x 12V DC IN
> > * 1x Power switch
> > * 1x 12V fan header (3-pin, power only)
> > * 1x mini-PCIe LED header (2x0.1" pins)
> > * 1x M.2-2280 LED header (2x0.1" pins)
> > * 6x Bootstrap jumpers
> > * 1x Power LED (Green)
> > * 3x Tri-color RGB LEDs (Controllable)
> > * 1x Microchip ATECC608B secure element
> >
> > Note that 1G SFP does not work in U-boot as it would require Linux like
> > SFP support for parsing module interface and reconfiguring the PHY.
> >
> > Additionally, automatic import of the Marvell hw_info parameters is
> > enabled via the recently added mac command for Armada platforms.
> > The parameters stored in Marvell hw_info are usually the board serial
> > number and MAC addresses.
> >
> > Signed-off-by: Robert Marko
> > ---
> > Changes in v3:
> > * Rename "u-boot" partition to "firmware" which better describes its content
> > * Remove read-only flag from "hw-info" partition
> >
> > Changes in v2:
> > * Add DT compatible for hw-info to its partition
> > * Enable MTD support, command and SPI MTD command
> > ---
> > arch/arm/dts/Makefile| 1 +
> > arch/arm/dts/armada-7040-mochabin.dts| 304 +++
> > configs/mvebu_mochabin-88f7040_defconfig | 88 +++
> > 3 files changed, 393 insertions(+)
> > create mode 100644 arch/arm/dts/armada-7040-mochabin.dts
> > create mode 100644 configs/mvebu_mochabin-88f7040_defconfig
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index a88aecc5bd9..dea567d2595 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -232,6 +232,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
> > armada-38x-controlcenterdc.dtb \
> > armada-7040-db-nand.dtb \
> > armada-7040-db.dtb \
> > + armada-7040-mochabin.dtb\
> > armada-8040-clearfog-gt-8k.dtb \
> > armada-8040-db.dtb \
> > armada-8040-mcbin.dtb \
> > diff --git a/arch/arm/dts/armada-7040-mochabin.dts
> > b/arch/arm/dts/armada-7040-mochabin.dts
> > new file mode 100644
> > index 000..84f06363728
> > --- /dev/null
> > +++ b/arch/arm/dts/armada-7040-mochabin.dts
> > @@ -0,0 +1,304 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2016- 2021 Marvell International Ltd.
> > + * Copyright (C) 2021 Sartura Ltd.
> > + */
> > +
> > +/*
> > + * Device Tree file for Globalscale MOCHAbin
> > + * Boot device: SPI NOR, 0x32 (SW3)
> > + */
> > +
> > +#include "armada-7040.dtsi"
> > +
> > +/ {
> > + model = "Globalscale MOCHAbin";
> > + compatible = "globalscale,mochabin", "marvell,armada7040",
> > + "marvell,armada-ap806-quad", "marvell,armada-ap806";
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + aliases {
> > + i2c0 = _i2c0;
> > + spi0 = _spi1;
> > + };
> > +
> > + memory@ {
> > + device_type = "memory";
> > + reg = <0x0 0x0 0x0 0x8000>;
> > + };
> > +};
>
> Hm... Why is this DTS file different than newly merged DTS file in Linux?
> https://git.kernel.org/pub/scm/linux/kernel/git/gclement/mvebu.git/commit/?h=mvebu/dt64=73792919128304938412194d047be3c6176d
Fundamentally it's the same DTS, just adapted for U-boot.
Unsupported stuff was dropped and stuff like pinctrl and other
U-boot-specific things were added.
>
> Should not we have same file in Linux and U-Boot project and optional
> U-Boot modifications (if needed) in *-u-boot.dtsi file like it is for
> other arm boards?
> https://source.denx.de/u-boot/u-boot/-/tree/master/arch/arm/dts
That doesn't really work here as DTSI for SoC and CP is not synced
with the Linux one at all and the Linux DTS won't compile just like it
is.
To me, it also doesn't make sense to do it like that as what's the
purpose of having stuff that
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 12:23:21PM -0600, Simon Glass wrote: > Hi François, > > On Wed, 27 Oct 2021 at 09:14, François Ozog wrote: > > > > > > > > On Wed, 27 Oct 2021 at 16:08, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 00:07, François Ozog > >> wrote: > >> > > >> > Hi Simon > >> > > >> > Position unchanged on this series: adding fake dts for boards that > >> > generate their device tree in the dts directory is not good. If you have > >> > them in documentation the it is acceptable. > >> > >> I think we are going to have to disagree on this one. I actually used > >> the qemu one in testing/development recently. We have to have a way to > >> develop in-tree with U-Boot. It does not impinge on any of your use > >> cases, so far as I know. > > > > I am not the only one in disagreement... You just saw Alex Bénée from Qemu > > saying the same thing. > > I understand the advanced debug/development scenario you mention. > > But locating the DT files in the dts directory is mis-leading the > > contributors to think that they need to compile the DT for the targeted > > platforms. > > For your advanced scenario, you can still have the dts in the documentation > > area, or whatever directory (except dts). compile it and supply to U-Boot. > > We have this situation with rpi 1, 2 and 3 and I don't believe anyone > has noticed. U-Boot handles the build automatically. If you turn off > OF_BOARD, it will use the U-Boot devicetree always so you know what is > going on. That we have to have so many separate rpi build targets, and can't just use rpm_arm64 and add rpi_arm32 is not a good feature. The various rpi configs that use CONFIG_OF_EMBED are on your list of things that need to be cleaned up, yes? > We can add a message to U-Boot indicating where the devicetree came > from, perhaps? That might be useful given everything that is going on. > > As in this case, quite often in these discussions I struggle to > understand what is behind the objection. Is it that your customers are > demanding that devicetrees become private, secret data, not included > in open-source projects? Or is it just the strange case of QEMU that > is informing your thinking? I know of at least one project where the > first-stage bootloader produces a devicetree and no one has the source > for it. I believe TF-A was created for licensing reasons...so can you > be a bit clearer about what the problem actually is? If a board is > in-tree in U-Boot I would like it to have a devicetree there, at least > until we have a better option. At the very least, it MUST be > discoverable and it must be possible to undertake U-Boot development > easily without a lot of messing around. What I don't understand here is why or how U-Boot is supposed to become the source of truth for device trees. The general goal is that the device tree be something that actually comes along on the hardware, because it's stable enough and correct enough that it's not going to be changed from one OS kernel release to the next. That should be where the correct and true tree comes from, the device itself. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 06:36:12PM +0300, Tuomas Tynkkynen wrote: > Hi, > > On 27.10.2021 17.08, Simon Glass wrote: > > Hi François, > > > > On Tue, 26 Oct 2021 at 00:07, François Ozog > > wrote: > > > > > > Hi Simon > > > > > > Position unchanged on this series: adding fake dts for boards that > > > generate their device tree in the dts directory is not good. If you have > > > them in documentation the it is acceptable. > > > > I think we are going to have to disagree on this one. I actually used > > the qemu one in testing/development recently. We have to have a way to > > develop in-tree with U-Boot. It does not impinge on any of your use > > cases, so far as I know. > > > > How about having the file just contain a single line > > #include > > ... where this generated-*.dts is not checked to the source tree. Then of > course comments on how to generate it via qemu -dumpdtb + dtc, with > appropriate precautions (ie. must be regenerated if qemu command line is > changed or qemu is upgraded), intended use case, and so forth. That seems like it would help the development workflow, yes. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
On Wed, 27 Oct 2021 at 21:33, Simon Glass wrote: > > Hi François, > > On Wed, 27 Oct 2021 at 09:38, François Ozog wrote: > > > > Hi Simon, > > > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 09:57, François Ozog > >> wrote: > >> > > >> > > >> > > >> > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > >> >> > >> >> Hi François, > >> >> > >> >> On Tue, 26 Oct 2021 at 08:31, François Ozog > >> >> wrote: > >> >> > > >> >> > Hi Simon, > >> >> > > >> >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass wrote: > >> >> >> > >> >> >> At present some of the ideas and techniques behind devicetree in > >> >> >> U-Boot > >> >> >> are assumed, implied or unsaid. Add some documentation to cover how > >> >> >> devicetree is build, how it can be modified and the rules about using > >> >> >> the various CONFIG_OF_... options. > >> >> >> > >> > >> [..] > >> > >> >> >> +Why not have two devicetrees? > >> >> >> +- > >> >> >> + > >> >> >> +Setting aside the argument for restricting U-Boot from having its > >> >> >> own nodes and > >> >> >> +properties, another idea proposed is to have two devicetrees, one > >> >> >> for the > >> >> >> +U-Boot-specific bits (here called `special`) and one for everything > >> >> >> else (here > >> >> >> +called `linux`). > >> >> >> + > >> >> >> +On the positive side, it might quieten the discussion alluded to in > >> >> >> the section > >> >> >> +above. But there are many negatives to consider and many open > >> >> >> questions to > >> >> >> +resolve. > >> >> >> + > >> >> >> +- **Bindings** - Presumably the special devicetree would have its > >> >> >> own bindings. > >> >> >> + It would not be necessary to put a `u-boot,` prefix on anything. > >> >> >> People coming > >> >> >> + across the devicetree source would wonder how it fits in with the > >> >> >> Linux > >> >> >> + devicetree. > >> >> >> + > >> >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > >> >> >> devicetree. This > >> >> >> + would need to be expanded to support two trees. Features which > >> >> >> need to access > >> >> >> + both (such as a device driver which reads the special devicetree > >> >> >> to get some > >> >> >> + configuration info) could become quite confusing to read and > >> >> >> write. > >> >> >> + > >> >> >> +- **Merging** - Can the two devicetree be merged if a platform > >> >> >> desires it? If > >> >> >> + so, how is this managed in tooling? Does it happen during the > >> >> >> build, in which > >> >> >> + case they are not really separate at all. Or does U-Boot merge > >> >> >> them at > >> >> >> + runtime, in which case this adds time and memory? > >> >> >> + > >> >> >> +- **Efficiency** - A second device tree adds more code and more > >> >> >> code paths. It > >> >> >> + requires that both be made available to the code in U-Boot, e.g. > >> >> >> via a > >> >> >> + separate pointer or argument or API. Overall the separation would > >> >> >> certainly > >> >> >> + not speed up U-Boot, nor decrease its size. > >> >> >> + > >> >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > >> >> >> pieces needed for > >> >> >> + U-Boot for a particular board. Would we use these same files for > >> >> >> the special > >> >> >> + devicetree? > >> >> >> + > >> >> >> +- **Complexity** - Two devicetrees complicates the build system > >> >> >> since it must > >> >> >> + build and package them both. Errors must be reported in such a > >> >> >> way that it > >> >> >> + is obvious which one is failing. > >> >> >> + > >> >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > >> >> >> driver model > >> >> >> + are currently placed in the nodes they relate to. How would these > >> >> >> tags > >> >> >> + reference a node that is in a separate devicetree? What extra > >> >> >> validation would > >> >> >> + be needed? > >> >> >> + > >> >> >> +- **Storage** - How would the two devicetrees be stored in the > >> >> >> image? At present > >> >> >> + we simply concatenate the U-Boot binary and the devicetree. We > >> >> >> could add the > >> >> >> + special devicetree before the Linux one, so two are concatenated, > >> >> >> but it is > >> >> >> + not pretty. We could use binman to support more complex > >> >> >> arrangements, but only > >> >> >> + some boards use this at present, so it would be a big change. > >> >> >> + > >> >> >> +- **API** - How would another project provide two devicetree files > >> >> >> to U-Boot at > >> >> >> + runtime? Presumably this would just be too painful. But if it > >> >> >> doesn't, it > >> >> >> + would be unable to configure run-time features of U-Boot during > >> >> >> the boot. > >> >> >> + > >> >> >> +- **Confusion** - No other project has two devicetrees. U-Boot > >> >> >> would be in the > >> >> >> + unfortunate position of having to describe this fact to new > >> >> >> users, along with >
Re: [PATCH v5 26/26] fdt: Don't call board_fdt_blob_setup() without OF_BOARD
On Wed, Oct 27, 2021 at 10:17:05AM +0300, Ilias Apalodimas wrote:
> Hi Simon,
>
> On Tue, 26 Oct 2021 at 18:27, Simon Glass wrote:
> >
> > Hi Ilias,
> >
> > On Tue, 26 Oct 2021 at 07:56, Ilias Apalodimas
> > wrote:
> > >
> > > Hi Simon,
> > >
> > > As I said here [1], this is moving on an entirely different direction I
> > > had
> > > in mind. I'd much prefer starting the discussions for a solution that
> > > allows us to scale.
> >
> > I am missing the point here. Is there something in the plans that I
> > don't know about?
>
> I have some ideas, but haven't found time to code it and send patches yet.
>
> >
> > > FWIW I think the current code is still not clean for my taste. Commit
> > > 3b595da441cf ("fdtdec: allow board to provide fdt for CONFIG_OF_SEPARATE")
> > > allowed this function to be used regardless of the config options. IMHO
> > > we
> > > should have 2 clear options:
> > > - U-Boot provides the DTB
> >
> > Supported with: OF_SEPARATE
> >
> > > - It's somehow passed over to U-Boot
> >
> > Supported with: OF_SEPARATE + OF_BOARD
>
> That's exactly what I don't personally like. In your example OF_BOARD
> means "U-Boot has a DTB and here's a way to override it". In my head
In some ways this is just highlighting a long standing problem of
working with DTB. There's certainly been some places where splitting
the data from the driver has helped. But in some other places, my
goodness has it made the development cycle worse. Doing this on QEMU
probably is on one of the most annoying cases too since yes, you start
by dumping the dtb you would get, then iterating the dts outside of the
rest of the scope of your build. In the kernel you can "make dtbs" at
least and get things spit out. I'm just repeating over here that the
development cycle of working on device trees is at times not great.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 06:02:19PM +0200, François Ozog wrote: > Hi Mark, > > On Wed, 27 Oct 2021 at 17:10, Mark Kettenis wrote: > > > > From: François Ozog > > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > > > Hi, > > > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > > Hi all, > > > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini > > wrote: > > > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > > > Hi François, > > > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass < > > s...@chromium.org> > > > > a écrit : > > > > > > > > > >> > > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > > >> > > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini < > > tr...@konsulko.com> > > > > wrote: > > > > > > > > > >> > > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng > > wrote: > > > > > > > > > >> > > Hi Simon, > > > > > > > > > >> > > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > > s...@chromium.org> wrote: > > > > > > > > > >> > > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE > > and > > > > OF_HOSTFILE so > > > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > > > >> > > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the > > devicetree > > > > is built and > > > > > > > > > >> > > > appended to U-Boot > > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > > devicetree is embedded in > > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > > > >> > > > > > > > > > > > > >> > > > The last one is currently set up so that no > > devicetree > > > > is needed at all > > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > > > some don't. Some > > > > > > > > > >> > > > don't even provide instructions on how to boot on > > the > > > > board. > > > > > > > > > >> > > > > > > > > > > > > >> > > > The problems with this approach are documented at > > [1]. > > > > > > > > > >> > > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > > > OF_SEPARATE. Any board > > > > > > > > > >> > > > can obtain its devicetree at runtime, even it is > > has a > > > > devicetree built > > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a > > second-stage > > > > bootloader and its > > > > > > > > > >> > > > caller may have a better idea about the hardware > > > > available in the machine. > > > > > > > > > >> > > > This is the case with a few QEMU boards, for > > example. > > > > > > > > > >> > > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a > > 'choice'. It > > > > should be an > > > > > > > > > >> > > > option, available with either OF_SEPARATE or > > OF_EMBED. > > > > > > > > > >> > > > > > > > > > > > > >> > > > This series makes this change, adding various > > missing > > > > devicetree files > > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > > >> > > > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > > > hack to me. > > > > > > > > > >> > > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the > > fly > > > > based on > > > > > > > > > >> > > command line parameters, and the device tree you put > > in > > > > this series > > > > > > > > > >> > > has various hardcoded values which normally > > do > > > > not show up > > > > > > > > > >> > > in hand-written dts files. > > > > > > > > > >> > > > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > > > >> > > > > > > > > > > >> > I am also confused and do not like the idea of adding > > > > device trees for > > > > > > > > > >> > platforms that are capable of and can / do have a device > > > > tree to give us > > > > > > > > > >> > at run time. > > > > > > > > > >> > > > > > > > > > >> (I'll just reply to this one email, since the same points > > > > applies to > > > > > > > > > >> all replies I think) > > > > > > > > > >> > > > > > > > > > >> I have been thinking about this and discussing it with > > people > > > > for a > > > > > > > > > >> few months now. I've been signalling a change like this > > for > > > > over a > > > > > > > > > >> month now, on U-Boot contributor calls and in discussions > > > > with Linaro > > > > > > > > > >> people. I sent a patch
[PATCH u-boot-marvell 5/5] tools: kwboot: Do not use stack when setting baudrate back to default value
From: Pali Rohár
The ARM code we inject into the image to change baudrate back to the
default value of 115200 Baud, which is run after successful UART transfer
of the whole image, cannot use stack as at this stage stack pointer is not
initialized yet.
Stack can only be used when BootROM is executing binary header, to
preserve state of registers, since BootROM expects that.
Change the ARM baudrate code to not use stack at all and put binary
header specific pre + post code (which stores and restores registers) into
separate arrays.
The baudrate change code now jumps at it's end and expects that there is
either code which returns to the BootROM or jumps to the original exec
address.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
---
tools/kwboot.c | 112 -
1 file changed, 65 insertions(+), 47 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 62c218ef64..359b43c0d8 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -78,14 +78,7 @@ struct kwboot_block {
#define KWBOOT_BLK_RSP_TIMEO 1000 /* ms */
#define KWBOOT_HDR_RSP_TIMEO 1 /* ms */
-/* ARM code making baudrate changing function return to original exec address
*/
-static unsigned char kwboot_pre_baud_code[] = {
- /* exec_addr: */
- 0x00, 0x00, 0x00, 0x00, /* .word 0*/
- 0x0c, 0xe0, 0x1f, 0xe5, /* ldr lr, exec_addr */
-};
-
-/* ARM code for binary header injection to change baudrate */
+/* ARM code to change baudrate */
static unsigned char kwboot_baud_code[] = {
/* ; #define UART_BASE 0xd0012000 */
/* ; #define THR 0x00 */
@@ -123,14 +116,12 @@ static unsigned char kwboot_baud_code[] = {
/* ; return 0; */
/* ; }*/
- 0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */
-
/* ; r0 = UART_BASE */
0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd000 */
0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */
/* ; r2 = address of preamble string */
- 0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */
+ 0xc8, 0x20, 0x8f, 0xe2, /* adr r2, preamble */
/* ; Send preamble string over UART */
/* .Lloop_preamble: */
@@ -177,7 +168,7 @@ static unsigned char kwboot_baud_code[] = {
/* ; Read old baudrate value */
/* ; r2 = old_baudrate */
- 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */
+ 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */
/* ; Calculate base clock*/
/* ; r1 = r2 * r1*/
@@ -185,7 +176,7 @@ static unsigned char kwboot_baud_code[] = {
/* ; Read new baudrate value */
/* ; r2 = new_baudrate */
- 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */
+ 0x80, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */
/* ; Calculate new Divisor Latch */
/* ; r1 = DIV_ROUND(r1, r2) =*/
@@ -234,9 +225,7 @@ static unsigned char kwboot_baud_code[] = {
0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */
0xfc, 0xff, 0xff, 0x1a, /* bne .Lloop_sleep */
- /* ; Return 0 - no error */
- 0x00, 0x00, 0xa0, 0xe3, /* mov r0, #0 */
- 0xfe, 0x9f, 0xbd, 0xe8, /* pop { r1 - r12, pc } */
+ 0x05, 0x00, 0x00, 0xea, /* b end */
/* ; Preamble string */
/* preamble: */
@@ -252,10 +241,29 @@ static unsigned char kwboot_baud_code[] = {
/* ; Placeholder for new baudrate value */
/* new_baudrate: */
0x00, 0x00, 0x00, 0x00, /* .word 0*/
+
+ /* end: */
[PATCH u-boot-marvell 4/5] tools: kwboot: Replace ARM mov + movt instruction pair by mov + orr
From: Pali Rohár
Older Armada SoCs have custom ARMv5te compatible core which does not
support movt instruction. So replace mov + movt instruction pair used for
immediate move construction by mov + orr instructions which are supported
also by ARMv5te.
After this change kwboot ARM code should be compatible with any 32-bit ARM
core compatible by ARMv2 or new. At least GNU AS does not throw any error
or warning.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
---
tools/kwboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index 84294cadfe..62c218ef64 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -126,8 +126,8 @@ static unsigned char kwboot_baud_code[] = {
0xfe, 0x5f, 0x2d, 0xe9, /* push { r1 - r12, lr } */
/* ; r0 = UART_BASE */
- 0x02, 0x0a, 0xa0, 0xe3, /* mov r0, #0x2000 */
- 0x01, 0x00, 0x4d, 0xe3, /* movt r0, #0xd001 */
+ 0x0d, 0x02, 0xa0, 0xe3, /* mov r0, #0xd000 */
+ 0x12, 0x0a, 0x80, 0xe3, /* orr r0, r0, #0x12000 */
/* ; r2 = address of preamble string */
0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */
--
2.32.0
[PATCH u-boot-marvell 2/5] tools: kwboot: Do not call tcdrain() after each sent packet
From: Pali Rohár
Kwboot puts each xmodem packet to kernel queue, then waits until all bytes
of that packet are transmitted over UART and then waits for xmodem reply
until it is received into kernel queue.
If some reply is received during the time we are waiting until all bytes
are transmitted, then kernel puts them into the queue and returns it to
kwboot in next read() call.
So there is no need to wait (with tcdrain() function) until all bytes from
xmodem packet are transmitted over UART, since any reply received either
during that time or after is returned to kwboot with the next read().
Therefore do not call tcdrain() after each xmodem packet sent. Instead
directly wait for any reply after putting xmodem packet into write kernel
queue.
This change could speed up xmodem transfer in case tcdrain() function waits
for a longer time.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
---
tools/kwboot.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index b2c48812c3..a6bfd3d4ce 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -404,7 +404,7 @@ out:
}
static int
-kwboot_tty_send(int fd, const void *buf, size_t len)
+kwboot_tty_send(int fd, const void *buf, size_t len, int nodrain)
{
if (!buf)
return 0;
@@ -412,13 +412,16 @@ kwboot_tty_send(int fd, const void *buf, size_t len)
if (kwboot_write(fd, buf, len) < 0)
return -1;
+ if (nodrain)
+ return 0;
+
return tcdrain(fd);
}
static int
kwboot_tty_send_char(int fd, unsigned char c)
{
- return kwboot_tty_send(fd, , 1);
+ return kwboot_tty_send(fd, , 1, 0);
}
static speed_t
@@ -705,7 +708,7 @@ kwboot_bootmsg(int tty, void *msg)
break;
for (count = 0; count < 128; count++) {
- rc = kwboot_tty_send(tty, msg, 8);
+ rc = kwboot_tty_send(tty, msg, 8, 0);
if (rc) {
usleep(msg_req_delay * 1000);
continue;
@@ -737,7 +740,7 @@ kwboot_debugmsg(int tty, void *msg)
if (rc)
break;
- rc = kwboot_tty_send(tty, msg, 8);
+ rc = kwboot_tty_send(tty, msg, 8, 0);
if (rc) {
usleep(msg_req_delay * 1000);
continue;
@@ -929,7 +932,7 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int
allow_non_xm,
retries = 0;
do {
- rc = kwboot_tty_send(fd, block, sizeof(*block));
+ rc = kwboot_tty_send(fd, block, sizeof(*block), 1);
if (rc)
return rc;
--
2.32.0
[PATCH u-boot-marvell 3/5] tools: kwboot: Increase delay after changing baudrate in ARM code
From: Pali Rohár
Increase loop cycles from 60 to 2998272, which should increase delay
from 1ms to about 5ms on 1200 MHz CPU.
The Number 2998272 was chosen as the nearest value around 300 which can
be encoded into one ARM mov instruction. It avoids usage of movt instruction
which is not supported by ARMv5te cores.
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
---
tools/kwboot.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index a6bfd3d4ce..84294cadfe 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -119,7 +119,7 @@ static unsigned char kwboot_baud_code[] = {
/* ; writel(UART_BASE + DLL, new_dll); */
/* ; writel(UART_BASE + DLH, new_dlh); */
/* ; writel(UART_BASE + LCR, lcr & ~DLAB); */
- /* ; msleep(1); */
+ /* ; msleep(5); */
/* ; return 0; */
/* ; }*/
@@ -130,7 +130,7 @@ static unsigned char kwboot_baud_code[] = {
0x01, 0x00, 0x4d, 0xe3, /* movt r0, #0xd001 */
/* ; r2 = address of preamble string */
- 0xd0, 0x20, 0x8f, 0xe2, /* adr r2, preamble */
+ 0xcc, 0x20, 0x8f, 0xe2, /* adr r2, preamble */
/* ; Send preamble string over UART */
/* .Lloop_preamble: */
@@ -177,15 +177,15 @@ static unsigned char kwboot_baud_code[] = {
/* ; Read old baudrate value */
/* ; r2 = old_baudrate */
- 0x8c, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */
+ 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, old_baudrate */
/* ; Calculate base clock*/
/* ; r1 = r2 * r1*/
0x92, 0x01, 0x01, 0xe0, /* mul r1, r2, r1 */
/* ; Read new baudrate value */
- /* ; r2 = baudrate */
- 0x88, 0x20, 0x9f, 0xe5, /* ldr r2, baudrate */
+ /* ; r2 = new_baudrate */
+ 0x84, 0x20, 0x9f, 0xe5, /* ldr r2, new_baudrate */
/* ; Calculate new Divisor Latch */
/* ; r1 = DIV_ROUND(r1, r2) =*/
@@ -225,10 +225,10 @@ static unsigned char kwboot_baud_code[] = {
0x80, 0x10, 0xc1, 0xe3, /* bic r1, r1, #0x80*/
0x0c, 0x10, 0x80, 0xe5, /* str r1, [r0, #0x0c] */
- /* ; Sleep 1ms ~~ 60 cycles at 1200 MHz */
- /* ; r1 = 60 */
- 0x9f, 0x1d, 0xa0, 0xe3, /* mov r1, #0x27c0 */
- 0x09, 0x10, 0x40, 0xe3, /* movt r1, #0x0009 */
+ /* ; Loop 0x2dc000 (2998272) cycles */
+ /* ; which is about 5ms on 1200 MHz CPU */
+ /* ; r1 = 0x2dc000 */
+ 0xb7, 0x19, 0xa0, 0xe3, /* mov r1, #0x2dc000*/
/* .Lloop_sleep: */
0x01, 0x10, 0x41, 0xe2, /* sub r1, r1, #1 */
0x00, 0x00, 0x51, 0xe3, /* cmp r1, #0 */
--
2.32.0
[PATCH u-boot-marvell 1/5] tools: kwboot: Fix sending retry of last header packet
From: Pali Rohár
After the trasfer of last header packet, it is possible that baudrate
change pattern is received, and also that NAK byte is received so that
the packet should be sent again.
Thus we should not clear the baudrate change state when sending retry
of that packet.
Move code for initializing state variables from kwboot_xm_recv_reply()
to kwboot_xm_sendblock().
Signed-off-by: Pali Rohár
Reviewed-by: Marek Behún
---
tools/kwboot.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/tools/kwboot.c b/tools/kwboot.c
index bb7cae9f05..b2c48812c3 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -859,11 +859,6 @@ kwboot_xm_recv_reply(int fd, char *c, int nak_on_non_xm,
uint64_t recv_until = _now() + timeout;
int rc;
- if (non_xm_print)
- *non_xm_print = 0;
- if (baud_changed)
- *baud_changed = 0;
-
while (1) {
rc = kwboot_tty_recv(fd, c, 1, timeout);
if (rc) {
@@ -929,6 +924,8 @@ kwboot_xm_sendblock(int fd, struct kwboot_block *block, int
allow_non_xm,
char c;
*done_print = 0;
+ non_xm_print = 0;
+ baud_changed = 0;
retries = 0;
do {
--
2.32.0
[PATCH u-boot-marvell 0/5] kwboot fix for AXP + some others
From: Marek Behún Hi Stefan, here are some more Pali's changes for kwboot, reviewed by me. The last one should fix boot on AXP. Marek Pali Rohár (5): tools: kwboot: Fix sending retry of last header packet tools: kwboot: Do not call tcdrain() after each sent packet tools: kwboot: Increase delay after changing baudrate in ARM code tools: kwboot: Replace ARM mov + movt instruction pair by mov + orr tools: kwboot: Do not use stack when setting baudrate back to default value tools/kwboot.c | 148 +++-- 1 file changed, 83 insertions(+), 65 deletions(-) -- 2.32.0
Re: [PATCH 05/16] arm: qemu: Add a devicetree file for qemu_arm64
On Wed, Oct 27, 2021 at 12:34:26PM -0600, Simon Glass wrote: > Hi all, > > On Wed, 27 Oct 2021 at 08:56, Tom Rini wrote: > > > > On Wed, Oct 27, 2021 at 03:44:08PM +0100, Alex Bennée wrote: > > > > > > François Ozog writes: > > > > > > > Hi Simon > > > > > > > > The only place I could agree with this file presence is in the > > > > documentation directory, not in dts. It creates a mental picture for > > > > the reader > > > > an entirely bad mind scheme around Qemu and DT. > > > > > > > > And even in a documentation directory I would place a bug warning: > > > > don’t use this with any kernel , Qemu generates a DT dynamically > > > > based on cpu, memory and devices specified at the command line. > > > > > > Certainly for the arm, aarch64 and riscv "virt" machines you should > > > always use the QEMU generated DTB. I'm not entirely clear what a > > > qemu_arm and qemu_arm64 def targets are meant to be in this context. > > > > Agreed. We cannot include random device trees in U-Boot for devices > > that generate their own at run time or otherwise have the source of > > truth elsewhere. > > Until we have a way of bringing in the u-boot.dtsi that people in QEMU > can agree on, I don't see an alternative. I will send a series for the > bloblist handoff next week and I think you will all see what I mean. I think the alternative is that QEMU in U-Boot just can't be used for certain features. Which is annoying in that it would be good to use it to test certain feature, yes. It's generating a good and valid enough dtb for Linux, so it should be good enough for us in general. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 31/41] bootstd: Add an implementation of EFI boot
Hi Ilias, On Wed, 27 Oct 2021 at 08:48, Ilias Apalodimas wrote: > > Hi Simon, > > On Wed, Oct 27, 2021 at 08:09:04AM -0600, Simon Glass wrote: > > Hi Ilias, > > > > On Wed, 27 Oct 2021 at 02:36, Ilias Apalodimas > > wrote: > > > > > > Hi Simon, > > > > > > On Sun, 24 Oct 2021 at 02:27, Simon Glass wrote: > > > > > > > > Add a bootmeth driver which handles EFI boot, using EFI_LOADER. > > > > > > > > In effect, this provides the same functionality as the 'bootefi' command > > > > and shares the same code. But the interface into it is via a bootmeth, > > > > so it does not require any special scripts, etc. > > > > > > > > For now this requires the 'bootefi' command be enabled. Future work may > > > > tidy this up so that it can be used without CONFIG_CMDLINE being > > > > enabled. > > > > > > I'll leave this up to Heinrich, but personally I wouldn't include this > > > patch at all. EFI has it's bootmgr which can handle booting just fine. > > > I don't see why we should duplicate the functionality. The new boot > > > method can just have an entry called 'EFI' and then let the existing > > > EFI code to decide. > > > > This is needed so that EFI boot is actually invoked. If bootmgr starts > > being used then it can still be invoked from standard boot. The point > > is that there is a standard way of booting that supports EFI and other > > things. > > This patch tries to reason about the default naming EFI imposes on it's > boot files. distro_efi_read_bootflow() will try to find files following the > EFI naming convention (e.g bootaarch64.efi, bootarm.efi etc). If those are > found it will try to boot them right? That's not the right thing to do > though. > On the EFI spec these files are tried if no Boot variables are found. > So we can get rid of this entirely, add a dummy entry on the bootflow that > says 'boot the efi manager' (which is what the next patch does). > > The efibootmgr then will check Boot variables and if none are found, > it's going to fallback into loading bootaarch64.efi, bootarm.efi etc > essentially offering identical functionality. Yes that's fine, and when EFI's boot manager is in use I have a driver for that too, as you can see in the other patch. We may need to adjust the order, by the sound of it, if it needs to run before EFI things. But that is easy enough. But we do need to provide the existing functionality for now, as I understand it. > > This series is about replacing the scripts we currently have with a > > proper C implementation that uses driver model. > > Regards, Simon
Re: [PATCH v1 1/2] x86: tangier: pinmux: Move is_protected assignment closer to its user
On Wed, 27 Oct 2021 at 08:23, Andy Shevchenko wrote: > > Move is_protected assignment closer to its user. > This increases readability and makes maintenance easier. > > Signed-off-by: Andy Shevchenko > --- > arch/x86/cpu/tangier/pinmux.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Reviewed-by: Simon Glass (with a note that normally we would read the devicetree in the of_to_plat() method) > > diff --git a/arch/x86/cpu/tangier/pinmux.c b/arch/x86/cpu/tangier/pinmux.c > index 8385167b2b6b..883860f75e28 100644 > --- a/arch/x86/cpu/tangier/pinmux.c > +++ b/arch/x86/cpu/tangier/pinmux.c > @@ -162,8 +162,6 @@ static int mrfld_pinctrl_cfg_pin(ofnode pin_node) > u32 mask; > int ret; > > - is_protected = ofnode_read_bool(pin_node, "protected"); > - > pad_offset = ofnode_read_s32_default(pin_node, "pad-offset", -1); > if (pad_offset == -1) > return -EINVAL; > @@ -178,6 +176,7 @@ static int mrfld_pinctrl_cfg_pin(ofnode pin_node) > if (mode & ~mask) > return -ENOTSUPP; > > + is_protected = ofnode_read_bool(pin_node, "protected"); > if (is_protected) > ret = mrfld_pinconfig_protected(pad_offset, mask, mode); > else > -- > 2.33.0 >
Re: [PATCH v1 2/2] x86: tangier: pinmux: Move error message to the caller
On Wed, 27 Oct 2021 at 08:23, Andy Shevchenko
wrote:
>
> Move error message to the caller of mrfld_pinconfig*() in order
> to unify them in the future.
>
> Signed-off-by: Andy Shevchenko
> ---
> arch/x86/cpu/tangier/pinmux.c | 10 +++---
> 1 file changed, 3 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass
>
> diff --git a/arch/x86/cpu/tangier/pinmux.c b/arch/x86/cpu/tangier/pinmux.c
> index 883860f75e28..7b2c35fbf794 100644
> --- a/arch/x86/cpu/tangier/pinmux.c
> +++ b/arch/x86/cpu/tangier/pinmux.c
> @@ -117,13 +117,7 @@ static int mrfld_pinconfig_protected(unsigned int pin,
> u32 mask, u32 bits)
> debug("scu: v: 0x%x p: 0x%x bits: %d, mask: %d bufcfg: 0x%p\n",
> v, (u32)bufcfg, bits, mask, bufcfg);
>
> - ret = scu_ipc_raw_command(IPCMSG_INDIRECT_WRITE, 0, , 4,
> - NULL, 0, (u32)bufcfg, 0);
> - if (ret)
> - pr_err("Failed to set mode via SCU for pin %u (%d)\n",
> - pin, ret);
> -
> - return ret;
> + return scu_ipc_raw_command(IPCMSG_INDIRECT_WRITE, 0, , 4, NULL, 0,
> (u32)bufcfg, 0);
In general I like to separate out the error return so we can use something like:
ret = scu_...
if (ret)
return log_msg_ret("scu", ret);
return 0;
> }
>
> static int mrfld_pinconfig(unsigned int pin, u32 mask, u32 bits)
> @@ -181,6 +175,8 @@ static int mrfld_pinctrl_cfg_pin(ofnode pin_node)
> ret = mrfld_pinconfig_protected(pad_offset, mask, mode);
> else
> ret = mrfld_pinconfig(pad_offset, mask, mode);
> + if (ret)
> + pr_err("Failed to set mode for pin %u (%d)\n", pad_offset,
> ret);
>
> return ret;
> }
> --
> 2.33.0
>
Re: [PATCH 05/16] arm: qemu: Add a devicetree file for qemu_arm64
Hi all, On Wed, 27 Oct 2021 at 08:56, Tom Rini wrote: > > On Wed, Oct 27, 2021 at 03:44:08PM +0100, Alex Bennée wrote: > > > > François Ozog writes: > > > > > Hi Simon > > > > > > The only place I could agree with this file presence is in the > > > documentation directory, not in dts. It creates a mental picture for the > > > reader > > > an entirely bad mind scheme around Qemu and DT. > > > > > > And even in a documentation directory I would place a bug warning: don’t > > > use this with any kernel , Qemu generates a DT dynamically > > > based on cpu, memory and devices specified at the command line. > > > > Certainly for the arm, aarch64 and riscv "virt" machines you should > > always use the QEMU generated DTB. I'm not entirely clear what a > > qemu_arm and qemu_arm64 def targets are meant to be in this context. > > Agreed. We cannot include random device trees in U-Boot for devices > that generate their own at run time or otherwise have the source of > truth elsewhere. Until we have a way of bringing in the u-boot.dtsi that people in QEMU can agree on, I don't see an alternative. I will send a series for the bloblist handoff next week and I think you will all see what I mean. Perhaps all this will be easier a year or so, if we continue to make progress on the devicetree validation/sharing stuff, but for now, this seems like the only viable approach to me. Regards, Simon
Re: [PATCH v2 00/41] Initial implementation of standard boot
Hi Heinrich, On Wed, 27 Oct 2021 at 05:38, Heinrich Schuchardt wrote: > > > > On 10/24/21 01:25, Simon Glass wrote: > > > > The bootflow feature provide a built-in way for U-Boot to automatically > > boot an Operating System without custom scripting and other customisation. > > This is called 'standard boot' since it provides a standard way for > > U-Boot to boot a distro, without scripting. > > > > It introduces the following concepts: > > > > - bootdev - a device which can hold a distro > > - bootmeth - a method to scan a bootdev to find bootflows (owned by > > U-Boot) > > - bootflow - a description of how to boot (owned by the distro) > > Please, describe why you are suggesting this change. > > Replacing a script by a devicetree chunk is just decreasing flexibility > and increasing complexity. Where is the benefit? See previous email in thread. Continuing here on a few of your other points... > > I am missing a description here of where and how a boot flow shall be > defined. Describing some device-tree binding in patch 40/41 does not > replace describing the development and usage workflow. Who will provide > which bootflow information when? We already have this with distro boot, so nothing really changes there. Think of this as a generalised 'standard boot' implementation, which can support distro boot and anything else we need in the future. There is nothing required in the devicetree for normal operation. > > You still have an open discussion with Linaro about the source of > devicetrees. This discussion needs to be finalized before considering > this series. Why is that? They don't seem to be at all related to me. > > In my view bootflows cannot be defined in the devicetree because prior > firmware providing a devicetree is completely independent of any distro > and therefore cannot provide a distro specific bootflow. See previous post, they are not defined in the devicetree. Can you suggest how I can change the language to make that clear? Regards, Simon > > > > > This series provides an implementation of these, enabled to scan for > > bootflows from MMC, USB and Ethernet. It supports the existing distro > > boot as well as the EFI loader flow (bootefi/bootmgr). It works > > similiarly to the existing script-based approach, but is native to > > U-Boot. > > > > With this we can boot on a Raspberry Pi 3 with just one command: > > > > bootflow scan -lb > > > > which means to scan, listing (-l) each bootflow and trying to boot each > > one (-b). The final patch shows this. > > > > With a standard way to identify boot devices, booting become easier. It > > also should be possible to support U-Boot scripts, for backwards > > compatibility only. > > > > [..]
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
Hi François, On Wed, 27 Oct 2021 at 09:38, François Ozog wrote: > > Hi Simon, > > On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: >> >> Hi François, >> >> On Tue, 26 Oct 2021 at 09:57, François Ozog wrote: >> > >> > >> > >> > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: >> >> >> >> Hi François, >> >> >> >> On Tue, 26 Oct 2021 at 08:31, François Ozog >> >> wrote: >> >> > >> >> > Hi Simon, >> >> > >> >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass wrote: >> >> >> >> >> >> At present some of the ideas and techniques behind devicetree in U-Boot >> >> >> are assumed, implied or unsaid. Add some documentation to cover how >> >> >> devicetree is build, how it can be modified and the rules about using >> >> >> the various CONFIG_OF_... options. >> >> >> >> >> [..] >> >> >> >> +Why not have two devicetrees? >> >> >> +- >> >> >> + >> >> >> +Setting aside the argument for restricting U-Boot from having its own >> >> >> nodes and >> >> >> +properties, another idea proposed is to have two devicetrees, one for >> >> >> the >> >> >> +U-Boot-specific bits (here called `special`) and one for everything >> >> >> else (here >> >> >> +called `linux`). >> >> >> + >> >> >> +On the positive side, it might quieten the discussion alluded to in >> >> >> the section >> >> >> +above. But there are many negatives to consider and many open >> >> >> questions to >> >> >> +resolve. >> >> >> + >> >> >> +- **Bindings** - Presumably the special devicetree would have its own >> >> >> bindings. >> >> >> + It would not be necessary to put a `u-boot,` prefix on anything. >> >> >> People coming >> >> >> + across the devicetree source would wonder how it fits in with the >> >> >> Linux >> >> >> + devicetree. >> >> >> + >> >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the >> >> >> devicetree. This >> >> >> + would need to be expanded to support two trees. Features which need >> >> >> to access >> >> >> + both (such as a device driver which reads the special devicetree to >> >> >> get some >> >> >> + configuration info) could become quite confusing to read and write. >> >> >> + >> >> >> +- **Merging** - Can the two devicetree be merged if a platform >> >> >> desires it? If >> >> >> + so, how is this managed in tooling? Does it happen during the >> >> >> build, in which >> >> >> + case they are not really separate at all. Or does U-Boot merge them >> >> >> at >> >> >> + runtime, in which case this adds time and memory? >> >> >> + >> >> >> +- **Efficiency** - A second device tree adds more code and more code >> >> >> paths. It >> >> >> + requires that both be made available to the code in U-Boot, e.g. >> >> >> via a >> >> >> + separate pointer or argument or API. Overall the separation would >> >> >> certainly >> >> >> + not speed up U-Boot, nor decrease its size. >> >> >> + >> >> >> +- **Source code** - At present `u-boot.dtsi` files provide the pieces >> >> >> needed for >> >> >> + U-Boot for a particular board. Would we use these same files for >> >> >> the special >> >> >> + devicetree? >> >> >> + >> >> >> +- **Complexity** - Two devicetrees complicates the build system since >> >> >> it must >> >> >> + build and package them both. Errors must be reported in such a way >> >> >> that it >> >> >> + is obvious which one is failing. >> >> >> + >> >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by >> >> >> driver model >> >> >> + are currently placed in the nodes they relate to. How would these >> >> >> tags >> >> >> + reference a node that is in a separate devicetree? What extra >> >> >> validation would >> >> >> + be needed? >> >> >> + >> >> >> +- **Storage** - How would the two devicetrees be stored in the image? >> >> >> At present >> >> >> + we simply concatenate the U-Boot binary and the devicetree. We >> >> >> could add the >> >> >> + special devicetree before the Linux one, so two are concatenated, >> >> >> but it is >> >> >> + not pretty. We could use binman to support more complex >> >> >> arrangements, but only >> >> >> + some boards use this at present, so it would be a big change. >> >> >> + >> >> >> +- **API** - How would another project provide two devicetree files to >> >> >> U-Boot at >> >> >> + runtime? Presumably this would just be too painful. But if it >> >> >> doesn't, it >> >> >> + would be unable to configure run-time features of U-Boot during the >> >> >> boot. >> >> >> + >> >> >> +- **Confusion** - No other project has two devicetrees. U-Boot would >> >> >> be in the >> >> >> + unfortunate position of having to describe this fact to new users, >> >> >> along with >> >> >> + the (arguably contrived) reason for the arrangement. >> >> >> + >> >> > >> >> > False: >> >> > 1) projects in trustedfirmware.org are built to have multiple FDT >> >> > objects, some for "dynamic" configuration purposes. >> >> >> >> Can you provided a link and I can update this. >> > >> >
Re: [PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wednesday 27 October 2021 20:01:33 Robert Marko wrote:
> Globalscale MOCHAbin is a Armada 7040 based development board.
>
> Specifications:
> * Armada 7040 Quad core ARMv8 Cortex A-72 @ 1.4GHz
> * 2 / 4 / 8 GB of DDR4 DRAM
> * 16 GB eMMC
> * 4MB SPI-NOR (Bootloader)
> * 1x M.2-2280 B-key socket (for SSD expansion, SATA3 only)
> * 1x M.2-2250 B-key socket (for modems, USB2.0 and I2C only)
> * 1x Mini-PCIe 3.0 (x1, USB2.0 and I2C)
> * 1x SATA 7+15 socket (SATA3)
> * 1x 16-pin (2×8) MikroBus Connector
> * 1x SIM card slot (Connected to the mini-PCIe and both M.2 slots)
> * 2x USB3.0 Type-A ports via SMSC USB5434B hub
> * Cortex 2x5 JTAG
> * microUSB port for UART (PL2303GL/PL2303SA onboard)
> * 1x 10G SFP+
> * 1x 1G SFP (Connected to 88E1512 PHY)
> * 1x 1G RJ45 with PoE PD (Connected to 88E1512 PHY)
> * 4x 1G RJ45 ports via Topaz 88E6141 switch
> * RTC with battery holder (SoC provided, requires CR2032 battery)
> * 1x 12V DC IN
> * 1x Power switch
> * 1x 12V fan header (3-pin, power only)
> * 1x mini-PCIe LED header (2x0.1" pins)
> * 1x M.2-2280 LED header (2x0.1" pins)
> * 6x Bootstrap jumpers
> * 1x Power LED (Green)
> * 3x Tri-color RGB LEDs (Controllable)
> * 1x Microchip ATECC608B secure element
>
> Note that 1G SFP does not work in U-boot as it would require Linux like
> SFP support for parsing module interface and reconfiguring the PHY.
>
> Additionally, automatic import of the Marvell hw_info parameters is
> enabled via the recently added mac command for Armada platforms.
> The parameters stored in Marvell hw_info are usually the board serial
> number and MAC addresses.
>
> Signed-off-by: Robert Marko
> ---
> Changes in v3:
> * Rename "u-boot" partition to "firmware" which better describes its content
> * Remove read-only flag from "hw-info" partition
>
> Changes in v2:
> * Add DT compatible for hw-info to its partition
> * Enable MTD support, command and SPI MTD command
> ---
> arch/arm/dts/Makefile| 1 +
> arch/arm/dts/armada-7040-mochabin.dts| 304 +++
> configs/mvebu_mochabin-88f7040_defconfig | 88 +++
> 3 files changed, 393 insertions(+)
> create mode 100644 arch/arm/dts/armada-7040-mochabin.dts
> create mode 100644 configs/mvebu_mochabin-88f7040_defconfig
>
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> index a88aecc5bd9..dea567d2595 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -232,6 +232,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
> armada-38x-controlcenterdc.dtb \
> armada-7040-db-nand.dtb \
> armada-7040-db.dtb \
> + armada-7040-mochabin.dtb\
> armada-8040-clearfog-gt-8k.dtb \
> armada-8040-db.dtb \
> armada-8040-mcbin.dtb \
> diff --git a/arch/arm/dts/armada-7040-mochabin.dts
> b/arch/arm/dts/armada-7040-mochabin.dts
> new file mode 100644
> index 000..84f06363728
> --- /dev/null
> +++ b/arch/arm/dts/armada-7040-mochabin.dts
> @@ -0,0 +1,304 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2016- 2021 Marvell International Ltd.
> + * Copyright (C) 2021 Sartura Ltd.
> + */
> +
> +/*
> + * Device Tree file for Globalscale MOCHAbin
> + * Boot device: SPI NOR, 0x32 (SW3)
> + */
> +
> +#include "armada-7040.dtsi"
> +
> +/ {
> + model = "Globalscale MOCHAbin";
> + compatible = "globalscale,mochabin", "marvell,armada7040",
> + "marvell,armada-ap806-quad", "marvell,armada-ap806";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + aliases {
> + i2c0 = _i2c0;
> + spi0 = _spi1;
> + };
> +
> + memory@ {
> + device_type = "memory";
> + reg = <0x0 0x0 0x0 0x8000>;
> + };
> +};
Hm... Why is this DTS file different than newly merged DTS file in Linux?
https://git.kernel.org/pub/scm/linux/kernel/git/gclement/mvebu.git/commit/?h=mvebu/dt64=73792919128304938412194d047be3c6176d
Should not we have same file in Linux and U-Boot project and optional
U-Boot modifications (if needed) in *-u-boot.dtsi file like it is for
other arm boards?
https://source.denx.de/u-boot/u-boot/-/tree/master/arch/arm/dts
> +
> +_pinctl {
> +/* MPP Bus:
> + * SDIO [0-5]
> + * UART0 [11,19]
> + */
> + /* 0 1 2 3 4 5 6 7 8 9 */
> + pin-func = < 1 1 1 1 1 1 0 0 0 0
> + 0 3 0 0 0 0 0 0 0 3 >;
> +};
> +
> +/* microUSB UART console */
> + {
> + status = "okay";
> +};
> +
> +/* eMMC */
> +_sdhci0 {
> + status = "okay";
> +
> + bus-width = <4>;
> + no-1-8-v;
> + non-removable;
> +};
> +
> +_pinctl {
> + /* MPP Bus:
> + * E6341_RSTn[0]
> + * E6341_INT [1]
> + * I2C1 [2,3]
> + * PCIE_CLKREQn [5]
> +
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
Hi Tom Le mer. 27 oct. 2021 à 20:06, Tom Rini a écrit : > On Wed, Oct 27, 2021 at 09:24:25AM -0600, Simon Glass wrote: > > Hi Mark, > > > > On Wed, 27 Oct 2021 at 09:11, Mark Kettenis > wrote: > > > > > > > From: François Ozog > > > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > > > > > Hi, > > > > > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > > > Hi all, > > > > > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini > wrote: > > > > > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini > wrote: > > > > > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass > wrote: > > > > > > > > > > Hi François, > > > > > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass < > s...@chromium.org> > > > > > a écrit : > > > > > > > > > > >> > > > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > > > >> > > > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini < > tr...@konsulko.com> > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng > wrote: > > > > > > > > > > >> > > Hi Simon, > > > > > > > > > > >> > > > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > > > s...@chromium.org> wrote: > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped > OF_PRIOR_STAGE and > > > > > OF_HOSTFILE so > > > > > > > > > > >> > > > there are only three ways to obtain a > devicetree: > > > > > > > > > > >> > > > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the > devicetree > > > > > is built and > > > > > > > > > > >> > > > appended to U-Boot > > > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > > > devicetree is embedded in > > > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its > own > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > The last one is currently set up so that no > devicetree > > > > > is needed at all > > > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, > but > > > > > some don't. Some > > > > > > > > > > >> > > > don't even provide instructions on how to boot > on the > > > > > board. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > The problems with this approach are documented > at [1]. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct > from > > > > > OF_SEPARATE. Any board > > > > > > > > > > >> > > > can obtain its devicetree at runtime, even it > is has a > > > > > devicetree built > > > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a > second-stage > > > > > bootloader and its > > > > > > > > > > >> > > > caller may have a better idea about the hardware > > > > > available in the machine. > > > > > > > > > > >> > > > This is the case with a few QEMU boards, for > example. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a > 'choice'. It > > > > > should be an > > > > > > > > > > >> > > > option, available with either OF_SEPARATE or > OF_EMBED. > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > This series makes this change, adding various > missing > > > > > devicetree files > > > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > > > >> > > > > > > > > > > > > >> > > Adding device trees that are never used sounds > like a > > > > > hack to me. > > > > > > > > > > >> > > > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on > the fly > > > > > based on > > > > > > > > > > >> > > command line parameters, and the device tree you > put in > > > > > this series > > > > > > > > > > >> > > has various hardcoded values which > normally do > > > > > not show up > > > > > > > > > > >> > > in hand-written dts files. > > > > > > > > > > >> > > > > > > > > > > > > >> > > I am not sure I understand the whole point of > this. > > > > > > > > > > >> > > > > > > > > > > > >> > I am also confused and do not like the idea of > adding > > > > > device trees for > > > > > > > > > > >> > platforms that are capable of and can / do have a > device > > > > > tree to give us > > > > > > > > > > >> > at run time. > > > > > > > > > > >> > > > > > > > > > > >> (I'll just reply to this one email, since the same > points > > > > > applies to > > > > > > > > > > >> all replies I think) > > > > > > > > > > >> > > > > > > > > > > >> I have been thinking about this and discussing it > with people >
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 08:08:19AM -0600, Simon Glass wrote: [snip] > But trying to do any driver / core work for a board where you don't > have the devicetree is currently not possible. The devicetree is a > core component and being unable to modify it is simply not practical. > We are talking here about an option that can be enabled or disabled. > In my case I am able to disable it locally and do my development work. This certainly is an area where it's easier to work on arm32 platforms, where we aren't getting the DT passed in, than arm64, where we almost certainly are. -- Tom signature.asc Description: PGP signature
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
Hi François, On Wed, 27 Oct 2021 at 09:14, François Ozog wrote: > > > > On Wed, 27 Oct 2021 at 16:08, Simon Glass wrote: >> >> Hi François, >> >> On Tue, 26 Oct 2021 at 00:07, François Ozog wrote: >> > >> > Hi Simon >> > >> > Position unchanged on this series: adding fake dts for boards that >> > generate their device tree in the dts directory is not good. If you have >> > them in documentation the it is acceptable. >> >> I think we are going to have to disagree on this one. I actually used >> the qemu one in testing/development recently. We have to have a way to >> develop in-tree with U-Boot. It does not impinge on any of your use >> cases, so far as I know. > > I am not the only one in disagreement... You just saw Alex Bénée from Qemu > saying the same thing. > I understand the advanced debug/development scenario you mention. > But locating the DT files in the dts directory is mis-leading the > contributors to think that they need to compile the DT for the targeted > platforms. > For your advanced scenario, you can still have the dts in the documentation > area, or whatever directory (except dts). compile it and supply to U-Boot. We have this situation with rpi 1, 2 and 3 and I don't believe anyone has noticed. U-Boot handles the build automatically. If you turn off OF_BOARD, it will use the U-Boot devicetree always so you know what is going on. We can add a message to U-Boot indicating where the devicetree came from, perhaps? That might be useful given everything that is going on. As in this case, quite often in these discussions I struggle to understand what is behind the objection. Is it that your customers are demanding that devicetrees become private, secret data, not included in open-source projects? Or is it just the strange case of QEMU that is informing your thinking? I know of at least one project where the first-stage bootloader produces a devicetree and no one has the source for it. I believe TF-A was created for licensing reasons...so can you be a bit clearer about what the problem actually is? If a board is in-tree in U-Boot I would like it to have a devicetree there, at least until we have a better option. At the very least, it MUST be discoverable and it must be possible to undertake U-Boot development easily without a lot of messing around. >> >> >> But trying to do any driver / core work for a board where you don't >> have the devicetree is currently not possible. The devicetree is a >> core component and being unable to modify it is simply not practical. >> We are talking here about an option that can be enabled or disabled. >> In my case I am able to disable it locally and do my development work. >> >> >> BTW I've got the bloblist handoff working with a devicetree inside it, >> for qemu_arm. I need to try it on a real board to figure out what the >> difference is. >> > That's great news and much needed for stabilizing the inbound ABI from prior > loader to U-Boot. Let's create another thread to discuss this important topic. >> My scenario is not all that advanced and I am using qemu_arm to test the bloblist handoff stuff and include it in CI, with a suitable devicetree and a binman node. Regards, Simon >> > >> > Cheers >> > >> > FF >> > >> > Le mar. 26 oct. 2021 à 02:24, Simon Glass a écrit : >> >> >> >> With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so >> >> there are only three ways to obtain a devicetree: >> >> >> >>- OF_SEPARATE - the normal way, where the devicetree is built and >> >> appended to U-Boot >> >>- OF_EMBED - for development purposes, the devicetree is embedded in >> >> the ELF file (also used for EFI) >> >>- OF_BOARD - the board figures it out on its own >> >> >> >> The last one is currently set up so that no devicetree is needed at all >> >> in the U-Boot tree. Most boards do provide one, but some don't. Some >> >> don't even provide instructions on how to boot on the board. >> >> >> >> The problems with this approach are documented in another patch in this >> >> series: "doc: Add documentation about devicetree usage" >> >> >> >> In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board >> >> can obtain its devicetree at runtime, even it is has a devicetree built >> >> in U-Boot. This is because U-Boot may be a second-stage bootloader and its >> >> caller may have a better idea about the hardware available in the machine. >> >> This is the case with a few QEMU boards, for example. >> >> >> >> So it makes no sense to have OF_BOARD as a 'choice'. It should be an >> >> option, available with either OF_SEPARATE or OF_EMBED. >> >> >> >> This series makes this change, adding various missing devicetree files >> >> (and placeholders) to make the build work. >> >> >> >> Note: If board maintainers are able to add their own patch to add the >> >> files, some patches in this series can be dropped. >> >> >> >> It also provides a few qemu clean-ups discovered along the way. >> >> >> >> Note: This
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 08:08:19AM -0600, Simon Glass wrote: > Hi François, > > On Tue, 26 Oct 2021 at 00:07, François Ozog wrote: > > > > Hi Simon > > > > Position unchanged on this series: adding fake dts for boards that generate > > their device tree in the dts directory is not good. If you have them in > > documentation the it is acceptable. > > I think we are going to have to disagree on this one. I'm not convinced either that we want or should have checked in device trees for all of the platforms where the source of truth is elsewhere. > I actually used > the qemu one in testing/development recently. We have to have a way to > develop in-tree with U-Boot. It does not impinge on any of your use > cases, so far as I know. It's certainly true that the "edit, rebuild, re-pass the dtb" cycle has been sub-optimal since inception. It's not even U-Boot related. I can count a number of times recently working with customers and just for Linux, having to spend a number of hours on the edit, rebuild, really make sure it gets populated where it's supposed to go, verify that yes really our modified dtb is the one present cycle. It's a very generic problem. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] video: fix positioning in TrueType console
Dear Anatolij,
somehow you lost track of this patch since March:
https://patchwork.ozlabs.org/project/uboot/patch/20210203161254.970-1-xypron.g...@gmx.de/
Could you, please, review it and consider it for merging.
Best regards
Heinrich
On 2/3/21 17:12, Heinrich Schuchardt wrote:
With the patch accurate positioning is possible for mono-typed fonts:
Fix the return value of console_truetype_putc_xy(). The current position
is passed as parameter x. Some part of x represents a fractional pixel.
The return value represents how much the character position must be
advanced. This should only comprise the width of the current character and
not the preexisting fractional pixel position.
Characters are not square. As all characters of a mono-type font we can
take the width of any character. 'W' as one of the widest ANSI characters
provides also a good value for variable width fonts.
The character width must be a float.
Signed-off-by: Heinrich Schuchardt
---
drivers/video/console_truetype.c | 27 ---
include/video_console.h | 2 +-
2 files changed, 21 insertions(+), 8 deletions(-)
--
2.30.1
diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c
index 318a060926..56939af849 100644
--- a/drivers/video/console_truetype.c
+++ b/drivers/video/console_truetype.c
@@ -211,7 +211,7 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
int width_frac, linenum;
struct pos_info *pos;
u8 *bits, *data;
- int advance;
+ int advance, kern_adv;
void *start, *line, *sync_start, *sync_end;
int row, ret;
int bg_r, bg_g, bg_b;
@@ -226,8 +226,11 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
* this character */
xpos = frac(VID_TO_PIXEL((double)x));
if (vc_priv->last_ch) {
- xpos += priv->scale * stbtt_GetCodepointKernAdvance(font,
- vc_priv->last_ch, ch);
+ kern_adv = stbtt_GetCodepointKernAdvance(font, vc_priv->last_ch,
+ch);
+ xpos += priv->scale * kern_adv;
+ } else {
+ kern_adv = 0;
}
/*
@@ -238,8 +241,8 @@ static int console_truetype_putc_xy(struct udevice *dev,
uint x, uint y,
*/
x_shift = xpos - (double)tt_floor(xpos);
xpos += advance * priv->scale;
- width_frac = (int)VID_TO_POS(xpos);
- if (x + width_frac >= vc_priv->xsize_frac)
+ width_frac = VID_TO_POS(priv->scale * (kern_adv + advance));
+ if (x + (int)VID_TO_POS(xpos) >= vc_priv->xsize_frac)
return -EAGAIN;
/* Write the current cursor position into history */
@@ -591,20 +594,21 @@ static int console_truetype_probe(struct udevice *dev)
struct udevice *vid_dev = dev->parent;
struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev);
stbtt_fontinfo *font = >font;
- int ascent;
+ int advance, ascent, lsb;
debug("%s: start\n", __func__);
+
if (vid_priv->font_size)
priv->font_size = vid_priv->font_size;
else
priv->font_size = CONFIG_CONSOLE_TRUETYPE_SIZE;
+
priv->font_data = console_truetype_find_font();
if (!priv->font_data) {
debug("%s: Could not find any fonts\n", __func__);
return -EBFONT;
}
- vc_priv->x_charsize = priv->font_size;
vc_priv->y_charsize = priv->font_size;
vc_priv->xstart_frac = VID_TO_POS(2);
vc_priv->cols = vid_priv->xsize / priv->font_size;
@@ -618,6 +622,15 @@ static int console_truetype_probe(struct udevice *dev)
/* Pre-calculate some things we will need regularly */
priv->scale = stbtt_ScaleForPixelHeight(font, priv->font_size);
+
+ /* Assuming that 'W' is the widest character */
+ stbtt_GetCodepointHMetrics(font, 'W', , );
+ advance += stbtt_GetCodepointKernAdvance(font, 'W', 'W');
+ vc_priv->cols =
+ (int)VID_TO_POS(vid_priv->xsize - 2) /
+ (int)VID_TO_POS(advance * priv->scale);
+ vc_priv->x_charsize = advance * priv->scale;
+
stbtt_GetFontVMetrics(font, , 0, 0);
priv->baseline = (int)(ascent * priv->scale);
debug("%s: ready\n", __func__);
diff --git a/include/video_console.h b/include/video_console.h
index 8747299d61..cf306e5ca2 100644
--- a/include/video_console.h
+++ b/include/video_console.h
@@ -68,7 +68,7 @@ struct vidconsole_priv {
int ycur;
int rows;
int cols;
- int x_charsize;
+ double x_charsize;
int y_charsize;
int tab_width_frac;
int xsize_frac;
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 09:24:25AM -0600, Simon Glass wrote: > Hi Mark, > > On Wed, 27 Oct 2021 at 09:11, Mark Kettenis wrote: > > > > > From: François Ozog > > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > > > Hi, > > > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > > Hi all, > > > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > > Hi Tom, > > > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini wrote: > > > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > > > Hi François, > > > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass > > > > > > > > > > > > > > a écrit : > > > > > > > > > >> > > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > > >> > > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini > > > > wrote: > > > > > > > > > >> > > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng wrote: > > > > > > > > > >> > > Hi Simon, > > > > > > > > > >> > > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > > s...@chromium.org> wrote: > > > > > > > > > >> > > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE > > > > > > > > > >> > > > and > > > > OF_HOSTFILE so > > > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > > > >> > > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the > > > > > > > > > >> > > > devicetree > > > > is built and > > > > > > > > > >> > > > appended to U-Boot > > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > > devicetree is embedded in > > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > > > >> > > > > > > > > > > > > >> > > > The last one is currently set up so that no > > > > > > > > > >> > > > devicetree > > > > is needed at all > > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > > > some don't. Some > > > > > > > > > >> > > > don't even provide instructions on how to boot on the > > > > board. > > > > > > > > > >> > > > > > > > > > > > > >> > > > The problems with this approach are documented at > > > > > > > > > >> > > > [1]. > > > > > > > > > >> > > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > > > OF_SEPARATE. Any board > > > > > > > > > >> > > > can obtain its devicetree at runtime, even it is has > > > > > > > > > >> > > > a > > > > devicetree built > > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a > > > > > > > > > >> > > > second-stage > > > > bootloader and its > > > > > > > > > >> > > > caller may have a better idea about the hardware > > > > available in the machine. > > > > > > > > > >> > > > This is the case with a few QEMU boards, for example. > > > > > > > > > >> > > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a 'choice'. > > > > > > > > > >> > > > It > > > > should be an > > > > > > > > > >> > > > option, available with either OF_SEPARATE or > > > > > > > > > >> > > > OF_EMBED. > > > > > > > > > >> > > > > > > > > > > > > >> > > > This series makes this change, adding various missing > > > > devicetree files > > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > > >> > > > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > > > hack to me. > > > > > > > > > >> > > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the > > > > > > > > > >> > > fly > > > > based on > > > > > > > > > >> > > command line parameters, and the device tree you put in > > > > this series > > > > > > > > > >> > > has various hardcoded values which normally > > > > > > > > > >> > > do > > > > not show up > > > > > > > > > >> > > in hand-written dts files. > > > > > > > > > >> > > > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > > > >> > > > > > > > > > > >> > I am also confused and do not like the idea of adding > > > > device trees for > > > > > > > > > >> > platforms that are capable of and can / do have a device > > > > tree to give us > > > > > > > > > >> > at run time. > > > > > > > > > >> > > > > > > > > > >> (I'll just reply to this one email, since the same points > > > > applies to > > > > > > > > > >> all replies I think) > > > > > > > > > >> > > > > > > > > > >> I have been thinking about this and discussing it with > > > > > > > > > >> people > > > > for a > > > > > > > > > >> few months now. I've
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 05:02:39PM +0200, Heinrich Schuchardt wrote: > On 10/27/21 16:55, Tom Rini wrote: > > On Wed, Oct 27, 2021 at 03:23:01PM +0200, Heinrich Schuchardt wrote: > > > > [snip] > > > One passed to U-Boot for fixups and further passed to the OS. This > > > devicetree may originate from a prior boot stage, from a file loaded by > > > U-Boot, or from a later bootstage, e.g systemd-boot's devicetree command. > > > > I assume systemd-boot is implementing the same logic that extlinux.conf > > has used for forever, yes? > > It is loading the file and then calls U-Boot's implementation of the EFI > Device Tree Fixup Protocol for fixups before passing the device-tree to the > OS. So it's using https://systemd.io/BOOT_LOADER_SPECIFICATION/ OK. > > > This devicetree will not contain any U-Boot specific information. > > > > To repeat, it must only have official bindings, yes, regardless of what > > project they come from. > > > > Don't expect prior firmware stages to provide any U-Boot specific stuff > whatever official or non-official U-Boot specific bindings exist. Failure to implement official bindings may result in failure to boot, which would be pretty silly. -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands
On Wed, 27 Oct 2021 10:58:35 -0700 Roman Bacik wrote: > Thank you very much for your suggestion. To simplify, we will decouple > this issue from the current bnxt driver submission. Very well, that seems reasonable. I am willing to help you with this afterwards. Marek
[PATCH v4 2/2] arm: mvebu: add Globalscale MOCHAbin support
Globalscale MOCHAbin is a Armada 7040 based development board.
Specifications:
* Armada 7040 Quad core ARMv8 Cortex A-72 @ 1.4GHz
* 2 / 4 / 8 GB of DDR4 DRAM
* 16 GB eMMC
* 4MB SPI-NOR (Bootloader)
* 1x M.2-2280 B-key socket (for SSD expansion, SATA3 only)
* 1x M.2-2250 B-key socket (for modems, USB2.0 and I2C only)
* 1x Mini-PCIe 3.0 (x1, USB2.0 and I2C)
* 1x SATA 7+15 socket (SATA3)
* 1x 16-pin (2×8) MikroBus Connector
* 1x SIM card slot (Connected to the mini-PCIe and both M.2 slots)
* 2x USB3.0 Type-A ports via SMSC USB5434B hub
* Cortex 2x5 JTAG
* microUSB port for UART (PL2303GL/PL2303SA onboard)
* 1x 10G SFP+
* 1x 1G SFP (Connected to 88E1512 PHY)
* 1x 1G RJ45 with PoE PD (Connected to 88E1512 PHY)
* 4x 1G RJ45 ports via Topaz 88E6141 switch
* RTC with battery holder (SoC provided, requires CR2032 battery)
* 1x 12V DC IN
* 1x Power switch
* 1x 12V fan header (3-pin, power only)
* 1x mini-PCIe LED header (2x0.1" pins)
* 1x M.2-2280 LED header (2x0.1" pins)
* 6x Bootstrap jumpers
* 1x Power LED (Green)
* 3x Tri-color RGB LEDs (Controllable)
* 1x Microchip ATECC608B secure element
Note that 1G SFP does not work in U-boot as it would require Linux like
SFP support for parsing module interface and reconfiguring the PHY.
Additionally, automatic import of the Marvell hw_info parameters is
enabled via the recently added mac command for Armada platforms.
The parameters stored in Marvell hw_info are usually the board serial
number and MAC addresses.
Signed-off-by: Robert Marko
---
Changes in v3:
* Rename "u-boot" partition to "firmware" which better describes its content
* Remove read-only flag from "hw-info" partition
Changes in v2:
* Add DT compatible for hw-info to its partition
* Enable MTD support, command and SPI MTD command
---
arch/arm/dts/Makefile| 1 +
arch/arm/dts/armada-7040-mochabin.dts| 304 +++
configs/mvebu_mochabin-88f7040_defconfig | 88 +++
3 files changed, 393 insertions(+)
create mode 100644 arch/arm/dts/armada-7040-mochabin.dts
create mode 100644 configs/mvebu_mochabin-88f7040_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index a88aecc5bd9..dea567d2595 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -232,6 +232,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
armada-38x-controlcenterdc.dtb \
armada-7040-db-nand.dtb \
armada-7040-db.dtb \
+ armada-7040-mochabin.dtb\
armada-8040-clearfog-gt-8k.dtb \
armada-8040-db.dtb \
armada-8040-mcbin.dtb \
diff --git a/arch/arm/dts/armada-7040-mochabin.dts
b/arch/arm/dts/armada-7040-mochabin.dts
new file mode 100644
index 000..84f06363728
--- /dev/null
+++ b/arch/arm/dts/armada-7040-mochabin.dts
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2016- 2021 Marvell International Ltd.
+ * Copyright (C) 2021 Sartura Ltd.
+ */
+
+/*
+ * Device Tree file for Globalscale MOCHAbin
+ * Boot device: SPI NOR, 0x32 (SW3)
+ */
+
+#include "armada-7040.dtsi"
+
+/ {
+ model = "Globalscale MOCHAbin";
+ compatible = "globalscale,mochabin", "marvell,armada7040",
+"marvell,armada-ap806-quad", "marvell,armada-ap806";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ aliases {
+ i2c0 = _i2c0;
+ spi0 = _spi1;
+ };
+
+ memory@ {
+ device_type = "memory";
+ reg = <0x0 0x0 0x0 0x8000>;
+ };
+};
+
+_pinctl {
+ /* MPP Bus:
+ * SDIO [0-5]
+ * UART0 [11,19]
+ */
+ /* 0 1 2 3 4 5 6 7 8 9 */
+ pin-func = < 1 1 1 1 1 1 0 0 0 0
+0 3 0 0 0 0 0 0 0 3 >;
+};
+
+/* microUSB UART console */
+ {
+ status = "okay";
+};
+
+/* eMMC */
+_sdhci0 {
+ status = "okay";
+
+ bus-width = <4>;
+ no-1-8-v;
+ non-removable;
+};
+
+_pinctl {
+ /* MPP Bus:
+* E6341_RSTn[0]
+* E6341_INT [1]
+* I2C1 [2,3]
+* PCIE_CLKREQn [5]
+* UART0 [6,7]
+* PCIE_WAKEn[8]
+* PCIE_RSTOUTn [9] (GPIO)
+* DEVSLP_M2-2 [10]
+* PHY0_RSTn [12] (88E1512 PHY RST)
+* SPI1 [13-16]
+* PHY0_INT [24] (88E1512 PHY INT)
+* 9554_INT [27] (PCA9554 INT)
+* IS_CLK_VBM[29] (is31fl3199-clk-vbm)
+* IS_SDB[30] (is31fl3199-sdb)
+* MKR_PWM [31] (mikroBus PWM)
+* MKR_INT [32] (mikroBus INT)
+* MKR_RST [33] (mikroBus RST)
+
[PATCH v4 1/2] arm: mvebu: Implement the mac command (Marvell hw_info)
From: Luka Kovacic
The mac command is implemented to enable parsing Marvell hw_info formatted
environments. This format is often used on Marvell Armada devices to store
parameters like the board serial number, factory MAC addresses and some
other information.
These parameters are usually written to the flash in the factory.
Currently the mac command supports reading/writing parameters and dumping
the current hw_info parameters.
EEPROM config pattern and checksum aren't supported.
This functionality has been tested on the GST ESPRESSOBin-Ultra board
successfully, both reading the stock U-Boot parameters in mainline U-Boot
and reading the parameters written by this command in the stock U-Boot.
Support for this command is added for Marvell Armada A37XX and 7K/8K
devices.
Usage example:
=> mac read
=> saveenv
Signed-off-by: Luka Kovacic
Cc: Luka Perkov
Cc: Robert Marko
Acked-by: Pali Rohár
---
Changes in v4:
* Add forgotten ACK from Pali
Changes in v2:
* Use DTS to find the hw_info partition offset instead of using KConfig
---
arch/arm/mach-mvebu/Kconfig | 1 +
board/Marvell/common/Kconfig| 20 ++
board/Marvell/common/Makefile | 5 +
board/Marvell/common/mac.c | 418
include/configs/mvebu_armada-37xx.h | 7 +
include/configs/mvebu_armada-8k.h | 7 +
lib/hashtable.c | 2 +-
7 files changed, 459 insertions(+), 1 deletion(-)
create mode 100644 board/Marvell/common/Kconfig
create mode 100644 board/Marvell/common/Makefile
create mode 100644 board/Marvell/common/mac.c
diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index d23cc0c760f..39a91dd41c9 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -339,6 +339,7 @@ config SECURED_MODE_CSK_INDEX
default 0
depends on SECURED_MODE_IMAGE
+source "board/Marvell/common/Kconfig"
source "board/solidrun/clearfog/Kconfig"
source "board/kobol/helios4/Kconfig"
diff --git a/board/Marvell/common/Kconfig b/board/Marvell/common/Kconfig
new file mode 100644
index 000..9887e25fb1f
--- /dev/null
+++ b/board/Marvell/common/Kconfig
@@ -0,0 +1,20 @@
+menu "Marvell Armada common configuration"
+depends on TARGET_MVEBU_ARMADA_37XX || TARGET_MVEBU_ARMADA_8K
+
+config MVEBU_MAC_HW_INFO
+ bool "Marvell hw_info (mac) support"
+ depends on SPI_FLASH && ENV_IS_IN_SPI_FLASH && ARCH_MVEBU
+ default n
+ help
+ Enable loading of the Marvell hw_info parameters from the
+ SPI flash hw_info area. Parameters (usually the board serial
+ number and MAC addresses) are then imported into the
+ existing U-Boot environment.
+ Implementation of this command is compatible with the
+ original Marvell U-Boot command. Reading and writing is
+ supported.
+ EEPROM config pattern and checksum aren't supported.
+ After enabled, these parameters are managed from the common
+ U-Boot mac command.
+
+endmenu
diff --git a/board/Marvell/common/Makefile b/board/Marvell/common/Makefile
new file mode 100644
index 000..072c3e49de7
--- /dev/null
+++ b/board/Marvell/common/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2021 Sartura Ltd.
+
+obj-$(CONFIG_ID_EEPROM) += mac.o
diff --git a/board/Marvell/common/mac.c b/board/Marvell/common/mac.c
new file mode 100644
index 000..590c44c882e
--- /dev/null
+++ b/board/Marvell/common/mac.c
@@ -0,0 +1,418 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Marvell hw_info (mac) command implementation
+ * Helper command for interfacing with the Marvell hw_info parameters
+ *
+ * Copyright (c) 2021 Sartura Ltd.
+ * Copyright (c) 2018 Marvell International Ltd.
+ *
+ * Author: Luka Kovacic
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define HW_INFO_MAX_ENV_SIZE 0x1F0
+#define HW_INFO_ENV_OFFSET 0xA
+#define HW_INFO_ENV_SEP0x20
+
+#define HW_INFO_MAX_NAME_LEN 32
+
+#define HW_INFO_MERGED_VARIABLE"read_board_hw_info"
+
+static char hw_info_allowed_parameters[][HW_INFO_MAX_NAME_LEN] = {
+ "pcb_slm",
+ "pcb_rev",
+ "eco_rev",
+ "pcb_sn",
+ "ethaddr",
+ "eth1addr",
+ "eth2addr",
+ "eth3addr",
+ "eth4addr",
+ "eth5addr",
+ "eth6addr",
+ "eth7addr",
+ "eth8addr",
+ "eth9addr",
+};
+
+static int hw_info_allowed_param_count = (sizeof(hw_info_allowed_parameters) /
+ sizeof(hw_info_allowed_parameters[0]));
+
+static int hw_info_check_parameter(char *name)
+{
+ int idx;
+
+ for (idx = 0; idx < hw_info_allowed_param_count; idx++) {
+ if (strcmp(name, hw_info_allowed_parameters[idx]) == 0)
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+/**
+ * read_spi_flash_offset() - Read data from the SPI
RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands
Hi Marek, > -Original Message- > From: Marek Behún > Sent: Wednesday, October 27, 2021 10:40 AM > To: Roman Bacik > Cc: Simon Glass ; U-Boot Mailing List b...@lists.denx.de>; Bharat Gooty ; > Aswath Govindraju ; Bin Meng > ; Franck LENORMAND > ; Heinrich Schuchardt > ; Kory Maincent ; > Michal Simek ; Patrick Delaunay > ; Peng Fan ; Priyanka > Jain ; Rayagonda Kokatanur > ; Sean Anderson > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > On Wed, 27 Oct 2021 10:02:41 -0700 > Roman Bacik wrote: > > > Marek, Simon, > > > > Thank you very much for your comments. We will remove bnxt commands > and will > > probe bnxt driver each boot in the next version. > > Thanks, > > Roman > > I think that the idea of not loading fw or initializing the controller > during every boot, but only when needed, is quite reasonable. > > But it has to be done without the need to call custom commands, which > the user may not know about. > > It has to be done in such a way that if the user calls for example the > dhcp > command, it will work. > > I think this could be achieved by creating a new DM driver flag, and > wiring the DM and/or PCI code so that when this flag is set, the PCI > subsystem won't probe the driver, only bind the driver. Thank you very much for your suggestion. To simplify, we will decouple this issue from the current bnxt driver submission. > > That way U-Boot will know that there is another ethernet controller > which can be used by network commands when the `ethact` variable is set > to point to that controller. > > Marek Thanks, Roman -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands
On Wed, 27 Oct 2021 10:02:41 -0700 Roman Bacik wrote: > Marek, Simon, > > Thank you very much for your comments. We will remove bnxt commands and will > probe bnxt driver each boot in the next version. > Thanks, Roman I think that the idea of not loading fw or initializing the controller during every boot, but only when needed, is quite reasonable. But it has to be done without the need to call custom commands, which the user may not know about. It has to be done in such a way that if the user calls for example the dhcp command, it will work. I think this could be achieved by creating a new DM driver flag, and wiring the DM and/or PCI code so that when this flag is set, the PCI subsystem won't probe the driver, only bind the driver. That way U-Boot will know that there is another ethernet controller which can be used by network commands when the `ethact` variable is set to point to that controller. Marek
Re: [PATCH v4 1/4] tools: Separate image types which depend on OpenSSL
On Fri, Oct 22, 2021 at 12:46:59PM -0700, Vagrant Cascadian wrote: > On 2021-10-22, Andre Przywara wrote: > > On Fri, 22 Oct 2021 09:47:35 -0700 > > Vagrant Cascadian wrote: > >> On 2021-10-22, Tom Rini wrote: > >> > On Fri, Oct 22, 2021 at 04:56:09PM +0100, Andre Przywara wrote: > >> >> On Fri, 22 Oct 2021 11:09:27 -0400 > >> >> Tom Rini wrote: > >> > >> >> > On Fri, Oct 22, 2021 at 04:59:22PM +0200, Marek Behún wrote: > >> >> > > On Fri, 22 Oct 2021 12:09:19 +0200 > >> >> > > Heinrich Schuchardt wrote: > >> >> > > > >> >> > > > On 10/21/21 15:00, Marek Behún wrote: > >> >> > > > > BTW, wouldn't it be enough to simply imply TOOLS_LIBCRYPTO for > >> >> > > > > mvebu > >> >> > > > > platform in Kconfig? > >> >> > > > > > >> >> > > > > >> >> > > > We should only use 'imply' for suggested settings and never for > >> >> > > > hard > >> >> > > > requirements. TOOLS_LIBCRYPTO already defaults to 'Y'. So > >> >> > > > implying it > >> >> > > > for mvebu would be redundant. > >> >> > > > > >> >> > > > In an OS distribution we only want to ship a single version of > >> >> > > > mkimage. > >> >> > > > So it is good to elimate symbol CONFIG_MXS. > >> >> > > > > >> >> > > > How mkimage is built should not depend on CONFIG_TOOLS_LIBCRYPTO. > >> >> > > > > >> >> > > > Tom wrote regarding this aspect in > >> >> > > > https://lists.denx.de/pipermail/u-boot/2021-September/460251.html: > >> >> > > > > >> >> > > > "if we're building a generically useful tool, we don't want > >> >> > > > another > >> >> > > > symbol for it." > >> >> > > > >> >> > > OK, so mkimage and dumpimage should be always generic and always > >> >> > > support all platforms, that makes sense, since the tools can be > >> >> > > installed as a distribution package. > >> >> > > > >> >> > > But I still think it should be possible to cripple these tools if > >> >> > > the > >> >> > > developer wants to disable libcrypto due to embedded environment. > >> >> > > > >> >> > >> >> Well, I don't think this is the real question here, is it? > >> >> I think the tools part is clear: distros want to build just mkimage, > >> >> supporting as many platforms as possible, and might need to avoid > >> >> OpenSSL. > >> >> This should be covered by TOOLS_LIBCRYPTO=[yn] and "make > >> >> tools-only_defconfg && make tools", and Samuel's patch actually fixes > >> >> the > >> >> build (at least somewhat, I still get link errors). > >> > > >> > The problem is, are distros doing a tools-only build, for tools, or are > >> > they doing it per board? Like, hey, ugh, OpenEmbedded uses > >> > sandbox_defconfig and cross_tools as the targets. That's not quite what > >> > I was hoping to see. So I want to know everyone else is doing, rather > >> > than we hope they're doing. > >> > >> Thanks for bringing this to my attention! > >> > >> In Debian, the u-boot-tools package is built using tools-only, and for > >> each of the board-specific targets, it still ends up building the > >> relevent tools, but we throw them away and do not ship them in any > >> packages. > >> > >> With 2021.10, the board-specific builds made it harder to avoid openssl > >> with the corresponding tools, and I reluctantly added a dependency on > >> openssl... (which is technically permitted in Debian, having declared > >> openssl as a system library to avoid the GPL incompatibilities, but > >> ... meh.) > > > > But this is purely a *build-time* dependency only, right? The resulting > > images do not have any openssl code in them, they were just *created* > > (signed) using that code. > > I don't think this a legal issue? > > The various .h includes are all that I saw, and I *think* all in the > tools/ directory, but yeah, if this is really the case that no openssl > code ends up in the board-specific binaries, that simplifies things > considerably. > > > > The problems are about *shipping* openssl code, which you only do for > > u-boot-tools - where it now can be disabled. > > Probably won't disable it for u-boot-tools in Debian (reluctantly riding > on the system library exception), but the tools builds that are part of > the build process would be nice to be able to disable. > > > > >> I also have been doing some packaging of u-boot for GNU Guix, where I > >> suspect the stance wouldn't be as willing to accept such a compromise... > >> > >> So... I would *love* an option to be able to build a board-only config > >> without any of the tools; > > > > Why is this a problem (see above)? Who is building board builds? It's > > either the maintainer when creating the binary package, or a curious user, > > right? And they can surely *use* OpenSSL during build time - if it's > > needed by the board. > > Sure, if there is no actual openssl code embedded in the resulting > binary with GPLv2 code, it shouldn't be a problem... > > > It's a mess of an issue to tease out exactly what codepaths trigger and > do not trigger the compatibility issues between
RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands
Marek, Simon, Thank you very much for your comments. We will remove bnxt commands and will probe bnxt driver each boot in the next version. Thanks, Roman > -Original Message- > From: Roman Bacik > Sent: Wednesday, October 27, 2021 9:47 AM > To: Marek Behún > Cc: Simon Glass ; U-Boot Mailing List b...@lists.denx.de>; Bharat Gooty ; > Aswath Govindraju ; Bin Meng > ; Franck LENORMAND > ; Heinrich Schuchardt > ; Kory Maincent ; > Michal Simek ; Patrick Delaunay > ; Peng Fan ; Priyanka > Jain ; Rayagonda Kokatanur > ; Sean Anderson > > Subject: RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > Hi Marek > > > -Original Message- > > From: Marek Behún > > Sent: Wednesday, October 27, 2021 9:36 AM > > To: Roman Bacik > > Cc: Simon Glass ; U-Boot Mailing List > b...@lists.denx.de>; Bharat Gooty ; > > Aswath Govindraju ; Bin Meng > > ; Franck LENORMAND > > ; Heinrich Schuchardt > > ; Kory Maincent ; > > Michal Simek ; Patrick Delaunay > > ; Peng Fan ; > Priyanka > > Jain ; Rayagonda Kokatanur > > ; Sean Anderson > > > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > > > On Wed, 27 Oct 2021 08:05:11 -0700 > > Roman Bacik wrote: > > > > > chimp_ld_secure #this command loads FW, which is necessary for PCIe to > > > enumerate it > > > pci enum #this command is necessary to call bnxt_bind > > > > Wait, so what is this firmware for? Is it firmware for the netXtreme > > controller or for PCIe controller itself? > > It is FW for netXtreme controller. > > > > > If it is for the ethernet controller, you should be able to determine > > whether there is netXtreme card present on the PCI bus without loading > > the firmware, by looking at PCI vendor / device ID. > > The current code fails to enumerate PCI if FW is not loaded. Loading FW > and trying to enumerate again still fails. Only reset helps after that. > > > > > In that case the firmware should be loaded when the ethernet controller > > is requested for, i.e. the ethact env variable points to that > > controller and a network command is executed (dhcp, tftpboot, ...). > > > > (The ethact variable is used to set which ethernet controller is used > > for network commands when there are multiple etherent controllers.) > > Thanks, > > Roman -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
[PATCH 2/2] Revert "arch: arm: use dt and UCLASS_SYSCON to get gic lpi details"
Stop using the device tree as a source for ad-hoc information.
This reverts commit 2ae7adc659f7fca9ea65df4318e5bca2b8274310.
Signed-off-by: Michael Walle
---
arch/arm/Kconfig| 2 -
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 27 +-
arch/arm/include/asm/gic-v3.h | 4 +-
arch/arm/lib/gic-v3-its.c | 66 +++--
4 files changed, 36 insertions(+), 63 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 02f8306f15..86c1ebde05 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -82,8 +82,6 @@ config GICV3
config GIC_V3_ITS
bool "ARM GICV3 ITS"
- select REGMAP
- select SYSCON
select IRQ
help
ARM GICV3 Interrupt translation service (ITS).
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index c0e100d21c..a08ed3f544 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -41,11 +41,36 @@ DECLARE_GLOBAL_DATA_PTR;
#endif
#ifdef CONFIG_GIC_V3_ITS
+#define PENDTABLE_MAX_SZ ALIGN(BIT(ITS_MAX_LPI_NRBITS), SZ_64K)
+#define PROPTABLE_MAX_SZ ALIGN(BIT(ITS_MAX_LPI_NRBITS) / 8, SZ_64K)
+#define GIC_LPI_SIZE ALIGN(cpu_numcores() * PENDTABLE_MAX_SZ + \
+ PROPTABLE_MAX_SZ, SZ_1M)
+static int fdt_add_resv_mem_gic_rd_tables(void *blob, u64 base, size_t size)
+{
+ int err;
+ struct fdt_memory gic_rd_tables;
+
+ gic_rd_tables.start = base;
+ gic_rd_tables.end = base + size - 1;
+ err = fdtdec_add_reserved_memory(blob, "gic-rd-tables", _rd_tables,
+NULL, 0, NULL, 0);
+ if (err < 0)
+ debug("%s: failed to add reserved memory: %d\n", __func__, err);
+
+ return err;
+}
+
int ls_gic_rd_tables_init(void *blob)
{
+ u64 gic_lpi_base;
int ret;
- ret = gic_lpi_tables_init();
+ gic_lpi_base = ALIGN(gd->arch.resv_ram - GIC_LPI_SIZE, SZ_64K);
+ ret = fdt_add_resv_mem_gic_rd_tables(blob, gic_lpi_base, GIC_LPI_SIZE);
+ if (ret)
+ return ret;
+
+ ret = gic_lpi_tables_init(gic_lpi_base, cpu_numcores());
if (ret)
debug("%s: failed to init gic-lpi-tables\n", __func__);
diff --git a/arch/arm/include/asm/gic-v3.h b/arch/arm/include/asm/gic-v3.h
index 35efec78c3..5131fabec4 100644
--- a/arch/arm/include/asm/gic-v3.h
+++ b/arch/arm/include/asm/gic-v3.h
@@ -127,9 +127,9 @@
#define GIC_REDISTRIBUTOR_OFFSET 0x2
#ifdef CONFIG_GIC_V3_ITS
-int gic_lpi_tables_init(void);
+int gic_lpi_tables_init(u64 base, u32 max_redist);
#else
-int gic_lpi_tables_init(void)
+int gic_lpi_tables_init(u64 base, u32 max_redist)
{
return 0;
}
diff --git a/arch/arm/lib/gic-v3-its.c b/arch/arm/lib/gic-v3-its.c
index 2d3fdb600e..f6211a2d92 100644
--- a/arch/arm/lib/gic-v3-its.c
+++ b/arch/arm/lib/gic-v3-its.c
@@ -5,8 +5,6 @@
#include
#include
#include
-#include
-#include
#include
#include
#include
@@ -19,22 +17,15 @@ static u32 lpi_id_bits;
#define LPI_PROPBASE_SZALIGN(BIT(LPI_NRBITS), SZ_64K)
#define LPI_PENDBASE_SZALIGN(BIT(LPI_NRBITS) / 8, SZ_64K)
-/* Number of GIC re-distributors */
-#define MAX_GIC_REDISTRIBUTORS 8
-
/*
* gic_v3_its_priv - gic details
*
* @gicd_base: gicd base address
* @gicr_base: gicr base address
- * @lpi_base: gic lpi base address
- * @num_redist: number of gic re-distributors
*/
struct gic_v3_its_priv {
ulong gicd_base;
ulong gicr_base;
- ulong lpi_base;
- u32 num_redist;
};
static int gic_v3_its_get_gic_addr(struct gic_v3_its_priv *priv)
@@ -68,39 +59,13 @@ static int gic_v3_its_get_gic_addr(struct gic_v3_its_priv
*priv)
return 0;
}
-static int gic_v3_its_get_gic_lpi_addr(struct gic_v3_its_priv *priv)
-{
- struct regmap *regmap;
- struct udevice *dev;
- int ret;
-
- ret = uclass_get_device_by_driver(UCLASS_SYSCON,
- DM_DRIVER_GET(gic_lpi_syscon), );
- if (ret) {
- pr_err("%s: failed to get %s syscon device\n", __func__,
- DM_DRIVER_GET(gic_lpi_syscon)->name);
- return ret;
- }
-
- regmap = syscon_get_regmap(dev);
- if (!regmap) {
- pr_err("%s: failed to regmap for %s syscon device\n", __func__,
- DM_DRIVER_GET(gic_lpi_syscon)->name);
- return -ENODEV;
- }
- priv->lpi_base = regmap->ranges[0].start;
-
- priv->num_redist = dev_read_u32_default(dev, "max-gic-redistributors",
- MAX_GIC_REDISTRIBUTORS);
-
- return 0;
-}
-
/*
* Program the GIC LPI configuration tables for all
* the re-distributors and enable the LPI table
+ * base: Configuration table address
+ * num_redist: number of redistributors
*/
-int
[PATCH 1/2] Revert "arm64: Layerscape: Survive LPI one-way reset workaround"
From: Tom Rini
Ad-hoc bindings that are not part of the upstream device tree / bindings
are not allowed in-tree. Only bindings that are in-progress with
upstream and then re-synced once agreed upon are.
This reverts commit af288cb291da3abef6be0875527729296f7de7a0.
Cc: Hou Zhiqiang
Cc: Priyanka Jain
Reported-by: Michael Walle
Signed-off-by: Tom Rini
---
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 18 +-
arch/arm/dts/fsl-ls1028a.dtsi | 6 --
arch/arm/dts/fsl-ls1088a.dtsi | 6 --
arch/arm/dts/fsl-ls2080a.dtsi | 6 --
arch/arm/dts/fsl-lx2160a.dtsi | 6 --
5 files changed, 1 insertion(+), 41 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
index 9820d3290e..c0e100d21c 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/soc.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/soc.c
@@ -43,23 +43,7 @@ DECLARE_GLOBAL_DATA_PTR;
#ifdef CONFIG_GIC_V3_ITS
int ls_gic_rd_tables_init(void *blob)
{
- struct fdt_memory lpi_base;
- fdt_addr_t addr;
- fdt_size_t size;
- int offset, ret;
-
- offset = fdt_path_offset(gd->fdt_blob, "/syscon@0x8000");
- addr = fdtdec_get_addr_size_auto_noparent(gd->fdt_blob, offset, "reg",
- 0, , false);
-
- lpi_base.start = addr;
- lpi_base.end = addr + size - 1;
- ret = fdtdec_add_reserved_memory(blob, "lpi_rd_table", _base, NULL,
-0, NULL, 0);
- if (ret) {
- debug("%s: failed to add reserved memory\n", __func__);
- return ret;
- }
+ int ret;
ret = gic_lpi_tables_init();
if (ret)
diff --git a/arch/arm/dts/fsl-ls1028a.dtsi b/arch/arm/dts/fsl-ls1028a.dtsi
index 50f9b527cd..53b052ed32 100644
--- a/arch/arm/dts/fsl-ls1028a.dtsi
+++ b/arch/arm/dts/fsl-ls1028a.dtsi
@@ -44,12 +44,6 @@
IRQ_TYPE_LEVEL_LOW)>;
};
- gic_lpi_base: syscon@0x8000 {
- compatible = "gic-lpi-base";
- reg = <0x0 0x8000 0x0 0x10>;
- max-gic-redistributors = <2>;
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = ;
};
- gic_lpi_base: syscon@0x8000 {
- compatible = "gic-lpi-base";
- reg = <0x0 0x8000 0x0 0x10>;
- max-gic-redistributors = <8>;
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = <1 13 0x8>, /* Physical Secure PPI, active-low */
diff --git a/arch/arm/dts/fsl-ls2080a.dtsi b/arch/arm/dts/fsl-ls2080a.dtsi
index 7374d580e0..278daeeb6e 100644
--- a/arch/arm/dts/fsl-ls2080a.dtsi
+++ b/arch/arm/dts/fsl-ls2080a.dtsi
@@ -27,12 +27,6 @@
interrupts = <1 9 0x4>;
};
- gic_lpi_base: syscon@0x8000 {
- compatible = "gic-lpi-base";
- reg = <0x0 0x8000 0x0 0x10>;
- max-gic-redistributors = <8>;
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = <1 13 0x8>, /* Physical Secure PPI, active-low */
diff --git a/arch/arm/dts/fsl-lx2160a.dtsi b/arch/arm/dts/fsl-lx2160a.dtsi
index a6f0e9bc56..3b5f0d119e 100644
--- a/arch/arm/dts/fsl-lx2160a.dtsi
+++ b/arch/arm/dts/fsl-lx2160a.dtsi
@@ -43,12 +43,6 @@
interrupts = <1 9 0x4>;
};
- gic_lpi_base: syscon@0x8000 {
- compatible = "gic-lpi-base";
- reg = <0x0 0x8000 0x0 0x20>;
- max-gic-redistributors = <16>;
- };
-
timer {
compatible = "arm,armv8-timer";
interrupts = <1 13 0x8>, /* Physical Secure PPI, active-low */
--
2.30.2
[PATCH 0/2] arch: arm: gic-v3-its: stop abusing the device tree
Please stop throwing every ad-hoc information in the device tree. Use the
official bindings (or maybe some bindings which will get approved soon).
On the quest of syncing the device tree used in u-boot with the one used in
linux, there is this nice piece:
gic_lpi_base: syscon@0x8000 {
compatible = "gic-lpi-base";
reg = <0x0 0x8000 0x0 0x10>;
max-gic-redistributors = <2>;
};
There is no offical binding for it. Also, the chances that there will be
one are virtually zero. We need to get rid of it. In fact, most information
there are already known or can be deduced via the offical binding.
More than a month ago NXP [1] said they will look into it and try to get
the bindings together. I don't think this will happen. Actually, I don't
think the culprit is that commit, but rather the one which introduced that
broken binding in the first place [2]. Therefore, revert it, too.
The funny thing is, I don't even know why this is needed at all. Linux will
happily setup the LPI for us. At least for the LS1028A (but I guess also
for all other layerscape SoC) the u-boot LPI setup code is only called
right before we jump to linux. So u-boot doesn't even seem to use the
interrupts. Now I can't speak of the Broadcom NS3 SoC where this 'feature'
was introduced in the first place [3]. Unfortunately, it's not mentioned in
the commit log *why* it was introduced. But this also seem to have crept
into the layerscape SoCs [4]. All layerscape boards have CONFIG_GIC_V3_ITS
enabled, well except for one; mine, the kontron_sl28 (which has a LS1028A).
I haven't noticed anything out of the ordinary, though. Why is
CONFIG_GIC_V3_ITS needed?
I briefly tested this on my board with CONFIG_GIC_V3_ITS enabled, at least
linux prints:
[0.00] GICv3: Using preallocated redistributor tables
and there will be a reserved memory area:
reserved-memory {
#address-cells = <0x02>;
#size-cells = <0x02>;
ranges;
gic-rd-tables@20,fff0 {
reg = <0x20 0xfff0 0x00 0x10>;
};
};
Also please note, that the reverts needed some manual adjustments because
there were changes in the meantime.
While this will hopefully not break the layerscape boards, it will
definetly break the Broadcom NS3, because the gic_lpi_tables_init() is
called without arguments. But, these information are also not available in
u-boot't device tree for the ns3. S..
[1] https://lore.kernel.org/all/20210825210510.24766-1-tr...@konsulko.com/
[2]
https://lore.kernel.org/u-boot/20200726170733.30214-1-rayagonda.kokata...@broadcom.com/
[3]
https://lore.kernel.org/u-boot/20200610104120.30668-10-rayagonda.kokata...@broadcom.com/
[4] https://lore.kernel.org/u-boot/20200428021935.27659-1-zhiqiang@nxp.com/
Michael Walle (1):
Revert "arch: arm: use dt and UCLASS_SYSCON to get gic lpi details"
Tom Rini (1):
Revert "arm64: Layerscape: Survive LPI one-way reset workaround"
arch/arm/Kconfig| 2 -
arch/arm/cpu/armv8/fsl-layerscape/soc.c | 41 +--
arch/arm/dts/fsl-ls1028a.dtsi | 6 ---
arch/arm/dts/fsl-ls1088a.dtsi | 6 ---
arch/arm/dts/fsl-ls2080a.dtsi | 6 ---
arch/arm/dts/fsl-lx2160a.dtsi | 6 ---
arch/arm/include/asm/gic-v3.h | 4 +-
arch/arm/lib/gic-v3-its.c | 66 +++--
8 files changed, 35 insertions(+), 102 deletions(-)
--
2.30.2
RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands
Hi Marek > -Original Message- > From: Marek Behún > Sent: Wednesday, October 27, 2021 9:36 AM > To: Roman Bacik > Cc: Simon Glass ; U-Boot Mailing List b...@lists.denx.de>; Bharat Gooty ; > Aswath Govindraju ; Bin Meng > ; Franck LENORMAND > ; Heinrich Schuchardt > ; Kory Maincent ; > Michal Simek ; Patrick Delaunay > ; Peng Fan ; Priyanka > Jain ; Rayagonda Kokatanur > ; Sean Anderson > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > On Wed, 27 Oct 2021 08:05:11 -0700 > Roman Bacik wrote: > > > chimp_ld_secure #this command loads FW, which is necessary for PCIe to > > enumerate it > > pci enum #this command is necessary to call bnxt_bind > > Wait, so what is this firmware for? Is it firmware for the netXtreme > controller or for PCIe controller itself? It is FW for netXtreme controller. > > If it is for the ethernet controller, you should be able to determine > whether there is netXtreme card present on the PCI bus without loading > the firmware, by looking at PCI vendor / device ID. The current code fails to enumerate PCI if FW is not loaded. Loading FW and trying to enumerate again still fails. Only reset helps after that. > > In that case the firmware should be loaded when the ethernet controller > is requested for, i.e. the ethact env variable points to that > controller and a network command is executed (dhcp, tftpboot, ...). > > (The ethact variable is used to set which ethernet controller is used > for network commands when there are multiple etherent controllers.) Thanks, Roman -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands
On Wed, 27 Oct 2021 08:05:11 -0700 Roman Bacik wrote: > chimp_ld_secure #this command loads FW, which is necessary for PCIe to > enumerate it > pci enum #this command is necessary to call bnxt_bind Wait, so what is this firmware for? Is it firmware for the netXtreme controller or for PCIe controller itself? If it is for the ethernet controller, you should be able to determine whether there is netXtreme card present on the PCI bus without loading the firmware, by looking at PCI vendor / device ID. In that case the firmware should be loaded when the ethernet controller is requested for, i.e. the ethact env variable points to that controller and a network command is executed (dhcp, tftpboot, ...). (The ethact variable is used to set which ethernet controller is used for network commands when there are multiple etherent controllers.)
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
Hi Mark, On Wed, 27 Oct 2021 at 17:10, Mark Kettenis wrote: > > From: François Ozog > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > Hi, > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > Hi all, > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini > wrote: > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > > Hi François, > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass < > s...@chromium.org> > > > a écrit : > > > > > > > > >> > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > >> > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini < > tr...@konsulko.com> > > > wrote: > > > > > > > > >> > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng > wrote: > > > > > > > > >> > > Hi Simon, > > > > > > > > >> > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > s...@chromium.org> wrote: > > > > > > > > >> > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE > and > > > OF_HOSTFILE so > > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > > >> > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the > devicetree > > > is built and > > > > > > > > >> > > > appended to U-Boot > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > devicetree is embedded in > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > > >> > > > > > > > > > > > >> > > > The last one is currently set up so that no > devicetree > > > is needed at all > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > > some don't. Some > > > > > > > > >> > > > don't even provide instructions on how to boot on > the > > > board. > > > > > > > > >> > > > > > > > > > > > >> > > > The problems with this approach are documented at > [1]. > > > > > > > > >> > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > > OF_SEPARATE. Any board > > > > > > > > >> > > > can obtain its devicetree at runtime, even it is > has a > > > devicetree built > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a > second-stage > > > bootloader and its > > > > > > > > >> > > > caller may have a better idea about the hardware > > > available in the machine. > > > > > > > > >> > > > This is the case with a few QEMU boards, for > example. > > > > > > > > >> > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a > 'choice'. It > > > should be an > > > > > > > > >> > > > option, available with either OF_SEPARATE or > OF_EMBED. > > > > > > > > >> > > > > > > > > > > > >> > > > This series makes this change, adding various > missing > > > devicetree files > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > >> > > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > > hack to me. > > > > > > > > >> > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the > fly > > > based on > > > > > > > > >> > > command line parameters, and the device tree you put > in > > > this series > > > > > > > > >> > > has various hardcoded values which normally > do > > > not show up > > > > > > > > >> > > in hand-written dts files. > > > > > > > > >> > > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > > >> > > > > > > > > > >> > I am also confused and do not like the idea of adding > > > device trees for > > > > > > > > >> > platforms that are capable of and can / do have a device > > > tree to give us > > > > > > > > >> > at run time. > > > > > > > > >> > > > > > > > > >> (I'll just reply to this one email, since the same points > > > applies to > > > > > > > > >> all replies I think) > > > > > > > > >> > > > > > > > > >> I have been thinking about this and discussing it with > people > > > for a > > > > > > > > >> few months now. I've been signalling a change like this > for > > > over a > > > > > > > > >> month now, on U-Boot contributor calls and in discussions > > > with Linaro > > > > > > > > >> people. I sent a patch (below) to try to explain things. I > > > hope it is > > > > > > > > >> not a surprise! > > > > > > > > >> > > > > > > > > >> The issue here is that we need a devicetree in-tree in > > > U-Boot, to > > > > > > > > >> avoid the mess that has been created by OF_PRIOR_STAGE, > > > OF_BOARD, > > > > > > > > >> BINMAN_STANDALONE_FDT
RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands
Hi Simon, > -Original Message- > From: Simon Glass > Sent: Wednesday, October 27, 2021 8:41 AM > To: Roman Bacik > Cc: Marek Behún ; U-Boot Mailing List b...@lists.denx.de>; Bharat Gooty ; > Aswath Govindraju ; Bin Meng > ; Franck LENORMAND > ; Heinrich Schuchardt > ; Kory Maincent ; > Michal Simek ; Patrick Delaunay > ; Peng Fan ; Priyanka > Jain ; Rayagonda Kokatanur > ; Sean Anderson > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > Hi Roman, > > On Wed, 27 Oct 2021 at 09:05, Roman Bacik > wrote: > > > > > -Original Message- > > > From: Marek Behún > > > Sent: Tuesday, October 26, 2021 9:50 AM > > > To: Roman Bacik > > > Cc: Simon Glass ; U-Boot Mailing List > > b...@lists.denx.de>; Bharat Gooty ; > > > Aswath Govindraju ; Bin Meng > > > ; Franck LENORMAND > > > ; Heinrich Schuchardt > > > ; Kory Maincent ; > > > Michal Simek ; Patrick Delaunay > > > ; Peng Fan ; > Priyanka > > > Jain ; Rayagonda Kokatanur > > > ; Sean Anderson > > > > > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > > > > > On Tue, 26 Oct 2021 09:02:54 -0700 > > > Roman Bacik wrote: > > > > > > > On Tue, Oct 26, 2021 at 8:55 AM Marek Behún > wrote: > > > > > > > > > > On Tue, 26 Oct 2021 08:14:28 -0700 > > > > > Roman Bacik wrote: > > > > > > > > > > > Hi Marek, > > > > > > > > > > > > We do not want this driver to be automatically probed. It is not > > > > > > needed > > > > > > all the time and also slows down the boot time. We have stripped > > > > > > down > > > > > > everything else to bare minimum. > > > > > > Thanks, > > > > > > > > > > > > Roman > > > > > > > > > > Hi Roman, > > > > > > > > > > OK, that is reasonable, but not reasonable enough to introduce a > > > > > new > > > > > vendor specific command. > > > > > > > > > > Still NAK. > > > > > > > > > > So you have the bnxt_drv_probe method defined in the driver, but > you > > > > > don't set a pointer to it into the U_BOOT_DRIVER structure, and > > > > > instead > > > > > you call this method when "brcm probe" command is called. > > > > > > > > > > I think this introduction of another vendor specific command is > > > > > wrong. > > > > > > > > > > If probing takes too much time and should be done only when the > device > > > > > is needed, there are 2 things you could do: > > > > > > > > > > - you can create new driver flag saying that the device should be > > > > > probeb only when needed, wire necessary code and add this flag > > > > > to > > > your > > > > > driver (this could get very complicated, though) > > > > > - you can do minimum stuff in probe method, and move the stuff > > > > > that > > > > > takes long time into bnxt_start(), which is called only when > > > > > network > > > > > via this ethernet controller is requested for by U-Boot > > > > > commands. > > > > > > > > So renaming bnxt probe/remove to bnxt start/stop will do, right? > > > > > > No. The whole idea of adding the new "bnxt" command is wrong, > because > > > the command is *vendor specific*. The ethernet controller should work > > > out of the box with standard U-Boot commands, i.e. it if I use the > > > dhcp > > > command, it should work, without needing to call the "bnxt" command. > > > > Hi Marek, > > > > In order to speed up the boot, we do not load bnxt driver on each boot. > Also > > we do not need to load FW and initialize PCI required to bind bnxt. When > > bnxt is required, then we execute these commands: > > > > chimp_ld_secure #this command loads FW, which is necessary for PCIe to > > enumerate it > > pci enum #this command is necessary to call bnxt_bind > > bnxt 0 probe #this command would probe/load the driver > > > > Do you have a suggestion on how to make this work without introducing > bnxt > > commands if we do not want to enumerate PCIe, load FW and load bnxt on > each > > boot? Currently we boot to uboot prompt in 1s, which is our requirement. > > Thanks, > > It sounds like your ethernet driver will not even be bound in the > non-PCI case. Is that right? Yes correct > > What about the PCI driver? Does that get bound initially? If so, I > think U-Boot will normally probe PCI. But we should provide a way to > not auto--probe PCI. Would that help? PCI is probed automatically but PCI must be enumerated only after FW is loaded, which we do not want to be executed each boot. > > You could put the firmware loading (or a call to it) in your PCI > driver, perhaps. Then bringing up Ethernet could be something like: > > ret = uclass_first_device_err(UCLASS_PCI, ); Where would we call this from? Will it include calling "pci enum" required for bnxt to bind? We need this sequence somewhere: load fw -> pci enum (calls bnxt_bind which sets device name) -> bnxp_probe (needs bound device by name) > > I think your use case is not that unusual and we should be able to > support it in a standard way. If there are features missing for that I > am very happy to help figure it out. > > Regards, > Simon Maybe probing on
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
Hi Mark, On Wed, 27 Oct 2021 at 09:11, Mark Kettenis wrote: > > > From: François Ozog > > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > > > Hi, > > > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > > Hi all, > > > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > > Hi Tom, > > > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini wrote: > > > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > > Hi François, > > > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass > > > a écrit : > > > > > > > > >> > > > > > > > > >> Hi Tom, Bin,François, > > > > > > > > >> > > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini > > > wrote: > > > > > > > > >> > > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng wrote: > > > > > > > > >> > > Hi Simon, > > > > > > > > >> > > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > > s...@chromium.org> wrote: > > > > > > > > >> > > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and > > > OF_HOSTFILE so > > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > > >> > > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the devicetree > > > is built and > > > > > > > > >> > > > appended to U-Boot > > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > > devicetree is embedded in > > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > > >> > > > > > > > > > > > >> > > > The last one is currently set up so that no devicetree > > > is needed at all > > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > > some don't. Some > > > > > > > > >> > > > don't even provide instructions on how to boot on the > > > board. > > > > > > > > >> > > > > > > > > > > > >> > > > The problems with this approach are documented at [1]. > > > > > > > > >> > > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > > OF_SEPARATE. Any board > > > > > > > > >> > > > can obtain its devicetree at runtime, even it is has a > > > devicetree built > > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a second-stage > > > bootloader and its > > > > > > > > >> > > > caller may have a better idea about the hardware > > > available in the machine. > > > > > > > > >> > > > This is the case with a few QEMU boards, for example. > > > > > > > > >> > > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a 'choice'. It > > > should be an > > > > > > > > >> > > > option, available with either OF_SEPARATE or OF_EMBED. > > > > > > > > >> > > > > > > > > > > > >> > > > This series makes this change, adding various missing > > > devicetree files > > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > > >> > > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > > hack to me. > > > > > > > > >> > > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the fly > > > based on > > > > > > > > >> > > command line parameters, and the device tree you put in > > > this series > > > > > > > > >> > > has various hardcoded values which normally do > > > not show up > > > > > > > > >> > > in hand-written dts files. > > > > > > > > >> > > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > > >> > > > > > > > > > >> > I am also confused and do not like the idea of adding > > > device trees for > > > > > > > > >> > platforms that are capable of and can / do have a device > > > tree to give us > > > > > > > > >> > at run time. > > > > > > > > >> > > > > > > > > >> (I'll just reply to this one email, since the same points > > > applies to > > > > > > > > >> all replies I think) > > > > > > > > >> > > > > > > > > >> I have been thinking about this and discussing it with people > > > for a > > > > > > > > >> few months now. I've been signalling a change like this for > > > over a > > > > > > > > >> month now, on U-Boot contributor calls and in discussions > > > with Linaro > > > > > > > > >> people. I sent a patch (below) to try to explain things. I > > > hope it is > > > > > > > > >> not a surprise! > > > > > > > > >> > > > > > > > > >> The issue here is that we need a devicetree in-tree in > > > U-Boot, to > > > > > > > > >> avoid the mess that has been created by OF_PRIOR_STAGE, > > > OF_BOARD, > > > > > > > > >> BINMAN_STANDALONE_FDT and to a lesser extent, OF_HOSTFILE. > > > Between > > > > > > > > >> Ilias'
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
> From: François Ozog > Date: Wed, 27 Oct 2021 15:15:01 +0200 > > Hi, > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > Hi all, > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini wrote: > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > Hi François, > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass > > a écrit : > > > > > > > >> > > > > > > > >> Hi Tom, Bin,François, > > > > > > > >> > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini > > wrote: > > > > > > > >> > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng wrote: > > > > > > > >> > > Hi Simon, > > > > > > > >> > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > s...@chromium.org> wrote: > > > > > > > >> > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and > > OF_HOSTFILE so > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > >> > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the devicetree > > is built and > > > > > > > >> > > > appended to U-Boot > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > devicetree is embedded in > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > >> > > > > > > > > > > >> > > > The last one is currently set up so that no devicetree > > is needed at all > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > some don't. Some > > > > > > > >> > > > don't even provide instructions on how to boot on the > > board. > > > > > > > >> > > > > > > > > > > >> > > > The problems with this approach are documented at [1]. > > > > > > > >> > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > OF_SEPARATE. Any board > > > > > > > >> > > > can obtain its devicetree at runtime, even it is has a > > devicetree built > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a second-stage > > bootloader and its > > > > > > > >> > > > caller may have a better idea about the hardware > > available in the machine. > > > > > > > >> > > > This is the case with a few QEMU boards, for example. > > > > > > > >> > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a 'choice'. It > > should be an > > > > > > > >> > > > option, available with either OF_SEPARATE or OF_EMBED. > > > > > > > >> > > > > > > > > > > >> > > > This series makes this change, adding various missing > > devicetree files > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > >> > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > hack to me. > > > > > > > >> > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the fly > > based on > > > > > > > >> > > command line parameters, and the device tree you put in > > this series > > > > > > > >> > > has various hardcoded values which normally do > > not show up > > > > > > > >> > > in hand-written dts files. > > > > > > > >> > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > >> > > > > > > > > >> > I am also confused and do not like the idea of adding > > device trees for > > > > > > > >> > platforms that are capable of and can / do have a device > > tree to give us > > > > > > > >> > at run time. > > > > > > > >> > > > > > > > >> (I'll just reply to this one email, since the same points > > applies to > > > > > > > >> all replies I think) > > > > > > > >> > > > > > > > >> I have been thinking about this and discussing it with people > > for a > > > > > > > >> few months now. I've been signalling a change like this for > > over a > > > > > > > >> month now, on U-Boot contributor calls and in discussions > > with Linaro > > > > > > > >> people. I sent a patch (below) to try to explain things. I > > hope it is > > > > > > > >> not a surprise! > > > > > > > >> > > > > > > > >> The issue here is that we need a devicetree in-tree in > > U-Boot, to > > > > > > > >> avoid the mess that has been created by OF_PRIOR_STAGE, > > OF_BOARD, > > > > > > > >> BINMAN_STANDALONE_FDT and to a lesser extent, OF_HOSTFILE. > > Between > > > > > > > >> Ilias' series and this one we can get ourselves on a stronger > > footing. > > > > > > > >> There is just OF_SEPARATE, with OF_EMBED for debugging/ELF > > use. > > > > > > > >> For more context: > > > > > > > >> > > > > > > > >> > >
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On 10/27/21 16:55, Tom Rini wrote: On Wed, Oct 27, 2021 at 03:23:01PM +0200, Heinrich Schuchardt wrote: [snip] One passed to U-Boot for fixups and further passed to the OS. This devicetree may originate from a prior boot stage, from a file loaded by U-Boot, or from a later bootstage, e.g systemd-boot's devicetree command. I assume systemd-boot is implementing the same logic that extlinux.conf has used for forever, yes? It is loading the file and then calls U-Boot's implementation of the EFI Device Tree Fixup Protocol for fixups before passing the device-tree to the OS. This devicetree will not contain any U-Boot specific information. To repeat, it must only have official bindings, yes, regardless of what project they come from. Don't expect prior firmware stages to provide any U-Boot specific stuff whatever official or non-official U-Boot specific bindings exist. Best regards Heinrich
Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands
Hi Roman, On Wed, 27 Oct 2021 at 09:05, Roman Bacik wrote: > > > -Original Message- > > From: Marek Behún > > Sent: Tuesday, October 26, 2021 9:50 AM > > To: Roman Bacik > > Cc: Simon Glass ; U-Boot Mailing List > b...@lists.denx.de>; Bharat Gooty ; > > Aswath Govindraju ; Bin Meng > > ; Franck LENORMAND > > ; Heinrich Schuchardt > > ; Kory Maincent ; > > Michal Simek ; Patrick Delaunay > > ; Peng Fan ; Priyanka > > Jain ; Rayagonda Kokatanur > > ; Sean Anderson > > > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > > > On Tue, 26 Oct 2021 09:02:54 -0700 > > Roman Bacik wrote: > > > > > On Tue, Oct 26, 2021 at 8:55 AM Marek Behún wrote: > > > > > > > > On Tue, 26 Oct 2021 08:14:28 -0700 > > > > Roman Bacik wrote: > > > > > > > > > Hi Marek, > > > > > > > > > > We do not want this driver to be automatically probed. It is not > > > > > needed > > > > > all the time and also slows down the boot time. We have stripped > > > > > down > > > > > everything else to bare minimum. > > > > > Thanks, > > > > > > > > > > Roman > > > > > > > > Hi Roman, > > > > > > > > OK, that is reasonable, but not reasonable enough to introduce a new > > > > vendor specific command. > > > > > > > > Still NAK. > > > > > > > > So you have the bnxt_drv_probe method defined in the driver, but you > > > > don't set a pointer to it into the U_BOOT_DRIVER structure, and > > > > instead > > > > you call this method when "brcm probe" command is called. > > > > > > > > I think this introduction of another vendor specific command is wrong. > > > > > > > > If probing takes too much time and should be done only when the device > > > > is needed, there are 2 things you could do: > > > > > > > > - you can create new driver flag saying that the device should be > > > > probeb only when needed, wire necessary code and add this flag to > > your > > > > driver (this could get very complicated, though) > > > > - you can do minimum stuff in probe method, and move the stuff that > > > > takes long time into bnxt_start(), which is called only when network > > > > via this ethernet controller is requested for by U-Boot commands. > > > > > > So renaming bnxt probe/remove to bnxt start/stop will do, right? > > > > No. The whole idea of adding the new "bnxt" command is wrong, because > > the command is *vendor specific*. The ethernet controller should work > > out of the box with standard U-Boot commands, i.e. it if I use the > > dhcp > > command, it should work, without needing to call the "bnxt" command. > > Hi Marek, > > In order to speed up the boot, we do not load bnxt driver on each boot. Also > we do not need to load FW and initialize PCI required to bind bnxt. When > bnxt is required, then we execute these commands: > > chimp_ld_secure #this command loads FW, which is necessary for PCIe to > enumerate it > pci enum #this command is necessary to call bnxt_bind > bnxt 0 probe #this command would probe/load the driver > > Do you have a suggestion on how to make this work without introducing bnxt > commands if we do not want to enumerate PCIe, load FW and load bnxt on each > boot? Currently we boot to uboot prompt in 1s, which is our requirement. > Thanks, It sounds like your ethernet driver will not even be bound in the non-PCI case. Is that right? What about the PCI driver? Does that get bound initially? If so, I think U-Boot will normally probe PCI. But we should provide a way to not auto--probe PCI. Would that help? You could put the firmware loading (or a call to it) in your PCI driver, perhaps. Then bringing up Ethernet could be something like: ret = uclass_first_device_err(UCLASS_PCI, ); I think your use case is not that unusual and we should be able to support it in a standard way. If there are features missing for that I am very happy to help figure it out. Regards, Simon
Re: [PATCH v5 02/26] doc: Add documentation about devicetree usage
Hi Simon, On Wed, 27 Oct 2021 at 16:13, Simon Glass wrote: > Hi François, > > On Tue, 26 Oct 2021 at 09:57, François Ozog > wrote: > > > > > > > > On Tue, 26 Oct 2021 at 17:27, Simon Glass wrote: > >> > >> Hi François, > >> > >> On Tue, 26 Oct 2021 at 08:31, François Ozog > wrote: > >> > > >> > Hi Simon, > >> > > >> > On Tue, 26 Oct 2021 at 02:25, Simon Glass wrote: > >> >> > >> >> At present some of the ideas and techniques behind devicetree in > U-Boot > >> >> are assumed, implied or unsaid. Add some documentation to cover how > >> >> devicetree is build, how it can be modified and the rules about using > >> >> the various CONFIG_OF_... options. > >> >> > > [..] > > >> >> +Why not have two devicetrees? > >> >> +- > >> >> + > >> >> +Setting aside the argument for restricting U-Boot from having its > own nodes and > >> >> +properties, another idea proposed is to have two devicetrees, one > for the > >> >> +U-Boot-specific bits (here called `special`) and one for everything > else (here > >> >> +called `linux`). > >> >> + > >> >> +On the positive side, it might quieten the discussion alluded to in > the section > >> >> +above. But there are many negatives to consider and many open > questions to > >> >> +resolve. > >> >> + > >> >> +- **Bindings** - Presumably the special devicetree would have its > own bindings. > >> >> + It would not be necessary to put a `u-boot,` prefix on anything. > People coming > >> >> + across the devicetree source would wonder how it fits in with the > Linux > >> >> + devicetree. > >> >> + > >> >> +- **Access** - U-Boot has a nice `ofnode` API for accessing the > devicetree. This > >> >> + would need to be expanded to support two trees. Features which > need to access > >> >> + both (such as a device driver which reads the special devicetree > to get some > >> >> + configuration info) could become quite confusing to read and > write. > >> >> + > >> >> +- **Merging** - Can the two devicetree be merged if a platform > desires it? If > >> >> + so, how is this managed in tooling? Does it happen during the > build, in which > >> >> + case they are not really separate at all. Or does U-Boot merge > them at > >> >> + runtime, in which case this adds time and memory? > >> >> + > >> >> +- **Efficiency** - A second device tree adds more code and more > code paths. It > >> >> + requires that both be made available to the code in U-Boot, e.g. > via a > >> >> + separate pointer or argument or API. Overall the separation would > certainly > >> >> + not speed up U-Boot, nor decrease its size. > >> >> + > >> >> +- **Source code** - At present `u-boot.dtsi` files provide the > pieces needed for > >> >> + U-Boot for a particular board. Would we use these same files for > the special > >> >> + devicetree? > >> >> + > >> >> +- **Complexity** - Two devicetrees complicates the build system > since it must > >> >> + build and package them both. Errors must be reported in such a > way that it > >> >> + is obvious which one is failing. > >> >> + > >> >> +- **Referencing each other** - The `u-boot,dm-xxx` tags used by > driver model > >> >> + are currently placed in the nodes they relate to. How would these > tags > >> >> + reference a node that is in a separate devicetree? What extra > validation would > >> >> + be needed? > >> >> + > >> >> +- **Storage** - How would the two devicetrees be stored in the > image? At present > >> >> + we simply concatenate the U-Boot binary and the devicetree. We > could add the > >> >> + special devicetree before the Linux one, so two are concatenated, > but it is > >> >> + not pretty. We could use binman to support more complex > arrangements, but only > >> >> + some boards use this at present, so it would be a big change. > >> >> + > >> >> +- **API** - How would another project provide two devicetree files > to U-Boot at > >> >> + runtime? Presumably this would just be too painful. But if it > doesn't, it > >> >> + would be unable to configure run-time features of U-Boot during > the boot. > >> >> + > >> >> +- **Confusion** - No other project has two devicetrees. U-Boot > would be in the > >> >> + unfortunate position of having to describe this fact to new > users, along with > >> >> + the (arguably contrived) reason for the arrangement. > >> >> + > >> > > >> > False: > >> > 1) projects in trustedfirmware.org are built to have multiple FDT > objects, some for "dynamic" configuration purposes. > >> > >> Can you provided a link and I can update this. > > > > > https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/index.html > > Bindings: > > for FCONF: > https://trustedfirmware-a.readthedocs.io/en/latest/components/fconf/fconf_properties.html > > for FF-A: > https://trustedfirmware-a.readthedocs.io/en/latest/components/ffa-manifest-binding.html > > For chain-of-trust: > https://trustedfirmware-a.readthedocs.io/en/latest/components/cot-binding.html > > > > For some code: > > >
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
Hi, On 27.10.2021 17.08, Simon Glass wrote: Hi François, On Tue, 26 Oct 2021 at 00:07, François Ozog wrote: Hi Simon Position unchanged on this series: adding fake dts for boards that generate their device tree in the dts directory is not good. If you have them in documentation the it is acceptable. I think we are going to have to disagree on this one. I actually used the qemu one in testing/development recently. We have to have a way to develop in-tree with U-Boot. It does not impinge on any of your use cases, so far as I know. How about having the file just contain a single line #include ... where this generated-*.dts is not checked to the source tree. Then of course comments on how to generate it via qemu -dumpdtb + dtc, with appropriate precautions (ie. must be regenerated if qemu command line is changed or qemu is upgraded), intended use case, and so forth.
Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements
On Wednesday 27 October 2021 17:27:41 Stefan Roese wrote:
> On 27.10.21 16:10, Pali Rohár wrote:
>
>
>
> > > Perfect! So it really looks like that issue is in the code which resets
> > > baudrate back to the value 115200.
> > >
> > > I have there another diff which removes usage of the stack in code which
> > > resets baudrate back to default value:
> > >
> > > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > > index b56c9a0c8104..8f0e50501398 100644
> > > --- a/tools/kwboot.c
> > > +++ b/tools/kwboot.c
> > > @@ -1444,6 +1444,11 @@ _inject_baudrate_change_code(void *img, size_t
> > > *size, int pre,
> > > memcpy(code, kwboot_baud_code, codesz - 8);
> > > *(uint32_t *)(code + codesz - 8) = cpu_to_le32(old_baud);
> > > *(uint32_t *)(code + codesz - 4) = cpu_to_le32(new_baud);
> > > +
> > > + if (!pre) {
> >
> > Ou, there is a mistake, it should be "if (pre) {"
> >
> > > + *(uint32_t *)code = cpu_to_le32(0xe1a0); /* arm nop */
> > > + *(uint32_t *)(code + codesz - 4*7) = cpu_to_le32(0xe12fff1e);
> > > /* bx lr */
> > > + }
> > > }
> > > static int
> > >
> > > Could you try to apply this change instead of my previous one?
>
> Tested with this corrected patch:
>
> 97 %
> [..]
> 99 % [... ]
> Done
> Finishing transfer
> Waiting 2s for baudrate change magic
>
> Changing baudrate back to 115200 Bd
>
> [Type Ctrl-\ + c to quit]
>
>
> U-Boot 2021.10-00916-gc6142e537e88-dirty (Oct 27 2021 - 17:23:24 +0200)
>
> SoC: MV78260-B0 at 1333 MHz
> I2C: ready
> DRAM: 2 GiB (667 MHz, 64-bit, ECC not enabled)
> Loading Environment from SPIFlash... SF: Detected m25p128 with page size 256
> Bytes, erase size 256 KiB, total 16 MiB
> OK
> Model: Marvell Armada XP theadorable
> ...
>
> Perfect. This works on Armada XP. :)
Nice! Therefore I will prepare proper patch for removing stack usage.
> Still I see no speed change. But this is a different story...
>
> Thanks,
> Stefan
Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements
On 27.10.21 16:10, Pali Rohár wrote:
Perfect! So it really looks like that issue is in the code which resets
baudrate back to the value 115200.
I have there another diff which removes usage of the stack in code which
resets baudrate back to default value:
diff --git a/tools/kwboot.c b/tools/kwboot.c
index b56c9a0c8104..8f0e50501398 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -1444,6 +1444,11 @@ _inject_baudrate_change_code(void *img, size_t *size,
int pre,
memcpy(code, kwboot_baud_code, codesz - 8);
*(uint32_t *)(code + codesz - 8) = cpu_to_le32(old_baud);
*(uint32_t *)(code + codesz - 4) = cpu_to_le32(new_baud);
+
+ if (!pre) {
Ou, there is a mistake, it should be "if (pre) {"
+ *(uint32_t *)code = cpu_to_le32(0xe1a0); /* arm nop */
+ *(uint32_t *)(code + codesz - 4*7) = cpu_to_le32(0xe12fff1e);
/* bx lr */
+ }
}
static int
Could you try to apply this change instead of my previous one?
Tested with this corrected patch:
97 %
[..]
99 % [...
]
Done
Finishing transfer
Waiting 2s for baudrate change magic
Changing baudrate back to 115200 Bd
[Type Ctrl-\ + c to quit]
U-Boot 2021.10-00916-gc6142e537e88-dirty (Oct 27 2021 - 17:23:24 +0200)
SoC: MV78260-B0 at 1333 MHz
I2C: ready
DRAM: 2 GiB (667 MHz, 64-bit, ECC not enabled)
Loading Environment from SPIFlash... SF: Detected m25p128 with page size
256 Bytes, erase size 256 KiB, total 16 MiB
OK
Model: Marvell Armada XP theadorable
...
Perfect. This works on Armada XP. :)
Still I see no speed change. But this is a different story...
Thanks,
Stefan
Re: [PATCH v2 00/41] Initial implementation of standard boot
Hi Heinrich, On Wed, 27 Oct 2021 at 13:38, Heinrich Schuchardt < heinrich.schucha...@canonical.com> wrote: > > > On 10/24/21 01:25, Simon Glass wrote: > > > > The bootflow feature provide a built-in way for U-Boot to automatically > > boot an Operating System without custom scripting and other > customisation. > > This is called 'standard boot' since it provides a standard way for > > U-Boot to boot a distro, without scripting. > > > > It introduces the following concepts: > > > > - bootdev - a device which can hold a distro > > - bootmeth - a method to scan a bootdev to find bootflows (owned by > > U-Boot) > > - bootflow - a description of how to boot (owned by the distro) > > Please, describe why you are suggesting this change. > > Replacing a script by a devicetree chunk is just decreasing flexibility > and increasing complexity. Where is the benefit? > > I am missing a description here of where and how a boot flow shall be > defined. Describing some device-tree binding in patch 40/41 does not > replace describing the development and usage workflow. Who will provide > which bootflow information when? > > You still have an open discussion with Linaro about the source of > devicetrees. This discussion needs to be finalized before considering > this series. > > In my view bootflows cannot be defined in the devicetree because prior > firmware providing a devicetree is completely independent of any distro > and therefore cannot provide a distro specific bootflow. > Agreed if we talk about the DT passed to the OS. There are discussions in System Device Tree to represent partition booting orchestration information in a System Device Tree. This information may end-up being integrated in "a" device tree that will be used only by hypervisor as dynamic config. The same applies to secure world partitions: manifest are declared in "a" device tree that is consumed only by secure hypervisor (hafnium for the moment) as dynamic configuration, passed as a separate entity from a FIP section (I think it is TOS_CONFIG) https://trustedfirmware-a.readthedocs.io/en/latest/components/ffa-manifest-binding.html The other pattern is firmware component DT binding can be standardized separately from platform device tree. > > Best regards > > Heinrich > > > > > This series provides an implementation of these, enabled to scan for > > bootflows from MMC, USB and Ethernet. It supports the existing distro > > boot as well as the EFI loader flow (bootefi/bootmgr). It works > > similiarly to the existing script-based approach, but is native to > > U-Boot. > > > > With this we can boot on a Raspberry Pi 3 with just one command: > > > > bootflow scan -lb > > > > which means to scan, listing (-l) each bootflow and trying to boot each > > one (-b). The final patch shows this. > > > > With a standard way to identify boot devices, booting become easier. It > > also should be possible to support U-Boot scripts, for backwards > > compatibility only. > > > > This series relies on the PXE clean-up series, posted here: > > > > https://patchwork.ozlabs.org/project/uboot/list/?series=267078 > > > > For documentation, see the 'doc' patch. > > > > For version 2, a new naming scheme is used as above: > > > > - bootdev is used instead of bootdevice, because 'device' is > overused, > > is everywhere in U-Boot, can be confused with udevice > > - bootmeth - because 'method' is too vanilla, appears 1300 times in > > U-Boot > > > > Also in version 2, drivers are introduced for the boot methods, to make > > it more extensible. Booting a custom OS is simply a matter of creating a > > bootmeth for it and implementing the read_file() and boot() methods. > > > > The design is described in these two documents: > > > > > https://drive.google.com/file/d/1ggW0KJpUOR__vBkj3l61L2dav4ZkNC12/view?usp=sharing > > > > > https://drive.google.com/file/d/1kTrflO9vvGlKp-ZH_jlgb9TY3WYG6FF9/view?usp=sharing > > > > The series is available at u-boot-dm/bme-working > > > > Sample log on rpi_3_32b: > > > > U-Boot 2021.10-rc2-00043-gccd453aa918-dirty (Aug 28 2021 - 13:58:46 > -0600) > > > > DRAM: 992 MiB > > RPI 3 Model B (0xa22082) > > MMC: mmc@7e202000: 0, sdhci@7e30: 1 > > Loading Environment from FAT... Unable to read "uboot.env" from > mmc0:1... In:serial > > Out: vidconsole > > Err: vidconsole > > Net: No ethernet found. > > starting USB... > > Bus usb@7e98: USB DWC2 > > scanning bus usb@7e98 for devices... usb_kbd usb_kbd: Timeout poll > on interrupt endpoint > > Failed to get keyboard state from device 0c40:8000 > > 4 USB Device(s) found > > scanning usb for storage devices... 0 Storage Device(s) found > > Hit any key to stop autoboot: 0 > > Scanning for bootflows in all bootmethods > > Seq Type State UclassPart Name > Filename > > --- --- -- > > > Scanning bootmethod
Re: [PATCH v5 00/26] fdt: Make OF_BOARD a boolean option
On Wed, 27 Oct 2021 at 16:08, Simon Glass wrote: > Hi François, > > On Tue, 26 Oct 2021 at 00:07, François Ozog > wrote: > > > > Hi Simon > > > > Position unchanged on this series: adding fake dts for boards that > generate their device tree in the dts directory is not good. If you have > them in documentation the it is acceptable. > > I think we are going to have to disagree on this one. I actually used > the qemu one in testing/development recently. We have to have a way to > develop in-tree with U-Boot. It does not impinge on any of your use > cases, so far as I know. > I am not the only one in disagreement... You just saw Alex Bénée from Qemu saying the same thing. I understand the advanced debug/development scenario you mention. But locating the DT files in the dts directory is mis-leading the contributors to think that they need to compile the DT for the targeted platforms. For your advanced scenario, you can still have the dts in the documentation area, or whatever directory (except dts). compile it and supply to U-Boot. > > But trying to do any driver / core work for a board where you don't > have the devicetree is currently not possible. The devicetree is a > core component and being unable to modify it is simply not practical. > We are talking here about an option that can be enabled or disabled. > In my case I am able to disable it locally and do my development work. > BTW I've got the bloblist handoff working with a devicetree inside it, > for qemu_arm. I need to try it on a real board to figure out what the > difference is. > > That's great news and much needed for stabilizing the inbound ABI from prior loader to U-Boot. Let's create another thread to discuss this important topic. > Regards, > Simon > > > > > > > > > > > Cheers > > > > FF > > > > Le mar. 26 oct. 2021 à 02:24, Simon Glass a écrit : > >> > >> With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so > >> there are only three ways to obtain a devicetree: > >> > >>- OF_SEPARATE - the normal way, where the devicetree is built and > >> appended to U-Boot > >>- OF_EMBED - for development purposes, the devicetree is embedded in > >> the ELF file (also used for EFI) > >>- OF_BOARD - the board figures it out on its own > >> > >> The last one is currently set up so that no devicetree is needed at all > >> in the U-Boot tree. Most boards do provide one, but some don't. Some > >> don't even provide instructions on how to boot on the board. > >> > >> The problems with this approach are documented in another patch in this > >> series: "doc: Add documentation about devicetree usage" > >> > >> In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board > >> can obtain its devicetree at runtime, even it is has a devicetree built > >> in U-Boot. This is because U-Boot may be a second-stage bootloader and > its > >> caller may have a better idea about the hardware available in the > machine. > >> This is the case with a few QEMU boards, for example. > >> > >> So it makes no sense to have OF_BOARD as a 'choice'. It should be an > >> option, available with either OF_SEPARATE or OF_EMBED. > >> > >> This series makes this change, adding various missing devicetree files > >> (and placeholders) to make the build work. > >> > >> Note: If board maintainers are able to add their own patch to add the > >> files, some patches in this series can be dropped. > >> > >> It also provides a few qemu clean-ups discovered along the way. > >> > >> Note: This breaks the qemu-riscv64_spl test, which still needs > >> investigation. > >> > >> [1] > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/ > >> > >> Changes in v5: > >> - Bring into the OF_BOARD series > >> - Rebase to master and drop mention of OF_PRIOR_STAGE, since removed > >> - Refer to the 'control' DTB in the first paragraph > >> - Use QEMU instead of qemu > >> - Merge RISC-V and ARM patches since they are similar > >> - Add new patches to clean up fdtdec_setup() and surrounds > >> > >> Changes in v3: > >> - Clarify the 'bug' refered to at the top > >> - Reword 'This means that there' paragraph to explain U-Boot-specific > things > >> - Move to doc/develop/devicetree now that OF_CONTROL is in the docs > >> > >> Changes in v2: > >> - Fix typos per Sean (thank you!) and a few others > >> - Add a 'Use of U-Boot /config node' section > >> - Drop mention of dm-verity since that actually uses the kernel cmdline > >> - Explain that OF_BOARD will still work after these changes (in > >> 'Once this bug is fixed...' paragraph) > >> - Expand a bit on the reason why the 'Current situation' is bad > >> - Clarify in a second place that Linux and U-Boot use the same > devicetree > >> in 'To be clear, while U-Boot...' > >> - Expand on why we should have rules for other projects in > >> 'Devicetree in another project' > >> - Add a comment as to why devicetree in U-Boot is not 'bad design' > >> - Reword 'in-tree U-Boot
Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements
On Wednesday 27 October 2021 17:08:35 Marek Behún wrote:
> On Wed, 27 Oct 2021 16:10:21 +0200
> Pali Rohár wrote:
>
> > On Wednesday 27 October 2021 15:52:47 Pali Rohár wrote:
> > > On Wednesday 27 October 2021 07:09:42 Stefan Roese wrote:
> > > > On 26.10.21 20:48, Pali Rohár wrote:
> > > > > On Tuesday 26 October 2021 16:21:02 Stefan Roese wrote:
> > > > > > On 26.10.21 14:40, Pali Rohár wrote:
> > > > > > > My another guess there could be a problem is usage of stack.
> > > > > > > Maybe it is
> > > > > > > possible that register with stack pointer is not initialized
> > > > > > > after the
> > > > > > > full transfer when going to execute main image. Same arm baudrate
> > > > > > > change
> > > > > > > code is used after the SPL and before main U-Boot, and the first
> > > > > > > instruction is "push". Maybe modifying the arm code to not use
> > > > > > > stack
> > > > > > > when switching the baudrate back to 115200 could also help. I
> > > > > > > will look
> > > > > > > at it.
> > > > > >
> > > > > > Thanks.
> > > > >
> > > > > Here is dirty hack patch which completely disable calling code for
> > > > > changing baudrate back to 115200 on ARM side:
> > > > >
> > > > > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > > > > index 5f4ff673972e..00d58a239c71 100644
> > > > > --- a/tools/kwboot.c
> > > > > +++ b/tools/kwboot.c
> > > > > @@ -1070,17 +1070,17 @@ kwboot_xmodem(int tty, const void *_img,
> > > > > size_t size, int baudrate)
> > > > > return rc;
> > > > > if (baudrate) {
> > > > > - char buf[sizeof(kwb_baud_magic)];
> > > > > -
> > > > > - kwboot_printv("Waiting 1s for baudrate change magic\n");
> > > > > - rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000);
> > > > > - if (rc)
> > > > > - return rc;
> > > > > -
> > > > > - if (memcmp(buf, kwb_baud_magic, sizeof(buf))) {
> > > > > - errno = EPROTO;
> > > > > - return -1;
> > > > > - }
> > > > > +// char buf[sizeof(kwb_baud_magic)];
> > > > > +//
> > > > > +// kwboot_printv("Waiting 1s for baudrate change magic\n");
> > > > > +// rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000);
> > > > > +// if (rc)
> > > > > +// return rc;
> > > > > +//
> > > > > +// if (memcmp(buf, kwb_baud_magic, sizeof(buf))) {
> > > > > +// errno = EPROTO;
> > > > > +// return -1;
> > > > > +// }
> > > > > kwboot_printv("\nChanging baudrate back to 115200
> > > > > Bd\n\n");
> > > > > rc = kwboot_tty_change_baudrate(tty, 115200);
> > > > > @@ -1551,8 +1551,8 @@ kwboot_img_patch(void *img, size_t *size, int
> > > > > baudrate)
> > > > >* This code is appended after the data part of the
> > > > > image and
> > > > >* execaddr is changed to execute this code before
> > > > > U-Boot proper.
> > > > >*/
> > > > > - kwboot_printv("Injecting code for changing baudrate
> > > > > back\n");
> > > > > - _copy_baudrate_change_code(hdr, size, 1, baudrate,
> > > > > 115200);
> > > > > +// kwboot_printv("Injecting code for changing baudrate
> > > > > back\n");
> > > > > +// _copy_baudrate_change_code(hdr, size, 1, baudrate,
> > > > > 115200);
> > > >
> > > > I do have this here in my version as well:
> > > >
> > > > /* Update the 32-bit data checksum */
> > > > *kwboot_img_csum32_ptr(img) = kwboot_img_csum32(img);
> > > >
> > > > /* recompute header size */
> > > > hdrsz = kwbheader_size(hdr);
> > > >
> > > > So I'm using the newer version, just to be sure.
> > >
> > > Ok, I probably generated diff against older version, but you have
> > > figured out how to apply it.
> > >
> > > > > /* recompute header size */
> > > > > hdrsz = kwbheader_size(hdr);
> > > > >
> > > > > As main U-Boot binary on ARM resets UART, it means that baudrate is
> > > > > properly set to 115200. Probably beginning of the U-Boot output could
> > > > > be
> > > > > lost but at least console should start.
> > > > >
> > > > > Could you try this patch if it starts working now?
> > > >
> > > > Okay, applied this patch and and booting with different baudrates works
> > > > on this board again (tested with 230400):
> > > >
> > > > 96 %
> > > > [..]
> > > > 98 %
> > > > [..]
> > > > 99 % [ ]
> > > > Done
> > > > Finishing transfer
> > > >
> > > > Changing baudrate back to 115200 Bd
> > > >
> > > > [Type Ctrl-\ + c to quit]
> > > >
> > > >
> > > > U-Boot 2021.10-00908-gc129aa2f173a-dirty (Oct 27 2021 - 07:05:39 +0200)
> > > >
> > > > SoC:
Re: [PATCH u-boot-marvell 00/13] Yet another kwboot improvements
On Wed, 27 Oct 2021 16:10:21 +0200
Pali Rohár wrote:
> On Wednesday 27 October 2021 15:52:47 Pali Rohár wrote:
> > On Wednesday 27 October 2021 07:09:42 Stefan Roese wrote:
> > > On 26.10.21 20:48, Pali Rohár wrote:
> > > > On Tuesday 26 October 2021 16:21:02 Stefan Roese wrote:
> > > > > On 26.10.21 14:40, Pali Rohár wrote:
> > > > > > My another guess there could be a problem is usage of stack. Maybe
> > > > > > it is
> > > > > > possible that register with stack pointer is not initialized after
> > > > > > the
> > > > > > full transfer when going to execute main image. Same arm baudrate
> > > > > > change
> > > > > > code is used after the SPL and before main U-Boot, and the first
> > > > > > instruction is "push". Maybe modifying the arm code to not use stack
> > > > > > when switching the baudrate back to 115200 could also help. I will
> > > > > > look
> > > > > > at it.
> > > > >
> > > > > Thanks.
> > > >
> > > > Here is dirty hack patch which completely disable calling code for
> > > > changing baudrate back to 115200 on ARM side:
> > > >
> > > > diff --git a/tools/kwboot.c b/tools/kwboot.c
> > > > index 5f4ff673972e..00d58a239c71 100644
> > > > --- a/tools/kwboot.c
> > > > +++ b/tools/kwboot.c
> > > > @@ -1070,17 +1070,17 @@ kwboot_xmodem(int tty, const void *_img, size_t
> > > > size, int baudrate)
> > > > return rc;
> > > > if (baudrate) {
> > > > - char buf[sizeof(kwb_baud_magic)];
> > > > -
> > > > - kwboot_printv("Waiting 1s for baudrate change magic\n");
> > > > - rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000);
> > > > - if (rc)
> > > > - return rc;
> > > > -
> > > > - if (memcmp(buf, kwb_baud_magic, sizeof(buf))) {
> > > > - errno = EPROTO;
> > > > - return -1;
> > > > - }
> > > > +// char buf[sizeof(kwb_baud_magic)];
> > > > +//
> > > > +// kwboot_printv("Waiting 1s for baudrate change magic\n");
> > > > +// rc = kwboot_tty_recv(tty, buf, sizeof(buf), 1000);
> > > > +// if (rc)
> > > > +// return rc;
> > > > +//
> > > > +// if (memcmp(buf, kwb_baud_magic, sizeof(buf))) {
> > > > +// errno = EPROTO;
> > > > +// return -1;
> > > > +// }
> > > > kwboot_printv("\nChanging baudrate back to 115200
> > > > Bd\n\n");
> > > > rc = kwboot_tty_change_baudrate(tty, 115200);
> > > > @@ -1551,8 +1551,8 @@ kwboot_img_patch(void *img, size_t *size, int
> > > > baudrate)
> > > > * This code is appended after the data part of the
> > > > image and
> > > > * execaddr is changed to execute this code before
> > > > U-Boot proper.
> > > > */
> > > > - kwboot_printv("Injecting code for changing baudrate
> > > > back\n");
> > > > - _copy_baudrate_change_code(hdr, size, 1, baudrate,
> > > > 115200);
> > > > +// kwboot_printv("Injecting code for changing baudrate
> > > > back\n");
> > > > +// _copy_baudrate_change_code(hdr, size, 1, baudrate,
> > > > 115200);
> > >
> > > I do have this here in my version as well:
> > >
> > > /* Update the 32-bit data checksum */
> > > *kwboot_img_csum32_ptr(img) = kwboot_img_csum32(img);
> > >
> > > /* recompute header size */
> > > hdrsz = kwbheader_size(hdr);
> > >
> > > So I'm using the newer version, just to be sure.
> >
> > Ok, I probably generated diff against older version, but you have
> > figured out how to apply it.
> >
> > > > /* recompute header size */
> > > > hdrsz = kwbheader_size(hdr);
> > > >
> > > > As main U-Boot binary on ARM resets UART, it means that baudrate is
> > > > properly set to 115200. Probably beginning of the U-Boot output could be
> > > > lost but at least console should start.
> > > >
> > > > Could you try this patch if it starts working now?
> > >
> > > Okay, applied this patch and and booting with different baudrates works
> > > on this board again (tested with 230400):
> > >
> > > 96 %
> > > [..]
> > > 98 %
> > > [..]
> > > 99 % [ ]
> > > Done
> > > Finishing transfer
> > >
> > > Changing baudrate back to 115200 Bd
> > >
> > > [Type Ctrl-\ + c to quit]
> > >
> > >
> > > U-Boot 2021.10-00908-gc129aa2f173a-dirty (Oct 27 2021 - 07:05:39 +0200)
> > >
> > > SoC: MV78260-B0 at 1333 MHz
> > > I2C: ready
> > > DRAM: 2 GiB (667 MHz, 64-bit, ECC not enabled)
> > > Loading Environment from SPIFlash... SF: Detected m25p128 with page size
> > > 256
> > > Bytes, erase size 256 KiB, total 16 MiB
> > > OK
> > > Model: Marvell Armada XP
RE: [PATCH v3 2/2] cmd: brcm: netXtreme commands
> -Original Message- > From: Marek Behún > Sent: Tuesday, October 26, 2021 9:50 AM > To: Roman Bacik > Cc: Simon Glass ; U-Boot Mailing List b...@lists.denx.de>; Bharat Gooty ; > Aswath Govindraju ; Bin Meng > ; Franck LENORMAND > ; Heinrich Schuchardt > ; Kory Maincent ; > Michal Simek ; Patrick Delaunay > ; Peng Fan ; Priyanka > Jain ; Rayagonda Kokatanur > ; Sean Anderson > > Subject: Re: [PATCH v3 2/2] cmd: brcm: netXtreme commands > > On Tue, 26 Oct 2021 09:02:54 -0700 > Roman Bacik wrote: > > > On Tue, Oct 26, 2021 at 8:55 AM Marek Behún wrote: > > > > > > On Tue, 26 Oct 2021 08:14:28 -0700 > > > Roman Bacik wrote: > > > > > > > Hi Marek, > > > > > > > > We do not want this driver to be automatically probed. It is not > > > > needed > > > > all the time and also slows down the boot time. We have stripped > > > > down > > > > everything else to bare minimum. > > > > Thanks, > > > > > > > > Roman > > > > > > Hi Roman, > > > > > > OK, that is reasonable, but not reasonable enough to introduce a new > > > vendor specific command. > > > > > > Still NAK. > > > > > > So you have the bnxt_drv_probe method defined in the driver, but you > > > don't set a pointer to it into the U_BOOT_DRIVER structure, and > > > instead > > > you call this method when "brcm probe" command is called. > > > > > > I think this introduction of another vendor specific command is wrong. > > > > > > If probing takes too much time and should be done only when the device > > > is needed, there are 2 things you could do: > > > > > > - you can create new driver flag saying that the device should be > > > probeb only when needed, wire necessary code and add this flag to > your > > > driver (this could get very complicated, though) > > > - you can do minimum stuff in probe method, and move the stuff that > > > takes long time into bnxt_start(), which is called only when network > > > via this ethernet controller is requested for by U-Boot commands. > > > > So renaming bnxt probe/remove to bnxt start/stop will do, right? > > No. The whole idea of adding the new "bnxt" command is wrong, because > the command is *vendor specific*. The ethernet controller should work > out of the box with standard U-Boot commands, i.e. it if I use the > dhcp > command, it should work, without needing to call the "bnxt" command. Hi Marek, In order to speed up the boot, we do not load bnxt driver on each boot. Also we do not need to load FW and initialize PCI required to bind bnxt. When bnxt is required, then we execute these commands: chimp_ld_secure #this command loads FW, which is necessary for PCIe to enumerate it pci enum #this command is necessary to call bnxt_bind bnxt 0 probe #this command would probe/load the driver Do you have a suggestion on how to make this work without introducing bnxt commands if we do not want to enumerate PCIe, load FW and load bnxt on each boot? Currently we boot to uboot prompt in 1s, which is our requirement. Thanks, Roman -- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it. smime.p7s Description: S/MIME Cryptographic Signature
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 03:23:01PM +0200, Heinrich Schuchardt wrote: [snip] > One passed to U-Boot for fixups and further passed to the OS. This > devicetree may originate from a prior boot stage, from a file loaded by > U-Boot, or from a later bootstage, e.g systemd-boot's devicetree command. I assume systemd-boot is implementing the same logic that extlinux.conf has used for forever, yes? > This devicetree will not contain any U-Boot specific information. To repeat, it must only have official bindings, yes, regardless of what project they come from. -- Tom signature.asc Description: PGP signature
Re: [PATCH 00/16] fdt: Make OF_BOARD a boolean option
On Wed, Oct 27, 2021 at 03:15:01PM +0200, François Ozog wrote: > Hi, > > On Wed, 27 Oct 2021 at 14:48, Tom Rini wrote: > > > On Fri, Oct 15, 2021 at 12:03:44PM -0600, Simon Glass wrote: > > > Hi all, > > > > > > On Thu, 14 Oct 2021 at 09:28, Tom Rini wrote: > > > > > > > > On Thu, Oct 14, 2021 at 09:17:52AM -0600, Simon Glass wrote: > > > > > Hi Tom, > > > > > > > > > > On Thu, 14 Oct 2021 at 08:56, Tom Rini wrote: > > > > > > > > > > > > On Wed, Oct 13, 2021 at 12:06:02PM -0600, Simon Glass wrote: > > > > > > > Hi François, > > > > > > > > > > > > > > On Wed, 13 Oct 2021 at 11:35, François Ozog < > > francois.o...@linaro.org> wrote: > > > > > > > > > > > > > > > > Hi Simon > > > > > > > > > > > > > > > > Le mer. 13 oct. 2021 à 16:49, Simon Glass > > a écrit : > > > > > > > >> > > > > > > > >> Hi Tom, Bin,François, > > > > > > > >> > > > > > > > >> On Tue, 12 Oct 2021 at 19:34, Tom Rini > > wrote: > > > > > > > >> > > > > > > > > >> > On Wed, Oct 13, 2021 at 09:29:14AM +0800, Bin Meng wrote: > > > > > > > >> > > Hi Simon, > > > > > > > >> > > > > > > > > > >> > > On Wed, Oct 13, 2021 at 9:01 AM Simon Glass < > > s...@chromium.org> wrote: > > > > > > > >> > > > > > > > > > > >> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and > > OF_HOSTFILE so > > > > > > > >> > > > there are only three ways to obtain a devicetree: > > > > > > > >> > > > > > > > > > > >> > > >- OF_SEPARATE - the normal way, where the devicetree > > is built and > > > > > > > >> > > > appended to U-Boot > > > > > > > >> > > >- OF_EMBED - for development purposes, the > > devicetree is embedded in > > > > > > > >> > > > the ELF file (also used for EFI) > > > > > > > >> > > >- OF_BOARD - the board figures it out on its own > > > > > > > >> > > > > > > > > > > >> > > > The last one is currently set up so that no devicetree > > is needed at all > > > > > > > >> > > > in the U-Boot tree. Most boards do provide one, but > > some don't. Some > > > > > > > >> > > > don't even provide instructions on how to boot on the > > board. > > > > > > > >> > > > > > > > > > > >> > > > The problems with this approach are documented at [1]. > > > > > > > >> > > > > > > > > > > >> > > > In practice, OF_BOARD is not really distinct from > > OF_SEPARATE. Any board > > > > > > > >> > > > can obtain its devicetree at runtime, even it is has a > > devicetree built > > > > > > > >> > > > in U-Boot. This is because U-Boot may be a second-stage > > bootloader and its > > > > > > > >> > > > caller may have a better idea about the hardware > > available in the machine. > > > > > > > >> > > > This is the case with a few QEMU boards, for example. > > > > > > > >> > > > > > > > > > > >> > > > So it makes no sense to have OF_BOARD as a 'choice'. It > > should be an > > > > > > > >> > > > option, available with either OF_SEPARATE or OF_EMBED. > > > > > > > >> > > > > > > > > > > >> > > > This series makes this change, adding various missing > > devicetree files > > > > > > > >> > > > (and placeholders) to make the build work. > > > > > > > >> > > > > > > > > > >> > > Adding device trees that are never used sounds like a > > hack to me. > > > > > > > >> > > > > > > > > > >> > > For QEMU, device tree is dynamically generated on the fly > > based on > > > > > > > >> > > command line parameters, and the device tree you put in > > this series > > > > > > > >> > > has various hardcoded values which normally do > > not show up > > > > > > > >> > > in hand-written dts files. > > > > > > > >> > > > > > > > > > >> > > I am not sure I understand the whole point of this. > > > > > > > >> > > > > > > > > >> > I am also confused and do not like the idea of adding > > device trees for > > > > > > > >> > platforms that are capable of and can / do have a device > > tree to give us > > > > > > > >> > at run time. > > > > > > > >> > > > > > > > >> (I'll just reply to this one email, since the same points > > applies to > > > > > > > >> all replies I think) > > > > > > > >> > > > > > > > >> I have been thinking about this and discussing it with people > > for a > > > > > > > >> few months now. I've been signalling a change like this for > > over a > > > > > > > >> month now, on U-Boot contributor calls and in discussions > > with Linaro > > > > > > > >> people. I sent a patch (below) to try to explain things. I > > hope it is > > > > > > > >> not a surprise! > > > > > > > >> > > > > > > > >> The issue here is that we need a devicetree in-tree in > > U-Boot, to > > > > > > > >> avoid the mess that has been created by OF_PRIOR_STAGE, > > OF_BOARD, > > > > > > > >> BINMAN_STANDALONE_FDT and to a lesser extent, OF_HOSTFILE. > > Between > > > > > > > >> Ilias' series and this one we can get ourselves on a stronger > > footing. > > > > > > > >> There is just OF_SEPARATE, with OF_EMBED for debugging/ELF > > use. > > > > > > > >> For more context: > > > > > > > >> > > > > > > > >> > >
Re: [PATCH 05/16] arm: qemu: Add a devicetree file for qemu_arm64
On Wed, Oct 27, 2021 at 03:44:08PM +0100, Alex Bennée wrote: > > François Ozog writes: > > > Hi Simon > > > > The only place I could agree with this file presence is in the > > documentation directory, not in dts. It creates a mental picture for the > > reader > > an entirely bad mind scheme around Qemu and DT. > > > > And even in a documentation directory I would place a bug warning: don’t > > use this with any kernel , Qemu generates a DT dynamically > > based on cpu, memory and devices specified at the command line. > > Certainly for the arm, aarch64 and riscv "virt" machines you should > always use the QEMU generated DTB. I'm not entirely clear what a > qemu_arm and qemu_arm64 def targets are meant to be in this context. Agreed. We cannot include random device trees in U-Boot for devices that generate their own at run time or otherwise have the source of truth elsewhere. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 31/41] bootstd: Add an implementation of EFI boot
Hi Simon, On Wed, Oct 27, 2021 at 08:09:04AM -0600, Simon Glass wrote: > Hi Ilias, > > On Wed, 27 Oct 2021 at 02:36, Ilias Apalodimas > wrote: > > > > Hi Simon, > > > > On Sun, 24 Oct 2021 at 02:27, Simon Glass wrote: > > > > > > Add a bootmeth driver which handles EFI boot, using EFI_LOADER. > > > > > > In effect, this provides the same functionality as the 'bootefi' command > > > and shares the same code. But the interface into it is via a bootmeth, > > > so it does not require any special scripts, etc. > > > > > > For now this requires the 'bootefi' command be enabled. Future work may > > > tidy this up so that it can be used without CONFIG_CMDLINE being enabled. > > > > I'll leave this up to Heinrich, but personally I wouldn't include this > > patch at all. EFI has it's bootmgr which can handle booting just fine. > > I don't see why we should duplicate the functionality. The new boot > > method can just have an entry called 'EFI' and then let the existing > > EFI code to decide. > > This is needed so that EFI boot is actually invoked. If bootmgr starts > being used then it can still be invoked from standard boot. The point > is that there is a standard way of booting that supports EFI and other > things. This patch tries to reason about the default naming EFI imposes on it's boot files. distro_efi_read_bootflow() will try to find files following the EFI naming convention (e.g bootaarch64.efi, bootarm.efi etc). If those are found it will try to boot them right? That's not the right thing to do though. On the EFI spec these files are tried if no Boot variables are found. So we can get rid of this entirely, add a dummy entry on the bootflow that says 'boot the efi manager' (which is what the next patch does). The efibootmgr then will check Boot variables and if none are found, it's going to fallback into loading bootaarch64.efi, bootarm.efi etc essentially offering identical functionality. Regards /Ilias > > This series is about replacing the scripts we currently have with a > proper C implementation that uses driver model. > > > Regards > > /Ilias > > > [..] > > Regards, > Simon
Re: [PATCH 05/16] arm: qemu: Add a devicetree file for qemu_arm64
François Ozog writes: > Hi Simon > > The only place I could agree with this file presence is in the documentation > directory, not in dts. It creates a mental picture for the reader > an entirely bad mind scheme around Qemu and DT. > > And even in a documentation directory I would place a bug warning: don’t use > this with any kernel , Qemu generates a DT dynamically > based on cpu, memory and devices specified at the command line. Certainly for the arm, aarch64 and riscv "virt" machines you should always use the QEMU generated DTB. I'm not entirely clear what a qemu_arm and qemu_arm64 def targets are meant to be in this context. -- Alex Bennée
Re: [PATCH v2 2/2] arm: mvebu: add Globalscale MOCHAbin support
On Wed, Oct 27, 2021 at 4:04 PM Pali Rohár wrote:
>
> On Wednesday 27 October 2021 12:08:13 Robert Marko wrote:
> > Globalscale MOCHAbin is a Armada 7040 based development board.
> >
> > Specifications:
> > * Armada 7040 Quad core ARMv8 Cortex A-72 @ 1.4GHz
> > * 2 / 4 / 8 GB of DDR4 DRAM
> > * 16 GB eMMC
> > * 4MB SPI-NOR (Bootloader)
> > * 1x M.2-2280 B-key socket (for SSD expansion, SATA3 only)
> > * 1x M.2-2250 B-key socket (for modems, USB2.0 and I2C only)
> > * 1x Mini-PCIe 3.0 (x1, USB2.0 and I2C)
> > * 1x SATA 7+15 socket (SATA3)
> > * 1x 16-pin (2×8) MikroBus Connector
> > * 1x SIM card slot (Connected to the mini-PCIe and both M.2 slots)
> > * 2x USB3.0 Type-A ports via SMSC USB5434B hub
> > * Cortex 2x5 JTAG
> > * microUSB port for UART (PL2303GL/PL2303SA onboard)
> > * 1x 10G SFP+
> > * 1x 1G SFP (Connected to 88E1512 PHY)
> > * 1x 1G RJ45 with PoE PD (Connected to 88E1512 PHY)
> > * 4x 1G RJ45 ports via Topaz 88E6141 switch
> > * RTC with battery holder (SoC provided, requires CR2032 battery)
> > * 1x 12V DC IN
> > * 1x Power switch
> > * 1x 12V fan header (3-pin, power only)
> > * 1x mini-PCIe LED header (2x0.1" pins)
> > * 1x M.2-2280 LED header (2x0.1" pins)
> > * 6x Bootstrap jumpers
> > * 1x Power LED (Green)
> > * 3x Tri-color RGB LEDs (Controllable)
> > * 1x Microchip ATECC608B secure element
> >
> > Note that 1G SFP does not work in U-boot as it would require Linux like
> > SFP support for parsing module interface and reconfiguring the PHY.
> >
> > Additionally, automatic import of the Marvell hw_info parameters is
> > enabled via the recently added mac command for Armada platforms.
> > The parameters stored in Marvell hw_info are usually the board serial
> > number and MAC addresses.
> >
> > Signed-off-by: Robert Marko
> > ---
> > Changes in v2:
> > * Add DT compatible for hw-info to its partition
> > * Enable MTD support, command and SPI MTD command
> > ---
> > arch/arm/dts/Makefile| 1 +
> > arch/arm/dts/armada-7040-mochabin.dts| 305 +++
> > configs/mvebu_mochabin-88f7040_defconfig | 88 +++
> > 3 files changed, 394 insertions(+)
> > create mode 100644 arch/arm/dts/armada-7040-mochabin.dts
> > create mode 100644 configs/mvebu_mochabin-88f7040_defconfig
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index a88aecc5bd9..dea567d2595 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -232,6 +232,7 @@ dtb-$(CONFIG_ARCH_MVEBU) += \
> > armada-38x-controlcenterdc.dtb \
> > armada-7040-db-nand.dtb \
> > armada-7040-db.dtb \
> > + armada-7040-mochabin.dtb\
> > armada-8040-clearfog-gt-8k.dtb \
> > armada-8040-db.dtb \
> > armada-8040-mcbin.dtb \
> > diff --git a/arch/arm/dts/armada-7040-mochabin.dts
> > b/arch/arm/dts/armada-7040-mochabin.dts
> > new file mode 100644
> > index 000..75e3331efb7
> > --- /dev/null
> > +++ b/arch/arm/dts/armada-7040-mochabin.dts
> > @@ -0,0 +1,305 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (C) 2016- 2021 Marvell International Ltd.
> > + * Copyright (C) 2021 Sartura Ltd.
> > + */
> > +
> > +/*
> > + * Device Tree file for Globalscale MOCHAbin
> > + * Boot device: SPI NOR, 0x32 (SW3)
> > + */
> > +
> > +#include "armada-7040.dtsi"
> > +
> > +/ {
> > + model = "Globalscale MOCHAbin";
> > + compatible = "globalscale,mochabin", "marvell,armada7040",
> > + "marvell,armada-ap806-quad", "marvell,armada-ap806";
> > +
> > + chosen {
> > + stdout-path = "serial0:115200n8";
> > + };
> > +
> > + aliases {
> > + i2c0 = _i2c0;
> > + spi0 = _spi1;
> > + };
> > +
> > + memory@ {
> > + device_type = "memory";
> > + reg = <0x0 0x0 0x0 0x8000>;
> > + };
> > +};
> > +
> > +_pinctl {
> > +/* MPP Bus:
> > + * SDIO [0-5]
> > + * UART0 [11,19]
> > + */
> > + /* 0 1 2 3 4 5 6 7 8 9 */
> > + pin-func = < 1 1 1 1 1 1 0 0 0 0
> > + 0 3 0 0 0 0 0 0 0 3 >;
> > +};
> > +
> > +/* microUSB UART console */
> > + {
> > + status = "okay";
> > +};
> > +
> > +/* eMMC */
> > +_sdhci0 {
> > + status = "okay";
> > +
> > + bus-width = <4>;
> > + no-1-8-v;
> > + non-removable;
> > +};
> > +
> > +_pinctl {
> > + /* MPP Bus:
> > + * E6341_RSTn[0]
> > + * E6341_INT [1]
> > + * I2C1 [2,3]
> > + * PCIE_CLKREQn [5]
> > + * UART0 [6,7]
> > + * PCIE_WAKEn[8]
> > + * PCIE_RSTOUTn [9] (GPIO)
> > + * DEVSLP_M2-2 [10]
> > + * PHY0_RSTn [12] (88E1512 PHY RST)
> > + * SPI1
