[PATCH u-boot-marvell 0/8] Turris Omnia: Add support for configuring mSATA and WWAN slots via env variables

[PATCH u-boot-marvell 0/8] Turris Omnia: Add support for configuring mSATA and WWAN slots via env variables

[Pali Rohár]

Some mPCIe cards are broken and their PIN 43 incorrectly that card is
mSATA. U-Boot SPL on Turris Omnia is using PIN 43 for configuring PCIe
vs SATA functionality on SerDes. Allow to configure functionality via
additional env variable "omnia_msata_slot" which may override PIN 43.

To force PCIe mode for broken MiniPCIe cards, call U-Boot commands:

  => setenv omnia_msata_slot pcie
  => saveenv
  => reset


PCI-SIG in PCIe Mini CEM 2.1 spec added support for USB3.0 mode to mPCIe
cards. PCIe and USB3.0 functions share same pins (like SATA pins on mSATA
with PCIe on mPCIe) and therefore only one function may be active at the
same time. This mode was introduced by PCI-SIG specially for LTE/5G modems.

One mPCIe slot on Turris Omnia is connected to A385 CPU SerDes which
can be configured for PCIe or USB3.0 function. It is the slot with SIM
card suitable for cellular modems. As PCIe Mini CEM 2.1 spec say that
detection of PCIe vs USB3.0 functionality is platform specific, add a
new U-Boot variable "omnia_wwan_slot" for configuring functionality in
this slot. By default PCIe is used like before.

To set this WWAN slot to USB3.0 mode, call U-Boot commands:

  => setenv omnia_wwan_slot usb3
  => saveenv
  => reset


Pali Rohár (8):
  env: sf: Allow to use env_sf_init_addr() at any stage
  arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function
  arm: mvebu: turris_omnia: Enable ENV support in SPL
  arm: mvebu: turris_omnia: Define only one serdes map variable
  arm: mvebu: turris_omnia: Allow to configure mSATA slot via env
    variable
  arm: mvebu: turris_omnia: Extract code for disabling sata/pcie
  arm: mvebu: turris_omnia: Signal error when sata/pcie DT mode
  arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe
    slot

 board/CZ.NIC/turris_omnia/turris_omnia.c | 207 +++++++++++++++++------
 configs/turris_omnia_defconfig           |   1 +
 env/sf.c                                 |  22 +--
 3 files changed, 163 insertions(+), 67 deletions(-)

-- 
2.20.1

[PATCH u-boot-marvell 1/8] env: sf: Allow to use env_sf_init_addr() at any stage

[Pali Rohár]

In some cases it makes sense to use env_sf_init_addr() also in SPL mode.
Allow it for boards by providing custom implementation of weak function
env_sf_get_env_addr(). When this function returns NULL it signals that
address is invalid, like config option CONFIG_ENV_ADDR.

There is no change in default behavior or in config options.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 env/sf.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/env/sf.c b/env/sf.c
index 6a4bb756f006..d2c07cd71687 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -24,10 +24,6 @@
 #include <dm/device-internal.h>
 #include <u-boot/crc.h>
 
-#ifndef CONFIG_SPL_BUILD
-#define INITENV
-#endif
-
 #define	OFFSET_INVALID		(~(u32)0)
 
 #ifdef CONFIG_ENV_OFFSET_REDUND
@@ -322,14 +318,15 @@ done:
 	return ret;
 }
 
-#if CONFIG_ENV_ADDR != 0x0
 __weak void *env_sf_get_env_addr(void)
 {
+#ifndef CONFIG_SPL_BUILD
 	return (void *)CONFIG_ENV_ADDR;
-}
+#else
+	return NULL;
 #endif
+}
 
-#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
 /*
  * check if Environment on CONFIG_ENV_ADDR is valid.
  */
@@ -337,6 +334,9 @@ static int env_sf_init_addr(void)
 {
 	env_t *env_ptr = (env_t *)env_sf_get_env_addr();
 
+	if (!env_ptr)
+		return -ENOENT;
+
 	if (crc32(0, env_ptr->data, ENV_SIZE) == env_ptr->crc) {
 		gd->env_addr = (ulong)&(env_ptr->data);
 		gd->env_valid = ENV_VALID;
@@ -346,7 +346,6 @@ static int env_sf_init_addr(void)
 
 	return 0;
 }
-#endif
 
 #if defined(CONFIG_ENV_SPI_EARLY)
 /*
@@ -432,9 +431,10 @@ out:
 
 static int env_sf_init(void)
 {
-#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
-	return env_sf_init_addr();
-#elif defined(CONFIG_ENV_SPI_EARLY)
+	int ret = env_sf_init_addr();
+	if (ret != -ENOENT)
+		return ret;
+#ifdef CONFIG_ENV_SPI_EARLY
 	return env_sf_init_early();
 #endif
 	/*
-- 
2.20.1

[PATCH u-boot-marvell 2/8] arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function

[Pali Rohár]

BootROM maps SPI Flash to fixed address 0xD4000000 and this mapping is
active also when BootROM is executing binary kwbimage headers, which
includes also U-Boot SPL.

Therefore no initialization code is required to access SPI Flags from
U-Boot SPL. In proper U-Boot it is remapped to other location.

So in mvebu implementation of env_sf_get_env_addr() function returns
0xD4000000 when running in SPL and NULL when in proper U-Boot.

This change would allow to use U-Boot ENV in U-Boot SPL. Normally it is not
possible to read ENV because it is too big and U-Boot SPL does not have
such big malloc() pool to real all ENV variables.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 33cec6587e19..a93af6c5b877 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -243,6 +243,16 @@ static bool omnia_detect_sata(void)
 	return stsword & MSATA_IND_STSBIT ? true : false;
 }
 
+void *env_sf_get_env_addr(void)
+{
+	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
+#ifdef CONFIG_SPL_BUILD
+	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
+#else
+	return NULL;
+#endif
+}
+
 int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 {
 	if (omnia_detect_sata()) {
-- 
2.20.1

[PATCH u-boot-marvell 3/8] arm: mvebu: turris_omnia: Enable ENV support in SPL

[Pali Rohár]

Allow to read ENV variables also in SPL on Turris Omnia.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 configs/turris_omnia_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig
index 280dd55f001b..9f633583022c 100644
--- a/configs/turris_omnia_defconfig
+++ b/configs/turris_omnia_defconfig
@@ -37,6 +37,7 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_MISC_INIT_R=y
 CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_I2C=y
 CONFIG_CMD_MEMTEST=y
 CONFIG_SYS_ALT_MEMTEST=y
-- 
2.20.1

[PATCH u-boot-marvell 4/8] arm: mvebu: turris_omnia: Define only one serdes map variable

[Pali Rohár]

By default use primary serdes map with PCIe function in combined
miniPCIe/mSATA slot. When SATA is detected change serdes map variable at
runtime.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index a93af6c5b877..d4c41bb1797a 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -93,7 +93,7 @@ enum status_word_bits {
 #define OMNIA_GPP_POL_LOW	0x0
 #define OMNIA_GPP_POL_MID	0x0
 
-static struct serdes_map board_serdes_map_pex[] = {
+static struct serdes_map board_serdes_map[] = {
 	{PEX0, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
 	{USB3_HOST0, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
 	{PEX1, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
@@ -102,15 +102,6 @@ static struct serdes_map board_serdes_map_pex[] = {
 	{SGMII2, SERDES_SPEED_1_25_GBPS, SERDES_DEFAULT_MODE, 0, 0}
 };
 
-static struct serdes_map board_serdes_map_sata[] = {
-	{SATA0, SERDES_SPEED_6_GBPS, SERDES_DEFAULT_MODE, 0, 0},
-	{USB3_HOST0, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
-	{PEX1, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
-	{USB3_HOST1, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
-	{PEX2, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
-	{SGMII2, SERDES_SPEED_1_25_GBPS, SERDES_DEFAULT_MODE, 0, 0}
-};
-
 static struct udevice *omnia_get_i2c_chip(const char *name, uint addr,
 					  uint offset_len)
 {
@@ -256,13 +247,15 @@ void *env_sf_get_env_addr(void)
 int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 {
 	if (omnia_detect_sata()) {
-		*serdes_map_array = board_serdes_map_sata;
-		*count = ARRAY_SIZE(board_serdes_map_sata);
-	} else {
-		*serdes_map_array = board_serdes_map_pex;
-		*count = ARRAY_SIZE(board_serdes_map_pex);
+		/* Change SerDes for first mPCIe port (mSATA) from PCIe to SATA */
+		board_serdes_map[0].serdes_type = SATA0;
+		board_serdes_map[0].serdes_speed = SERDES_SPEED_6_GBPS;
+		board_serdes_map[0].serdes_mode = SERDES_DEFAULT_MODE;
 	}
 
+	*serdes_map_array = board_serdes_map;
+	*count = ARRAY_SIZE(board_serdes_map);
+
 	return 0;
 }
 
-- 
2.20.1

[PATCH u-boot-marvell 5/8] arm: mvebu: turris_omnia: Allow to configure mSATA slot via env variable

[Pali Rohár]

Some PCIe-based MiniPCIe cards are broken and they do not ground PIN 43
which is required by PCIe mini CEM specs. Such broken cards are incorrectly
detected as mSATA cards because SATA specs requires that PIN 43 on mSATA
cards has to be disconnected.

PIN 43 on Turris Omnia is used only for MiniPCIe/mSATA card detection by
software in U-Boot SPL. Allow to override that U-Boot SPL detection by a
new "omnia_msata_slot" env variable (to value "pcie" or "sata") so broken
MiniPCIe cards can be used in combo mSATA/MiniPCIe slot too.

As configuration of PCIe vs SATA functionality is done in U-Boot SPL,
it is required to change env variable in permanent storage and reset the
board to take effect.

To force PCIe mode for broken MiniPCIe cards, call U-Boot commands:

  => setenv omnia_msata_slot pcie
  => saveenv
  => reset

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 27 ++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index d4c41bb1797a..6a057ea7dd70 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -207,13 +207,25 @@ static bool disable_mcu_watchdog(void)
 	return true;
 }
 
-static bool omnia_detect_sata(void)
+static bool omnia_detect_sata(const char *msata_slot)
 {
 	int ret;
 	u16 stsword;
 
 	puts("MiniPCIe/mSATA card detection... ");
 
+	if (msata_slot) {
+		if (strcmp(msata_slot, "pcie") == 0) {
+			puts("forced to MiniPCIe via env\n");
+			return false;
+		} else if (strcmp(msata_slot, "sata") == 0) {
+			puts("forced to mSATA via env\n");
+			return true;
+		} else if (strcmp(msata_slot, "auto") != 0) {
+			printf("unsupported env value '%s', fallback to... ", msata_slot);
+		}
+	}
+
 	ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &stsword, sizeof(stsword));
 	if (ret) {
 		printf("omnia_mcu_read failed: %i, defaulting to MiniPCIe card\n",
@@ -246,7 +258,18 @@ void *env_sf_get_env_addr(void)
 
 int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 {
-	if (omnia_detect_sata()) {
+#ifdef CONFIG_SPL_ENV_SUPPORT
+	/* Do not use env_load() as malloc() pool is too small at this stage */
+	bool has_env = (env_init() == 0);
+#endif
+	const char *env_value = NULL;
+
+#ifdef CONFIG_SPL_ENV_SUPPORT
+	/* beware that env_get() returns static allocated memory */
+	env_value = has_env ? env_get("omnia_msata_slot") : NULL;
+#endif
+
+	if (omnia_detect_sata(env_value)) {
 		/* Change SerDes for first mPCIe port (mSATA) from PCIe to SATA */
 		board_serdes_map[0].serdes_type = SATA0;
 		board_serdes_map[0].serdes_speed = SERDES_SPEED_6_GBPS;
-- 
2.20.1

[PATCH u-boot-marvell 6/8] arm: mvebu: turris_omnia: Extract code for disabling sata/pcie

[Pali Rohár]

Move code for disabling sata and pcie DT nodes to own functions, so this
code can be called from other places in follow up patches.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 96 +++++++++++++-----------
 1 file changed, 52 insertions(+), 44 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 6a057ea7dd70..57b797db4a8e 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -518,11 +518,54 @@ void spl_board_init(void)
 
 #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP)
 
-static void fixup_serdes_0_nodes(void *blob)
+static void disable_sata_node(void *blob)
 {
-	bool mode_sata;
 	int node;
 
+	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-380-ahci") {
+		if (!fdtdec_get_is_enabled(blob, node))
+			continue;
+
+		if (fdt_status_disabled(blob, node) < 0)
+			printf("Cannot disable SATA DT node!\n");
+		else
+			debug("Disabled SATA DT node\n");
+
+		break;
+	}
+}
+
+static void disable_pcie_node(void *blob, int port)
+{
+	int node;
+
+	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-370-pcie") {
+		int port_node;
+
+		if (!fdtdec_get_is_enabled(blob, node))
+			continue;
+
+		fdt_for_each_subnode (port_node, blob, node) {
+			if (!fdtdec_get_is_enabled(blob, port_node))
+				continue;
+
+			if (fdtdec_get_int(blob, port_node, "marvell,pcie-port", -1) != port)
+				continue;
+
+			if (fdt_status_disabled(blob, port_node) < 0)
+				printf("Cannot disable PCIe port %d DT node!\n", port);
+			else
+				debug("Disabled PCIe port %d DT node\n", port);
+
+			return;
+		}
+	}
+}
+
+static void fixup_msata_port_nodes(void *blob)
+{
+	bool mode_sata;
+
 	/*
 	 * Determine if SerDes 0 is configured to SATA mode.
 	 * We do this instead of calling omnia_detect_sata() to avoid another
@@ -541,47 +584,12 @@ static void fixup_serdes_0_nodes(void *blob)
 		return;
 	}
 
-	/* If mSATA card is not present, disable SATA DT node */
 	if (!mode_sata) {
-		fdt_for_each_node_by_compatible(node, blob, -1,
-						"marvell,armada-380-ahci") {
-			if (!fdtdec_get_is_enabled(blob, node))
-				continue;
-
-			if (fdt_status_disabled(blob, node) < 0)
-				printf("Cannot disable SATA DT node!\n");
-			else
-				debug("Disabled SATA DT node\n");
-
-			break;
-		}
-
-		return;
-	}
-
-	/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
-	fdt_for_each_node_by_compatible(node, blob, -1,
-					"marvell,armada-370-pcie") {
-		int port;
-
-		if (!fdtdec_get_is_enabled(blob, node))
-			continue;
-
-		fdt_for_each_subnode (port, blob, node) {
-			if (!fdtdec_get_is_enabled(blob, port))
-				continue;
-
-			if (fdtdec_get_int(blob, port, "marvell,pcie-port",
-					   -1) != 0)
-				continue;
-
-			if (fdt_status_disabled(blob, port) < 0)
-				printf("Cannot disable PCIe port 0 DT node!\n");
-			else
-				debug("Disabled PCIe port 0 DT node\n");
-
-			return;
-		}
+		/* If mSATA card is not present, disable SATA DT node */
+		disable_sata_node(blob);
+	} else {
+		/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
+		disable_pcie_node(blob, 0);
 	}
 }
 
@@ -590,7 +598,7 @@ static void fixup_serdes_0_nodes(void *blob)
 #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP)
 int board_fix_fdt(void *blob)
 {
-	fixup_serdes_0_nodes(blob);
+	fixup_msata_port_nodes(blob);
 
 	return 0;
 }
@@ -828,7 +836,7 @@ fail:
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
 	fixup_spi_nor_partitions(blob);
-	fixup_serdes_0_nodes(blob);
+	fixup_msata_port_nodes(blob);
 
 	return 0;
 }
-- 
2.20.1

[PATCH u-boot-marvell 7/8] arm: mvebu: turris_omnia: Signal error when sata/pcie DT mode

[Pali Rohár]

Show error message when DT file does not contain sata or pcie node which
should be explicitly disabled. This can happen when U-Boot code for finding
those nodes is incomplete or when those DT nodes are in different
unexpected location. In any case it is needed to know if DT not was not
explicitly disabled as it could mean that combo slots where setup
incorrectly.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 57b797db4a8e..e2f4daa827ed 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -531,8 +531,10 @@ static void disable_sata_node(void *blob)
 		else
 			debug("Disabled SATA DT node\n");
 
-		break;
+		return;
 	}
+
+	printf("Cannot find SATA DT node!\n");
 }
 
 static void disable_pcie_node(void *blob, int port)
@@ -560,6 +562,8 @@ static void disable_pcie_node(void *blob, int port)
 			return;
 		}
 	}
+
+	printf("Cannot find PCIe port %d DT node!\n", port);
 }
 
 static void fixup_msata_port_nodes(void *blob)
-- 
2.20.1

[PATCH u-boot-marvell 8/8] arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe slot

[Pali Rohár]

PCIe Mini CEM 2.1 spec added support for USB3.0 mode on MiniPCIe cards.
USB3.0 and PCIe share same pins and only one function can be active at the
same time. PCIe Mini CEM 2.1 spec says that determining function is
platform specific and spec does not define any dedicated pin which could
say if card is USB3.0-based or PCIe-based.

Implement this platform specific decision (USB3.0 vs PCIe) for WWAN
MiniPCIe slot on Turris Omnia via U-Boot env variable "omnia_wwan_slot",
similarly like is implemented forced mode for MiniPCIe/mSATA slot via
"omnia_msata_slot" env variable. Value "usb3" for "omnia_wwan_slot" would
mean to set USB3.0 mode and value "pcie" original PCIe mode.

A385 SoC on Turris Omnia has configurable fifth SerDes line (exported to
MiniPCIe WWAN slot with SIM card) either to USB3.0 or PCIe functionality,
so implementation of this new PCIe Mini CEM 2.1 feature is simple, by just
configuring SerDes to USB 3.0 mode.

Other twos MiniPCIe slots on Turris Omnia do not have this new
functionality as their SerDes lines cannot be switched to USB3.0
functionality.

Note that A385 SoC does not have too many USB3.0 blocks, so activating
USB3.0 in MiniPCIe cause that one external USB3.0 USB-A port would loose
USB3.0 functionality and would be downgraded just to USB2.0.

By default this MiniPCIe WWAN slot is in PCIe mode, like before.

To set this MiniPCIe WWAN slot to USB3.0 mode, call U-Boot commands:

  => setenv omnia_wwan_slot usb3
  => saveenv
  => reset

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 board/CZ.NIC/turris_omnia/turris_omnia.c | 57 ++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
index e2f4daa827ed..83cfc80d1930 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -246,6 +246,22 @@ static bool omnia_detect_sata(const char *msata_slot)
 	return stsword & MSATA_IND_STSBIT ? true : false;
 }
 
+static bool omnia_detect_wwan_usb3(const char *wwan_slot)
+{
+	puts("WWAN slot configuration... ");
+
+	if (wwan_slot && strcmp(wwan_slot, "usb3") == 0) {
+		puts("USB3.0\n");
+		return true;
+	}
+
+	if (wwan_slot && strcmp(wwan_slot, "pcie") != 0)
+		printf("unsupported env value '%s', fallback to... ", wwan_slot);
+
+	puts("PCIe+USB2.0\n");
+	return false;
+}
+
 void *env_sf_get_env_addr(void)
 {
 	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
@@ -276,6 +292,20 @@ int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
 		board_serdes_map[0].serdes_mode = SERDES_DEFAULT_MODE;
 	}
 
+#ifdef CONFIG_SPL_ENV_SUPPORT
+	/* beware that env_get() returns static allocated memory */
+	env_value = has_env ? env_get("omnia_wwan_slot") : NULL;
+#endif
+
+	if (omnia_detect_wwan_usb3(env_value)) {
+		/* Disable SerDes for USB 3.0 pins on the front USB-A port */
+		board_serdes_map[1].serdes_type = DEFAULT_SERDES;
+		/* Change SerDes for third mPCIe port (WWAN) from PCIe to USB 3.0 */
+		board_serdes_map[4].serdes_type = USB3_HOST0;
+		board_serdes_map[4].serdes_speed = SERDES_SPEED_5_GBPS;
+		board_serdes_map[4].serdes_mode = SERDES_DEFAULT_MODE;
+	}
+
 	*serdes_map_array = board_serdes_map;
 	*count = ARRAY_SIZE(board_serdes_map);
 
@@ -597,12 +627,38 @@ static void fixup_msata_port_nodes(void *blob)
 	}
 }
 
+static void fixup_wwan_port_nodes(void *blob)
+{
+	bool mode_usb3;
+
+	/* Determine if SerDes 4 is configured to USB3 mode */
+	mode_usb3 = ((readl(MVEBU_REGISTER(0x183fc)) & GENMASK(19, 16)) >> 16) == 4;
+
+	/* If SerDes 4 is not configured to USB3 mode then nothing is needed to fixup */
+	if (!mode_usb3)
+		return;
+
+	/*
+	 * We're either adding status = "disabled" property, or changing
+	 * status = "okay" to status = "disabled". In both cases we'll need more
+	 * space. Increase the size a little.
+	 */
+	if (fdt_increase_size(blob, 32) < 0) {
+		printf("Cannot increase FDT size!\n");
+		return;
+	}
+
+	/* Disable PCIe port 2 DT node (WWAN) */
+	disable_pcie_node(blob, 2);
+}
+
 #endif
 
 #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP)
 int board_fix_fdt(void *blob)
 {
 	fixup_msata_port_nodes(blob);
+	fixup_wwan_port_nodes(blob);
 
 	return 0;
 }
@@ -841,6 +897,7 @@ int ft_board_setup(void *blob, struct bd_info *bd)
 {
 	fixup_spi_nor_partitions(blob);
 	fixup_msata_port_nodes(blob);
+	fixup_wwan_port_nodes(blob);
 
 	return 0;
 }
-- 
2.20.1

Re: [PATCH u-boot-marvell 5/8] arm: mvebu: turris_omnia: Allow to configure mSATA slot via env variable

[Marek Behún]

>  {
> -	if (omnia_detect_sata()) {
> +#ifdef CONFIG_SPL_ENV_SUPPORT
> +	/* Do not use env_load() as malloc() pool is too small at this stage */
> +	bool has_env = (env_init() == 0);
> +#endif
> +	const char *env_value = NULL;
> +
> +#ifdef CONFIG_SPL_ENV_SUPPORT
> +	/* beware that env_get() returns static allocated memory */
> +	env_value = has_env ? env_get("omnia_msata_slot") : NULL;
> +#endif

Can we use
  if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT))
instead of macros here?

Marek

Re: [PATCH u-boot-marvell 2/8] arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function

[Marek Behún]

On Wed,  2 Mar 2022 12:47:52 +0100
Pali Rohár <pali@kernel.org> wrote:

> BootROM maps SPI Flash to fixed address 0xD4000000 and this mapping is
> active also when BootROM is executing binary kwbimage headers, which
> includes also U-Boot SPL.
> 
> Therefore no initialization code is required to access SPI Flags from
> U-Boot SPL. In proper U-Boot it is remapped to other location.
> 
> So in mvebu implementation of env_sf_get_env_addr() function returns
> 0xD4000000 when running in SPL and NULL when in proper U-Boot.
> 
> This change would allow to use U-Boot ENV in U-Boot SPL. Normally it is not
> possible to read ENV because it is too big and U-Boot SPL does not have
> such big malloc() pool to real all ENV variables.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  board/CZ.NIC/turris_omnia/turris_omnia.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 33cec6587e19..a93af6c5b877 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -243,6 +243,16 @@ static bool omnia_detect_sata(void)
>  	return stsword & MSATA_IND_STSBIT ? true : false;
>  }
>  
> +void *env_sf_get_env_addr(void)
> +{
> +	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> +#ifdef CONFIG_SPL_BUILD
> +	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> +#else
> +	return NULL;
> +#endif

if (IS_ENABLED(CONFIG_SPL_BUILD))
	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
else
	return NULL;

Marek

Re: [PATCH u-boot-marvell 6/8] arm: mvebu: turris_omnia: Extract code for disabling sata/pcie

[Marek Behún]

On Wed,  2 Mar 2022 12:47:56 +0100
Pali Rohár <pali@kernel.org> wrote:

> Move code for disabling sata and pcie DT nodes to own functions, so this
> code can be called from other places in follow up patches.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  board/CZ.NIC/turris_omnia/turris_omnia.c | 96 +++++++++++++-----------
>  1 file changed, 52 insertions(+), 44 deletions(-)
> 
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index 6a057ea7dd70..57b797db4a8e 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -518,11 +518,54 @@ void spl_board_init(void)
>  
>  #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP)
>  
> -static void fixup_serdes_0_nodes(void *blob)
> +static void disable_sata_node(void *blob)
>  {
> -	bool mode_sata;
>  	int node;
>  
> +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-380-ahci") {
> +		if (!fdtdec_get_is_enabled(blob, node))
> +			continue;
> +
> +		if (fdt_status_disabled(blob, node) < 0)
> +			printf("Cannot disable SATA DT node!\n");
> +		else
> +			debug("Disabled SATA DT node\n");
> +
> +		break;
> +	}
> +}
> +
> +static void disable_pcie_node(void *blob, int port)
> +{
> +	int node;
> +
> +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-370-pcie") {
> +		int port_node;
> +
> +		if (!fdtdec_get_is_enabled(blob, node))
> +			continue;
> +
> +		fdt_for_each_subnode (port_node, blob, node) {
> +			if (!fdtdec_get_is_enabled(blob, port_node))
> +				continue;
> +
> +			if (fdtdec_get_int(blob, port_node, "marvell,pcie-port", -1) != port)
> +				continue;
> +
> +			if (fdt_status_disabled(blob, port_node) < 0)
> +				printf("Cannot disable PCIe port %d DT node!\n", port);
> +			else
> +				debug("Disabled PCIe port %d DT node\n", port);
> +
> +			return;
> +		}
> +	}
> +}
> +
> +static void fixup_msata_port_nodes(void *blob)
> +{
> +	bool mode_sata;
> +
>  	/*
>  	 * Determine if SerDes 0 is configured to SATA mode.
>  	 * We do this instead of calling omnia_detect_sata() to avoid another
> @@ -541,47 +584,12 @@ static void fixup_serdes_0_nodes(void *blob)
>  		return;
>  	}
>  
> -	/* If mSATA card is not present, disable SATA DT node */
>  	if (!mode_sata) {
> -		fdt_for_each_node_by_compatible(node, blob, -1,
> -						"marvell,armada-380-ahci") {
> -			if (!fdtdec_get_is_enabled(blob, node))
> -				continue;
> -
> -			if (fdt_status_disabled(blob, node) < 0)
> -				printf("Cannot disable SATA DT node!\n");
> -			else
> -				debug("Disabled SATA DT node\n");
> -
> -			break;
> -		}
> -
> -		return;
> -	}
> -
> -	/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
> -	fdt_for_each_node_by_compatible(node, blob, -1,
> -					"marvell,armada-370-pcie") {
> -		int port;
> -
> -		if (!fdtdec_get_is_enabled(blob, node))
> -			continue;
> -
> -		fdt_for_each_subnode (port, blob, node) {
> -			if (!fdtdec_get_is_enabled(blob, port))
> -				continue;
> -
> -			if (fdtdec_get_int(blob, port, "marvell,pcie-port",
> -					   -1) != 0)
> -				continue;
> -
> -			if (fdt_status_disabled(blob, port) < 0)
> -				printf("Cannot disable PCIe port 0 DT node!\n");
> -			else
> -				debug("Disabled PCIe port 0 DT node\n");
> -
> -			return;
> -		}
> +		/* If mSATA card is not present, disable SATA DT node */
> +		disable_sata_node(blob);
> +	} else {
> +		/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
> +		disable_pcie_node(blob, 0);
>  	}

If I am looking at this correctly, there is only one statement both in
if and in else clause. Can we drop the {} parentheses?

Marek

Re: [PATCH u-boot-marvell 8/8] arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe slot

[Marek Behún]

On Wed,  2 Mar 2022 12:47:58 +0100
Pali Rohár <pali@kernel.org> wrote:

> PCIe Mini CEM 2.1 spec added support for USB3.0 mode on MiniPCIe cards.
> USB3.0 and PCIe share same pins and only one function can be active at the
> same time. PCIe Mini CEM 2.1 spec says that determining function is
> platform specific and spec does not define any dedicated pin which could
> say if card is USB3.0-based or PCIe-based.
> 
> Implement this platform specific decision (USB3.0 vs PCIe) for WWAN
> MiniPCIe slot on Turris Omnia via U-Boot env variable "omnia_wwan_slot",
> similarly like is implemented forced mode for MiniPCIe/mSATA slot via
> "omnia_msata_slot" env variable. Value "usb3" for "omnia_wwan_slot" would
> mean to set USB3.0 mode and value "pcie" original PCIe mode.
> 
> A385 SoC on Turris Omnia has configurable fifth SerDes line (exported to
> MiniPCIe WWAN slot with SIM card) either to USB3.0 or PCIe functionality,
> so implementation of this new PCIe Mini CEM 2.1 feature is simple, by just
> configuring SerDes to USB 3.0 mode.
> 
> Other twos MiniPCIe slots on Turris Omnia do not have this new
> functionality as their SerDes lines cannot be switched to USB3.0
> functionality.
> 
> Note that A385 SoC does not have too many USB3.0 blocks, so activating
> USB3.0 in MiniPCIe cause that one external USB3.0 USB-A port would loose
> USB3.0 functionality and would be downgraded just to USB2.0.
> 
> By default this MiniPCIe WWAN slot is in PCIe mode, like before.
> 
> To set this MiniPCIe WWAN slot to USB3.0 mode, call U-Boot commands:
> 
>   => setenv omnia_wwan_slot usb3
>   => saveenv
>   => reset  
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  board/CZ.NIC/turris_omnia/turris_omnia.c | 57 ++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> index e2f4daa827ed..83cfc80d1930 100644
> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> @@ -246,6 +246,22 @@ static bool omnia_detect_sata(const char *msata_slot)
>  	return stsword & MSATA_IND_STSBIT ? true : false;
>  }
>  
> +static bool omnia_detect_wwan_usb3(const char *wwan_slot)
> +{
> +	puts("WWAN slot configuration... ");
> +
> +	if (wwan_slot && strcmp(wwan_slot, "usb3") == 0) {
> +		puts("USB3.0\n");
> +		return true;
> +	}
> +
> +	if (wwan_slot && strcmp(wwan_slot, "pcie") != 0)
> +		printf("unsupported env value '%s', fallback to... ", wwan_slot);

If  I recall correctly, Linux' and U-Boot's code style (in contrast to
TF-A) normally wants
  if (expr)        instead of       if (expr != 0)
and
  if (!expr)       instead of       if (expr == 0)

Sometimes in spite of this style it is better to use == or != operator,
for example in cases where one wants to compare a character to multiple
literals:
  if (c == 0x13 || c == 0x12 || c == 0x00)

But for strcmp() and friends, we normaly just use
  if (!strcmp())

This is just a nitpick, I don't mind that much. Just saying this so
you are aware of this in the future...


> +
> +	puts("PCIe+USB2.0\n");
> +	return false;
> +}
> +
>  void *env_sf_get_env_addr(void)
>  {
>  	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> @@ -276,6 +292,20 @@ int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
>  		board_serdes_map[0].serdes_mode = SERDES_DEFAULT_MODE;
>  	}
>  
> +#ifdef CONFIG_SPL_ENV_SUPPORT
> +	/* beware that env_get() returns static allocated memory */
> +	env_value = has_env ? env_get("omnia_wwan_slot") : NULL;
> +#endif

Use
  if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT))
instead of macro wherever possible, so that an error can be caught by
the compiler even if it is in the disabled part of the code.

Marek

Re: [PATCH u-boot-marvell 6/8] arm: mvebu: turris_omnia: Extract code for disabling sata/pcie

[Pali Rohár]

On Wednesday 02 March 2022 13:38:48 Marek Behún wrote:
> On Wed,  2 Mar 2022 12:47:56 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
> > Move code for disabling sata and pcie DT nodes to own functions, so this
> > code can be called from other places in follow up patches.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  board/CZ.NIC/turris_omnia/turris_omnia.c | 96 +++++++++++++-----------
> >  1 file changed, 52 insertions(+), 44 deletions(-)
> > 
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index 6a057ea7dd70..57b797db4a8e 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -518,11 +518,54 @@ void spl_board_init(void)
> >  
> >  #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP)
> >  
> > -static void fixup_serdes_0_nodes(void *blob)
> > +static void disable_sata_node(void *blob)
> >  {
> > -	bool mode_sata;
> >  	int node;
> >  
> > +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-380-ahci") {
> > +		if (!fdtdec_get_is_enabled(blob, node))
> > +			continue;
> > +
> > +		if (fdt_status_disabled(blob, node) < 0)
> > +			printf("Cannot disable SATA DT node!\n");
> > +		else
> > +			debug("Disabled SATA DT node\n");
> > +
> > +		break;
> > +	}
> > +}
> > +
> > +static void disable_pcie_node(void *blob, int port)
> > +{
> > +	int node;
> > +
> > +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-370-pcie") {
> > +		int port_node;
> > +
> > +		if (!fdtdec_get_is_enabled(blob, node))
> > +			continue;
> > +
> > +		fdt_for_each_subnode (port_node, blob, node) {
> > +			if (!fdtdec_get_is_enabled(blob, port_node))
> > +				continue;
> > +
> > +			if (fdtdec_get_int(blob, port_node, "marvell,pcie-port", -1) != port)
> > +				continue;
> > +
> > +			if (fdt_status_disabled(blob, port_node) < 0)
> > +				printf("Cannot disable PCIe port %d DT node!\n", port);
> > +			else
> > +				debug("Disabled PCIe port %d DT node\n", port);
> > +
> > +			return;
> > +		}
> > +	}
> > +}
> > +
> > +static void fixup_msata_port_nodes(void *blob)
> > +{
> > +	bool mode_sata;
> > +
> >  	/*
> >  	 * Determine if SerDes 0 is configured to SATA mode.
> >  	 * We do this instead of calling omnia_detect_sata() to avoid another
> > @@ -541,47 +584,12 @@ static void fixup_serdes_0_nodes(void *blob)
> >  		return;
> >  	}
> >  
> > -	/* If mSATA card is not present, disable SATA DT node */
> >  	if (!mode_sata) {
> > -		fdt_for_each_node_by_compatible(node, blob, -1,
> > -						"marvell,armada-380-ahci") {
> > -			if (!fdtdec_get_is_enabled(blob, node))
> > -				continue;
> > -
> > -			if (fdt_status_disabled(blob, node) < 0)
> > -				printf("Cannot disable SATA DT node!\n");
> > -			else
> > -				debug("Disabled SATA DT node\n");
> > -
> > -			break;
> > -		}
> > -
> > -		return;
> > -	}
> > -
> > -	/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
> > -	fdt_for_each_node_by_compatible(node, blob, -1,
> > -					"marvell,armada-370-pcie") {
> > -		int port;
> > -
> > -		if (!fdtdec_get_is_enabled(blob, node))
> > -			continue;
> > -
> > -		fdt_for_each_subnode (port, blob, node) {
> > -			if (!fdtdec_get_is_enabled(blob, port))
> > -				continue;
> > -
> > -			if (fdtdec_get_int(blob, port, "marvell,pcie-port",
> > -					   -1) != 0)
> > -				continue;
> > -
> > -			if (fdt_status_disabled(blob, port) < 0)
> > -				printf("Cannot disable PCIe port 0 DT node!\n");
> > -			else
> > -				debug("Disabled PCIe port 0 DT node\n");
> > -
> > -			return;
> > -		}
> > +		/* If mSATA card is not present, disable SATA DT node */
> > +		disable_sata_node(blob);
> > +	} else {
> > +		/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
> > +		disable_pcie_node(blob, 0);
> >  	}
> 
> If I am looking at this correctly, there is only one statement both in
> if and in else clause. Can we drop the {} parentheses?
> 
> Marek

Due to comment above which belongs to the code, I put there {} parentheses.

Re: [PATCH u-boot-marvell 6/8] arm: mvebu: turris_omnia: Extract code for disabling sata/pcie

[Stefan Roese]

On 4/22/22 11:20, Pali Rohár wrote:
> On Wednesday 02 March 2022 13:38:48 Marek Behún wrote:
>> On Wed,  2 Mar 2022 12:47:56 +0100
>> Pali Rohár <pali@kernel.org> wrote:
>>
>>> Move code for disabling sata and pcie DT nodes to own functions, so this
>>> code can be called from other places in follow up patches.
>>>
>>> Signed-off-by: Pali Rohár <pali@kernel.org>
>>> ---
>>>   board/CZ.NIC/turris_omnia/turris_omnia.c | 96 +++++++++++++-----------
>>>   1 file changed, 52 insertions(+), 44 deletions(-)
>>>
>>> diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> index 6a057ea7dd70..57b797db4a8e 100644
>>> --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
>>> @@ -518,11 +518,54 @@ void spl_board_init(void)
>>>   
>>>   #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP)
>>>   
>>> -static void fixup_serdes_0_nodes(void *blob)
>>> +static void disable_sata_node(void *blob)
>>>   {
>>> -	bool mode_sata;
>>>   	int node;
>>>   
>>> +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-380-ahci") {
>>> +		if (!fdtdec_get_is_enabled(blob, node))
>>> +			continue;
>>> +
>>> +		if (fdt_status_disabled(blob, node) < 0)
>>> +			printf("Cannot disable SATA DT node!\n");
>>> +		else
>>> +			debug("Disabled SATA DT node\n");
>>> +
>>> +		break;
>>> +	}
>>> +}
>>> +
>>> +static void disable_pcie_node(void *blob, int port)
>>> +{
>>> +	int node;
>>> +
>>> +	fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-370-pcie") {
>>> +		int port_node;
>>> +
>>> +		if (!fdtdec_get_is_enabled(blob, node))
>>> +			continue;
>>> +
>>> +		fdt_for_each_subnode (port_node, blob, node) {
>>> +			if (!fdtdec_get_is_enabled(blob, port_node))
>>> +				continue;
>>> +
>>> +			if (fdtdec_get_int(blob, port_node, "marvell,pcie-port", -1) != port)
>>> +				continue;
>>> +
>>> +			if (fdt_status_disabled(blob, port_node) < 0)
>>> +				printf("Cannot disable PCIe port %d DT node!\n", port);
>>> +			else
>>> +				debug("Disabled PCIe port %d DT node\n", port);
>>> +
>>> +			return;
>>> +		}
>>> +	}
>>> +}
>>> +
>>> +static void fixup_msata_port_nodes(void *blob)
>>> +{
>>> +	bool mode_sata;
>>> +
>>>   	/*
>>>   	 * Determine if SerDes 0 is configured to SATA mode.
>>>   	 * We do this instead of calling omnia_detect_sata() to avoid another
>>> @@ -541,47 +584,12 @@ static void fixup_serdes_0_nodes(void *blob)
>>>   		return;
>>>   	}
>>>   
>>> -	/* If mSATA card is not present, disable SATA DT node */
>>>   	if (!mode_sata) {
>>> -		fdt_for_each_node_by_compatible(node, blob, -1,
>>> -						"marvell,armada-380-ahci") {
>>> -			if (!fdtdec_get_is_enabled(blob, node))
>>> -				continue;
>>> -
>>> -			if (fdt_status_disabled(blob, node) < 0)
>>> -				printf("Cannot disable SATA DT node!\n");
>>> -			else
>>> -				debug("Disabled SATA DT node\n");
>>> -
>>> -			break;
>>> -		}
>>> -
>>> -		return;
>>> -	}
>>> -
>>> -	/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
>>> -	fdt_for_each_node_by_compatible(node, blob, -1,
>>> -					"marvell,armada-370-pcie") {
>>> -		int port;
>>> -
>>> -		if (!fdtdec_get_is_enabled(blob, node))
>>> -			continue;
>>> -
>>> -		fdt_for_each_subnode (port, blob, node) {
>>> -			if (!fdtdec_get_is_enabled(blob, port))
>>> -				continue;
>>> -
>>> -			if (fdtdec_get_int(blob, port, "marvell,pcie-port",
>>> -					   -1) != 0)
>>> -				continue;
>>> -
>>> -			if (fdt_status_disabled(blob, port) < 0)
>>> -				printf("Cannot disable PCIe port 0 DT node!\n");
>>> -			else
>>> -				debug("Disabled PCIe port 0 DT node\n");
>>> -
>>> -			return;
>>> -		}
>>> +		/* If mSATA card is not present, disable SATA DT node */
>>> +		disable_sata_node(blob);
>>> +	} else {
>>> +		/* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
>>> +		disable_pcie_node(blob, 0);
>>>   	}
>>
>> If I am looking at this correctly, there is only one statement both in
>> if and in else clause. Can we drop the {} parentheses?
>>
>> Marek
> 
> Due to comment above which belongs to the code, I put there {} parentheses.

Agreed. I am also in favor of putting parentheses on multi-line
statements (not depending on single- or multi-statement).

Thanks,
Stefan

Re: [PATCH u-boot-marvell 8/8] arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe slot

[Pali Rohár]

On Wednesday 02 March 2022 13:46:07 Marek Behún wrote:
> On Wed,  2 Mar 2022 12:47:58 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
> > PCIe Mini CEM 2.1 spec added support for USB3.0 mode on MiniPCIe cards.
> > USB3.0 and PCIe share same pins and only one function can be active at the
> > same time. PCIe Mini CEM 2.1 spec says that determining function is
> > platform specific and spec does not define any dedicated pin which could
> > say if card is USB3.0-based or PCIe-based.
> > 
> > Implement this platform specific decision (USB3.0 vs PCIe) for WWAN
> > MiniPCIe slot on Turris Omnia via U-Boot env variable "omnia_wwan_slot",
> > similarly like is implemented forced mode for MiniPCIe/mSATA slot via
> > "omnia_msata_slot" env variable. Value "usb3" for "omnia_wwan_slot" would
> > mean to set USB3.0 mode and value "pcie" original PCIe mode.
> > 
> > A385 SoC on Turris Omnia has configurable fifth SerDes line (exported to
> > MiniPCIe WWAN slot with SIM card) either to USB3.0 or PCIe functionality,
> > so implementation of this new PCIe Mini CEM 2.1 feature is simple, by just
> > configuring SerDes to USB 3.0 mode.
> > 
> > Other twos MiniPCIe slots on Turris Omnia do not have this new
> > functionality as their SerDes lines cannot be switched to USB3.0
> > functionality.
> > 
> > Note that A385 SoC does not have too many USB3.0 blocks, so activating
> > USB3.0 in MiniPCIe cause that one external USB3.0 USB-A port would loose
> > USB3.0 functionality and would be downgraded just to USB2.0.
> > 
> > By default this MiniPCIe WWAN slot is in PCIe mode, like before.
> > 
> > To set this MiniPCIe WWAN slot to USB3.0 mode, call U-Boot commands:
> > 
> >   => setenv omnia_wwan_slot usb3
> >   => saveenv
> >   => reset  
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  board/CZ.NIC/turris_omnia/turris_omnia.c | 57 ++++++++++++++++++++++++
> >  1 file changed, 57 insertions(+)
> > 
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index e2f4daa827ed..83cfc80d1930 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -246,6 +246,22 @@ static bool omnia_detect_sata(const char *msata_slot)
> >  	return stsword & MSATA_IND_STSBIT ? true : false;
> >  }
> >  
> > +static bool omnia_detect_wwan_usb3(const char *wwan_slot)
> > +{
> > +	puts("WWAN slot configuration... ");
> > +
> > +	if (wwan_slot && strcmp(wwan_slot, "usb3") == 0) {
> > +		puts("USB3.0\n");
> > +		return true;
> > +	}
> > +
> > +	if (wwan_slot && strcmp(wwan_slot, "pcie") != 0)
> > +		printf("unsupported env value '%s', fallback to... ", wwan_slot);
> 
> If  I recall correctly, Linux' and U-Boot's code style (in contrast to
> TF-A) normally wants
>   if (expr)        instead of       if (expr != 0)
> and
>   if (!expr)       instead of       if (expr == 0)

I guess that this applies for functions which return boolean value. And
not for strcmp() function which is not failure expression. But instead
it returns comparison value.

> Sometimes in spite of this style it is better to use == or != operator,
> for example in cases where one wants to compare a character to multiple
> literals:
>   if (c == 0x13 || c == 0x12 || c == 0x00)
> 
> But for strcmp() and friends, we normaly just use
>   if (!strcmp())
> 
> This is just a nitpick, I don't mind that much. Just saying this so
> you are aware of this in the future...
> 
> 
> > +
> > +	puts("PCIe+USB2.0\n");
> > +	return false;
> > +}
> > +
> >  void *env_sf_get_env_addr(void)
> >  {
> >  	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> > @@ -276,6 +292,20 @@ int hws_board_topology_load(struct serdes_map **serdes_map_array, u8 *count)
> >  		board_serdes_map[0].serdes_mode = SERDES_DEFAULT_MODE;
> >  	}
> >  
> > +#ifdef CONFIG_SPL_ENV_SUPPORT
> > +	/* beware that env_get() returns static allocated memory */
> > +	env_value = has_env ? env_get("omnia_wwan_slot") : NULL;
> > +#endif
> 
> Use
>   if (IS_ENABLED(CONFIG_SPL_ENV_SUPPORT))
> instead of macro wherever possible, so that an error can be caught by
> the compiler even if it is in the disabled part of the code.
> 
> Marek

Re: [PATCH u-boot-marvell 2/8] arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function

[Pali Rohár]

On Wednesday 02 March 2022 13:37:25 Marek Behún wrote:
> On Wed,  2 Mar 2022 12:47:52 +0100
> Pali Rohár <pali@kernel.org> wrote:
> 
> > BootROM maps SPI Flash to fixed address 0xD4000000 and this mapping is
> > active also when BootROM is executing binary kwbimage headers, which
> > includes also U-Boot SPL.
> > 
> > Therefore no initialization code is required to access SPI Flags from
> > U-Boot SPL. In proper U-Boot it is remapped to other location.
> > 
> > So in mvebu implementation of env_sf_get_env_addr() function returns
> > 0xD4000000 when running in SPL and NULL when in proper U-Boot.
> > 
> > This change would allow to use U-Boot ENV in U-Boot SPL. Normally it is not
> > possible to read ENV because it is too big and U-Boot SPL does not have
> > such big malloc() pool to real all ENV variables.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  board/CZ.NIC/turris_omnia/turris_omnia.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > index 33cec6587e19..a93af6c5b877 100644
> > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > @@ -243,6 +243,16 @@ static bool omnia_detect_sata(void)
> >  	return stsword & MSATA_IND_STSBIT ? true : false;
> >  }
> >  
> > +void *env_sf_get_env_addr(void)
> > +{
> > +	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> > +#ifdef CONFIG_SPL_BUILD
> > +	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> > +#else
> > +	return NULL;
> > +#endif
> 
> if (IS_ENABLED(CONFIG_SPL_BUILD))
> 	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> else
> 	return NULL;
> 
> Marek

This does not work. CONFIG_ENV_OFFSET is not defined.

All this code needs to be filtered out via preprocessor.

Re: [PATCH u-boot-marvell 8/8] arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe slot

[Marek Behún]

On Fri, 22 Apr 2022 13:47:33 +0200
Pali Rohár <pali@kernel.org> wrote:

> On Wednesday 02 March 2022 13:46:07 Marek Behún wrote:
> > On Wed,  2 Mar 2022 12:47:58 +0100
> > Pali Rohár <pali@kernel.org> wrote:
> >   
> > > PCIe Mini CEM 2.1 spec added support for USB3.0 mode on MiniPCIe cards.
> > > USB3.0 and PCIe share same pins and only one function can be active at the
> > > same time. PCIe Mini CEM 2.1 spec says that determining function is
> > > platform specific and spec does not define any dedicated pin which could
> > > say if card is USB3.0-based or PCIe-based.
> > > 
> > > Implement this platform specific decision (USB3.0 vs PCIe) for WWAN
> > > MiniPCIe slot on Turris Omnia via U-Boot env variable "omnia_wwan_slot",
> > > similarly like is implemented forced mode for MiniPCIe/mSATA slot via
> > > "omnia_msata_slot" env variable. Value "usb3" for "omnia_wwan_slot" would
> > > mean to set USB3.0 mode and value "pcie" original PCIe mode.
> > > 
> > > A385 SoC on Turris Omnia has configurable fifth SerDes line (exported to
> > > MiniPCIe WWAN slot with SIM card) either to USB3.0 or PCIe functionality,
> > > so implementation of this new PCIe Mini CEM 2.1 feature is simple, by just
> > > configuring SerDes to USB 3.0 mode.
> > > 
> > > Other twos MiniPCIe slots on Turris Omnia do not have this new
> > > functionality as their SerDes lines cannot be switched to USB3.0
> > > functionality.
> > > 
> > > Note that A385 SoC does not have too many USB3.0 blocks, so activating
> > > USB3.0 in MiniPCIe cause that one external USB3.0 USB-A port would loose
> > > USB3.0 functionality and would be downgraded just to USB2.0.
> > > 
> > > By default this MiniPCIe WWAN slot is in PCIe mode, like before.
> > > 
> > > To set this MiniPCIe WWAN slot to USB3.0 mode, call U-Boot commands:
> > >   
> > >   => setenv omnia_wwan_slot usb3
> > >   => saveenv
> > >   => reset    
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  board/CZ.NIC/turris_omnia/turris_omnia.c | 57 ++++++++++++++++++++++++
> > >  1 file changed, 57 insertions(+)
> > > 
> > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > index e2f4daa827ed..83cfc80d1930 100644
> > > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > @@ -246,6 +246,22 @@ static bool omnia_detect_sata(const char *msata_slot)
> > >  	return stsword & MSATA_IND_STSBIT ? true : false;
> > >  }
> > >  
> > > +static bool omnia_detect_wwan_usb3(const char *wwan_slot)
> > > +{
> > > +	puts("WWAN slot configuration... ");
> > > +
> > > +	if (wwan_slot && strcmp(wwan_slot, "usb3") == 0) {
> > > +		puts("USB3.0\n");
> > > +		return true;
> > > +	}
> > > +
> > > +	if (wwan_slot && strcmp(wwan_slot, "pcie") != 0)
> > > +		printf("unsupported env value '%s', fallback to... ", wwan_slot);  
> > 
> > If  I recall correctly, Linux' and U-Boot's code style (in contrast to
> > TF-A) normally wants
> >   if (expr)        instead of       if (expr != 0)
> > and
> >   if (!expr)       instead of       if (expr == 0)  
> 
> I guess that this applies for functions which return boolean value. And
> not for strcmp() function which is not failure expression. But instead
> it returns comparison value.

Again, this was an unimportant nitpick from me, feel free to ignore
this. But since you replied, I shall entertain you with an opinion I
have to share.

I am sure that I remember people from TF-A requesting to use
(x == 0) or (x != 0), while people from Linux requesting (x) or (!x),
not only for functions which return boolean value. I am not sure now
for what kind of function it may have been exactly, but I think it was
someting like
  err = func();
  if (err == 0)
    ..
and the request was to change it to
  if (!err)

I guess that functions which return 0 on success and negative error
code on failure may be considered returning "boolean" in a sense of
success/failure boolean.

But anyway if seems that in the usage of strcmp(), Linux uses both
variants, although the one with the comparison operator is used a
little bit less:
  $ git grep 'strcmp' | wc -l
  6971
  $ git grep 'strcmp.* [=!]= 0' | wc -l
  2100

Mostly the strcmp() function is used to determine whether two strings
are equal or not, which is a binary/boolean decision, and so I prefer to
check the result as such. But strcmp() can be also used to compare
strings for sorting, and that is the case where ==, < and > operators
would make sense to me.

Again, feel free to ignore, since this is a matter of personal
preference.

Marek

Re: [PATCH u-boot-marvell 2/8] arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function

[Marek Behún]

On Fri, 22 Apr 2022 13:49:43 +0200
Pali Rohár <pali@kernel.org> wrote:

> On Wednesday 02 March 2022 13:37:25 Marek Behún wrote:
> > On Wed,  2 Mar 2022 12:47:52 +0100
> > Pali Rohár <pali@kernel.org> wrote:
> >   
> > > BootROM maps SPI Flash to fixed address 0xD4000000 and this mapping is
> > > active also when BootROM is executing binary kwbimage headers, which
> > > includes also U-Boot SPL.
> > > 
> > > Therefore no initialization code is required to access SPI Flags from
> > > U-Boot SPL. In proper U-Boot it is remapped to other location.
> > > 
> > > So in mvebu implementation of env_sf_get_env_addr() function returns
> > > 0xD4000000 when running in SPL and NULL when in proper U-Boot.
> > > 
> > > This change would allow to use U-Boot ENV in U-Boot SPL. Normally it is not
> > > possible to read ENV because it is too big and U-Boot SPL does not have
> > > such big malloc() pool to real all ENV variables.
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > ---
> > >  board/CZ.NIC/turris_omnia/turris_omnia.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > index 33cec6587e19..a93af6c5b877 100644
> > > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
> > > @@ -243,6 +243,16 @@ static bool omnia_detect_sata(void)
> > >  	return stsword & MSATA_IND_STSBIT ? true : false;
> > >  }
> > >  
> > > +void *env_sf_get_env_addr(void)
> > > +{
> > > +	/* SPI Flash is mapped to address 0xD4000000 only in SPL */
> > > +#ifdef CONFIG_SPL_BUILD
> > > +	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> > > +#else
> > > +	return NULL;
> > > +#endif  
> > 
> > if (IS_ENABLED(CONFIG_SPL_BUILD))
> > 	return (void *)0xD4000000 + CONFIG_ENV_OFFSET;
> > else
> > 	return NULL;
> > 
> > Marek  
> 
> This does not work. CONFIG_ENV_OFFSET is not defined.
> 
> All this code needs to be filtered out via preprocessor.

OK.

In that case:

Reviewed-by: Marek Behún <marek.behun@nic.cz>

Re: [PATCH u-boot-marvell 0/8] Turris Omnia: Add support for configuring mSATA and WWAN slots via env variables

[Pali Rohár]

On Wednesday 02 March 2022 12:47:50 Pali Rohár wrote:
> Some mPCIe cards are broken and their PIN 43 incorrectly that card is
> mSATA. U-Boot SPL on Turris Omnia is using PIN 43 for configuring PCIe
> vs SATA functionality on SerDes. Allow to configure functionality via
> additional env variable "omnia_msata_slot" which may override PIN 43.
> 
> To force PCIe mode for broken MiniPCIe cards, call U-Boot commands:
> 
>   => setenv omnia_msata_slot pcie
>   => saveenv
>   => reset
> 
> 
> PCI-SIG in PCIe Mini CEM 2.1 spec added support for USB3.0 mode to mPCIe
> cards. PCIe and USB3.0 functions share same pins (like SATA pins on mSATA
> with PCIe on mPCIe) and therefore only one function may be active at the
> same time. This mode was introduced by PCI-SIG specially for LTE/5G modems.
> 
> One mPCIe slot on Turris Omnia is connected to A385 CPU SerDes which
> can be configured for PCIe or USB3.0 function. It is the slot with SIM
> card suitable for cellular modems. As PCIe Mini CEM 2.1 spec say that
> detection of PCIe vs USB3.0 functionality is platform specific, add a
> new U-Boot variable "omnia_wwan_slot" for configuring functionality in
> this slot. By default PCIe is used like before.
> 
> To set this WWAN slot to USB3.0 mode, call U-Boot commands:
> 
>   => setenv omnia_wwan_slot usb3
>   => saveenv
>   => reset
> 

PING? Any more comments on this patch series? It is there for two months.

> Pali Rohár (8):
>   env: sf: Allow to use env_sf_init_addr() at any stage
>   arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function
>   arm: mvebu: turris_omnia: Enable ENV support in SPL
>   arm: mvebu: turris_omnia: Define only one serdes map variable
>   arm: mvebu: turris_omnia: Allow to configure mSATA slot via env
>     variable
>   arm: mvebu: turris_omnia: Extract code for disabling sata/pcie
>   arm: mvebu: turris_omnia: Signal error when sata/pcie DT mode
>   arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe
>     slot
> 
>  board/CZ.NIC/turris_omnia/turris_omnia.c | 207 +++++++++++++++++------
>  configs/turris_omnia_defconfig           |   1 +
>  env/sf.c                                 |  22 +--
>  3 files changed, 163 insertions(+), 67 deletions(-)
> 
> -- 
> 2.20.1
> 

Re: [PATCH u-boot-marvell 0/8] Turris Omnia: Add support for configuring mSATA and WWAN slots via env variables

[Stefan Roese]

On 02.03.22 12:47, Pali Rohár wrote:
> Some mPCIe cards are broken and their PIN 43 incorrectly that card is
> mSATA. U-Boot SPL on Turris Omnia is using PIN 43 for configuring PCIe
> vs SATA functionality on SerDes. Allow to configure functionality via
> additional env variable "omnia_msata_slot" which may override PIN 43.
> 
> To force PCIe mode for broken MiniPCIe cards, call U-Boot commands:
> 
>    => setenv omnia_msata_slot pcie
>    => saveenv
>    => reset
> 
> 
> PCI-SIG in PCIe Mini CEM 2.1 spec added support for USB3.0 mode to mPCIe
> cards. PCIe and USB3.0 functions share same pins (like SATA pins on mSATA
> with PCIe on mPCIe) and therefore only one function may be active at the
> same time. This mode was introduced by PCI-SIG specially for LTE/5G modems.
> 
> One mPCIe slot on Turris Omnia is connected to A385 CPU SerDes which
> can be configured for PCIe or USB3.0 function. It is the slot with SIM
> card suitable for cellular modems. As PCIe Mini CEM 2.1 spec say that
> detection of PCIe vs USB3.0 functionality is platform specific, add a
> new U-Boot variable "omnia_wwan_slot" for configuring functionality in
> this slot. By default PCIe is used like before.
> 
> To set this WWAN slot to USB3.0 mode, call U-Boot commands:
> 
>    => setenv omnia_wwan_slot usb3
>    => saveenv
>    => reset
> 
> 
> Pali Rohár (8):
>    env: sf: Allow to use env_sf_init_addr() at any stage
>    arm: mvebu: turris_omnia: Provide env_sf_get_env_addr() function
>    arm: mvebu: turris_omnia: Enable ENV support in SPL
>    arm: mvebu: turris_omnia: Define only one serdes map variable
>    arm: mvebu: turris_omnia: Allow to configure mSATA slot via env
>      variable
>    arm: mvebu: turris_omnia: Extract code for disabling sata/pcie
>    arm: mvebu: turris_omnia: Signal error when sata/pcie DT mode
>    arm: mvebu: turris_omnia: Add support for USB3.0 mode in WWAN MiniPCIe
>      slot
> 
>   board/CZ.NIC/turris_omnia/turris_omnia.c | 207 +++++++++++++++++------
>   configs/turris_omnia_defconfig           |   1 +
>   env/sf.c                                 |  22 +--
>   3 files changed, 163 insertions(+), 67 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan