[PATCH u-boot-marvell 0/8] Turris Omnia: Add support for configuring mSATA and WWAN slots via env variables
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(-)
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 /*
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()) {
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
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.
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
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
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; }
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;
{
- 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
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; }
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");elsedebug("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);elsedebug("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");elsedebug("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");elsedebug("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
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");elsedebug("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);elsedebug("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");elsedebug("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");elsedebug("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.
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");elsedebug("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);elsedebug("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");elsedebug("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");elsedebug("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
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)
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; }
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
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
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
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
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
participants (3)
-
Marek Behún -
Pali Rohár -
Stefan Roese