[PATCH 0/8] Patches for Toradex boards

[PATCH 0/8] Patches for Toradex boards

[Oleksandr Suvorov]

There is the set of patches for Toradex boards.


Marcel Ziswiler (1):
  board: apalis-tk1: launch toradex easy installer in usb recovery

Ming Liu (1):
  board: colibri_imx6: add board_fit_config_name_match to support FIT in
    SPL

Oleksandr Suvorov (3):
  board: colibri-imx8x: update building documentation
  configs: toradex: add default rootpath
  board: colibri_vf: fix compiling warning

Stefan Agner (3):
  board: colibri_vf: synchronize/improve memory options
  board: colibri_imx7: use SDP if USB serial downloader has been used
  board: toradex: make USB PID from config block optional

 board/toradex/apalis-tk1/apalis-tk1.c     | 13 +++++++++--
 board/toradex/colibri_imx6/colibri_imx6.c | 10 +++++++++
 board/toradex/colibri_imx7/colibri_imx7.c | 10 +++++++++
 board/toradex/colibri_vf/colibri_vf.c     |  2 +-
 board/toradex/common/Kconfig              | 10 +++++++++
 board/toradex/common/tdx-common.c         |  2 +-
 doc/board/toradex/colibri-imx8x.rst       | 27 +++++++----------------
 include/configs/apalis-imx8.h             |  1 +
 include/configs/apalis-imx8x.h            |  1 +
 include/configs/apalis-tk1.h              |  1 +
 include/configs/apalis_imx6.h             |  1 +
 include/configs/colibri-imx8x.h           |  1 +
 include/configs/colibri_imx6.h            |  1 +
 include/configs/colibri_imx7.h            |  1 +
 include/configs/colibri_vf.h              |  2 ++
 15 files changed, 60 insertions(+), 23 deletions(-)

-- 
2.31.1

[PATCH 1/8] board: colibri_vf: synchronize/improve memory options

[Oleksandr Suvorov]

From: Stefan Agner <stefan.agner@toradex.com>

Synchronize initramfs related configuration options and improve
memory layout. The memory layout with an offset of 16MiB allows
to boot bigger kernels. With AUTO_ZRELADDR, which is pretty much
the standard nowadays, the kernel relacates itself to PC masked
with 0xf8000000 plus a text offset of 0x8000 (hence 0x80008000 for
Vybrid). With a 16MiB offset from the start of memory and a 16MiB
distance to the device-tree, we allow kernel up to ~16MiB.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 include/configs/colibri_vf.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
index 5bd440f1db..7fb54a442e 100644
--- a/include/configs/colibri_vf.h
+++ b/include/configs/colibri_vf.h
@@ -47,6 +47,8 @@
 #define MEM_LAYOUT_ENV_SETTINGS \
 	"bootm_size=0x10000000\0" \
 	"fdt_addr_r=0x82000000\0" \
+	"fdt_high=0xfffffff0\0" \
+	"initrd_high=0xfffffff0\0" \
 	"kernel_addr_r=0x81000000\0" \
 	"pxefile_addr_r=0x87100000\0" \
 	"ramdisk_addr_r=0x82100000\0" \
-- 
2.31.1

[PATCH 2/8] board: colibri_imx7: use SDP if USB serial downloader has been used

[Oleksandr Suvorov]

From: Stefan Agner <stefan.agner@toradex.com>

In case USB serial downloader has been used to load U-Boot start the
serial download protocol (SDP) emulation. This allows to download
complete images such as Toradex Easy Installer over USB SDP as well.
This code uses the boot ROM provided boot information to reliably
detect USB serial downloader.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 board/toradex/colibri_imx7/colibri_imx7.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/board/toradex/colibri_imx7/colibri_imx7.c b/board/toradex/colibri_imx7/colibri_imx7.c
index 301b07d5b4..6822102d2c 100644
--- a/board/toradex/colibri_imx7/colibri_imx7.c
+++ b/board/toradex/colibri_imx7/colibri_imx7.c
@@ -355,12 +355,22 @@ int board_usb_phy_mode(int port)
 	}
 }
 
+#if defined(CONFIG_BOARD_LATE_INIT)
 int board_late_init(void)
 {
 #if defined(CONFIG_DM_VIDEO)
 	setup_lcd();
 #endif
+
+#if defined(CONFIG_CMD_USB_SDP)
+	if (is_boot_from_usb()) {
+		printf("Serial Downloader recovery mode, using sdp command\n");
+		env_set("bootdelay", "0");
+		env_set("bootcmd", "sdp 0");
+	}
+#endif
 	return 0;
 }
+#endif /* CONFIG_BOARD_LATE_INIT */
 
 #endif
-- 
2.31.1

[PATCH 3/8] board: toradex: make USB PID from config block optional

[Oleksandr Suvorov]

From: Stefan Agner <stefan.agner@toradex.com>

If config block support is enabled, USB gadget modes unconditionally
use Toradex Product ID as USB PID. Some applications might prefer a
different and/or static USB PID. Add a Kconfig configuration option
to descide whether to use USB PID from config block or the fallback
config option CONFIG_G_DNL_PRODUCT_NUM.

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 board/toradex/common/Kconfig      | 10 ++++++++++
 board/toradex/common/tdx-common.c |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/board/toradex/common/Kconfig b/board/toradex/common/Kconfig
index 36068d2e3b..1f6a5e4db5 100644
--- a/board/toradex/common/Kconfig
+++ b/board/toradex/common/Kconfig
@@ -84,4 +84,14 @@ config TDX_CFG_BLOCK_EXTRA
 	  Enables fetching auxilary config blocks from carrier board/display
 	  adapter EEPROMs.
 
+config TDX_CFG_BLOCK_USB_GADGET_PID
+	bool "Use config block product ID as USB product ID"
+	depends on USB_GADGET_DOWNLOAD
+	default y
+	help
+	  Use the Toradex product ID learned from the config block as USB
+	  product ID. An offset of 0x4000 is added to the product ID since
+	  inside the Toradex vendor ID (0x1b67) the range starting from
+	  offset 0x4000 is reserved for Colibri/Apalis modules.
+
 endif
diff --git a/board/toradex/common/tdx-common.c b/board/toradex/common/tdx-common.c
index a3d287ed5e..061abf7537 100644
--- a/board/toradex/common/tdx-common.c
+++ b/board/toradex/common/tdx-common.c
@@ -147,7 +147,7 @@ int show_board_info(void)
 	return 0;
 }
 
-#ifdef CONFIG_USB_GADGET_DOWNLOAD
+#ifdef CONFIG_TDX_CFG_BLOCK_USB_GADGET_PID
 int g_dnl_bind_fixup(struct usb_device_descriptor *dev, const char *name)
 {
 	unsigned short usb_pid;
-- 
2.31.1

[PATCH 4/8] board: colibri-imx8x: update building documentation

[Oleksandr Suvorov]

Update the documentation on how to build the u-boot image for
Colibri iMX8QXP, adding support of V1.0D revision of the module.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 doc/board/toradex/colibri-imx8x.rst | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/doc/board/toradex/colibri-imx8x.rst b/doc/board/toradex/colibri-imx8x.rst
index 616f40ae0f..545568c844 100644
--- a/doc/board/toradex/colibri-imx8x.rst
+++ b/doc/board/toradex/colibri-imx8x.rst
@@ -1,6 +1,6 @@
 .. SPDX-License-Identifier: GPL-2.0+
 
-Colibri iMX8QXP V1.0B Module
+Colibri iMX8QXP V1.0D Module
 ============================
 
 Quick Start
@@ -18,34 +18,23 @@ Get and Build the ARM Trusted Firmware
 
 .. code-block:: bash
 
-    $ git clone -b imx_4.14.78_1.0.0_ga https://source.codeaurora.org/external/imx/imx-atf
-    $ cd imx-atf/
-    $ make PLAT=imx8qxp bl31
+    $ git clone -b toradex_imx_5.4.70_2.3.0 http://git.toradex.com/cgit/imx-atf.git/
+    $ make PLAT=imx8qx bl31 -C imx-atf
 
 Get scfw_tcm.bin and ahab-container.img
 ---------------------------------------
 .. code-block:: bash
 
-    $ wget https://github.com/toradex/meta-fsl-bsp-release/blob/
-           toradex-sumo-4.14.78-1.0.0_ga-bringup/imx/meta-bsp/recipes-
-           bsp/imx-sc-firmware/files/mx8qx-colibri-scfw-tcm.bin?raw=true
-    $ mv mx8qx-colibri-scfw-tcm.bin\?raw\=true mx8qx-colibri-scfw-tcm.bin
-    $ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/firmware-imx-8.0.bin
-    $ chmod +x firmware-imx-8.0.bin
-    $ ./firmware-imx-8.0.bin
-
-Copy the following binaries to the U-Boot folder:
-
-.. code-block:: bash
-
-    $ cp imx-atf/build/imx8qxp/release/bl31.bin .
-    $ cp u-boot/u-boot.bin .
+    $ wget https://github.com/toradex/i.MX-System-Controller-Firmware/raw/master/src/scfw_export_mx8qx_b0/build_mx8qx_b0/mx8qx-colibri-scfw-tcm.bin
+    $ wget https://www.nxp.com/lgfiles/NMG/MAD/YOCTO/imx-seco-3.7.4.bin
+    $ sh imx-seco-3.7.4.bin --auto-accept
 
 Copy the following firmware to the U-Boot folder:
 
 .. code-block:: bash
 
-    $ cp firmware-imx-8.0/firmware/seco/ahab-container.img .
+    $ cp imx-atf/build/imx8qx/release/bl31.bin .
+    $ cp imx-seco-3.7.4/firmware/seco/mx8qxc0-ahab-container.img mx8qx-ahab-container.img
 
 Build U-Boot
 ------------
-- 
2.31.1

[PATCH 5/8] board: colibri_imx6: add board_fit_config_name_match to support FIT in SPL

[Oleksandr Suvorov]

From: Ming Liu <ming.liu@toradex.com>

Only one dtb is currently supported, so match with imx6-colibri.

Signed-off-by: Ming Liu <ming.liu@toradex.com>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 board/toradex/colibri_imx6/colibri_imx6.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/board/toradex/colibri_imx6/colibri_imx6.c b/board/toradex/colibri_imx6/colibri_imx6.c
index c0e7754469..587d92a8e9 100644
--- a/board/toradex/colibri_imx6/colibri_imx6.c
+++ b/board/toradex/colibri_imx6/colibri_imx6.c
@@ -1081,6 +1081,16 @@ void board_init_f(ulong dummy)
 	board_init_r(NULL, 0);
 }
 
+#ifdef CONFIG_SPL_LOAD_FIT
+int board_fit_config_name_match(const char *name)
+{
+	if (!strcmp(name, "imx6-colibri"))
+		return 0;
+
+	return -1;
+}
+#endif
+
 void reset_cpu(void)
 {
 }
-- 
2.31.1

[PATCH 6/8] configs: toradex: add default rootpath

[Oleksandr Suvorov]

The rootpath is used by distroboot script as a default path to a
rootfs on an NFS server. Set this properly for all Toradex apalis
and colibri modules, that support booting with Distro Boot script.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 include/configs/apalis-imx8.h   | 1 +
 include/configs/apalis-imx8x.h  | 1 +
 include/configs/apalis-tk1.h    | 1 +
 include/configs/apalis_imx6.h   | 1 +
 include/configs/colibri-imx8x.h | 1 +
 include/configs/colibri_imx6.h  | 1 +
 include/configs/colibri_imx7.h  | 1 +
 7 files changed, 7 insertions(+)

diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
index b04a03f76d..5a90e03edb 100644
--- a/include/configs/apalis-imx8.h
+++ b/include/configs/apalis-imx8.h
@@ -24,6 +24,7 @@
 #define CONFIG_IPADDR			192.168.10.2
 #define CONFIG_NETMASK			255.255.255.0
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #define MEM_LAYOUT_ENV_SETTINGS \
 	"fdt_addr_r=0x84000000\0" \
diff --git a/include/configs/apalis-imx8x.h b/include/configs/apalis-imx8x.h
index 2ad4ca3418..ad3b77853c 100644
--- a/include/configs/apalis-imx8x.h
+++ b/include/configs/apalis-imx8x.h
@@ -22,6 +22,7 @@
 #define CONFIG_IPADDR			192.168.10.2
 #define CONFIG_NETMASK			255.255.255.0
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #define MEM_LAYOUT_ENV_SETTINGS \
 	"kernel_addr_r=0x80280000\0" \
diff --git a/include/configs/apalis-tk1.h b/include/configs/apalis-tk1.h
index 57192649ec..9c5c89a6e3 100644
--- a/include/configs/apalis-tk1.h
+++ b/include/configs/apalis-tk1.h
@@ -44,6 +44,7 @@
 #define CONFIG_NETMASK		255.255.255.0
 #undef CONFIG_SERVERIP
 #define CONFIG_SERVERIP		192.168.10.1
+#define CONFIG_ROOTPATH		"/srv/nfs"
 
 #define DFU_ALT_EMMC_INFO	"apalis-tk1.img raw 0x0 0x500 mmcpart 1; " \
 				"boot part 0 1 mmcpart 0; " \
diff --git a/include/configs/apalis_imx6.h b/include/configs/apalis_imx6.h
index 12de0105c6..9bd3d7a848 100644
--- a/include/configs/apalis_imx6.h
+++ b/include/configs/apalis_imx6.h
@@ -79,6 +79,7 @@
 #define CONFIG_NETMASK			255.255.255.0
 #undef CONFIG_SERVERIP
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #define CONFIG_LOADADDR			0x12000000
 
diff --git a/include/configs/colibri-imx8x.h b/include/configs/colibri-imx8x.h
index cb22b3c75a..acd3694a44 100644
--- a/include/configs/colibri-imx8x.h
+++ b/include/configs/colibri-imx8x.h
@@ -24,6 +24,7 @@
 #define CONFIG_IPADDR			192.168.10.2
 #define CONFIG_NETMASK			255.255.255.0
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #define MEM_LAYOUT_ENV_SETTINGS \
 	"fdt_addr_r=0x83000000\0" \
diff --git a/include/configs/colibri_imx6.h b/include/configs/colibri_imx6.h
index 804a144a03..02f3bf41b8 100644
--- a/include/configs/colibri_imx6.h
+++ b/include/configs/colibri_imx6.h
@@ -67,6 +67,7 @@
 #define CONFIG_NETMASK			255.255.255.0
 #undef CONFIG_SERVERIP
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #define CONFIG_LOADADDR			0x12000000
 
diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h
index 2fffaa39c0..008d8749c0 100644
--- a/include/configs/colibri_imx7.h
+++ b/include/configs/colibri_imx7.h
@@ -31,6 +31,7 @@
 #define CONFIG_IPADDR			192.168.10.2
 #define CONFIG_NETMASK			255.255.255.0
 #define CONFIG_SERVERIP			192.168.10.1
+#define CONFIG_ROOTPATH			"/srv/nfs"
 
 #if defined(CONFIG_TARGET_COLIBRI_IMX7_EMMC)
 #define UBOOT_UPDATE \
-- 
2.31.1

[PATCH 7/8] board: colibri_vf: fix compiling warning

[Oleksandr Suvorov]

This patch fixes the following compiler warning:
=============
board/toradex/colibri_vf/colibri_vf.c: In function 'ft_board_setup':
board/toradex/colibri_vf/colibri_vf.c:436:6: warning: unused variable 'ret' [-Wunused-variable]
=============

Fixes: be3f1a56bf8 ("video: fsl_dcu_fb: add DM_VIDEO support")
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 board/toradex/colibri_vf/colibri_vf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/toradex/colibri_vf/colibri_vf.c b/board/toradex/colibri_vf/colibri_vf.c
index e9c5bc0632..c09591e543 100644
--- a/board/toradex/colibri_vf/colibri_vf.c
+++ b/board/toradex/colibri_vf/colibri_vf.c
@@ -433,7 +433,7 @@ int checkboard(void)
 #if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
-#ifndef CONFIG_DM_VIDEO
+#if defined(CONFIG_VIDEO_FSL_DCU_FB) && !defined(CONFIG_DM_VIDEO)
 	int ret = 0;
 #endif
 #ifdef CONFIG_FDT_FIXUP_PARTITIONS
-- 
2.31.1

[PATCH 8/8] board: apalis-tk1: launch toradex easy installer in usb recovery

[Oleksandr Suvorov]

From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

The USB recovery mode is used by Toradex to load the Toradex Easy
Installer image which supports further system images installation.
Prepare for loading and launching the Toradex Easy Installer if the
USB Recovery mode is activated.

Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
---

 board/toradex/apalis-tk1/apalis-tk1.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/board/toradex/apalis-tk1/apalis-tk1.c b/board/toradex/apalis-tk1/apalis-tk1.c
index b97617cfca..f418f2c39f 100644
--- a/board/toradex/apalis-tk1/apalis-tk1.c
+++ b/board/toradex/apalis-tk1/apalis-tk1.c
@@ -38,8 +38,17 @@
 int arch_misc_init(void)
 {
 	if (readl(NV_PA_BASE_SRAM + NVBOOTINFOTABLE_BOOTTYPE) ==
-	    NVBOOTTYPE_RECOVERY)
-		printf("USB recovery mode\n");
+	    NVBOOTTYPE_RECOVERY) {
+		printf("USB recovery mode, attempting to boot Toradex Easy Installer\n");
+		env_set("bootdelay", "-2");
+		env_set("defargs", "pcie_aspm=off user_debug=30");
+		env_set("fdt_high", "");
+		env_set("initrd_high", "");
+		env_set("setup", "env set setupargs igb_mac=${ethaddr} consoleblank=0 no_console_suspend=1 console=${console},${baudrate}n8 ${memargs}");
+		env_set("teziargs", "rootfstype=squashfs root=/dev/ram quiet autoinstall");
+		env_set("vidargs", "video=HDMI-A-1:640x480-16@60D");
+		env_set("bootcmd", "run setup; env set bootargs ${defargs} ${setupargs} ${vidargs} ${teziargs}; bootm 0x80208000#config@${soc}-${fdt_module}-${fdt_board}.dtb");
+	}
 
 	/* PCB Version Indication: V1.2 and later have GPIO_PV0 wired to GND */
 	gpio_request(TEGRA_GPIO(V, 0), "PCB Version Indication");
-- 
2.31.1

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Marek Vasut]

On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:

[...]

> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
> index b04a03f76d..5a90e03edb 100644
> --- a/include/configs/apalis-imx8.h
> +++ b/include/configs/apalis-imx8.h
> @@ -24,6 +24,7 @@
>   #define CONFIG_IPADDR			192.168.10.2
>   #define CONFIG_NETMASK			255.255.255.0
>   #define CONFIG_SERVERIP			192.168.10.1
> +#define CONFIG_ROOTPATH			"/srv/nfs"

Rather, all this should be removed, since all these IP addresses and 
root path settings are invalid for most places where the system will be 
used. The user should define their own. It is not recommended to hard 
code this in board config.

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Oleksandr Suvorov]

Hello Marek,

On Thu, Jul 22, 2021 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>
> On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:
>
> [...]
>
> > diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
> > index b04a03f76d..5a90e03edb 100644
> > --- a/include/configs/apalis-imx8.h
> > +++ b/include/configs/apalis-imx8.h
> > @@ -24,6 +24,7 @@
> >   #define CONFIG_IPADDR                       192.168.10.2
> >   #define CONFIG_NETMASK                      255.255.255.0
> >   #define CONFIG_SERVERIP                     192.168.10.1
> > +#define CONFIG_ROOTPATH                      "/srv/nfs"
>
> Rather, all this should be removed, since all these IP addresses and
> root path settings are invalid for most places where the system will be
> used. The user should define their own. It is not recommended to hard
> code this in board config.

Is it acceptable to move this stuff to Toradex Kconfigs as default values?

Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Marek Vasut]

On 7/22/21 6:32 PM, Oleksandr Suvorov wrote:
> Hello Marek,
> 
> On Thu, Jul 22, 2021 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:
>>
>> [...]
>>
>>> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
>>> index b04a03f76d..5a90e03edb 100644
>>> --- a/include/configs/apalis-imx8.h
>>> +++ b/include/configs/apalis-imx8.h
>>> @@ -24,6 +24,7 @@
>>>    #define CONFIG_IPADDR                       192.168.10.2
>>>    #define CONFIG_NETMASK                      255.255.255.0
>>>    #define CONFIG_SERVERIP                     192.168.10.1
>>> +#define CONFIG_ROOTPATH                      "/srv/nfs"
>>
>> Rather, all this should be removed, since all these IP addresses and
>> root path settings are invalid for most places where the system will be
>> used. The user should define their own. It is not recommended to hard
>> code this in board config.
> 
> Is it acceptable to move this stuff to Toradex Kconfigs as default values?

The general recommendation is to not set such default values at all, 
just let the user configure what they need.

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Oleksandr Suvorov]

On Thu, Jul 22, 2021 at 9:06 PM Marek Vasut <marex@denx.de> wrote:
>
> On 7/22/21 6:32 PM, Oleksandr Suvorov wrote:
> > Hello Marek,
> >
> > On Thu, Jul 22, 2021 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:
> >>
> >> [...]
> >>
> >>> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
> >>> index b04a03f76d..5a90e03edb 100644
> >>> --- a/include/configs/apalis-imx8.h
> >>> +++ b/include/configs/apalis-imx8.h
> >>> @@ -24,6 +24,7 @@
> >>>    #define CONFIG_IPADDR                       192.168.10.2
> >>>    #define CONFIG_NETMASK                      255.255.255.0
> >>>    #define CONFIG_SERVERIP                     192.168.10.1
> >>> +#define CONFIG_ROOTPATH                      "/srv/nfs"
> >>
> >> Rather, all this should be removed, since all these IP addresses and
> >> root path settings are invalid for most places where the system will be
> >> used. The user should define their own. It is not recommended to hard
> >> code this in board config.
> >
> > Is it acceptable to move this stuff to Toradex Kconfigs as default values?
>
> The general recommendation is to not set such default values at all,
> just let the user configure what they need.

Thanks for your feedback! I'll remove this patch from the next version
of the patchset.

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Marek Vasut]

On 7/22/21 8:10 PM, Oleksandr Suvorov wrote:
> On Thu, Jul 22, 2021 at 9:06 PM Marek Vasut <marex@denx.de> wrote:
>>
>> On 7/22/21 6:32 PM, Oleksandr Suvorov wrote:
>>> Hello Marek,
>>>
>>> On Thu, Jul 22, 2021 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
>>>>
>>>> On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:
>>>>
>>>> [...]
>>>>
>>>>> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
>>>>> index b04a03f76d..5a90e03edb 100644
>>>>> --- a/include/configs/apalis-imx8.h
>>>>> +++ b/include/configs/apalis-imx8.h
>>>>> @@ -24,6 +24,7 @@
>>>>>     #define CONFIG_IPADDR                       192.168.10.2
>>>>>     #define CONFIG_NETMASK                      255.255.255.0
>>>>>     #define CONFIG_SERVERIP                     192.168.10.1
>>>>> +#define CONFIG_ROOTPATH                      "/srv/nfs"
>>>>
>>>> Rather, all this should be removed, since all these IP addresses and
>>>> root path settings are invalid for most places where the system will be
>>>> used. The user should define their own. It is not recommended to hard
>>>> code this in board config.
>>>
>>> Is it acceptable to move this stuff to Toradex Kconfigs as default values?
>>
>> The general recommendation is to not set such default values at all,
>> just let the user configure what they need.
> 
> Thanks for your feedback! I'll remove this patch from the next version
> of the patchset.

You likely also want to remove all those CONFIG_IPADDR and co.

Re: [PATCH 6/8] configs: toradex: add default rootpath

[Oleksandr Suvorov]

On Thu, Jul 22, 2021 at 9:18 PM Marek Vasut <marex@denx.de> wrote:
>
> On 7/22/21 8:10 PM, Oleksandr Suvorov wrote:
> > On Thu, Jul 22, 2021 at 9:06 PM Marek Vasut <marex@denx.de> wrote:
> >>
> >> On 7/22/21 6:32 PM, Oleksandr Suvorov wrote:
> >>> Hello Marek,
> >>>
> >>> On Thu, Jul 22, 2021 at 7:08 PM Marek Vasut <marex@denx.de> wrote:
> >>>>
> >>>> On 7/22/21 5:49 PM, Oleksandr Suvorov wrote:
> >>>>
> >>>> [...]
> >>>>
> >>>>> diff --git a/include/configs/apalis-imx8.h b/include/configs/apalis-imx8.h
> >>>>> index b04a03f76d..5a90e03edb 100644
> >>>>> --- a/include/configs/apalis-imx8.h
> >>>>> +++ b/include/configs/apalis-imx8.h
> >>>>> @@ -24,6 +24,7 @@
> >>>>>     #define CONFIG_IPADDR                       192.168.10.2
> >>>>>     #define CONFIG_NETMASK                      255.255.255.0
> >>>>>     #define CONFIG_SERVERIP                     192.168.10.1
> >>>>> +#define CONFIG_ROOTPATH                      "/srv/nfs"
> >>>>
> >>>> Rather, all this should be removed, since all these IP addresses and
> >>>> root path settings are invalid for most places where the system will be
> >>>> used. The user should define their own. It is not recommended to hard
> >>>> code this in board config.
> >>>
> >>> Is it acceptable to move this stuff to Toradex Kconfigs as default values?
> >>
> >> The general recommendation is to not set such default values at all,
> >> just let the user configure what they need.
> >
> > Thanks for your feedback! I'll remove this patch from the next version
> > of the patchset.
>
> You likely also want to remove all those CONFIG_IPADDR and co.

Yes, thanks, probably by the next release :)

Re: [PATCH 1/8] board: colibri_vf: synchronize/improve memory options

[Tom Rini]

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

On Thu, Jul 22, 2021 at 06:49:36PM +0300, Oleksandr Suvorov wrote:

> From: Stefan Agner <stefan.agner@toradex.com>
> 
> Synchronize initramfs related configuration options and improve
> memory layout. The memory layout with an offset of 16MiB allows
> to boot bigger kernels. With AUTO_ZRELADDR, which is pretty much
> the standard nowadays, the kernel relacates itself to PC masked
> with 0xf8000000 plus a text offset of 0x8000 (hence 0x80008000 for
> Vybrid). With a 16MiB offset from the start of memory and a 16MiB
> distance to the device-tree, we allow kernel up to ~16MiB.
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> ---
> 
>  include/configs/colibri_vf.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
> index 5bd440f1db..7fb54a442e 100644
> --- a/include/configs/colibri_vf.h
> +++ b/include/configs/colibri_vf.h
> @@ -47,6 +47,8 @@
>  #define MEM_LAYOUT_ENV_SETTINGS \
>  	"bootm_size=0x10000000\0" \
>  	"fdt_addr_r=0x82000000\0" \
> +	"fdt_high=0xfffffff0\0" \
> +	"initrd_high=0xfffffff0\0" \
>  	"kernel_addr_r=0x81000000\0" \
>  	"pxefile_addr_r=0x87100000\0" \
>  	"ramdisk_addr_r=0x82100000\0" \

NAK.  You cannot disable device tree relocation by default.  You're
already setting bootm_size, which is good.  I don't like disabling
initrd, but that's at least an easier to debug problem, and also much
more performance sensitive.

-- 
Tom

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

Re: [PATCH 1/8] board: colibri_vf: synchronize/improve memory options

[Oleksandr Suvorov]

Hello Tom,

On Thu, Jul 22, 2021 at 8:41 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Jul 22, 2021 at 06:49:36PM +0300, Oleksandr Suvorov wrote:
>
> > From: Stefan Agner <stefan.agner@toradex.com>
> >
> > Synchronize initramfs related configuration options and improve
> > memory layout. The memory layout with an offset of 16MiB allows
> > to boot bigger kernels. With AUTO_ZRELADDR, which is pretty much
> > the standard nowadays, the kernel relacates itself to PC masked
> > with 0xf8000000 plus a text offset of 0x8000 (hence 0x80008000 for
> > Vybrid). With a 16MiB offset from the start of memory and a 16MiB
> > distance to the device-tree, we allow kernel up to ~16MiB.
> >
> > Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@toradex.com>
> > ---
> >
> >  include/configs/colibri_vf.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/include/configs/colibri_vf.h b/include/configs/colibri_vf.h
> > index 5bd440f1db..7fb54a442e 100644
> > --- a/include/configs/colibri_vf.h
> > +++ b/include/configs/colibri_vf.h
> > @@ -47,6 +47,8 @@
> >  #define MEM_LAYOUT_ENV_SETTINGS \
> >       "bootm_size=0x10000000\0" \
> >       "fdt_addr_r=0x82000000\0" \
> > +     "fdt_high=0xfffffff0\0" \
> > +     "initrd_high=0xfffffff0\0" \
> >       "kernel_addr_r=0x81000000\0" \
> >       "pxefile_addr_r=0x87100000\0" \
> >       "ramdisk_addr_r=0x82100000\0" \
>
> NAK.  You cannot disable device tree relocation by default.  You're
> already setting bootm_size, which is good. I don't like disabling
> initrd, but that's at least an easier to debug problem, and also much
> more performance sensitive.

Thanks for your notes! I'll remove this patch from the next patchset
version and think over how to rework
it later.

>
> --
> Tom

Best regards
Oleksandr Suvorov

Toradex AG
Ebenaustrasse 10 | 6048 Horw | Switzerland | T: +41 41 500 48 00