On 3/27/24 17:23, Marek Behún wrote:

U-Boot builds for Turris Omnia are approaching the limit of 0xf0000 bytes, which is the size of the U-Boot partition on Omnia.

Enable LTO to get more size optimized binaries.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

configs/turris_omnia_defconfig | 1 + 1 file changed, 1 insertion(+)

diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig index 29148402a1..39e15043df 100644 --- a/configs/turris_omnia_defconfig +++ b/configs/turris_omnia_defconfig @@ -31,6 +31,7 @@ CONFIG_AHCI=y CONFIG_OF_BOARD_FIXUP=y CONFIG_SYS_MEMTEST_START=0x00800000 CONFIG_SYS_MEMTEST_END=0x00ffffff +CONFIG_LTO=y CONFIG_HAS_BOARD_SIZE_LIMIT=y CONFIG_BOARD_SIZE_LIMIT=983040 CONFIG_FIT=y

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Add header containing all MCU command definitions and use it in board code and in MCU driver.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_omnia/turris_omnia.c | 81 +++----- drivers/gpio/turris_omnia_mcu.c | 54 +---- include/turris-omnia-mcu-interface.h | 248 +++++++++++++++++++++++ 3 files changed, 272 insertions(+), 111 deletions(-) create mode 100644 include/turris-omnia-mcu-interface.h

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index adeb69a205..6c2d7da528 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -23,6 +23,7 @@ #include <fdt_support.h> #include <hexdump.h> #include <time.h> +#include <turris-omnia-mcu-interface.h> #include <linux/bitops.h> #include <linux/delay.h> #include <u-boot/crc.h> @@ -59,46 +60,6 @@ DECLARE_GLOBAL_DATA_PTR; #define A385_WD_RSTOUT_UNMASK MVEBU_REGISTER(0x20704) #define A385_WD_RSTOUT_UNMASK_GLOBAL BIT(8)

-enum mcu_commands {

• CMD_GET_STATUS_WORD = 0x01,
• CMD_GET_RESET = 0x09,
• CMD_GET_FW_VERSION_APP = 0x0a,
• CMD_WATCHDOG_STATE = 0x0b,
• CMD_GET_FW_VERSION_BOOT = 0x0e,
• /* available if STS_FEATURES_SUPPORTED bit set in status word */
• CMD_GET_FEATURES = 0x10,
• /* available if EXT_CMD bit set in features */
• CMD_EXT_CONTROL = 0x12,

-};

-enum status_word_bits {

• STS_MCU_TYPE_MASK = GENMASK(1, 0),
• STS_MCU_TYPE_STM32 = 0,
• STS_MCU_TYPE_GD32 = 1,
• STS_MCU_TYPE_MKL = 2,
• STS_MCU_TYPE_UNKN = 3,
• STS_FEATURES_SUPPORTED = BIT(2),
• CARD_DET_STSBIT = 0x0010,
• MSATA_IND_STSBIT = 0x0020,

-};

-/* CMD_GET_FEATURES */ -enum features_e {

• FEAT_PERIPH_MCU = BIT(0),
• FEAT_EXT_CMDS = BIT(1),

-};

-/* CMD_EXT_CONTROL */ -enum ext_ctl_e {

• EXT_CTL_nRES_LAN = BIT(1),
• EXT_CTL_nRES_PHY = BIT(2),
• EXT_CTL_nPERST0 = BIT(3),
• EXT_CTL_nPERST1 = BIT(4),
• EXT_CTL_nPERST2 = BIT(5),

-};

• /*
  • Those values and defines are taken from the Marvell U-Boot version
  • "u-boot-2013.01-2014_T3.0"

@@ -219,7 +180,7 @@ static bool disable_mcu_watchdog(void)

puts("Disabling MCU watchdog... ");

• ret = omnia_mcu_write(CMD_WATCHDOG_STATE, "\x00", 1);

• ret = omnia_mcu_write(CMD_SET_WATCHDOG_STATE, "\x00", 1); if (ret) { printf("omnia_mcu_write failed: %i\n", ret); return false;

@@ -256,17 +217,17 @@ static bool omnia_detect_sata(const char *msata_slot) return false; }

• if (!(stsword & CARD_DET_STSBIT)) {

• if (!(stsword & STS_CARD_DET)) { puts("none\n"); return false; }

• if (stsword & MSATA_IND_STSBIT)

• if (stsword & STS_MSATA_IND) puts("mSATA\n"); else puts("MiniPCIe\n");

• return stsword & MSATA_IND_STSBIT ? true : false;

• return stsword & STS_MSATA_IND; }

  static bool omnia_detect_wwan_usb3(const char *wwan_slot)

@@ -393,18 +354,7 @@ static int omnia_get_ram_size_gb(void)

static const char * const omnia_get_mcu_type(void) {

• static const char * const mcu_types[] = {

• [STS_MCU_TYPE_STM32] = "STM32",
  

• [STS_MCU_TYPE_GD32]  = "GD32",
  

• [STS_MCU_TYPE_MKL]   = "MKL",
  

• [STS_MCU_TYPE_UNKN]  = "unknown",
  

• };
• static const char * const mcu_types_with_perip_resets[] = {

• [STS_MCU_TYPE_STM32] = "STM32 (with peripheral resets)",
  

• [STS_MCU_TYPE_GD32]  = "GD32 (with peripheral resets)",
  

• [STS_MCU_TYPE_MKL]   = "MKL (with peripheral resets)",
  

• [STS_MCU_TYPE_UNKN]  = "unknown (with peripheral resets)",
  

• };

• static char result[] = "xxxxxxx (with peripheral resets)"; u16 stsword, features; int ret;

@@ -412,13 +362,28 @@ static const char * const omnia_get_mcu_type(void) if (ret) return "unknown";

• switch (stsword & STS_MCU_TYPE_MASK) {
• case STS_MCU_TYPE_STM32:

• strcpy(result, "STM32");
  

• break;
  

• case STS_MCU_TYPE_GD32:

• strcpy(result, "GD32");
  

• break;
  

• case STS_MCU_TYPE_MKL:

• strcpy(result, "MKL");
  

• break;
  

• default:

• strcpy(result, "unknown");
  

• break;
  

• }
• if (stsword & STS_FEATURES_SUPPORTED) { ret = omnia_mcu_read(CMD_GET_FEATURES, &features, sizeof(features)); if (ret == 0 && (features & FEAT_PERIPH_MCU))

• 	return mcu_types_with_perip_resets[stsword & STS_MCU_TYPE_MASK];
  

• 	strcat(result, " (with peripheral resets)");
  

  }

• return mcu_types[stsword & STS_MCU_TYPE_MASK];

• return result; }

  static const char * const omnia_get_mcu_version(void)

diff --git a/drivers/gpio/turris_omnia_mcu.c b/drivers/gpio/turris_omnia_mcu.c index 2d2bf2d1dd..da9a6efe6d 100644 --- a/drivers/gpio/turris_omnia_mcu.c +++ b/drivers/gpio/turris_omnia_mcu.c @@ -4,62 +4,10 @@ #include <common.h> #include <dm.h> #include <i2c.h> +#include <turris-omnia-mcu-interface.h> #include <asm/gpio.h> #include <linux/log2.h>

-enum commands_e {

• CMD_GET_STATUS_WORD = 0x01,
• CMD_GENERAL_CONTROL = 0x02,
• /* available if STS_FEATURES_SUPPORTED bit set in status word */
• CMD_GET_FEATURES = 0x10,
• /* available if FEAT_EXT_CMDS bit is set in features */
• CMD_GET_EXT_STATUS_DWORD = 0x11,
• /* available if FEAT_EXT_CMDS and FEAT_PERIPH_MCU bits are set in featurs */
• CMD_EXT_CONTROL = 0x12,
• CMD_GET_EXT_CONTROL_STATUS = 0x13,

-};

-/* CMD_GET_STATUS_WORD */ -enum sts_word_e {

• STS_MCU_TYPE_MASK = GENMASK(1, 0),
• STS_MCU_TYPE_STM32 = 0,
• STS_MCU_TYPE_GD32 = 1,
• STS_MCU_TYPE_MKL = 2,
• STS_FEATURES_SUPPORTED = BIT(2),
• STS_USER_REGULATOR_NOT_SUPPORTED = BIT(3),
• STS_CARD_DET = BIT(4),
• STS_MSATA_IND = BIT(5),
• STS_USB30_OVC = BIT(6),
• STS_USB31_OVC = BIT(7),
• STS_USB30_PWRON = BIT(8),
• STS_USB31_PWRON = BIT(9),
• STS_ENABLE_4V5 = BIT(10),
• STS_BUTTON_MODE = BIT(11),
• STS_BUTTON_PRESSED = BIT(12),
• STS_BUTTON_COUNTER_MASK = GENMASK(15, 13)

-};

-/* CMD_GENERAL_CONTROL */ -enum ctl_byte_e {

• CTL_LIGHT_RST = BIT(0),
• CTL_HARD_RST = BIT(1),
• /*CTL_RESERVED = BIT(2),*/
• CTL_USB30_PWRON = BIT(3),
• CTL_USB31_PWRON = BIT(4),
• CTL_ENABLE_4V5 = BIT(5),
• CTL_BUTTON_MODE = BIT(6),
• CTL_BOOTLOADER = BIT(7)

-};

-/* CMD_GET_FEATURES */ -enum features_e {

• FEAT_PERIPH_MCU = BIT(0),
• FEAT_EXT_CMDS = BIT(1),

-};

• struct turris_omnia_mcu_info { u16 features; };

diff --git a/include/turris-omnia-mcu-interface.h b/include/turris-omnia-mcu-interface.h new file mode 100644 index 0000000000..3c4638f732 --- /dev/null +++ b/include/turris-omnia-mcu-interface.h @@ -0,0 +1,248 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/*

• • CZ.NIC's Turris Omnia MCU I2C interface commands definitions
• • 2023 by Marek Behún kabel@kernel.org
• */

+#ifndef __TURRIS_OMNIA_MCU_INTERFACE_H +#define __TURRIS_OMNIA_MCU_INTERFACE_H

+#include <linux/bitops.h>

+enum omnia_commands_e {

• CMD_GET_STATUS_WORD = 0x01, /* slave sends status word back */
• CMD_GENERAL_CONTROL = 0x02,
• CMD_LED_MODE = 0x03, /* default/user */
• CMD_LED_STATE = 0x04, /* LED on/off */
• CMD_LED_COLOR = 0x05, /* LED number + RED + GREEN + BLUE */
• CMD_USER_VOLTAGE = 0x06,
• CMD_SET_BRIGHTNESS = 0x07,
• CMD_GET_BRIGHTNESS = 0x08,
• CMD_GET_RESET = 0x09,
• CMD_GET_FW_VERSION_APP = 0x0A, /* 20B git hash number */
• CMD_SET_WATCHDOG_STATE = 0x0B, /* 0 - disable

• 				 * 1 - enable / ping
  

• 				 * after boot watchdog is started
  

• 				 * with 2 minutes timeout
  

• 				 */
  

• /* CMD_WATCHDOG_STATUS = 0x0C, not implemented anymore */
• CMD_GET_WATCHDOG_STATE = 0x0D,
• CMD_GET_FW_VERSION_BOOT = 0x0E, /* 20B git hash number */
• CMD_GET_FW_CHECKSUM = 0x0F, /* 4B length, 4B checksum */
• /* available if FEATURES_SUPPORTED bit set in status word */
• CMD_GET_FEATURES = 0x10,
• /* available if EXT_CMD bit set in features */
• CMD_GET_EXT_STATUS_DWORD = 0x11,
• CMD_EXT_CONTROL = 0x12,
• CMD_GET_EXT_CONTROL_STATUS = 0x13,
• /* available if NEW_INT_API bit set in features */
• CMD_GET_INT_AND_CLEAR = 0x14,
• CMD_GET_INT_MASK = 0x15,
• CMD_SET_INT_MASK = 0x16,
• /* available if FLASHING bit set in features */
• CMD_FLASH = 0x19,
• /* available if WDT_PING bit set in features */
• CMD_SET_WDT_TIMEOUT = 0x20,
• CMD_GET_WDT_TIMELEFT = 0x21,
• /* available if POWEROFF_WAKEUP bit set in features */
• CMD_SET_WAKEUP = 0x22,
• CMD_GET_UPTIME_AND_WAKEUP = 0x23,
• CMD_POWER_OFF = 0x24,
• /* available if USB_OVC_PROT_SETTING bit set in features */
• CMD_SET_USB_OVC_PROT = 0x25,
• CMD_GET_USB_OVC_PROT = 0x26,
• /* available if TRNG bit set in features */
• CMD_TRNG_COLLECT_ENTROPY = 0x28,
• /* available if CRYPTO bit set in features */
• CMD_CRYPTO_GET_PUBLIC_KEY = 0x29,
• CMD_CRYPTO_SIGN_MESSAGE = 0x2A,
• CMD_CRYPTO_COLLECT_SIGNATURE = 0x2B,
• /* available if BOARD_INFO it set in features */
• CMD_BOARD_INFO_GET = 0x2C,
• CMD_BOARD_INFO_BURN = 0x2D,
• /* available only at address 0x2b (led-controller) */
• /* available only if LED_GAMMA_CORRECTION bit set in features */
• CMD_SET_GAMMA_CORRECTION = 0x30,
• CMD_GET_GAMMA_CORRECTION = 0x31,
• /* available only at address 0x2b (led-controller) */
• /* available only if PER_LED_CORRECTION bit set in features */
• /* available only if FROM_BIT_16_INVALID bit NOT set in features */
• CMD_SET_LED_CORRECTIONS = 0x32,
• CMD_GET_LED_CORRECTIONS = 0x33,

+};

+enum omnia_flashing_commands_e {

• FLASH_CMD_UNLOCK = 0x01,
• FLASH_CMD_SIZE_AND_CSUM = 0x02,
• FLASH_CMD_PROGRAM = 0x03,
• FLASH_CMD_RESET = 0x04,

+};

+enum omnia_sts_word_e {

• STS_MCU_TYPE_MASK = GENMASK(1, 0),
• STS_MCU_TYPE_STM32 = 0 << 0,
• STS_MCU_TYPE_GD32 = 1 << 0,
• STS_MCU_TYPE_MKL = 2 << 0,
• STS_FEATURES_SUPPORTED = BIT(2),
• STS_USER_REGULATOR_NOT_SUPPORTED = BIT(3),
• STS_CARD_DET = BIT(4),
• STS_MSATA_IND = BIT(5),
• STS_USB30_OVC = BIT(6),
• STS_USB31_OVC = BIT(7),
• STS_USB30_PWRON = BIT(8),
• STS_USB31_PWRON = BIT(9),
• STS_ENABLE_4V5 = BIT(10),
• STS_BUTTON_MODE = BIT(11),
• STS_BUTTON_PRESSED = BIT(12),
• STS_BUTTON_COUNTER_MASK = GENMASK(15, 13)

+};

+enum omnia_ctl_byte_e {

• CTL_LIGHT_RST = BIT(0),
• CTL_HARD_RST = BIT(1),
• /* BIT(2) is currently reserved */
• CTL_USB30_PWRON = BIT(3),
• CTL_USB31_PWRON = BIT(4),
• CTL_ENABLE_4V5 = BIT(5),
• CTL_BUTTON_MODE = BIT(6),
• CTL_BOOTLOADER = BIT(7)

+};

+enum omnia_features_e {

• FEAT_PERIPH_MCU = BIT(0),
• FEAT_EXT_CMDS = BIT(1),
• FEAT_WDT_PING = BIT(2),
• FEAT_LED_STATE_EXT_MASK = GENMASK(4, 3),
• FEAT_LED_STATE_EXT = 1 << 3,
• FEAT_LED_STATE_EXT_V32 = 2 << 3,
• FEAT_LED_GAMMA_CORRECTION = BIT(5),
• FEAT_NEW_INT_API = BIT(6),
• FEAT_BOOTLOADER = BIT(7),
• FEAT_FLASHING = BIT(8),
• FEAT_NEW_MESSAGE_API = BIT(9),
• FEAT_BRIGHTNESS_INT = BIT(10),
• FEAT_POWEROFF_WAKEUP = BIT(11),
• FEAT_CAN_OLD_MESSAGE_API = BIT(12),
• FEAT_TRNG = BIT(13),
• FEAT_CRYPTO = BIT(14),
• FEAT_BOARD_INFO = BIT(15),
• /*

• * Orginally the features command replied only 16 bits. If more were
  

• * read, either the I2C transaction failed or 0xff bytes were sent.
  

• * Therefore to consider bits 16 - 31 valid, one bit (20) was reserved
  

• * to be zero.
  

• */
  

• /* Bits 16 - 19 correspond to bits 0 - 3 of status word */
• FEAT_MCU_TYPE_MASK = GENMASK(17, 16),
• FEAT_MCU_TYPE_STM32 = 0 << 16,
• FEAT_MCU_TYPE_GD32 = 1 << 16,
• FEAT_MCU_TYPE_MKL = 2 << 16,
• FEAT_FEATURES_SUPPORTED = BIT(18),
• FEAT_USER_REGULATOR_NOT_SUPPORTED = BIT(19),
• /* must not be set */
• FEAT_FROM_BIT_16_INVALID = BIT(20),
• FEAT_PER_LED_CORRECTION = BIT(21),
• FEAT_USB_OVC_PROT_SETTING = BIT(22),

+};

+enum omnia_ext_sts_dword_e {

• EXT_STS_SFP_nDET = BIT(0),
• EXT_STS_LED_STATES_MASK = GENMASK(31, 12),
• EXT_STS_WLAN0_MSATA_LED = BIT(12),
• EXT_STS_WLAN1_LED = BIT(13),
• EXT_STS_WLAN2_LED = BIT(14),
• EXT_STS_WPAN0_LED = BIT(15),
• EXT_STS_WPAN1_LED = BIT(16),
• EXT_STS_WPAN2_LED = BIT(17),
• EXT_STS_WAN_LED0 = BIT(18),
• EXT_STS_WAN_LED1 = BIT(19),
• EXT_STS_LAN0_LED0 = BIT(20),
• EXT_STS_LAN0_LED1 = BIT(21),
• EXT_STS_LAN1_LED0 = BIT(22),
• EXT_STS_LAN1_LED1 = BIT(23),
• EXT_STS_LAN2_LED0 = BIT(24),
• EXT_STS_LAN2_LED1 = BIT(25),
• EXT_STS_LAN3_LED0 = BIT(26),
• EXT_STS_LAN3_LED1 = BIT(27),
• EXT_STS_LAN4_LED0 = BIT(28),
• EXT_STS_LAN4_LED1 = BIT(29),
• EXT_STS_LAN5_LED0 = BIT(30),
• EXT_STS_LAN5_LED1 = BIT(31),

+};

+enum omnia_ext_ctl_e {

• EXT_CTL_nRES_MMC = BIT(0),
• EXT_CTL_nRES_LAN = BIT(1),
• EXT_CTL_nRES_PHY = BIT(2),
• EXT_CTL_nPERST0 = BIT(3),
• EXT_CTL_nPERST1 = BIT(4),
• EXT_CTL_nPERST2 = BIT(5),
• EXT_CTL_PHY_SFP = BIT(6),
• EXT_CTL_PHY_SFP_AUTO = BIT(7),
• EXT_CTL_nVHV_CTRL = BIT(8),

+};

+enum omnia_int_e {

• INT_CARD_DET = BIT(0),
• INT_MSATA_IND = BIT(1),
• INT_USB30_OVC = BIT(2),
• INT_USB31_OVC = BIT(3),
• INT_BUTTON_PRESSED = BIT(4),
• INT_SFP_nDET = BIT(5),
• INT_BRIGHTNESS_CHANGED = BIT(6),
• INT_TRNG = BIT(7),
• INT_MESSAGE_SIGNED = BIT(8),
• INT_LED_STATES_MASK = GENMASK(31, 12),
• INT_WLAN0_MSATA_LED = BIT(12),
• INT_WLAN1_LED = BIT(13),
• INT_WLAN2_LED = BIT(14),
• INT_WPAN0_LED = BIT(15),
• INT_WPAN1_LED = BIT(16),
• INT_WPAN2_LED = BIT(17),
• INT_WAN_LED0 = BIT(18),
• INT_WAN_LED1 = BIT(19),
• INT_LAN0_LED0 = BIT(20),
• INT_LAN0_LED1 = BIT(21),
• INT_LAN1_LED0 = BIT(22),
• INT_LAN1_LED1 = BIT(23),
• INT_LAN2_LED0 = BIT(24),
• INT_LAN2_LED1 = BIT(25),
• INT_LAN3_LED0 = BIT(26),
• INT_LAN3_LED1 = BIT(27),
• INT_LAN4_LED0 = BIT(28),
• INT_LAN4_LED1 = BIT(29),
• INT_LAN5_LED0 = BIT(30),
• INT_LAN5_LED1 = BIT(31),

+};

+enum omnia_cmd_poweroff_e {

• CMD_POWER_OFF_POWERON_BUTTON = BIT(0),
• CMD_POWER_OFF_MAGIC = 0xdead,

+};

+enum cmd_usb_ovc_prot_e {

• CMD_xET_USB_OVC_PROT_PORT_MASK = GENMASK(3, 0),
• CMD_xET_USB_OVC_PROT_ENABLE = BIT(4),

+};

+#endif /* __TURRIS_OMNIA_MCU_INTERFACE_H */

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Since commit 8cd4bf7dc9ba ("turris: Use checkboard() instead of show_board_info()") the model is show two times during boot: Model: Turris Omnia Model: Turris Omnia

This is because the common function show_board_info() shows it, and Turris' checkboard() also does.

Remove the second print.

Fixes: 8cd4bf7dc9ba ("turris: Use checkboard() instead of show_board_info()") Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_mox/turris_mox.c | 5 +---- board/CZ.NIC/turris_omnia/turris_omnia.c | 1 - 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/board/CZ.NIC/turris_mox/turris_mox.c b/board/CZ.NIC/turris_mox/turris_mox.c index 3489bdd74b..1a2f60e3d1 100644 --- a/board/CZ.NIC/turris_mox/turris_mox.c +++ b/board/CZ.NIC/turris_mox/turris_mox.c @@ -565,13 +565,10 @@ static void handle_reset_button(void) int checkboard(void) { int i, ret, board_version, ram_size, is_sd;

• const char *pub_key, *model;

• const char *pub_key; const u8 *topology; u64 serial_number;

• model = fdt_getprop(gd->fdt_blob, 0, "model", NULL);
• printf("Model: %s\n", model);
• ret = mbox_sp_get_board_info(&serial_number, NULL, NULL, &board_version, &ram_size, NULL); if (ret < 0) {

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 6c2d7da528..87e33d88c4 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -933,7 +933,6 @@ int checkboard(void) int err;

err = turris_atsha_otp_get_serial_number(serial);

• printf("Model: Turris Omnia\n"); printf(" MCU type: %s\n", omnia_get_mcu_type()); printf(" MCU version: %s\n", omnia_get_mcu_version()); printf(" RAM size: %i MiB\n", omnia_get_ram_size_gb() * 1024);

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Refactor MCU status word and MCU firmware features reading to make it simpler to use.

Try reading 32 bits of features, if that fails, read 16 bits. Older MCU firmware supports only 16-bit wide features, and if more bytes are read, either 0xff is sent or I2C transaction fails. Handle both cases.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_omnia/turris_omnia.c | 100 +++++++++++++++-------- 1 file changed, 68 insertions(+), 32 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 87e33d88c4..6dfde5ee7a 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -133,6 +133,59 @@ static int omnia_mcu_write(u8 cmd, const void *buf, int len) return dm_i2c_write(chip, cmd, buf, len); }

+static int omnia_mcu_get_sts_and_features(u16 *psts, u32 *pfeatures) +{

• u16 sts, feat16;
• int ret;
• ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &sts, sizeof(sts));
• if (ret)

• return ret;
  

• if (psts)

• *psts = sts;
  

• if (!pfeatures)

• return 0;
  

• if (sts & STS_FEATURES_SUPPORTED) {

• /* try read 32-bit features */
  

• ret = omnia_mcu_read(CMD_GET_FEATURES, pfeatures,
  

• 		     sizeof(*pfeatures));
  

• if (ret) {
  

• 	/* try read 16-bit features */
  

• 	ret = omnia_mcu_read(CMD_GET_FEATURES, &feat16,
  

• 			     sizeof(&feat16));
  

• 	if (ret)
  

• 		return ret;
  

• 	*pfeatures = feat16;
  

• } else {
  

• 	if (*pfeatures & FEAT_FROM_BIT_16_INVALID)
  

• 		*pfeatures &= GENMASK(15, 0);
  

• }
  

• } else {

• *pfeatures = 0;
  

• }
• return 0;

+}

+static int omnia_mcu_get_sts(u16 *sts) +{

• return omnia_mcu_get_sts_and_features(sts, NULL);

+}

+static bool omnia_mcu_has_feature(u32 feature) +{

• u32 features;
• if (omnia_mcu_get_sts_and_features(NULL, &features))

• return false;
  

• return feature & features;

+}

• static void enable_a385_watchdog(unsigned int timeout_minutes) { struct sar_freq_modes sar_freq;

@@ -194,7 +247,7 @@ static bool disable_mcu_watchdog(void) static bool omnia_detect_sata(const char *msata_slot) { int ret;

• u16 stsword;

• u16 sts;

  puts("MiniPCIe/mSATA card detection... ");

@@ -210,24 +263,24 @@ static bool omnia_detect_sata(const char *msata_slot) } }

• ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &stsword, sizeof(stsword));

• ret = omnia_mcu_get_sts(&sts); if (ret) { printf("omnia_mcu_read failed: %i, defaulting to MiniPCIe card\n", ret); return false; }

• if (!(stsword & STS_CARD_DET)) {

• if (!(sts & STS_CARD_DET)) { puts("none\n"); return false; }

• if (stsword & STS_MSATA_IND)

• if (sts & STS_MSATA_IND) puts("mSATA\n"); else puts("MiniPCIe\n");

• return stsword & STS_MSATA_IND;

• return sts & STS_MSATA_IND; }

  static bool omnia_detect_wwan_usb3(const char *wwan_slot)

@@ -355,14 +408,14 @@ static int omnia_get_ram_size_gb(void) static const char * const omnia_get_mcu_type(void) { static char result[] = "xxxxxxx (with peripheral resets)";

• u16 stsword, features;

• u16 sts; int ret;

• ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &stsword, sizeof(stsword));

• ret = omnia_mcu_get_sts(&sts); if (ret) return "unknown";

• switch (stsword & STS_MCU_TYPE_MASK) {

• switch (sts & STS_MCU_TYPE_MASK) { case STS_MCU_TYPE_STM32: strcpy(result, "STM32"); break;

@@ -377,11 +430,8 @@ static const char * const omnia_get_mcu_type(void) break; }

• if (stsword & STS_FEATURES_SUPPORTED) {

• ret = omnia_mcu_read(CMD_GET_FEATURES, &features, sizeof(features));
  

• if (ret == 0 && (features & FEAT_PERIPH_MCU))
  

• 	strcat(result, " (with peripheral resets)");
  

• }

• if (omnia_mcu_has_feature(FEAT_PERIPH_MCU))

• strcat(result, " (with peripheral resets)");
  
  

  return result; }

@@ -660,9 +710,6 @@ int board_early_init_f(void)

void spl_board_init(void) {

• u16 val;
• int ret;
• /*
  • If booting from UART, disable MCU watchdog in SPL, since uploading
  • U-Boot proper can take too much time and trigger it. Instead enable

@@ -679,12 +726,8 @@ void spl_board_init(void) * resets then LAN eth switch is initialized automatically by bootstrap * pins when A385 is released from the reset. */

• ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &val, sizeof(val));
• if (ret == 0 && (val & STS_FEATURES_SUPPORTED)) {

• ret = omnia_mcu_read(CMD_GET_FEATURES, &val, sizeof(val));
  

• if (ret == 0 && (val & FEAT_PERIPH_MCU))
  

• 	initialize_switch();
  

• }

• if (omnia_mcu_has_feature(FEAT_PERIPH_MCU))

• initialize_switch();
  

  }

  #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP)

@@ -884,16 +927,9 @@ static int fixup_mcu_gpio_in_eth_wan_node(void *blob) #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) int board_fix_fdt(void *blob) {

• u16 val;
• int ret;
• ret = omnia_mcu_read(CMD_GET_STATUS_WORD, &val, sizeof(val));
• if (ret == 0 && (val & STS_FEATURES_SUPPORTED)) {

• ret = omnia_mcu_read(CMD_GET_FEATURES, &val, sizeof(val));
  

• if (ret == 0 && (val & FEAT_PERIPH_MCU)) {
  

• 	fixup_mcu_gpio_in_pcie_nodes(blob);
  

• 	fixup_mcu_gpio_in_eth_wan_node(blob);
  

• }
  

• if (omnia_mcu_has_feature(FEAT_PERIPH_MCU)) {

• fixup_mcu_gpio_in_pcie_nodes(blob);
  

• fixup_mcu_gpio_in_eth_wan_node(blob);
  

  }

  fixup_msata_port_nodes(blob);

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Implement reading board serial number, first MAC address and board version from MCU. MCU supports board information if the FEAT_BOARD_INFO feature bit is set in MCU features.

Prefer getting board information from MCU if supported, fallback to Atmel SHA chip.

Signed-off-by: Marek Behún kabel@kernel.org

Minor comment below. Other than this:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_atsha_otp.c | 27 +------ board/CZ.NIC/turris_omnia/Makefile | 2 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 94 +++++++++++++++++++++++- 3 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/board/CZ.NIC/turris_atsha_otp.c b/board/CZ.NIC/turris_atsha_otp.c index a29fe36231..85eebcdf18 100644 --- a/board/CZ.NIC/turris_atsha_otp.c +++ b/board/CZ.NIC/turris_atsha_otp.c @@ -11,6 +11,7 @@ #include <atsha204a-i2c.h>

#include "turris_atsha_otp.h" +#include "turris_common.h"

#define TURRIS_ATSHA_OTP_VERSION 0 #define TURRIS_ATSHA_OTP_SERIAL 1 @@ -32,26 +33,6 @@ static struct udevice *get_atsha204a_dev(void) return dev; }

-static void increment_mac(u8 *mac) -{

• int i;
• for (i = 5; i >= 3; i--) {

• mac[i] += 1;
  

• if (mac[i])
  

• 	break;
  

• }

-}

-static void set_mac_if_invalid(int i, u8 *mac) -{

• u8 oldmac[6];
• if (is_valid_ethaddr(mac) &&

•    !eth_env_get_enetaddr_by_index("eth", i, oldmac))
  

• eth_env_set_enetaddr_by_index("eth", i, mac);
  

-}

• int turris_atsha_otp_init_mac_addresses(int first_idx) { struct udevice *dev = get_atsha204a_dev();

@@ -84,11 +65,7 @@ int turris_atsha_otp_init_mac_addresses(int first_idx) mac[4] = mac1[2]; mac[5] = mac1[3];

• set_mac_if_invalid((first_idx + 0) % 3, mac);
• increment_mac(mac);
• set_mac_if_invalid((first_idx + 1) % 3, mac);
• increment_mac(mac);
• set_mac_if_invalid((first_idx + 2) % 3, mac);

• turris_init_mac_addresses(first_idx, mac);

  return 0; }

diff --git a/board/CZ.NIC/turris_omnia/Makefile b/board/CZ.NIC/turris_omnia/Makefile index dc39b44ae1..341378b4e5 100644 --- a/board/CZ.NIC/turris_omnia/Makefile +++ b/board/CZ.NIC/turris_omnia/Makefile @@ -2,4 +2,4 @@ # # Copyright (C) 2017 Marek Behún kabel@kernel.org

-obj-y := turris_omnia.o ../turris_atsha_otp.o +obj-y := turris_omnia.o ../turris_atsha_otp.o ../turris_common.o diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 6dfde5ee7a..f63640ad64 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -18,6 +18,7 @@ #include <asm/io.h> #include <asm/arch/cpu.h> #include <asm/arch/soc.h> +#include <asm/unaligned.h> #include <dm/uclass.h> #include <dt-bindings/gpio/gpio.h> #include <fdt_support.h> @@ -25,12 +26,14 @@ #include <time.h> #include <turris-omnia-mcu-interface.h> #include <linux/bitops.h> +#include <linux/bitrev.h> #include <linux/delay.h> #include <u-boot/crc.h>

#include "../drivers/ddr/marvell/a38x/ddr3_init.h" #include <../serdes/a38x/high_speed_env_spec.h> #include "../turris_atsha_otp.h" +#include "../turris_common.h"

DECLARE_GLOBAL_DATA_PTR;

@@ -186,6 +189,70 @@ static bool omnia_mcu_has_feature(u32 feature) return feature & features; }

+static u32 omnia_mcu_crc32(const void *p, size_t len) +{

• u32 val, crc = 0;
• compiletime_assert(!(len % 4), "length has to be a multiple of 4");
• while (len) {

• val = bitrev32(get_unaligned_le32(p));
  

• crc = crc32(crc, (void *)&val, 4);
  

• p += 4;
  

• len -= 4;
  

• }
• return ~bitrev32(crc);

+}

+/* Can only be called after relocation, since it needs cleared BSS */ +static int omnia_mcu_board_info(char *serial, u8 *mac, char *version) +{

• static u8 reply[17];
• static bool cached;
• if (!cached) {

• u8 csum;
  

• int ret;
  

• ret = omnia_mcu_read(CMD_BOARD_INFO_GET, reply, sizeof(reply));
  

• if (ret)
  

• 	return ret;
  

• if (reply[0] != 16)
  

• 	return -EBADMSG;
  

• csum = reply[16];
  

• reply[16] = 0;
  

• if ((omnia_mcu_crc32(&reply[1], 16) & 0xff) != csum)
  

• 	return -EBADMSG;
  

• cached = true;
  

• }
• if (serial) {

• const char *serial_env;
  

• serial_env = env_get("serial#");
  

• if (serial_env && strlen(serial_env) == 16) {
  

• 	strcpy(serial, serial_env);
  

Usage of strcpy() is discouraged AFAIK.

• } else {
  

• 	sprintf(serial, "%016llX",
  

• 		get_unaligned_le64(&reply[1]));
  

• 	env_set("serial#", serial);
  

• }
  

• }
• if (mac)

• memcpy(mac, &reply[9], ETH_ALEN);
  

• if (version)

• sprintf(version, "%u", reply[15]);
  

• return 0;

+}

• static void enable_a385_watchdog(unsigned int timeout_minutes) { struct sar_freq_modes sar_freq;

@@ -965,13 +1032,23 @@ int board_late_init(void)

int checkboard(void) {

• char serial[17];

• char serial[17], version[4];
• bool has_version; int err;

• err = turris_atsha_otp_get_serial_number(serial); printf(" MCU type: %s\n", omnia_get_mcu_type()); printf(" MCU version: %s\n", omnia_get_mcu_version()); printf(" RAM size: %i MiB\n", omnia_get_ram_size_gb() * 1024);

• if (omnia_mcu_has_feature(FEAT_BOARD_INFO)) {

• err = omnia_mcu_board_info(serial, NULL, version);
  

• has_version = !err;
  

• } else {

• err = turris_atsha_otp_get_serial_number(serial);
  

• has_version = false;
  

• }

• printf(" Board version: %s\n", has_version ? version : "unknown"); printf(" Serial Number: %s\n", !err ? serial : "unknown");

  return 0;

@@ -979,8 +1056,17 @@ int checkboard(void)

int misc_init_r(void) {

• turris_atsha_otp_init_mac_addresses(1);
• turris_atsha_otp_init_serial_number();

• if (omnia_mcu_has_feature(FEAT_BOARD_INFO)) {

• char serial[17];
  

• u8 first_mac[6];
  

• if (!omnia_mcu_board_info(serial, first_mac, NULL))
  

• 	turris_init_mac_addresses(1, first_mac);
  

• } else {

• turris_atsha_otp_init_mac_addresses(1);
  

• turris_atsha_otp_init_serial_number();
  

• }
• return 0; }

Viele Grüße, Stefan Roese

On 3/28/24 12:17, Marek Behún wrote:

On Thu, 28 Mar 2024 10:56:01 +0100 Stefan Roese sr@denx.de wrote:

...

On 3/27/24 17:23, Marek Behún wrote:

...

Implement reading board serial number, first MAC address and board version from MCU. MCU supports board information if the FEAT_BOARD_INFO feature bit is set in MCU features.

Prefer getting board information from MCU if supported, fallback to Atmel SHA chip.

Signed-off-by: Marek Behún kabel@kernel.org

Minor comment below. Other than this:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

...

---

board/CZ.NIC/turris_atsha_otp.c | 27 +------ board/CZ.NIC/turris_omnia/Makefile | 2 +- board/CZ.NIC/turris_omnia/turris_omnia.c | 94 +++++++++++++++++++++++- 3 files changed, 93 insertions(+), 30 deletions(-)

diff --git a/board/CZ.NIC/turris_atsha_otp.c b/board/CZ.NIC/turris_atsha_otp.c index a29fe36231..85eebcdf18 100644 --- a/board/CZ.NIC/turris_atsha_otp.c +++ b/board/CZ.NIC/turris_atsha_otp.c @@ -11,6 +11,7 @@ #include <atsha204a-i2c.h>

#include "turris_atsha_otp.h" +#include "turris_common.h"

#define TURRIS_ATSHA_OTP_VERSION 0 #define TURRIS_ATSHA_OTP_SERIAL 1 @@ -32,26 +33,6 @@ static struct udevice *get_atsha204a_dev(void) return dev; }

-static void increment_mac(u8 *mac) -{

• int i;
• for (i = 5; i >= 3; i--) {

• mac[i] += 1;
  

• if (mac[i])
  

• 	break;
  

• }

-}

-static void set_mac_if_invalid(int i, u8 *mac) -{

• u8 oldmac[6];
• if (is_valid_ethaddr(mac) &&

•    !eth_env_get_enetaddr_by_index("eth", i, oldmac))
  

• eth_env_set_enetaddr_by_index("eth", i, mac);
  

-}

• int turris_atsha_otp_init_mac_addresses(int first_idx) { struct udevice *dev = get_atsha204a_dev();

@@ -84,11 +65,7 @@ int turris_atsha_otp_init_mac_addresses(int first_idx) mac[4] = mac1[2]; mac[5] = mac1[3];

• set_mac_if_invalid((first_idx + 0) % 3, mac);
• increment_mac(mac);
• set_mac_if_invalid((first_idx + 1) % 3, mac);
• increment_mac(mac);
• set_mac_if_invalid((first_idx + 2) % 3, mac);

• turris_init_mac_addresses(first_idx, mac);

  return 0; }

diff --git a/board/CZ.NIC/turris_omnia/Makefile b/board/CZ.NIC/turris_omnia/Makefile index dc39b44ae1..341378b4e5 100644 --- a/board/CZ.NIC/turris_omnia/Makefile +++ b/board/CZ.NIC/turris_omnia/Makefile @@ -2,4 +2,4 @@ # # Copyright (C) 2017 Marek Behún kabel@kernel.org

-obj-y := turris_omnia.o ../turris_atsha_otp.o +obj-y := turris_omnia.o ../turris_atsha_otp.o ../turris_common.o diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index 6dfde5ee7a..f63640ad64 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -18,6 +18,7 @@ #include <asm/io.h> #include <asm/arch/cpu.h> #include <asm/arch/soc.h> +#include <asm/unaligned.h> #include <dm/uclass.h> #include <dt-bindings/gpio/gpio.h> #include <fdt_support.h> @@ -25,12 +26,14 @@ #include <time.h> #include <turris-omnia-mcu-interface.h> #include <linux/bitops.h> +#include <linux/bitrev.h> #include <linux/delay.h> #include <u-boot/crc.h>

#include "../drivers/ddr/marvell/a38x/ddr3_init.h" #include <../serdes/a38x/high_speed_env_spec.h> #include "../turris_atsha_otp.h" +#include "../turris_common.h"

DECLARE_GLOBAL_DATA_PTR;

@@ -186,6 +189,70 @@ static bool omnia_mcu_has_feature(u32 feature) return feature & features; }

+static u32 omnia_mcu_crc32(const void *p, size_t len) +{

• u32 val, crc = 0;
• compiletime_assert(!(len % 4), "length has to be a multiple of 4");
• while (len) {

• val = bitrev32(get_unaligned_le32(p));
  

• crc = crc32(crc, (void *)&val, 4);
  

• p += 4;
  

• len -= 4;
  

• }
• return ~bitrev32(crc);

+}

+/* Can only be called after relocation, since it needs cleared BSS */ +static int omnia_mcu_board_info(char *serial, u8 *mac, char *version) +{

• static u8 reply[17];
• static bool cached;
• if (!cached) {

• u8 csum;
  

• int ret;
  

• ret = omnia_mcu_read(CMD_BOARD_INFO_GET, reply, sizeof(reply));
  

• if (ret)
  

• 	return ret;
  

• if (reply[0] != 16)
  

• 	return -EBADMSG;
  

• csum = reply[16];
  

• reply[16] = 0;
  

• if ((omnia_mcu_crc32(&reply[1], 16) & 0xff) != csum)
  

• 	return -EBADMSG;
  

• cached = true;
  

• }
• if (serial) {

• const char *serial_env;
  

• serial_env = env_get("serial#");
  

• if (serial_env && strlen(serial_env) == 16) {
  

• 	strcpy(serial, serial_env);
  

Usage of strcpy() is discouraged AFAIK.

Yeah, it would make sense to use strncpy, but the potential leak is prevented by strlen() check.

strncpy is also not optimal. Please see here if you are interested:

https://www.geeksforgeeks.org/why-strcpy-and-strncpy-are-not-safe-to-use/

AFAIU, there is no pressing need to change this implementation here though right now. I just wanted to make this comment.

Thanks, Stefan

On 3/27/24 17:23, Marek Behún wrote:

If MCU supports the FEAT_CRYPTO feature, read board ECDSA public key from MCU and print it.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_omnia/turris_omnia.c | 25 +++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index f63640ad64..b2f0088e5e 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -253,6 +253,24 @@ static int omnia_mcu_board_info(char *serial, u8 *mac, char *version) return 0; }

+static int omnia_mcu_get_board_public_key(char pub_key[static 67]) +{

• u8 reply[34];
• int ret;
• ret = omnia_mcu_read(CMD_CRYPTO_GET_PUBLIC_KEY, reply, sizeof(reply));
• if (ret)

• return ret;
  

• if (reply[0] != 33)

• return -EBADMSG;
  

• bin2hex(pub_key, &reply[1], 33);
• pub_key[66] = '\0';
• return 0;

+}

• static void enable_a385_watchdog(unsigned int timeout_minutes) { struct sar_freq_modes sar_freq;

@@ -1032,7 +1050,7 @@ int board_late_init(void)

int checkboard(void) {

• char serial[17], version[4];

• char serial[17], version[4], pub_key[67]; bool has_version; int err;

@@ -1051,6 +1069,11 @@ int checkboard(void) printf(" Board version: %s\n", has_version ? version : "unknown"); printf(" Serial Number: %s\n", !err ? serial : "unknown");

• if (omnia_mcu_has_feature(FEAT_CRYPTO)) {

• err = omnia_mcu_get_board_public_key(pub_key);
  

• printf("  ECDSA Public Key: %s\n", !err ? pub_key : "unknown");
  

• }
• return 0; }

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

If the FEAT_CRYPTO feature bit is present in MCU features, the board crypto is implemented by MCU and the Atmel SHA chip is not present. Disable Atmel SHA device-tree node in that case.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

board/CZ.NIC/turris_omnia/turris_omnia.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c b/board/CZ.NIC/turris_omnia/turris_omnia.c index b2f0088e5e..3b7a71bdad 100644 --- a/board/CZ.NIC/turris_omnia/turris_omnia.c +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c @@ -1007,6 +1007,25 @@ static int fixup_mcu_gpio_in_eth_wan_node(void *blob) return 0; }

+static void fixup_atsha_node(void *blob) +{

• int node;
• if (!omnia_mcu_has_feature(FEAT_CRYPTO))

• return;
  

• node = fdt_node_offset_by_compatible(blob, -1, "atmel,atsha204a");
• if (node < 0) {

• printf("Cannot find ATSHA204A node!\n");
  

• return;
  

• }
• if (fdt_status_disabled(blob, node) < 0)

• printf("Cannot disable ATSHA204A node!\n");
  

• else

• debug("Disabled ATSHA204A node\n");
  

+}

• #endif

  #if IS_ENABLED(CONFIG_OF_BOARD_FIXUP)

@@ -1020,6 +1039,8 @@ int board_fix_fdt(void *blob) fixup_msata_port_nodes(blob); fixup_wwan_port_nodes(blob);

• fixup_atsha_node(blob);
• return 0; } #endif

@@ -1211,6 +1232,8 @@ int ft_board_setup(void *blob, struct bd_info *bd) fixup_msata_port_nodes(blob); fixup_wwan_port_nodes(blob);

• fixup_atsha_node(blob);
• return 0; } #endif

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Commit 35e29e89a386 ("arm: mvebu: Implement simple mvebu-reset driver for enabling/disabling PCIe ports") made it so that the mvebu reset driver for enabling/disabling PCIe ports is build if CONFIG_DM_RESET is enabled. This is because PCI_MVEBU depends on DM_RESET.

But the driver should not be built for SPL. Indeed the PCI_MVEBU driver is not supported in SPL now, and so the mvebu-reset driver is not needed.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

arch/arm/mach-mvebu/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index 0584ed2be5..ef790d97fe 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -22,13 +22,14 @@ else # CONFIG_ARCH_KIRKWOOD obj-y = cpu.o obj-y += dram.o obj-y += lowlevel.o -obj-$(CONFIG_DM_RESET) += system-controller.o ifndef CONFIG_SPL_BUILD obj-$(CONFIG_ARMADA_375) += ../../../drivers/ddr/marvell/axp/xor.o obj-$(CONFIG_ARMADA_38X) += ../../../drivers/ddr/marvell/a38x/xor.o obj-$(CONFIG_ARMADA_XP) += ../../../drivers/ddr/marvell/axp/xor.o obj-$(CONFIG_ARMADA_MSYS) += ../../../drivers/ddr/marvell/axp/xor.o

+obj-$(CONFIG_DM_RESET) += system-controller.o

• ifdef CONFIG_ARMADA_38X obj-$(CONFIG_MVEBU_EFUSE) += efuse.o endif

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

The system-controller driver for 32-bit Armada is currently registered as UCLASS_RESET, since it only provides enabling/disabling PCIe ports.

Rework it as UCLASS_SYSCON and bind mvebu-reset as a child device.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

arch/arm/mach-mvebu/Kconfig | 11 ++++ arch/arm/mach-mvebu/Makefile | 2 +- arch/arm/mach-mvebu/system-controller.c | 76 ++++++++++++++----------- 3 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index 2058c95ca2..62a2bc5958 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -18,6 +18,7 @@ config ARMADA_32BIT select TOOLS_KWBIMAGE if SPL select SPL_SYS_NO_VECTOR_TABLE if SPL select ARCH_VERY_EARLY_INIT

• select ARMADA_32BIT_SYSCON_RESET if DM_RESET

  # ARMv7 SoCs... config ARMADA_375

@@ -456,6 +457,16 @@ config SF_DEFAULT_MODE default 0x0 depends on MVEBU_SPL_BOOT_DEVICE_SPI

+config ARMADA_32BIT_SYSCON_RESET

• bool "Support Armada XP/375/38x/39x reset controller"
• depends on ARMADA_32BIT
• depends on DM_RESET
• select REGMAP
• select SYSCON
• help

•  Build support for Armada XP/375/38x/39x reset controller. This is
  

•  needed for PCIe support.
  

• source "board/solidrun/clearfog/Kconfig" source "board/kobol/helios4/Kconfig"

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index ef790d97fe..d44ca3a0df 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -28,7 +28,7 @@ obj-$(CONFIG_ARMADA_38X) += ../../../drivers/ddr/marvell/a38x/xor.o obj-$(CONFIG_ARMADA_XP) += ../../../drivers/ddr/marvell/axp/xor.o obj-$(CONFIG_ARMADA_MSYS) += ../../../drivers/ddr/marvell/axp/xor.o

-obj-$(CONFIG_DM_RESET) += system-controller.o +obj-$(CONFIG_ARMADA_32BIT_SYSCON_RESET) += system-controller.o

ifdef CONFIG_ARMADA_38X obj-$(CONFIG_MVEBU_EFUSE) += efuse.o diff --git a/arch/arm/mach-mvebu/system-controller.c b/arch/arm/mach-mvebu/system-controller.c index 7cdde11cbd..c5c05922f2 100644 --- a/arch/arm/mach-mvebu/system-controller.c +++ b/arch/arm/mach-mvebu/system-controller.c @@ -1,19 +1,21 @@ // SPDX-License-Identifier: GPL-2.0+ -// (C) 2021 Pali Rohár pali@kernel.org +/*

• • Copyright (C) 2021 Pali Rohár pali@kernel.org
• • Copyright (C) 2024 Marek Behún kabel@kernel.org

• */

  #include <common.h> #include <dm.h>

+#include <dm/lists.h> +#include <regmap.h> #include <reset-uclass.h> +#include <syscon.h> #include <asm/io.h>

#define MVEBU_SOC_CONTROL_1_REG 0x4

#define MVEBU_PCIE_ID 0

-struct mvebu_reset_data {

• void *base;

-};

• static int mvebu_reset_of_xlate(struct reset_ctl *rst, struct ofnode_phandle_args *args) {

@@ -46,46 +48,33 @@ static int mvebu_reset_free(struct reset_ctl *rst)

static int mvebu_reset_assert(struct reset_ctl *rst) {

• struct mvebu_reset_data *data = dev_get_priv(rst->dev);

• struct regmap *regmap = syscon_get_regmap(rst->dev->parent);

• clrbits_32(data->base + MVEBU_SOC_CONTROL_1_REG, BIT(rst->data));
• return 0;

• return regmap_update_bits(regmap, MVEBU_SOC_CONTROL_1_REG,

• 		  BIT(rst->data), 0);
  

  }

  static int mvebu_reset_deassert(struct reset_ctl *rst) {

• struct mvebu_reset_data *data = dev_get_priv(rst->dev);

• struct regmap *regmap = syscon_get_regmap(rst->dev->parent);

• setbits_32(data->base + MVEBU_SOC_CONTROL_1_REG, BIT(rst->data));
• return 0;

• return regmap_update_bits(regmap, MVEBU_SOC_CONTROL_1_REG,

• 		  BIT(rst->data), BIT(rst->data));
  

  }

  static int mvebu_reset_status(struct reset_ctl *rst) {

• struct mvebu_reset_data *data = dev_get_priv(rst->dev);

• struct regmap *regmap = syscon_get_regmap(rst->dev->parent);
• uint val;
• int ret;

• return !(readl(data->base + MVEBU_SOC_CONTROL_1_REG) & BIT(rst->data));

-}

-static int mvebu_reset_of_to_plat(struct udevice *dev) -{

• struct mvebu_reset_data *data = dev_get_priv(dev);

• ret = regmap_read(regmap, MVEBU_SOC_CONTROL_1_REG, &val);
• if (ret < 0)

• return ret;
  
  

• data->base = dev_read_addr_ptr(dev);
• if (!data->base)

• return -EINVAL;
  

• return 0;

• return !(val & BIT(rst->data)); }

-static const struct udevice_id mvebu_reset_of_match[] = {

• { .compatible = "marvell,armada-370-xp-system-controller" },
• { .compatible = "marvell,armada-375-system-controller" },
• { .compatible = "marvell,armada-380-system-controller" },
• { .compatible = "marvell,armada-390-system-controller" },
• { },

-};

• static const struct reset_ops mvebu_reset_ops = { .of_xlate = mvebu_reset_of_xlate, .request = mvebu_reset_request,

@@ -98,8 +87,27 @@ static const struct reset_ops mvebu_reset_ops = { U_BOOT_DRIVER(mvebu_reset) = { .name = "mvebu-reset", .id = UCLASS_RESET,

• .of_match = mvebu_reset_of_match,
• .of_to_plat = mvebu_reset_of_to_plat,
• .priv_auto = sizeof(struct mvebu_reset_data), .ops = &mvebu_reset_ops, };

+static int mvebu_syscon_bind(struct udevice *dev) +{

• /* bind also mvebu-reset, with the same ofnode */
• return device_bind_driver_to_node(dev, "mvebu-reset", "mvebu-reset",

• 			  dev_ofnode(dev), NULL);
  

+}

+static const struct udevice_id mvebu_syscon_of_match[] = {

• { .compatible = "marvell,armada-370-xp-system-controller" },
• { .compatible = "marvell,armada-375-system-controller" },
• { .compatible = "marvell,armada-380-system-controller" },
• { .compatible = "marvell,armada-390-system-controller" },
• { },

+};

+U_BOOT_DRIVER(mvebu_syscon) = {

• .name = "mvebu-system-controller",
• .id = UCLASS_SYSCON,
• .of_match = mvebu_syscon_of_match,
• .bind = mvebu_syscon_bind,

+};

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

The mvebu-reset driver is only needed by the mvebu PCIe driver, but currently it is automatically selected if DM_RESET is enabled. Add the condition of PCI_MVEBU also being enabled for mvebu-reset to be selected.

Signed-off-by: Marek Behún kabel@kernel.org

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

arch/arm/mach-mvebu/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index 62a2bc5958..623432a60e 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -18,7 +18,7 @@ config ARMADA_32BIT select TOOLS_KWBIMAGE if SPL select SPL_SYS_NO_VECTOR_TABLE if SPL select ARCH_VERY_EARLY_INIT

• select ARMADA_32BIT_SYSCON_RESET if DM_RESET

• select ARMADA_32BIT_SYSCON_RESET if DM_RESET && PCI_MVEBU

  # ARMv7 SoCs... config ARMADA_375

Viele Grüße, Stefan Roese

On 3/27/24 17:23, Marek Behún wrote:

Add driver model support for sysreset via mvebu system controller. This is currently only available for U-Boot proper.

Signed-off-by: Marek Behún kabel@kernel.org

Only a minor comment below. Other than this:

Reviewed-by: Stefan Roese sr@denx.de

Thanks, Stefan

---

arch/arm/mach-mvebu/Kconfig | 18 +++++- arch/arm/mach-mvebu/Makefile | 2 +- arch/arm/mach-mvebu/cpu.c | 2 + arch/arm/mach-mvebu/system-controller.c | 74 +++++++++++++++++++++++-- 4 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig index 623432a60e..f15d3cc5ed 100644 --- a/arch/arm/mach-mvebu/Kconfig +++ b/arch/arm/mach-mvebu/Kconfig @@ -19,6 +19,7 @@ config ARMADA_32BIT select SPL_SYS_NO_VECTOR_TABLE if SPL select ARCH_VERY_EARLY_INIT select ARMADA_32BIT_SYSCON_RESET if DM_RESET && PCI_MVEBU

• select ARMADA_32BIT_SYSCON_SYSRESET if SYSRESET

  # ARMv7 SoCs... config ARMADA_375

@@ -457,16 +458,29 @@ config SF_DEFAULT_MODE default 0x0 depends on MVEBU_SPL_BOOT_DEVICE_SPI

+config ARMADA_32BIT_SYSCON

• bool
• depends on ARMADA_32BIT
• select REGMAP
• select SYSCON
• config ARMADA_32BIT_SYSCON_RESET bool "Support Armada XP/375/38x/39x reset controller" depends on ARMADA_32BIT depends on DM_RESET

• select REGMAP
• select SYSCON

• select ARMADA_32BIT_SYSCON help Build support for Armada XP/375/38x/39x reset controller. This is needed for PCIe support.

+config ARMADA_32BIT_SYSCON_SYSRESET

• bool "Support Armada XP/375/38x/39x sysreset via driver model"
• depends on ARMADA_32BIT
• depends on SYSRESET
• select ARMADA_32BIT_SYSCON
• help

•  Build support for Armada XP/375/38x/39x system reset via driver model.
  

• source "board/solidrun/clearfog/Kconfig" source "board/kobol/helios4/Kconfig"

diff --git a/arch/arm/mach-mvebu/Makefile b/arch/arm/mach-mvebu/Makefile index d44ca3a0df..329c2e4915 100644 --- a/arch/arm/mach-mvebu/Makefile +++ b/arch/arm/mach-mvebu/Makefile @@ -28,7 +28,7 @@ obj-$(CONFIG_ARMADA_38X) += ../../../drivers/ddr/marvell/a38x/xor.o obj-$(CONFIG_ARMADA_XP) += ../../../drivers/ddr/marvell/axp/xor.o obj-$(CONFIG_ARMADA_MSYS) += ../../../drivers/ddr/marvell/axp/xor.o

-obj-$(CONFIG_ARMADA_32BIT_SYSCON_RESET) += system-controller.o +obj-$(CONFIG_ARMADA_32BIT_SYSCON) += system-controller.o

ifdef CONFIG_ARMADA_38X obj-$(CONFIG_MVEBU_EFUSE) += efuse.o diff --git a/arch/arm/mach-mvebu/cpu.c b/arch/arm/mach-mvebu/cpu.c index 8e0de93538..7c62a5dbb6 100644 --- a/arch/arm/mach-mvebu/cpu.c +++ b/arch/arm/mach-mvebu/cpu.c @@ -52,6 +52,7 @@ void lowlevel_init(void) */ }

+#if defined(CONFIG_SPL_BUILD) || !defined(CONFIG_ARMADA_32BIT_SYSCON_SYSRESET) void reset_cpu(void) { struct mvebu_system_registers *reg = @@ -62,6 +63,7 @@ void reset_cpu(void) while (1) ; } +#endif

u32 get_boot_device(void) { diff --git a/arch/arm/mach-mvebu/system-controller.c b/arch/arm/mach-mvebu/system-controller.c index c5c05922f2..b5f8afb96d 100644 --- a/arch/arm/mach-mvebu/system-controller.c +++ b/arch/arm/mach-mvebu/system-controller.c @@ -10,11 +10,24 @@ #include <regmap.h> #include <reset-uclass.h> #include <syscon.h> +#include <sysreset.h> #include <asm/io.h>

-#define MVEBU_SOC_CONTROL_1_REG 0x4 +#define MVEBU_SOC_CONTROL_1_REG 0x4

-#define MVEBU_PCIE_ID 0 +#if defined(CONFIG_ARMADA_375) +# define MVEBU_RSTOUTN_MASK_REG 0x54 +# define MVEBU_SYS_SOFT_RST_REG 0x58 +#else +# define MVEBU_RSTOUTN_MASK_REG 0x60 +# define MVEBU_SYS_SOFT_RST_REG 0x64 +#endif

+#define MVEBU_GLOBAL_SOFT_RST_BIT BIT(0)

+#define MVEBU_PCIE_ID 0

+#if IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_RESET)

static int mvebu_reset_of_xlate(struct reset_ctl *rst, struct ofnode_phandle_args *args) @@ -90,11 +103,64 @@ U_BOOT_DRIVER(mvebu_reset) = { .ops = &mvebu_reset_ops, };

+#endif /* IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_RESET) */

+#if IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_SYSRESET)

+static int mvebu_sysreset_request(struct udevice *dev, enum sysreset_t type) +{

• struct regmap *regmap = syscon_get_regmap(dev->parent);
• uint bit;
• if (type != SYSRESET_COLD)

• return -EPROTONOSUPPORT;
  

• bit = MVEBU_GLOBAL_SOFT_RST_BIT;
• regmap_update_bits(regmap, MVEBU_RSTOUTN_MASK_REG, bit, bit);
• regmap_update_bits(regmap, MVEBU_SYS_SOFT_RST_REG, bit, bit);
• while (1)

• ;
  

A comment before this endless loop might be helpful here.

• return 0;

+}

+static struct sysreset_ops mvebu_sysreset_ops = {

• .request = mvebu_sysreset_request,

+};

+U_BOOT_DRIVER(mvebu_sysreset) = {

• .name = "mvebu-sysreset",
• .id = UCLASS_SYSRESET,
• .ops = &mvebu_sysreset_ops,

+};

+#endif /* IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_SYSRESET) */

• static int mvebu_syscon_bind(struct udevice *dev) {
• int ret = 0;
• /* bind also mvebu-reset, with the same ofnode */

• return device_bind_driver_to_node(dev, "mvebu-reset", "mvebu-reset",

• 			  dev_ofnode(dev), NULL);
  

• if (IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_RESET)) {

• ret = device_bind_driver_to_node(dev, "mvebu-reset",
  

• 				 "mvebu-reset", dev_ofnode(dev),
  

• 				 NULL);
  

• if (ret < 0)
  

• 	return ret;
  

• }

• /* bind also mvebu-sysreset, with the same ofnode */

• if (IS_ENABLED(CONFIG_ARMADA_32BIT_SYSCON_SYSRESET)) {

• ret = device_bind_driver_to_node(dev, "mvebu-sysreset",
  

• 				 "mvebu-sysreset",
  

• 				 dev_ofnode(dev), NULL);
  

• if (ret < 0)
  

• 	return ret;
  

• }

• return ret; }

  static const struct udevice_id mvebu_syscon_of_match[] = {

Viele Grüße, Stefan Roese