* [PATCH v2 0/5] Add SIMATIC IOT2050 board support
@ 2021-06-02 9:37 Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 1/5] arm: dts: Add IOT2050 device tree files Jan Kiszka
` (5 more replies)
0 siblings, 6 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
This is the baseline support for the SIMATIC IOT2050 devices.
Changes in v2:
- rebased
- sync with upstream-accepted DT
- add boot switch
- include watchdog support
Allows to boot mainline 5.10 kernels, but not the original BSP-derived
kernel we currently ship as reference. This is due to the TI sysfw ABI
breakages between 2.x and 3.x. We will soon provide a transitional
kernel that allows booting both firmware ABIs - as long as full upstream
kernel support is work in progress.
Note that this baseline support lacks Ethernet drivers. We are working
closely with TI to ensure that the to-be-upstreamed icssg-prueth driver
will work both with new SR2.0 AM65x silicon as well as with SR1.0 which
is used in the currently shipped IOT2050 devices.
A staging tree for complete IOT2050 support can be found at [1]. Full
image integration is available via [2].
Regarding patch 4: There has been some doubts on the proposed approach,
but there has been also no suggestion provided for a similarly
lightweight and secure embedding method. Therefore, I'm proposing our
solution once again.
Jan
[1] https://github.com/siemens/u-boot/commits/jan/iot2050
[2] https://github.com/siemens/meta-iot2050
Jan Kiszka (5):
arm: dts: Add IOT2050 device tree files
board: siemens: Add support for SIMATIC IOT2050 devices
arm64: dts: ti: k3-am65-mcu: Add RTI watchdog entry
watchdog: rti_wdt: Add support for loading firmware
configs: iot2050: Enable watchdog support, but do not auto-start it
arch/arm/dts/Makefile | 7 +-
arch/arm/dts/k3-am65-iot2050-boot-image.dtsi | 105 +++
.../dts/k3-am65-iot2050-common-u-boot.dtsi | 103 +++
arch/arm/dts/k3-am65-iot2050-common.dtsi | 655 ++++++++++++++++++
arch/arm/dts/k3-am65-iot2050-spl.dts | 16 +
arch/arm/dts/k3-am65-mcu.dtsi | 9 +
arch/arm/dts/k3-am6528-iot2050-basic.dts | 67 ++
arch/arm/dts/k3-am6548-iot2050-advanced.dts | 66 ++
arch/arm/mach-k3/Kconfig | 1 +
board/siemens/iot2050/Kconfig | 32 +
board/siemens/iot2050/MAINTAINERS | 8 +
board/siemens/iot2050/Makefile | 10 +
board/siemens/iot2050/README | 65 ++
board/siemens/iot2050/board.c | 278 ++++++++
board/siemens/iot2050/config.mk | 8 +
configs/iot2050_defconfig | 146 ++++
drivers/watchdog/Kconfig | 20 +
drivers/watchdog/Makefile | 5 +
drivers/watchdog/rti_wdt.c | 58 +-
drivers/watchdog/rti_wdt_fw.S | 20 +
include/configs/iot2050.h | 60 ++
21 files changed, 1737 insertions(+), 2 deletions(-)
create mode 100644 arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-common.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-spl.dts
create mode 100644 arch/arm/dts/k3-am6528-iot2050-basic.dts
create mode 100644 arch/arm/dts/k3-am6548-iot2050-advanced.dts
create mode 100644 board/siemens/iot2050/Kconfig
create mode 100644 board/siemens/iot2050/MAINTAINERS
create mode 100644 board/siemens/iot2050/Makefile
create mode 100644 board/siemens/iot2050/README
create mode 100644 board/siemens/iot2050/board.c
create mode 100644 board/siemens/iot2050/config.mk
create mode 100644 configs/iot2050_defconfig
create mode 100644 drivers/watchdog/rti_wdt_fw.S
create mode 100644 include/configs/iot2050.h
--
2.26.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2 1/5] arm: dts: Add IOT2050 device tree files
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
@ 2021-06-02 9:37 ` Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 2/5] board: siemens: Add support for SIMATIC IOT2050 devices Jan Kiszka
` (4 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
From: Jan Kiszka <jan.kiszka@siemens.com>
Prepares for the addition of the IOT2050 board which is based on the TI
AM65x. The board comes in two variants, Basic and Advanced, so there are
separate dts files. Furthermore, the SPL has its own device tree.
Based on original board support by Le Jin, Gao Nian and Chao Zeng.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
arch/arm/dts/Makefile | 7 +-
arch/arm/dts/k3-am65-iot2050-boot-image.dtsi | 105 +++
.../dts/k3-am65-iot2050-common-u-boot.dtsi | 103 +++
arch/arm/dts/k3-am65-iot2050-common.dtsi | 655 ++++++++++++++++++
arch/arm/dts/k3-am65-iot2050-spl.dts | 16 +
arch/arm/dts/k3-am6528-iot2050-basic.dts | 67 ++
arch/arm/dts/k3-am6548-iot2050-advanced.dts | 66 ++
7 files changed, 1018 insertions(+), 1 deletion(-)
create mode 100644 arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-common.dtsi
create mode 100644 arch/arm/dts/k3-am65-iot2050-spl.dts
create mode 100644 arch/arm/dts/k3-am6528-iot2050-basic.dts
create mode 100644 arch/arm/dts/k3-am6548-iot2050-advanced.dts
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 096068261d..003bc02633 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1065,7 +1065,12 @@ dtb-$(CONFIG_STM32MP15x) += \
stm32mp15xx-dhcom-picoitx.dtb \
stm32mp15xx-dhcor-avenger96.dtb
-dtb-$(CONFIG_SOC_K3_AM6) += k3-am654-base-board.dtb k3-am654-r5-base-board.dtb
+dtb-$(CONFIG_SOC_K3_AM6) += \
+ k3-am654-base-board.dtb \
+ k3-am654-r5-base-board.dtb \
+ k3-am65-iot2050-spl.dtb \
+ k3-am6528-iot2050-basic.dtb \
+ k3-am6548-iot2050-advanced.dtb
dtb-$(CONFIG_SOC_K3_J721E) += k3-j721e-common-proc-board.dtb \
k3-j721e-r5-common-proc-board.dtb \
k3-j7200-common-proc-board.dtb \
diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
new file mode 100644
index 0000000000..0565007597
--- /dev/null
+++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
@@ -0,0 +1,105 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2020-2021
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ */
+
+#include <config.h>
+
+/ {
+ binman {
+ filename = "flash.bin";
+ pad-byte = <0xff>;
+ size = <0x800000>;
+
+ blob-ext@0x000000 {
+ offset = <0x000000>;
+ filename = "tiboot3.bin";
+ };
+
+ blob@0x080000 {
+ offset = <0x080000>;
+ filename = "tispl.bin";
+ };
+
+ fit@0x280000 {
+ description = "U-Boot for IOT2050";
+ offset = <0x280000>;
+ images {
+ u-boot {
+ description = "U-Boot";
+ type = "standalone";
+ arch = "arm64";
+ os = "u-boot";
+ compression = "none";
+ load = <0x80800000>;
+ entry = <0x80800000>;
+ u-boot-nodtb {
+ };
+ };
+
+ fdt-iot2050-basic {
+ description = "k3-am6528-iot2050-basic.dtb";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ blob {
+ filename = "arch/arm/dts/k3-am6528-iot2050-basic.dtb";
+ };
+ };
+
+ fdt-iot2050-advanced {
+ description = "k3-am6548-iot2050-advanced.dtb";
+ type = "flat_dt";
+ arch = "arm64";
+ compression = "none";
+ blob {
+ filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
+ };
+ };
+ };
+
+ configurations {
+ default = "conf-iot2050-basic";
+
+ conf-iot2050-basic {
+ description = "iot2050-basic";
+ firmware = "u-boot";
+ fdt = "fdt-iot2050-basic";
+ };
+
+ conf-iot2050-advanced {
+ description = "iot2050-advanced";
+ firmware = "u-boot";
+ fdt = "fdt-iot2050-advanced";
+ };
+ };
+ };
+
+ /* primary env */
+ fill@0x680000 {
+ offset = <0x680000>;
+ size = <0x020000>;
+ fill-byte = [00];
+ };
+ /* secondary env */
+ fill@0x6a0000 {
+ offset = <0x6a0000>;
+ size = <0x020000>;
+ fill-byte = [00];
+ };
+
+ /* Sysfw Basic */
+ blob-ext@0x6c0000 {
+ offset = <0x6c0000>;
+ filename = "sysfw.itb";
+ };
+ /* Sysfw Advanced */
+ blob-ext@0x740000 {
+ offset = <0x740000>;
+ filename = "sysfw.itb_HS";
+ };
+ };
+};
diff --git a/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi b/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
new file mode 100644
index 0000000000..cb5d8532a0
--- /dev/null
+++ b/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ *
+ * Common U-Boot bits of the IOT2050 Basic and Advanced variants
+ */
+
+/dts-v1/;
+
+#include "k3-am65-iot2050-common.dtsi"
+
+/ {
+ aliases {
+ spi0 = &ospi0;
+ };
+
+ leds {
+ u-boot,dm-spl;
+ status-led-red {
+ u-boot,dm-spl;
+ };
+ status-led-green {
+ u-boot,dm-spl;
+ };
+ };
+};
+
+&cbass_mcu {
+ u-boot,dm-spl;
+};
+
+&cbass_wakeup {
+ u-boot,dm-spl;
+};
+
+&cbass_main {
+ u-boot,dm-spl;
+ main-navss {
+ u-boot,dm-spl;
+ };
+};
+
+&wkup_pmx0 {
+ u-boot,dm-spl;
+ mcu-fss0-ospi0-pins-default {
+ u-boot,dm-spl;
+ };
+};
+
+&main_pmx0 {
+ u-boot,dm-spl;
+ main-uart1-pins-default {
+ u-boot,dm-spl;
+ };
+};
+
+&main_uart1 {
+ u-boot,dm-spl;
+ current-speed = <115200>;
+};
+
+&wkup_gpio0 {
+ u-boot,dm-spl;
+};
+
+&ospi0 {
+ u-boot,dm-spl;
+ flash@0 {
+ u-boot,dm-spl;
+ };
+};
+
+&secure_proxy_main {
+ u-boot,dm-spl;
+};
+
+&dmsc {
+ u-boot,dm-spl;
+ k3_sysreset: sysreset-controller {
+ compatible = "ti,sci-sysreset";
+ u-boot,dm-spl;
+ };
+};
+
+&k3_pds {
+ u-boot,dm-spl;
+};
+
+&k3_clks {
+ u-boot,dm-spl;
+};
+
+&k3_reset {
+ u-boot,dm-spl;
+};
+
+&fss {
+ u-boot,dm-spl;
+};
diff --git a/arch/arm/dts/k3-am65-iot2050-common.dtsi b/arch/arm/dts/k3-am65-iot2050-common.dtsi
new file mode 100644
index 0000000000..de763ca925
--- /dev/null
+++ b/arch/arm/dts/k3-am65-iot2050-common.dtsi
@@ -0,0 +1,655 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ *
+ * Common bits of the IOT2050 Basic and Advanced variants
+ */
+
+/dts-v1/;
+
+#include "k3-am654.dtsi"
+#include <dt-bindings/phy/phy.h>
+
+/ {
+ aliases {
+ spi0 = &mcu_spi0;
+ };
+
+ chosen {
+ stdout-path = "serial3:115200n8";
+ bootargs = "earlycon=ns16550a,mmio32,0x02810000";
+ };
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ secure_ddr: secure-ddr@9e800000 {
+ reg = <0 0x9e800000 0 0x01800000>; /* for OP-TEE */
+ alignment = <0x1000>;
+ no-map;
+ };
+
+ mcu_r5fss0_core0_dma_memory_region: r5f-dma-memory@a0000000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0xa0000000 0 0x100000>;
+ no-map;
+ };
+
+ mcu_r5fss0_core0_memory_region: r5f-memory@a0100000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0xa0100000 0 0xf00000>;
+ no-map;
+ };
+
+ mcu_r5fss0_core1_dma_memory_region: r5f-dma-memory@a1000000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0xa1000000 0 0x100000>;
+ no-map;
+ };
+
+ mcu_r5fss0_core1_memory_region: r5f-memory@a1100000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0xa1100000 0 0xf00000>;
+ no-map;
+ };
+
+ rtos_ipc_memory_region: ipc-memories@a2000000 {
+ reg = <0x00 0xa2000000 0x00 0x00200000>;
+ alignment = <0x1000>;
+ no-map;
+ };
+ };
+
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&leds_pins_default>;
+
+ status-led-red {
+ gpios = <&wkup_gpio0 32 GPIO_ACTIVE_HIGH>;
+ panic-indicator;
+ };
+
+ status-led-green {
+ gpios = <&wkup_gpio0 24 GPIO_ACTIVE_HIGH>;
+ };
+
+ user-led1-red {
+ gpios = <&pcal9535_3 14 GPIO_ACTIVE_HIGH>;
+ };
+
+ user-led1-green {
+ gpios = <&pcal9535_2 15 GPIO_ACTIVE_HIGH>;
+ };
+
+ user-led2-red {
+ gpios = <&wkup_gpio0 17 GPIO_ACTIVE_HIGH>;
+ };
+
+ user-led2-green {
+ gpios = <&wkup_gpio0 22 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+ dp_refclk: clock {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <19200000>;
+ };
+};
+
+&wkup_pmx0 {
+ wkup_i2c0_pins_default: wkup-i2c0-pins-default {
+ pinctrl-single,pins = <
+ /* (AC7) WKUP_I2C0_SCL */
+ AM65X_WKUP_IOPAD(0x00e0, PIN_INPUT, 0)
+ /* (AD6) WKUP_I2C0_SDA */
+ AM65X_WKUP_IOPAD(0x00e4, PIN_INPUT, 0)
+ >;
+ };
+
+ mcu_i2c0_pins_default: mcu-i2c0-pins-default {
+ pinctrl-single,pins = <
+ /* (AD8) MCU_I2C0_SCL */
+ AM65X_WKUP_IOPAD(0x00e8, PIN_INPUT, 0)
+ /* (AD7) MCU_I2C0_SDA */
+ AM65X_WKUP_IOPAD(0x00ec, PIN_INPUT, 0)
+ >;
+ };
+
+ arduino_i2c_aio_switch_pins_default: arduino-i2c-aio-switch-pins-default {
+ pinctrl-single,pins = <
+ /* (R2) WKUP_GPIO0_21 */
+ AM65X_WKUP_IOPAD(0x0024, PIN_OUTPUT, 7)
+ >;
+ };
+
+ push_button_pins_default: push-button-pins-default {
+ pinctrl-single,pins = <
+ /* (T1) MCU_OSPI1_CLK.WKUP_GPIO0_25 */
+ AM65X_WKUP_IOPAD(0x0034, PIN_INPUT, 7)
+ >;
+ };
+
+ arduino_uart_pins_default: arduino-uart-pins-default {
+ pinctrl-single,pins = <
+ /* (P4) MCU_UART0_RXD */
+ AM65X_WKUP_IOPAD(0x0044, PIN_INPUT, 4)
+ /* (P5) MCU_UART0_TXD */
+ AM65X_WKUP_IOPAD(0x0048, PIN_OUTPUT, 4)
+ >;
+ };
+
+ arduino_io_d2_to_d3_pins_default: arduino-io-d2-to-d3-pins-default {
+ pinctrl-single,pins = <
+ /* (P1) WKUP_GPIO0_31 */
+ AM65X_WKUP_IOPAD(0x004C, PIN_OUTPUT, 7)
+ /* (N3) WKUP_GPIO0_33 */
+ AM65X_WKUP_IOPAD(0x0054, PIN_OUTPUT, 7)
+ >;
+ };
+
+ arduino_io_oe_pins_default: arduino-io-oe-pins-default {
+ pinctrl-single,pins = <
+ /* (N4) WKUP_GPIO0_34 */
+ AM65X_WKUP_IOPAD(0x0058, PIN_OUTPUT, 7)
+ /* (M2) WKUP_GPIO0_36 */
+ AM65X_WKUP_IOPAD(0x0060, PIN_OUTPUT, 7)
+ /* (M3) WKUP_GPIO0_37 */
+ AM65X_WKUP_IOPAD(0x0064, PIN_OUTPUT, 7)
+ /* (M4) WKUP_GPIO0_38 */
+ AM65X_WKUP_IOPAD(0x0068, PIN_OUTPUT, 7)
+ /* (M1) WKUP_GPIO0_41 */
+ AM65X_WKUP_IOPAD(0x0074, PIN_OUTPUT, 7)
+ >;
+ };
+
+ mcu_fss0_ospi0_pins_default: mcu-fss0-ospi0-pins-default {
+ pinctrl-single,pins = <
+ /* (V1) MCU_OSPI0_CLK */
+ AM65X_WKUP_IOPAD(0x0000, PIN_OUTPUT, 0)
+ /* (U2) MCU_OSPI0_DQS */
+ AM65X_WKUP_IOPAD(0x0008, PIN_INPUT, 0)
+ /* (U4) MCU_OSPI0_D0 */
+ AM65X_WKUP_IOPAD(0x000c, PIN_INPUT, 0)
+ /* (U5) MCU_OSPI0_D1 */
+ AM65X_WKUP_IOPAD(0x0010, PIN_INPUT, 0)
+ /* (R4) MCU_OSPI0_CSn0 */
+ AM65X_WKUP_IOPAD(0x002c, PIN_OUTPUT, 0)
+ >;
+ };
+
+ db9_com_mode_pins_default: db9-com-mode-pins-default {
+ pinctrl-single,pins = <
+ /* (AD3) WKUP_GPIO0_5, used as uart0 mode 0 */
+ AM65X_WKUP_IOPAD(0x00c4, PIN_OUTPUT, 7)
+ /* (AC3) WKUP_GPIO0_4, used as uart0 mode 1 */
+ AM65X_WKUP_IOPAD(0x00c0, PIN_OUTPUT, 7)
+ /* (AC1) WKUP_GPIO0_7, used as uart0 term */
+ AM65X_WKUP_IOPAD(0x00cc, PIN_OUTPUT, 7)
+ /* (AC2) WKUP_GPIO0_6, used as uart0 en */
+ AM65X_WKUP_IOPAD(0x00c8, PIN_OUTPUT, 7)
+ >;
+ };
+
+ leds_pins_default: leds-pins-default {
+ pinctrl-single,pins = <
+ /* (T2) WKUP_GPIO0_17, used as user led1 red */
+ AM65X_WKUP_IOPAD(0x0014, PIN_OUTPUT, 7)
+ /* (R3) WKUP_GPIO0_22, used as user led1 green */
+ AM65X_WKUP_IOPAD(0x0028, PIN_OUTPUT, 7)
+ /* (R5) WKUP_GPIO0_24, used as status led red */
+ AM65X_WKUP_IOPAD(0x0030, PIN_OUTPUT, 7)
+ /* (N2) WKUP_GPIO0_32, used as status led green */
+ AM65X_WKUP_IOPAD(0x0050, PIN_OUTPUT, 7)
+ >;
+ };
+
+ mcu_spi0_pins_default: mcu-spi0-pins-default {
+ pinctrl-single,pins = <
+ /* (Y1) MCU_SPI0_CLK */
+ AM65X_WKUP_IOPAD(0x0090, PIN_INPUT, 0)
+ /* (Y3) MCU_SPI0_D0 */
+ AM65X_WKUP_IOPAD(0x0094, PIN_INPUT, 0)
+ /* (Y2) MCU_SPI0_D1 */
+ AM65X_WKUP_IOPAD(0x0098, PIN_INPUT, 0)
+ /* (Y4) MCU_SPI0_CS0 */
+ AM65X_WKUP_IOPAD(0x009c, PIN_OUTPUT, 0)
+ >;
+ };
+
+ minipcie_pins_default: minipcie-pins-default {
+ pinctrl-single,pins = <
+ /* (P2) MCU_OSPI1_DQS.WKUP_GPIO0_27 */
+ AM65X_WKUP_IOPAD(0x003C, PIN_OUTPUT, 7)
+ >;
+ };
+};
+
+&main_pmx0 {
+ main_uart1_pins_default: main-uart1-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0174, PIN_INPUT, 6) /* (AE23) UART1_RXD */
+ AM65X_IOPAD(0x014c, PIN_OUTPUT, 6) /* (AD23) UART1_TXD */
+ AM65X_IOPAD(0x0178, PIN_INPUT, 6) /* (AD22) UART1_CTSn */
+ AM65X_IOPAD(0x017c, PIN_OUTPUT, 6) /* (AC21) UART1_RTSn */
+ >;
+ };
+
+ main_i2c3_pins_default: main-i2c3-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x01c0, PIN_INPUT, 2) /* (AF13) I2C3_SCL */
+ AM65X_IOPAD(0x01d4, PIN_INPUT, 2) /* (AG12) I2C3_SDA */
+ >;
+ };
+
+ main_mmc1_pins_default: main-mmc1-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x02d4, PIN_INPUT_PULLDOWN, 0) /* (C27) MMC1_CLK */
+ AM65X_IOPAD(0x02d8, PIN_INPUT_PULLUP, 0) /* (C28) MMC1_CMD */
+ AM65X_IOPAD(0x02d0, PIN_INPUT_PULLUP, 0) /* (D28) MMC1_DAT0 */
+ AM65X_IOPAD(0x02cc, PIN_INPUT_PULLUP, 0) /* (E27) MMC1_DAT1 */
+ AM65X_IOPAD(0x02c8, PIN_INPUT_PULLUP, 0) /* (D26) MMC1_DAT2 */
+ AM65X_IOPAD(0x02c4, PIN_INPUT_PULLUP, 0) /* (D27) MMC1_DAT3 */
+ AM65X_IOPAD(0x02dc, PIN_INPUT_PULLUP, 0) /* (B24) MMC1_SDCD */
+ AM65X_IOPAD(0x02e0, PIN_INPUT_PULLUP, 0) /* (C24) MMC1_SDWP */
+ >;
+ };
+
+ usb0_pins_default: usb0-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x02bc, PIN_OUTPUT, 0) /* (AD9) USB0_DRVVBUS */
+ >;
+ };
+
+ usb1_pins_default: usb1-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x02c0, PIN_OUTPUT, 0) /* (AC8) USB1_DRVVBUS */
+ >;
+ };
+
+ arduino_io_d4_to_d9_pins_default: arduino-io-d4-to-d9-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0084, PIN_OUTPUT, 7) /* (AG18) GPIO0_33 */
+ AM65X_IOPAD(0x008C, PIN_OUTPUT, 7) /* (AF17) GPIO0_35 */
+ AM65X_IOPAD(0x0098, PIN_OUTPUT, 7) /* (AH16) GPIO0_38 */
+ AM65X_IOPAD(0x00AC, PIN_OUTPUT, 7) /* (AH15) GPIO0_43 */
+ AM65X_IOPAD(0x00C0, PIN_OUTPUT, 7) /* (AG15) GPIO0_48 */
+ AM65X_IOPAD(0x00CC, PIN_OUTPUT, 7) /* (AD15) GPIO0_51 */
+ >;
+ };
+
+ dss_vout1_pins_default: dss-vout1-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0000, PIN_OUTPUT, 1) /* VOUT1_DATA0 */
+ AM65X_IOPAD(0x0004, PIN_OUTPUT, 1) /* VOUT1_DATA1 */
+ AM65X_IOPAD(0x0008, PIN_OUTPUT, 1) /* VOUT1_DATA2 */
+ AM65X_IOPAD(0x000c, PIN_OUTPUT, 1) /* VOUT1_DATA3 */
+ AM65X_IOPAD(0x0010, PIN_OUTPUT, 1) /* VOUT1_DATA4 */
+ AM65X_IOPAD(0x0014, PIN_OUTPUT, 1) /* VOUT1_DATA5 */
+ AM65X_IOPAD(0x0018, PIN_OUTPUT, 1) /* VOUT1_DATA6 */
+ AM65X_IOPAD(0x001c, PIN_OUTPUT, 1) /* VOUT1_DATA7 */
+ AM65X_IOPAD(0x0020, PIN_OUTPUT, 1) /* VOUT1_DATA8 */
+ AM65X_IOPAD(0x0024, PIN_OUTPUT, 1) /* VOUT1_DATA9 */
+ AM65X_IOPAD(0x0028, PIN_OUTPUT, 1) /* VOUT1_DATA10 */
+ AM65X_IOPAD(0x002c, PIN_OUTPUT, 1) /* VOUT1_DATA11 */
+ AM65X_IOPAD(0x0030, PIN_OUTPUT, 1) /* VOUT1_DATA12 */
+ AM65X_IOPAD(0x0034, PIN_OUTPUT, 1) /* VOUT1_DATA13 */
+ AM65X_IOPAD(0x0038, PIN_OUTPUT, 1) /* VOUT1_DATA14 */
+ AM65X_IOPAD(0x003c, PIN_OUTPUT, 1) /* VOUT1_DATA15 */
+ AM65X_IOPAD(0x0040, PIN_OUTPUT, 1) /* VOUT1_DATA16 */
+ AM65X_IOPAD(0x0044, PIN_OUTPUT, 1) /* VOUT1_DATA17 */
+ AM65X_IOPAD(0x0048, PIN_OUTPUT, 1) /* VOUT1_DATA18 */
+ AM65X_IOPAD(0x004c, PIN_OUTPUT, 1) /* VOUT1_DATA19 */
+ AM65X_IOPAD(0x0050, PIN_OUTPUT, 1) /* VOUT1_DATA20 */
+ AM65X_IOPAD(0x0054, PIN_OUTPUT, 1) /* VOUT1_DATA21 */
+ AM65X_IOPAD(0x0058, PIN_OUTPUT, 1) /* VOUT1_DATA22 */
+ AM65X_IOPAD(0x005c, PIN_OUTPUT, 1) /* VOUT1_DATA23 */
+ AM65X_IOPAD(0x0060, PIN_OUTPUT, 1) /* VOUT1_VSYNC */
+ AM65X_IOPAD(0x0064, PIN_OUTPUT, 1) /* VOUT1_HSYNC */
+ AM65X_IOPAD(0x0068, PIN_OUTPUT, 1) /* VOUT1_PCLK */
+ AM65X_IOPAD(0x006c, PIN_OUTPUT, 1) /* VOUT1_DE */
+ >;
+ };
+
+ dp_pins_default: dp-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0078, PIN_OUTPUT, 7) /* (AF18) DP rst_n */
+ >;
+ };
+
+ main_i2c2_pins_default: main-i2c2-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0074, PIN_INPUT, 5) /* (T27) I2C2_SCL */
+ AM65X_IOPAD(0x0070, PIN_INPUT, 5) /* (R25) I2C2_SDA */
+ >;
+ };
+};
+
+&main_pmx1 {
+ main_i2c0_pins_default: main-i2c0-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0000, PIN_INPUT, 0) /* (D20) I2C0_SCL */
+ AM65X_IOPAD(0x0004, PIN_INPUT, 0) /* (C21) I2C0_SDA */
+ >;
+ };
+
+ main_i2c1_pins_default: main-i2c1-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0008, PIN_INPUT, 0) /* (B21) I2C1_SCL */
+ AM65X_IOPAD(0x000c, PIN_INPUT, 0) /* (E21) I2C1_SDA */
+ >;
+ };
+
+ ecap0_pins_default: ecap0-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x0010, PIN_INPUT, 0) /* (D21) ECAP0_IN_APWM_OUT */
+ >;
+ };
+};
+
+&wkup_uart0 {
+ /* Wakeup UART is used by System firmware */
+ status = "reserved";
+};
+
+&main_uart1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_uart1_pins_default>;
+};
+
+&main_uart2 {
+ status = "disabled";
+};
+
+&mcu_uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&arduino_uart_pins_default>;
+};
+
+&main_gpio0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&arduino_io_d4_to_d9_pins_default>;
+ gpio-line-names =
+ "main_gpio0-base", "", "", "", "", "", "", "", "", "",
+ "", "", "", "", "", "", "", "", "", "",
+ "", "", "", "", "", "", "", "", "", "",
+ "", "", "", "IO4", "", "IO5", "", "", "IO6", "",
+ "", "", "", "IO7", "", "", "", "", "IO8", "",
+ "", "IO9";
+};
+
+&wkup_gpio0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <
+ &arduino_io_d2_to_d3_pins_default
+ &arduino_i2c_aio_switch_pins_default
+ &arduino_io_oe_pins_default
+ &push_button_pins_default
+ &db9_com_mode_pins_default
+ >;
+ gpio-line-names =
+ /* 0..9 */
+ "wkup_gpio0-base", "", "", "", "UART0-mode1", "UART0-mode0",
+ "UART0-enable", "UART0-terminate", "", "WIFI-disable",
+ /* 10..19 */
+ "", "", "", "", "", "", "", "", "", "",
+ /* 20..29 */
+ "", "A4A5-I2C-mux", "", "", "", "USER-button", "", "", "","IO0",
+ /* 30..39 */
+ "IO1", "IO2", "", "IO3", "IO17-direction", "A5",
+ "IO16-direction", "IO15-direction", "IO14-direction", "A3",
+ /* 40..49 */
+ "", "IO18-direction", "A4", "A2", "A1", "A0", "", "", "IO13",
+ "IO11",
+ /* 50..51 */
+ "IO12", "IO10";
+};
+
+&wkup_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&wkup_i2c0_pins_default>;
+ clock-frequency = <400000>;
+};
+
+&mcu_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_i2c0_pins_default>;
+ clock-frequency = <400000>;
+
+ psu: regulator@60 {
+ compatible = "ti,tps62363";
+ reg = <0x60>;
+ regulator-name = "tps62363-vout";
+ regulator-min-microvolt = <500000>;
+ regulator-max-microvolt = <1500000>;
+ regulator-boot-on;
+ ti,vsel0-state-high;
+ ti,vsel1-state-high;
+ ti,enable-vout-discharge;
+ };
+
+ /* D4200 */
+ pcal9535_1: gpio@20 {
+ compatible = "nxp,pcal9535";
+ reg = <0x20>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ gpio-line-names =
+ "A0-pull", "A1-pull", "A2-pull", "A3-pull", "A4-pull",
+ "A5-pull", "", "",
+ "IO14-enable", "IO15-enable", "IO16-enable",
+ "IO17-enable", "IO18-enable", "IO19-enable";
+ };
+
+ /* D4201 */
+ pcal9535_2: gpio@21 {
+ compatible = "nxp,pcal9535";
+ reg = <0x21>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ gpio-line-names =
+ "IO0-direction", "IO1-direction", "IO2-direction",
+ "IO3-direction", "IO4-direction", "IO5-direction",
+ "IO6-direction", "IO7-direction",
+ "IO8-direction", "IO9-direction", "IO10-direction",
+ "IO11-direction", "IO12-direction", "IO13-direction",
+ "IO19-direction";
+ };
+
+ /* D4202 */
+ pcal9535_3: gpio@25 {
+ compatible = "nxp,pcal9535";
+ reg = <0x25>;
+ #gpio-cells = <2>;
+ gpio-controller;
+ gpio-line-names =
+ "IO0-pull", "IO1-pull", "IO2-pull", "IO3-pull",
+ "IO4-pull", "IO5-pull", "IO6-pull", "IO7-pull",
+ "IO8-pull", "IO9-pull", "IO10-pull", "IO11-pull",
+ "IO12-pull", "IO13-pull";
+ };
+};
+
+&main_i2c0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c0_pins_default>;
+ clock-frequency = <400000>;
+
+ rtc: rtc8564@51 {
+ compatible = "nxp,pcf8563";
+ reg = <0x51>;
+ };
+
+ eeprom: eeprom@54 {
+ compatible = "atmel,24c08";
+ reg = <0x54>;
+ pagesize = <16>;
+ };
+};
+
+&main_i2c1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c1_pins_default>;
+ clock-frequency = <400000>;
+};
+
+&main_i2c2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c2_pins_default>;
+ clock-frequency = <400000>;
+};
+
+&main_i2c3 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c3_pins_default>;
+ clock-frequency = <400000>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ edp-bridge@f {
+ compatible = "toshiba,tc358767";
+ reg = <0x0f>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&dp_pins_default>;
+ reset-gpios = <&main_gpio0 30 GPIO_ACTIVE_HIGH>;
+
+ clock-names = "ref";
+ clocks = <&dp_refclk>;
+
+ toshiba,hpd-pin = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ bridge_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+ };
+ };
+};
+
+&mcu_cpsw {
+ status = "disabled";
+};
+
+&ecap0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&ecap0_pins_default>;
+};
+
+&sdhci1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mmc1_pins_default>;
+ ti,driver-strength-ohm = <50>;
+ disable-wp;
+};
+
+&usb0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb0_pins_default>;
+ dr_mode = "host";
+};
+
+&usb1 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb1_pins_default>;
+ dr_mode = "host";
+};
+
+&mcu_spi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_spi0_pins_default>;
+
+ #address-cells = <1>;
+ #size-cells= <0>;
+ ti,pindir-d0-out-d1-in = <1>;
+};
+
+&tscadc0 {
+ status = "disabled";
+};
+
+&tscadc1 {
+ adc {
+ ti,adc-channels = <0 1 2 3 4 5>;
+ };
+};
+
+&ospi0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&mcu_fss0_ospi0_pins_default>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0x0>;
+ spi-tx-bus-width = <1>;
+ spi-rx-bus-width = <1>;
+ spi-max-frequency = <50000000>;
+ cdns,tshsl-ns = <60>;
+ cdns,tsd2d-ns = <60>;
+ cdns,tchsh-ns = <60>;
+ cdns,tslch-ns = <60>;
+ cdns,read-delay = <2>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ };
+};
+
+&dss {
+ pinctrl-names = "default";
+ pinctrl-0 = <&dss_vout1_pins_default>;
+
+ assigned-clocks = <&k3_clks 67 2>;
+ assigned-clock-parents = <&k3_clks 67 5>;
+};
+
+&dss_ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@1 {
+ reg = <1>;
+
+ dpi_out: endpoint {
+ remote-endpoint = <&bridge_in>;
+ };
+ };
+};
+
+&serdes0 {
+ status = "disabled";
+};
+
+&pcie0_rc {
+ status = "disabled";
+};
+
+&pcie0_ep {
+ status = "disabled";
+};
+
+&pcie1_rc {
+ pinctrl-names = "default";
+ pinctrl-0 = <&minipcie_pins_default>;
+
+ num-lanes = <1>;
+ phys = <&serdes1 PHY_TYPE_PCIE 0>;
+ phy-names = "pcie-phy0";
+ reset-gpios = <&wkup_gpio0 27 GPIO_ACTIVE_HIGH>;
+};
+
+&pcie1_ep {
+ status = "disabled";
+};
diff --git a/arch/arm/dts/k3-am65-iot2050-spl.dts b/arch/arm/dts/k3-am65-iot2050-spl.dts
new file mode 100644
index 0000000000..b6bd148fd3
--- /dev/null
+++ b/arch/arm/dts/k3-am65-iot2050-spl.dts
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ */
+
+/dts-v1/;
+
+#include "k3-am65-iot2050-common-u-boot.dtsi"
+
+/ {
+ compatible = "siemens,iot2050", "ti,am654";
+ model = "Siemens IOT2050";
+};
diff --git a/arch/arm/dts/k3-am6528-iot2050-basic.dts b/arch/arm/dts/k3-am6528-iot2050-basic.dts
new file mode 100644
index 0000000000..4d1dd69719
--- /dev/null
+++ b/arch/arm/dts/k3-am6528-iot2050-basic.dts
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ *
+ * AM6528-based (dual-core) IOT2050 Basic variant
+ * 1 GB RAM, no eMMC, main_uart0 on connector X30
+ */
+
+/dts-v1/;
+
+#include "k3-am65-iot2050-common-u-boot.dtsi"
+#include "k3-am65-iot2050-boot-image.dtsi"
+
+/ {
+ compatible = "siemens,iot2050-basic", "ti,am654";
+ model = "SIMATIC IOT2050 Basic";
+
+ memory@80000000 {
+ device_type = "memory";
+ /* 1G RAM */
+ reg = <0x00000000 0x80000000 0x00000000 0x40000000>;
+ };
+
+ cpus {
+ cpu-map {
+ /delete-node/ cluster1;
+ };
+ /delete-node/ cpu@100;
+ /delete-node/ cpu@101;
+ };
+
+ /delete-node/ l2-cache1;
+};
+
+/* eMMC */
+&sdhci0 {
+ status = "disabled";
+};
+
+&main_pmx0 {
+ main_uart0_pins_default: main-uart0-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x01e4, PIN_INPUT, 0) /* (AF11) UART0_RXD */
+ AM65X_IOPAD(0x01e8, PIN_OUTPUT, 0) /* (AE11) UART0_TXD */
+ AM65X_IOPAD(0x01ec, PIN_INPUT, 0) /* (AG11) UART0_CTSn */
+ AM65X_IOPAD(0x01f0, PIN_OUTPUT, 0) /* (AD11) UART0_RTSn */
+ AM65X_IOPAD(0x0188, PIN_INPUT, 1) /* (D25) UART0_DCDn */
+ AM65X_IOPAD(0x018c, PIN_INPUT, 1) /* (B26) UART0_DSRn */
+ AM65X_IOPAD(0x0190, PIN_OUTPUT, 1) /* (A24) UART0_DTRn */
+ AM65X_IOPAD(0x0194, PIN_INPUT, 1) /* (E24) UART0_RIN */
+ >;
+ };
+};
+
+&main_uart0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_uart0_pins_default>;
+};
+
+&mcu_r5fss0 {
+ /* lock-step mode not supported on this board */
+ ti,cluster-mode = <0>;
+};
diff --git a/arch/arm/dts/k3-am6548-iot2050-advanced.dts b/arch/arm/dts/k3-am6548-iot2050-advanced.dts
new file mode 100644
index 0000000000..cd3e93fce1
--- /dev/null
+++ b/arch/arm/dts/k3-am6548-iot2050-advanced.dts
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ *
+ * AM6548-based (quad-core) IOT2050 Advanced variant
+ * 2 GB RAM, 16 GB eMMC, USB-serial converter on connector X30
+ */
+
+/dts-v1/;
+
+#include "k3-am65-iot2050-common-u-boot.dtsi"
+#include "k3-am65-iot2050-boot-image.dtsi"
+
+/ {
+ compatible = "siemens,iot2050-advanced", "ti,am654";
+ model = "SIMATIC IOT2050 Advanced";
+
+ aliases {
+ mmc0 = &sdhci1;
+ mmc1 = &sdhci0;
+ };
+
+ memory@80000000 {
+ device_type = "memory";
+ /* 2G RAM */
+ reg = <0x00000000 0x80000000 0x00000000 0x80000000>;
+ };
+};
+
+&main_pmx0 {
+ main_mmc0_pins_default: main-mmc0-pins-default {
+ pinctrl-single,pins = <
+ AM65X_IOPAD(0x01a8, PIN_INPUT_PULLDOWN, 0) /* (B25) MMC0_CLK */
+ AM65X_IOPAD(0x01ac, PIN_INPUT_PULLUP, 0) /* (B27) MMC0_CMD */
+ AM65X_IOPAD(0x01a4, PIN_INPUT_PULLUP, 0) /* (A26) MMC0_DAT0 */
+ AM65X_IOPAD(0x01a0, PIN_INPUT_PULLUP, 0) /* (E25) MMC0_DAT1 */
+ AM65X_IOPAD(0x019c, PIN_INPUT_PULLUP, 0) /* (C26) MMC0_DAT2 */
+ AM65X_IOPAD(0x0198, PIN_INPUT_PULLUP, 0) /* (A25) MMC0_DAT3 */
+ AM65X_IOPAD(0x0194, PIN_INPUT_PULLUP, 0) /* (E24) MMC0_DAT4 */
+ AM65X_IOPAD(0x0190, PIN_INPUT_PULLUP, 0) /* (A24) MMC0_DAT5 */
+ AM65X_IOPAD(0x018c, PIN_INPUT_PULLUP, 0) /* (B26) MMC0_DAT6 */
+ AM65X_IOPAD(0x0188, PIN_INPUT_PULLUP, 0) /* (D25) MMC0_DAT7 */
+ AM65X_IOPAD(0x01b8, PIN_OUTPUT_PULLUP, 7) /* (B23) MMC0_SDWP */
+ AM65X_IOPAD(0x01b4, PIN_INPUT_PULLUP, 0) /* (A23) MMC0_SDCD */
+ AM65X_IOPAD(0x01b0, PIN_INPUT, 0) /* (C25) MMC0_DS */
+ >;
+ };
+};
+
+/* eMMC */
+&sdhci0 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_mmc0_pins_default>;
+ bus-width = <8>;
+ non-removable;
+ ti,driver-strength-ohm = <50>;
+ disable-wp;
+};
+
+&main_uart0 {
+ status = "disabled";
+};
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 2/5] board: siemens: Add support for SIMATIC IOT2050 devices
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 1/5] arm: dts: Add IOT2050 device tree files Jan Kiszka
@ 2021-06-02 9:37 ` Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 3/5] arm64: dts: ti: k3-am65-mcu: Add RTI watchdog entry Jan Kiszka
` (3 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
From: Jan Kiszka <jan.kiszka@siemens.com>
This adds support for the IOT2050 Basic and Advanced devices. The Basic
used the dual-core AM6528 GP processor, the Advanced one the AM6548 HS
quad-core version.
Both variants are booted via a Siemens-provided FSBL that runs on the R5
cores. Consequently, U-Boot support is targeting the A53 cores. U-Boot
SPL, ATF and TEE have to reside in SPI flash.
Full integration into a bootable image can be found on
https://github.com/siemens/meta-iot2050
Based on original board support by Le Jin, Gao Nian and Chao Zeng.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
arch/arm/mach-k3/Kconfig | 1 +
board/siemens/iot2050/Kconfig | 32 ++++
board/siemens/iot2050/MAINTAINERS | 8 +
board/siemens/iot2050/Makefile | 10 ++
board/siemens/iot2050/README | 65 +++++++
board/siemens/iot2050/board.c | 278 ++++++++++++++++++++++++++++++
board/siemens/iot2050/config.mk | 8 +
configs/iot2050_defconfig | 140 +++++++++++++++
include/configs/iot2050.h | 60 +++++++
9 files changed, 602 insertions(+)
create mode 100644 board/siemens/iot2050/Kconfig
create mode 100644 board/siemens/iot2050/MAINTAINERS
create mode 100644 board/siemens/iot2050/Makefile
create mode 100644 board/siemens/iot2050/README
create mode 100644 board/siemens/iot2050/board.c
create mode 100644 board/siemens/iot2050/config.mk
create mode 100644 configs/iot2050_defconfig
create mode 100644 include/configs/iot2050.h
diff --git a/arch/arm/mach-k3/Kconfig b/arch/arm/mach-k3/Kconfig
index bfbce44bfa..a082460e04 100644
--- a/arch/arm/mach-k3/Kconfig
+++ b/arch/arm/mach-k3/Kconfig
@@ -150,4 +150,5 @@ config SYS_K3_SPL_ATF
source "board/ti/am65x/Kconfig"
source "board/ti/am64x/Kconfig"
source "board/ti/j721e/Kconfig"
+source "board/siemens/iot2050/Kconfig"
endif
diff --git a/board/siemens/iot2050/Kconfig b/board/siemens/iot2050/Kconfig
new file mode 100644
index 0000000000..8f634c172c
--- /dev/null
+++ b/board/siemens/iot2050/Kconfig
@@ -0,0 +1,32 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) Siemens AG, 2018-2021
+#
+# Authors:
+# Le Jin <le.jin@siemens.com>
+# Jan Kiszka <jan.kiszka@siemens.com>
+
+config TARGET_IOT2050_A53
+ bool "IOT2050 running on A53"
+ select ARM64
+ select SOC_K3_AM6
+ select BOARD_LATE_INIT
+ select SYS_DISABLE_DCACHE_OPS
+ select BINMAN
+
+if TARGET_IOT2050_A53
+
+config SYS_BOARD
+ default "iot2050"
+
+config SYS_VENDOR
+ default "siemens"
+
+config SYS_CONFIG_NAME
+ default "iot2050"
+
+config IOT2050_BOOT_SWITCH
+ bool "Disable eMMC boot via USER button (Advanced version only)"
+ default y
+
+endif
diff --git a/board/siemens/iot2050/MAINTAINERS b/board/siemens/iot2050/MAINTAINERS
new file mode 100644
index 0000000000..028fd22960
--- /dev/null
+++ b/board/siemens/iot2050/MAINTAINERS
@@ -0,0 +1,8 @@
+IOT2050 BOARD
+M: Le Jin <le.jin@siemens.com>
+M: Jan Kiszka <jan.kiszka@siemens.com>
+S: Maintained
+F: board/siemens/iot2050/
+F: include/configs/iot2050.h
+F: configs/iot2050_defconfig
+F: arch/arm/dts/iot2050-*
diff --git a/board/siemens/iot2050/Makefile b/board/siemens/iot2050/Makefile
new file mode 100644
index 0000000000..619594ab8e
--- /dev/null
+++ b/board/siemens/iot2050/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Makefile for Siemens IOT2050 board
+# Copyright (c) Siemens AG, 2018-2021
+#
+# Authors:
+# Le Jin <le.jin@siemens.com>
+#
+
+obj-y += board.o
diff --git a/board/siemens/iot2050/README b/board/siemens/iot2050/README
new file mode 100644
index 0000000000..b63f05b5cf
--- /dev/null
+++ b/board/siemens/iot2050/README
@@ -0,0 +1,65 @@
+SIMATIC IOT2050 BASIC and ADVANCED
+==================================
+
+The SIMATIC IOT2050 is an open industrial IoT gateway that is using the TI
+AM6528 GP (Basic variant) or the AM6548 HS (Advanced variant). The Advanced
+variant is prepared for secure boot.
+
+The IOT2050 starts only from OSPI. It loads a Siemens-provided bootloader
+called SE-Boot for the MCU domain (R5F cores), then hands over to ATF and
+OP-TEE, before booting U-Boot on the A53 cores. This describes how to build all
+open artifacts into a flashable image for the OSPI flash. The flash image will
+work on both variants.
+
+
+Dependencies
+------------
+
+ATF: Upstream release 2.4 or newer
+OP-TEE: Upstream release 3.10.0 or newer
+
+Binary dependencies can be found in
+https://github.com/siemens/meta-iot2050/tree/master/recipes-bsp/u-boot/files/prebuild.
+The following binaries from that source need to be present in the build folder:
+
+ - tiboot3.bin
+ - sysfw.itb
+ - sysfw.itb_HS
+
+
+Building
+--------
+
+Make sure that CROSS_COMPILE is set appropriately:
+
+ export CROSS_COMPILE=aarch64-linux-gnu-
+
+ATF:
+
+ make PLAT=k3 SPD=opteed K3_USART=1
+
+OP-TEE:
+
+ make PLATFORM=k3-am65x CFG_ARM64_core=y \
+ CFG_TEE_CORE_LOG_LEVEL=2 CFG_CONSOLE_UART=1
+
+U-Boot:
+
+ export ATF=/path/to/bl31.bin
+ export TEE=/path/to/tee-pager_v2.bin
+ make iot2050_defconfig
+ make
+
+
+Flashing
+--------
+
+Via U-Boot:
+
+ sf probe
+ load mmc 0:1 $loadaddr /path/to/flash.bin
+ sf update $loadaddr 0x0 $filesize
+
+Via external programmer Dediprog SF100 or SF600:
+
+ dpcmd --vcc 2 -v -u flash.bin
diff --git a/board/siemens/iot2050/board.c b/board/siemens/iot2050/board.c
new file mode 100644
index 0000000000..3d5b5ccb2e
--- /dev/null
+++ b/board/siemens/iot2050/board.c
@@ -0,0 +1,278 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Board specific initialization for IOT2050
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ */
+
+#include <common.h>
+#include <bootstage.h>
+#include <dm.h>
+#include <i2c.h>
+#include <led.h>
+#include <net.h>
+#include <phy.h>
+#include <spl.h>
+#include <version.h>
+#include <linux/delay.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/hardware.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+
+#define IOT2050_INFO_MAGIC 0x20502050
+
+struct iot2050_info {
+ u32 magic;
+ u16 size;
+ char name[20 + 1];
+ char serial[16 + 1];
+ char mlfb[18 + 1];
+ char uuid[32 + 1];
+ char a5e[18 + 1];
+ u8 mac_addr_cnt;
+ u8 mac_addr[8][ARP_HLEN];
+ char seboot_version[40 + 1];
+} __packed;
+
+/*
+ * Scratch SRAM (available before DDR RAM) contains extracted EEPROM data.
+ */
+#define IOT2050_INFO_DATA ((struct iot2050_info *) \
+ TI_SRAM_SCRATCH_BOARD_EEPROM_START)
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static bool board_is_advanced(void)
+{
+ struct iot2050_info *info = IOT2050_INFO_DATA;
+
+ return info->magic == IOT2050_INFO_MAGIC &&
+ !strcmp((char *)info->name, "IOT2050-ADVANCED");
+}
+
+static void remove_mmc1_target(void)
+{
+ const char *boot_targets = env_get("boot_targets");
+
+ if (strncmp(boot_targets, "mmc1 ", 5) == 0)
+ env_set("boot_targets", boot_targets + 5);
+}
+
+void set_board_info_env(void)
+{
+ struct iot2050_info *info = IOT2050_INFO_DATA;
+ u8 __maybe_unused mac_cnt;
+
+ if (info->magic != IOT2050_INFO_MAGIC) {
+ pr_err("IOT2050: Board info parsing error!\n");
+ return;
+ }
+
+ if (env_get("board_uuid"))
+ return;
+
+ env_set("board_name", info->name);
+ env_set("board_serial", info->serial);
+ env_set("mlfb", info->mlfb);
+ env_set("board_uuid", info->uuid);
+ env_set("board_a5e", info->a5e);
+ env_set("fw_version", PLAIN_VERSION);
+ env_set("seboot_version", info->seboot_version);
+
+#ifdef CONFIG_NET
+ /* MAC address */
+ for (mac_cnt = 0; mac_cnt < info->mac_addr_cnt; mac_cnt++) {
+ if (is_valid_ethaddr(info->mac_addr[mac_cnt]))
+ eth_env_set_enetaddr_by_index("eth", mac_cnt + 1,
+ info->mac_addr[mac_cnt]);
+ }
+
+ /*
+ * Set the MAC address environment variable that ICSSG0-PRU eth0 will
+ * use in u-boot
+ */
+ env_set("ethaddr", env_get("eth1addr"));
+#endif
+
+ if (board_is_advanced()) {
+ env_set("fdtfile", "ti/k3-am6548-iot2050-advanced.dtb");
+ } else {
+ env_set("fdtfile", "ti/k3-am6528-iot2050-basic.dtb");
+ /* remove the unavailable eMMC (mmc1) from the list */
+ remove_mmc1_target();
+ }
+
+ env_save();
+}
+
+int board_init(void)
+{
+ return 0;
+}
+
+int dram_init(void)
+{
+ if (board_is_advanced())
+ gd->ram_size = SZ_2G;
+ else
+ gd->ram_size = SZ_1G;
+
+ return 0;
+}
+
+int dram_init_banksize(void)
+{
+ dram_init();
+
+ /* Bank 0 declares the memory available in the DDR low region */
+ gd->bd->bi_dram[0].start = CONFIG_SYS_SDRAM_BASE;
+ gd->bd->bi_dram[0].size = gd->ram_size;
+
+ /* Bank 1 declares the memory available in the DDR high region */
+ gd->bd->bi_dram[1].start = 0;
+ gd->bd->bi_dram[1].size = 0;
+
+ return 0;
+}
+
+#ifdef CONFIG_SPL_LOAD_FIT
+int board_fit_config_name_match(const char *name)
+{
+ struct iot2050_info *info = IOT2050_INFO_DATA;
+ char upper_name[32];
+
+ if (info->magic != IOT2050_INFO_MAGIC ||
+ strlen(name) >= sizeof(upper_name))
+ return -1;
+
+ str_to_upper(name, upper_name, sizeof(upper_name));
+ if (!strcmp(upper_name, (char *)info->name))
+ return 0;
+
+ return -1;
+}
+#endif
+
+int do_board_detect(void)
+{
+ return 0;
+}
+
+#ifdef CONFIG_IOT2050_BOOT_SWITCH
+static bool user_button_pressed(void)
+{
+ struct udevice *red_led = NULL;
+ unsigned long count = 0;
+ struct gpio_desc gpio;
+
+ memset(&gpio, 0, sizeof(gpio));
+
+ if (dm_gpio_lookup_name("25", &gpio) < 0 ||
+ dm_gpio_request(&gpio, "USER button") < 0 ||
+ dm_gpio_set_dir_flags(&gpio, GPIOD_IS_IN) < 0)
+ return false;
+
+ if (dm_gpio_get_value(&gpio) == 1)
+ return false;
+
+ printf("USER button pressed - booting from external media only\n");
+
+ led_get_by_label("status-led-red", &red_led);
+
+ if (red_led)
+ led_set_state(red_led, LEDST_ON);
+
+ while (dm_gpio_get_value(&gpio) == 0 && count++ < 10000)
+ mdelay(1);
+
+ if (red_led)
+ led_set_state(red_led, LEDST_OFF);
+
+ return true;
+}
+#endif
+
+#define SERDES0_LANE_SELECT 0x00104080
+
+int board_late_init(void)
+{
+ /* change CTRL_MMR register to let serdes0 not output USB3.0 signals. */
+ writel(0x3, SERDES0_LANE_SELECT);
+
+ set_board_info_env();
+
+#ifdef CONFIG_IOT2050_BOOT_SWITCH
+ /* remove the eMMC if requested via button */
+ if (board_is_advanced() && user_button_pressed())
+ remove_mmc1_target();
+#endif
+
+ return 0;
+}
+
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+int ft_board_setup(void *blob, struct bd_info *bd)
+{
+ int ret;
+
+ ret = fdt_fixup_msmc_ram(blob, "/bus@100000", "sram@70000000");
+ if (ret < 0)
+ ret = fdt_fixup_msmc_ram(blob, "/interconnect@100000",
+ "sram@70000000");
+ if (ret)
+ pr_err("%s: fixing up msmc ram failed %d\n", __func__, ret);
+
+ return ret;
+}
+#endif
+
+void spl_board_init(void)
+{
+}
+
+/* RJ45 LED configuration */
+#define DP83867_DEVADDR 0x1f
+
+#define DP83867_LEDCR_1 0x0018
+
+#define DP83867_LED_1000_BT_LINK 0x5
+#define DP83867_LED_100_BTX_LINK 0x6
+#define DP83867_LED_LINK_BLINK_ACTIVE 0xb
+#define DP83867_LED_SET(n, v) ((v) << ((n) * 4))
+
+#define DP83867_LED_SETTINGS \
+ (DP83867_LED_SET(0, DP83867_LED_LINK_BLINK_ACTIVE) | \
+ DP83867_LED_SET(1, DP83867_LED_1000_BT_LINK) | \
+ DP83867_LED_SET(2, DP83867_LED_100_BTX_LINK) | \
+ DP83867_LED_SET(3, DP83867_LED_100_BTX_LINK))
+
+int board_phy_config(struct phy_device *phydev)
+{
+ return phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_LEDCR_1,
+ DP83867_LED_SETTINGS);
+}
+
+/*
+ * Indicate any error or (accidental?) entering of CLI via the red status LED.
+ */
+#if CONFIG_IS_ENABLED(LED)
+void show_boot_progress(int progress)
+{
+ struct udevice *dev;
+ int ret;
+
+ if (progress < 0 || progress == BOOTSTAGE_ID_ENTER_CLI_LOOP) {
+ ret = led_get_by_label("status-led-green", &dev);
+ if (ret == 0)
+ led_set_state(dev, LEDST_OFF);
+
+ ret = led_get_by_label("status-led-red", &dev);
+ if (ret == 0)
+ led_set_state(dev, LEDST_ON);
+ }
+}
+#endif
diff --git a/board/siemens/iot2050/config.mk b/board/siemens/iot2050/config.mk
new file mode 100644
index 0000000000..267ec76c4e
--- /dev/null
+++ b/board/siemens/iot2050/config.mk
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) Siemens AG, 2020-2021
+#
+# Authors:
+# Jan Kiszka <jan.kiszka@siemens.com>
+
+flash.bin: all
diff --git a/configs/iot2050_defconfig b/configs/iot2050_defconfig
new file mode 100644
index 0000000000..2ea13f3a39
--- /dev/null
+++ b/configs/iot2050_defconfig
@@ -0,0 +1,140 @@
+CONFIG_ARM=y
+CONFIG_ARCH_K3=y
+CONFIG_SPL_GPIO_SUPPORT=y
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_SYS_MALLOC_F_LEN=0x8000
+CONFIG_SOC_K3_AM6=y
+CONFIG_TARGET_IOT2050_A53=y
+CONFIG_IOT2050_BOOT_SWITCH=y
+CONFIG_ENV_SIZE=0x20000
+CONFIG_ENV_OFFSET=0x680000
+CONFIG_ENV_SECT_SIZE=0x20000
+CONFIG_SYS_SPI_U_BOOT_OFFS=0x280000
+CONFIG_DM_GPIO=y
+CONFIG_SPL_SERIAL_SUPPORT=y
+CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
+CONFIG_SPL_STACK_R_ADDR=0x82000000
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_ENV_OFFSET_REDUND=0x6a0000
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI_SUPPORT=y
+CONFIG_SPL_DM_SPI=y
+CONFIG_SPL_TEXT_BASE=0x80080000
+CONFIG_DISTRO_DEFAULTS=y
+# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
+CONFIG_SPL_LOAD_FIT=y
+# CONFIG_USE_SPL_FIT_GENERATOR is not set
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_CONSOLE_MUX=y
+# CONFIG_DISPLAY_CPUINFO is not set
+CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_STACK_R=y
+CONFIG_SPL_SEPARATE_BSS=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1400
+CONFIG_SPL_DM_MAILBOX=y
+CONFIG_SPL_DM_SPI_FLASH=y
+CONFIG_SPL_DM_RESET=y
+CONFIG_SPL_POWER_DOMAIN=y
+# CONFIG_SPL_SPI_FLASH_TINY is not set
+CONFIG_SPL_SPI_FLASH_SFDP_SUPPORT=y
+CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_PROMPT="IOT2050> "
+CONFIG_CMD_ASKENV=y
+CONFIG_CMD_DFU=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPT=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+# CONFIG_CMD_NET is not set
+CONFIG_CMD_PCI=y
+CONFIG_CMD_REMOTEPROC=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_TIME=y
+# CONFIG_ISO_PARTITION is not set
+CONFIG_OF_CONTROL=y
+CONFIG_SPL_OF_CONTROL=y
+CONFIG_DEFAULT_DEVICE_TREE="k3-am6528-iot2050-basic"
+CONFIG_OF_LIST="k3-am6528-iot2050-basic k3-am6548-iot2050-advanced"
+CONFIG_SPL_MULTI_DTB_FIT=y
+CONFIG_SPL_OF_LIST="k3-am65-iot2050-spl"
+CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_SYS_REDUNDAND_ENVIRONMENT=y
+# CONFIG_NET is not set
+CONFIG_DM=y
+CONFIG_SPL_DM=y
+CONFIG_SPL_DM_SEQ_ALIAS=y
+CONFIG_SPL_REGMAP=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_CLK=y
+CONFIG_SPL_CLK=y
+CONFIG_CLK_TI_SCI=y
+CONFIG_DFU_MMC=y
+CONFIG_DFU_RAM=y
+CONFIG_DFU_SF=y
+CONFIG_DMA_CHANNELS=y
+CONFIG_TI_K3_NAVSS_UDMA=y
+CONFIG_TI_SCI_PROTOCOL=y
+CONFIG_DA8XX_GPIO=y
+CONFIG_DM_PCA953X=y
+CONFIG_DM_I2C=y
+CONFIG_I2C_SET_DEFAULT_BUS_NUM=y
+CONFIG_SYS_I2C_OMAP24XX=y
+CONFIG_DM_KEYBOARD=y
+CONFIG_LED=y
+CONFIG_SPL_LED=y
+CONFIG_LED_GPIO=y
+CONFIG_SPL_LED_GPIO=y
+CONFIG_DM_MAILBOX=y
+CONFIG_K3_SEC_PROXY=y
+CONFIG_DM_MMC=y
+CONFIG_MMC_HS200_SUPPORT=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_ADMA=y
+CONFIG_MMC_SDHCI_AM654=y
+CONFIG_DM_SPI_FLASH=y
+CONFIG_SPI_FLASH_SFDP_SUPPORT=y
+CONFIG_SPI_FLASH_STMICRO=y
+CONFIG_SPI_FLASH_WINBOND=y
+# CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
+CONFIG_DM_ETH=y
+# CONFIG_TI_AM65_CPSW_NUSS is not set
+CONFIG_PCI=y
+CONFIG_DM_PCI=y
+CONFIG_PCI_KEYSTONE=y
+CONFIG_PHY=y
+CONFIG_AM654_PHY=y
+CONFIG_OMAP_USB2_PHY=y
+CONFIG_PINCTRL=y
+# CONFIG_PINCTRL_GENERIC is not set
+CONFIG_SPL_PINCTRL=y
+# CONFIG_SPL_PINCTRL_GENERIC is not set
+CONFIG_PINCTRL_SINGLE=y
+CONFIG_POWER_DOMAIN=y
+CONFIG_TI_SCI_POWER_DOMAIN=y
+CONFIG_REMOTEPROC_TI_K3_R5F=y
+CONFIG_DM_RESET=y
+CONFIG_RESET_TI_SCI=y
+CONFIG_DM_SERIAL=y
+CONFIG_SOC_DEVICE=y
+CONFIG_SOC_DEVICE_TI_K3=y
+CONFIG_SOC_TI=y
+CONFIG_SPI=y
+CONFIG_DM_SPI=y
+CONFIG_CADENCE_QSPI=y
+CONFIG_SYSRESET=y
+CONFIG_SPL_SYSRESET=y
+CONFIG_SYSRESET_TI_SCI=y
+CONFIG_USB=y
+CONFIG_DM_USB=y
+CONFIG_USB_XHCI_HCD=y
+CONFIG_USB_XHCI_DWC3=y
+CONFIG_USB_DWC3=y
+CONFIG_USB_DWC3_GENERIC=y
+CONFIG_USB_KEYBOARD=y
+CONFIG_FAT_WRITE=y
+CONFIG_OF_LIBFDT_OVERLAY=y
diff --git a/include/configs/iot2050.h b/include/configs/iot2050.h
new file mode 100644
index 0000000000..c4a8c301c3
--- /dev/null
+++ b/include/configs/iot2050.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Configuration header file for IOT2050
+ * Copyright (c) Siemens AG, 2018-2021
+ *
+ * Authors:
+ * Le Jin <le.jin@siemens.com>
+ * Jan Kiszka <jan.kiszk@siemens.com>
+ */
+
+#ifndef __CONFIG_IOT2050_H
+#define __CONFIG_IOT2050_H
+
+#include <linux/sizes.h>
+
+/* SPL Loader Configuration */
+#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SPL_TEXT_BASE + \
+ CONFIG_SYS_K3_NON_SECURE_MSRAM_SIZE)
+
+#define CONFIG_SKIP_LOWLEVEL_INIT
+
+#define CONFIG_SPL_MAX_SIZE CONFIG_SYS_K3_MAX_DOWNLODABLE_IMAGE_SIZE
+
+#define CONFIG_SYS_BOOTM_LEN SZ_64M
+
+/* U-Boot general configuration */
+#define EXTRA_ENV_IOT2050_BOARD_SETTINGS \
+ "loadaddr=0x80080000\0" \
+ "scriptaddr=0x83000000\0" \
+ "kernel_addr_r=0x80080000\0" \
+ "ramdisk_addr_r=0x81000000\0" \
+ "fdt_addr_r=0x82000000\0" \
+ "overlay_addr_r=0x83000000\0" \
+ "usb_pgood_delay=900\0"
+
+#ifndef CONFIG_SPL_BUILD
+
+/*
+ * This defines all MMC devices, even if the basic variant has no mmc1.
+ * The non-supported device will be removed from the boot targets during
+ * runtime, when that board was detected.
+ */
+#define BOOT_TARGET_DEVICES(func) \
+ func(MMC, mmc, 1) \
+ func(MMC, mmc, 0) \
+ func(USB, usb, 0) \
+ func(USB, usb, 1) \
+ func(USB, usb, 2)
+
+#include <config_distro_bootcmd.h>
+
+#endif
+
+#define CONFIG_EXTRA_ENV_SETTINGS \
+ BOOTENV \
+ EXTRA_ENV_IOT2050_BOARD_SETTINGS
+
+#include <configs/ti_armv7_common.h>
+
+#endif /* __CONFIG_IOT2050_H */
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 3/5] arm64: dts: ti: k3-am65-mcu: Add RTI watchdog entry
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 1/5] arm: dts: Add IOT2050 device tree files Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 2/5] board: siemens: Add support for SIMATIC IOT2050 devices Jan Kiszka
@ 2021-06-02 9:37 ` Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware Jan Kiszka
` (2 subsequent siblings)
5 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
From: Jan Kiszka <jan.kiszka@siemens.com>
Add the DT entry for a watchdog based on RTI1. It requires additional
firmware on the MCU R5F cores to handle the expiry, e.g.
https://github.com/siemens/k3-rti-wdt. As this firmware will also lock
the power domain to protect it against premature shutdown, mark it
shared.
Aligns us to the kernel's DT in this regard.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
arch/arm/dts/k3-am65-mcu.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm/dts/k3-am65-mcu.dtsi b/arch/arm/dts/k3-am65-mcu.dtsi
index 7454c8cec0..903796bf7d 100644
--- a/arch/arm/dts/k3-am65-mcu.dtsi
+++ b/arch/arm/dts/k3-am65-mcu.dtsi
@@ -308,4 +308,13 @@
ti,loczrama = <1>;
};
};
+
+ mcu_rti1: rti@40610000 {
+ compatible = "ti,j7-rti-wdt";
+ reg = <0x0 0x40610000 0x0 0x100>;
+ clocks = <&k3_clks 135 0>;
+ power-domains = <&k3_pds 135 TI_SCI_PD_SHARED>;
+ assigned-clocks = <&k3_clks 135 0>;
+ assigned-clock-parents = <&k3_clks 135 4>;
+ };
};
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
` (2 preceding siblings ...)
2021-06-02 9:37 ` [PATCH v2 3/5] arm64: dts: ti: k3-am65-mcu: Add RTI watchdog entry Jan Kiszka
@ 2021-06-02 9:37 ` Jan Kiszka
2021-06-07 10:03 ` Lokesh Vutla
2021-06-02 9:37 ` [PATCH v2 5/5] configs: iot2050: Enable watchdog support, but do not auto-start it Jan Kiszka
2021-06-11 14:30 ` [PATCH v2 0/5] Add SIMATIC IOT2050 board support Lokesh Vutla
5 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
From: Jan Kiszka <jan.kiszka@siemens.com>
To avoid the need of extra boot scripting on AM65x for loading a
watchdog firmware, add the required rproc init and loading logic for the
first R5F core to the watchdog start handler. In case the R5F cluster is
in lock-step mode, also initialize the second core. The firmware itself
is embedded into U-Boot binary to ease access to it and ensure it is
properly hashed in case of secure boot.
One possible firmware source is https://github.com/siemens/k3-rti-wdt.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
drivers/watchdog/Kconfig | 20 ++++++++++++
drivers/watchdog/Makefile | 5 +++
drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
4 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 drivers/watchdog/rti_wdt_fw.S
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index f0ff2612a6..1a1fddfe9f 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -209,6 +209,26 @@ config WDT_K3_RTI
Say Y here if you want to include support for the K3 watchdog
timer (RTI module) available in the K3 generation of processors.
+if WDT_K3_RTI
+
+config WDT_K3_RTI_LOAD_FW
+ bool "Load watchdog firmware"
+ depends on REMOTEPROC
+ help
+ Automatically load the specified firmware image into the MCU R5F
+ core 0. On the AM65x, this firmware is supposed to handle the expiry
+ of the watchdog timer, typically by resetting the system.
+
+config WDT_K3_RTI_FW_FILE
+ string "Watchdog firmware image file"
+ default "k3-rti-wdt.fw"
+ depends on WDT_K3_RTI_LOAD_FW
+ help
+ Firmware image to be embedded into U-Boot and loaded on watchdog
+ start.
+
+endif
+
config WDT_SANDBOX
bool "Enable Watchdog Timer support for Sandbox"
depends on SANDBOX && WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 5c7ef593fe..5016ee4708 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
+obj-$(CONFIG_WDT_K3_RTI_LOAD_FW) += rti_wdt_fw.o
obj-$(CONFIG_WDT_SP805) += sp805_wdt.o
obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o
obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
obj-$(CONFIG_WDT_XILINX) += xilinx_wwdt.o
+
+ifeq ($(CONFIG_WDT_K3_RTI_LOAD_FW),y)
+$(obj)/rti_wdt_fw.o: $(shell readlink -f $(CONFIG_WDT_K3_RTI_FW_FILE)) FORCE
+endif
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 8335b20ae8..97daf40145 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -11,9 +11,11 @@
#include <common.h>
#include <clk.h>
#include <dm.h>
+#include <dm/device_compat.h>
#include <power-domain.h>
#include <wdt.h>
#include <asm/io.h>
+#include <remoteproc.h>
/* Timer register set definition */
#define RTIDWDCTRL 0x90
@@ -42,15 +44,69 @@ struct rti_wdt_priv {
unsigned int clk_khz;
};
+extern const u32 rti_wdt_fw[];
+extern const int rti_wdt_fw_size;
+
static int rti_wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
{
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
+ struct udevice *rproc_dev;
+ int primary_core, ret;
+ u32 cluster_mode;
+ ofnode node;
+#endif
struct rti_wdt_priv *priv = dev_get_priv(dev);
u32 timer_margin;
- int ret;
if (readl(priv->regs + RTIDWDCTRL) == WDENABLE_KEY)
return -EBUSY;
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
+ node = ofnode_by_compatible(ofnode_null(), "ti,am654-r5fss");
+ if (!ofnode_valid(node)) {
+ dt_error:
+ dev_err(dev, "No compatible firmware target processor found\n");
+ return -ENODEV;
+ }
+
+ ret = ofnode_read_u32(node, "ti,cluster-mode", &cluster_mode);
+ if (ret)
+ cluster_mode = 1;
+
+ node = ofnode_by_compatible(node, "ti,am654-r5f");
+ if (!ofnode_valid(node))
+ goto dt_error;
+
+ ret = uclass_get_device_by_ofnode(UCLASS_REMOTEPROC, node, &rproc_dev);
+ if (ret)
+ return ret;
+
+ primary_core = dev_seq(rproc_dev);
+
+ ret = rproc_dev_init(primary_core);
+ if (ret) {
+ fw_error:
+ dev_err(dev, "Failed to load watchdog firmware into remote processor %d\n",
+ primary_core);
+ return ret;
+ }
+
+ if (cluster_mode == 1) {
+ ret = rproc_dev_init(primary_core + 1);
+ if (ret)
+ goto fw_error;
+ }
+
+ ret = rproc_load(primary_core, (ulong)rti_wdt_fw,
+ rti_wdt_fw_size);
+ if (ret)
+ goto fw_error;
+
+ ret = rproc_start(primary_core);
+ if (ret)
+ goto fw_error;
+#endif
+
timer_margin = timeout_ms * priv->clk_khz / 1000;
timer_margin >>= WDT_PRELOAD_SHIFT;
if (timer_margin > WDT_PRELOAD_MAX)
diff --git a/drivers/watchdog/rti_wdt_fw.S b/drivers/watchdog/rti_wdt_fw.S
new file mode 100644
index 0000000000..78d99ff9f2
--- /dev/null
+++ b/drivers/watchdog/rti_wdt_fw.S
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) Siemens AG, 2020
+ *
+ * Authors:
+ * Jan Kiszka <jan.kiszka@siemens.com>
+ */
+
+.section .rodata
+
+.global rti_wdt_fw
+.global rti_wdt_fw_size
+
+rti_wdt_fw:
+.align 4
+.incbin CONFIG_WDT_K3_RTI_FW_FILE
+rti_wdt_fw_end:
+
+rti_wdt_fw_size:
+.int rti_wdt_fw_end - rti_wdt_fw
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH v2 5/5] configs: iot2050: Enable watchdog support, but do not auto-start it
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
` (3 preceding siblings ...)
2021-06-02 9:37 ` [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware Jan Kiszka
@ 2021-06-02 9:37 ` Jan Kiszka
2021-06-11 14:30 ` [PATCH v2 0/5] Add SIMATIC IOT2050 board support Lokesh Vutla
5 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-02 9:37 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Lokesh Vutla
From: Jan Kiszka <jan.kiszka@siemens.com>
This allows to use the watchdog in custom scripts but does not enforce
that the OS has to support it as well.
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
configs/iot2050_defconfig | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/configs/iot2050_defconfig b/configs/iot2050_defconfig
index 2ea13f3a39..f228dc8a89 100644
--- a/configs/iot2050_defconfig
+++ b/configs/iot2050_defconfig
@@ -52,6 +52,7 @@ CONFIG_CMD_MMC=y
CONFIG_CMD_PCI=y
CONFIG_CMD_REMOTEPROC=y
CONFIG_CMD_USB=y
+CONFIG_CMD_WDT=y
# CONFIG_CMD_SETEXPR is not set
CONFIG_CMD_TIME=y
# CONFIG_ISO_PARTITION is not set
@@ -136,5 +137,10 @@ CONFIG_USB_XHCI_DWC3=y
CONFIG_USB_DWC3=y
CONFIG_USB_DWC3_GENERIC=y
CONFIG_USB_KEYBOARD=y
+# CONFIG_WATCHDOG is not set
+# CONFIG_WATCHDOG_AUTOSTART is not set
+CONFIG_WDT=y
+CONFIG_WDT_K3_RTI=y
+CONFIG_WDT_K3_RTI_LOAD_FW=y
CONFIG_FAT_WRITE=y
CONFIG_OF_LIBFDT_OVERLAY=y
--
2.26.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-02 9:37 ` [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware Jan Kiszka
@ 2021-06-07 10:03 ` Lokesh Vutla
2021-06-07 10:20 ` Jan Kiszka
2021-06-07 11:40 ` Tom Rini
0 siblings, 2 replies; 28+ messages in thread
From: Lokesh Vutla @ 2021-06-07 10:03 UTC (permalink / raw)
To: Jan Kiszka, U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Tom Rini
+Tom,
Hi Tom,
On 02/06/21 3:07 pm, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> To avoid the need of extra boot scripting on AM65x for loading a
> watchdog firmware, add the required rproc init and loading logic for the
> first R5F core to the watchdog start handler. In case the R5F cluster is
> in lock-step mode, also initialize the second core. The firmware itself
> is embedded into U-Boot binary to ease access to it and ensure it is
> properly hashed in case of secure boot.
>
> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> drivers/watchdog/Kconfig | 20 ++++++++++++
> drivers/watchdog/Makefile | 5 +++
> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> 4 files changed, 102 insertions(+), 1 deletion(-)
> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index f0ff2612a6..1a1fddfe9f 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> Say Y here if you want to include support for the K3 watchdog
> timer (RTI module) available in the K3 generation of processors.
>
> +if WDT_K3_RTI
> +
> +config WDT_K3_RTI_LOAD_FW
> + bool "Load watchdog firmware"
> + depends on REMOTEPROC
> + help
> + Automatically load the specified firmware image into the MCU R5F
> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> + of the watchdog timer, typically by resetting the system.
> +
> +config WDT_K3_RTI_FW_FILE
> + string "Watchdog firmware image file"
> + default "k3-rti-wdt.fw"
> + depends on WDT_K3_RTI_LOAD_FW
> + help
> + Firmware image to be embedded into U-Boot and loaded on watchdog
> + start.
I need your input on this proach. Is it okay to include the linker file unders
drivers?
> +
> +endif
> +
> config WDT_SANDBOX
> bool "Enable Watchdog Timer support for Sandbox"
> depends on SANDBOX && WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c7ef593fe..5016ee4708 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
> obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
> obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
> obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
> +obj-$(CONFIG_WDT_K3_RTI_LOAD_FW) += rti_wdt_fw.o
> obj-$(CONFIG_WDT_SP805) += sp805_wdt.o
> obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o
> obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
> obj-$(CONFIG_WDT_XILINX) += xilinx_wwdt.o
> +
[...snip..]
> timer_margin = timeout_ms * priv->clk_khz / 1000;
> timer_margin >>= WDT_PRELOAD_SHIFT;
> if (timer_margin > WDT_PRELOAD_MAX)
> diff --git a/drivers/watchdog/rti_wdt_fw.S b/drivers/watchdog/rti_wdt_fw.S
> new file mode 100644
> index 0000000000..78d99ff9f2
> --- /dev/null
> +++ b/drivers/watchdog/rti_wdt_fw.S
Isn't this usecase specific? IMHO, drivers might not be the right place. Did I
misunderstand something?
Thanks and regards,
Lokesh
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) Siemens AG, 2020
> + *
> + * Authors:
> + * Jan Kiszka <jan.kiszka@siemens.com>
> + */
> +
> +.section .rodata
> +
> +.global rti_wdt_fw
> +.global rti_wdt_fw_size
> +
> +rti_wdt_fw:
> +.align 4
> +.incbin CONFIG_WDT_K3_RTI_FW_FILE
> +rti_wdt_fw_end:
> +
> +rti_wdt_fw_size:
> +.int rti_wdt_fw_end - rti_wdt_fw
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-07 10:03 ` Lokesh Vutla
@ 2021-06-07 10:20 ` Jan Kiszka
2021-06-07 11:40 ` Tom Rini
1 sibling, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-07 10:20 UTC (permalink / raw)
To: Lokesh Vutla, U-Boot Mailing List
Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng, Tom Rini
On 07.06.21 12:03, Lokesh Vutla wrote:
> +Tom,
>
> Hi Tom,
>
> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> To avoid the need of extra boot scripting on AM65x for loading a
>> watchdog firmware, add the required rproc init and loading logic for the
>> first R5F core to the watchdog start handler. In case the R5F cluster is
>> in lock-step mode, also initialize the second core. The firmware itself
>> is embedded into U-Boot binary to ease access to it and ensure it is
>> properly hashed in case of secure boot.
>>
>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>> drivers/watchdog/Kconfig | 20 ++++++++++++
>> drivers/watchdog/Makefile | 5 +++
>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>> 4 files changed, 102 insertions(+), 1 deletion(-)
>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>
>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> index f0ff2612a6..1a1fddfe9f 100644
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>> Say Y here if you want to include support for the K3 watchdog
>> timer (RTI module) available in the K3 generation of processors.
>>
>> +if WDT_K3_RTI
>> +
>> +config WDT_K3_RTI_LOAD_FW
>> + bool "Load watchdog firmware"
>> + depends on REMOTEPROC
>> + help
>> + Automatically load the specified firmware image into the MCU R5F
>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>> + of the watchdog timer, typically by resetting the system.
>> +
>> +config WDT_K3_RTI_FW_FILE
>> + string "Watchdog firmware image file"
>> + default "k3-rti-wdt.fw"
>> + depends on WDT_K3_RTI_LOAD_FW
>> + help
>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>> + start.
>
> I need your input on this proach. Is it okay to include the linker file unders
> drivers?
>
>> +
>> +endif
>> +
>> config WDT_SANDBOX
>> bool "Enable Watchdog Timer support for Sandbox"
>> depends on SANDBOX && WDT
>> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
>> index 5c7ef593fe..5016ee4708 100644
>> --- a/drivers/watchdog/Makefile
>> +++ b/drivers/watchdog/Makefile
>> @@ -33,7 +33,12 @@ obj-$(CONFIG_WDT_OCTEONTX) += octeontx_wdt.o
>> obj-$(CONFIG_WDT_OMAP3) += omap_wdt.o
>> obj-$(CONFIG_WDT_SBSA) += sbsa_gwdt.o
>> obj-$(CONFIG_WDT_K3_RTI) += rti_wdt.o
>> +obj-$(CONFIG_WDT_K3_RTI_LOAD_FW) += rti_wdt_fw.o
>> obj-$(CONFIG_WDT_SP805) += sp805_wdt.o
>> obj-$(CONFIG_WDT_STM32MP) += stm32mp_wdt.o
>> obj-$(CONFIG_WDT_TANGIER) += tangier_wdt.o
>> obj-$(CONFIG_WDT_XILINX) += xilinx_wwdt.o
>> +
>
> [...snip..]
>
>> timer_margin = timeout_ms * priv->clk_khz / 1000;
>> timer_margin >>= WDT_PRELOAD_SHIFT;
>> if (timer_margin > WDT_PRELOAD_MAX)
>> diff --git a/drivers/watchdog/rti_wdt_fw.S b/drivers/watchdog/rti_wdt_fw.S
>> new file mode 100644
>> index 0000000000..78d99ff9f2
>> --- /dev/null
>> +++ b/drivers/watchdog/rti_wdt_fw.S
>
> Isn't this usecase specific? IMHO, drivers might not be the right place. Did I
> misunderstand something?
>
It's specific to this driver - on a subset of supported hardware
platforms, namely AM65x SR1.0.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-07 10:03 ` Lokesh Vutla
2021-06-07 10:20 ` Jan Kiszka
@ 2021-06-07 11:40 ` Tom Rini
2021-06-07 11:44 ` Jan Kiszka
1 sibling, 1 reply; 28+ messages in thread
From: Tom Rini @ 2021-06-07 11:40 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Jan Kiszka, U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao,
Chao Zeng, Simon Glass
[-- Attachment #1: Type: text/plain, Size: 2406 bytes --]
On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> +Tom,
>
> Hi Tom,
>
> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> > From: Jan Kiszka <jan.kiszka@siemens.com>
> >
> > To avoid the need of extra boot scripting on AM65x for loading a
> > watchdog firmware, add the required rproc init and loading logic for the
> > first R5F core to the watchdog start handler. In case the R5F cluster is
> > in lock-step mode, also initialize the second core. The firmware itself
> > is embedded into U-Boot binary to ease access to it and ensure it is
> > properly hashed in case of secure boot.
> >
> > One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >
> > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > ---
> > drivers/watchdog/Kconfig | 20 ++++++++++++
> > drivers/watchdog/Makefile | 5 +++
> > drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> > drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> > 4 files changed, 102 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index f0ff2612a6..1a1fddfe9f 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -209,6 +209,26 @@ config WDT_K3_RTI
> > Say Y here if you want to include support for the K3 watchdog
> > timer (RTI module) available in the K3 generation of processors.
> >
> > +if WDT_K3_RTI
> > +
> > +config WDT_K3_RTI_LOAD_FW
> > + bool "Load watchdog firmware"
> > + depends on REMOTEPROC
> > + help
> > + Automatically load the specified firmware image into the MCU R5F
> > + core 0. On the AM65x, this firmware is supposed to handle the expiry
> > + of the watchdog timer, typically by resetting the system.
> > +
> > +config WDT_K3_RTI_FW_FILE
> > + string "Watchdog firmware image file"
> > + default "k3-rti-wdt.fw"
> > + depends on WDT_K3_RTI_LOAD_FW
> > + help
> > + Firmware image to be embedded into U-Boot and loaded on watchdog
> > + start.
>
> I need your input on this proach. Is it okay to include the linker file unders
> drivers?
Maybe? I suppose the first thing that springs to mind is why aren't we
using binman and including this blob (which I happily see is GPLv2)
similar to how we do things with x86 for one example.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-07 11:40 ` Tom Rini
@ 2021-06-07 11:44 ` Jan Kiszka
2021-06-09 13:17 ` Jan Kiszka
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-06-07 11:44 UTC (permalink / raw)
To: Tom Rini, Lokesh Vutla
Cc: U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng,
Simon Glass
On 07.06.21 13:40, Tom Rini wrote:
> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>> +Tom,
>>
>> Hi Tom,
>>
>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> To avoid the need of extra boot scripting on AM65x for loading a
>>> watchdog firmware, add the required rproc init and loading logic for the
>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>> in lock-step mode, also initialize the second core. The firmware itself
>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>> properly hashed in case of secure boot.
>>>
>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>> drivers/watchdog/Makefile | 5 +++
>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>
>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>> index f0ff2612a6..1a1fddfe9f 100644
>>> --- a/drivers/watchdog/Kconfig
>>> +++ b/drivers/watchdog/Kconfig
>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>> Say Y here if you want to include support for the K3 watchdog
>>> timer (RTI module) available in the K3 generation of processors.
>>>
>>> +if WDT_K3_RTI
>>> +
>>> +config WDT_K3_RTI_LOAD_FW
>>> + bool "Load watchdog firmware"
>>> + depends on REMOTEPROC
>>> + help
>>> + Automatically load the specified firmware image into the MCU R5F
>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>> + of the watchdog timer, typically by resetting the system.
>>> +
>>> +config WDT_K3_RTI_FW_FILE
>>> + string "Watchdog firmware image file"
>>> + default "k3-rti-wdt.fw"
>>> + depends on WDT_K3_RTI_LOAD_FW
>>> + help
>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>> + start.
>>
>> I need your input on this proach. Is it okay to include the linker file unders
>> drivers?
>
> Maybe? I suppose the first thing that springs to mind is why aren't we
> using binman and including this blob (which I happily see is GPLv2)
> similar to how we do things with x86 for one example.
>
See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-07 11:44 ` Jan Kiszka
@ 2021-06-09 13:17 ` Jan Kiszka
2021-06-11 13:44 ` Lokesh Vutla
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-06-09 13:17 UTC (permalink / raw)
To: Tom Rini, Lokesh Vutla
Cc: U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng,
Simon Glass
On 07.06.21 13:44, Jan Kiszka wrote:
> On 07.06.21 13:40, Tom Rini wrote:
>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>> +Tom,
>>>
>>> Hi Tom,
>>>
>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>
>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>> properly hashed in case of secure boot.
>>>>
>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> ---
>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>> drivers/watchdog/Makefile | 5 +++
>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>
>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>> --- a/drivers/watchdog/Kconfig
>>>> +++ b/drivers/watchdog/Kconfig
>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>> Say Y here if you want to include support for the K3 watchdog
>>>> timer (RTI module) available in the K3 generation of processors.
>>>>
>>>> +if WDT_K3_RTI
>>>> +
>>>> +config WDT_K3_RTI_LOAD_FW
>>>> + bool "Load watchdog firmware"
>>>> + depends on REMOTEPROC
>>>> + help
>>>> + Automatically load the specified firmware image into the MCU R5F
>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>> + of the watchdog timer, typically by resetting the system.
>>>> +
>>>> +config WDT_K3_RTI_FW_FILE
>>>> + string "Watchdog firmware image file"
>>>> + default "k3-rti-wdt.fw"
>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>> + help
>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>> + start.
>>>
>>> I need your input on this proach. Is it okay to include the linker file unders
>>> drivers?
>>
>> Maybe? I suppose the first thing that springs to mind is why aren't we
>> using binman and including this blob (which I happily see is GPLv2)
>> similar to how we do things with x86 for one example.
>>
>
> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>
> Jan
>
Did this help to answer open questions? Otherwise, please let me know.
I'd also like to avoid that his patch alone blocks 1-3 of the series
needless - but I would also not mind getting everything in at once.
Thanks,
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-09 13:17 ` Jan Kiszka
@ 2021-06-11 13:44 ` Lokesh Vutla
2021-06-11 14:08 ` Tom Rini
0 siblings, 1 reply; 28+ messages in thread
From: Lokesh Vutla @ 2021-06-11 13:44 UTC (permalink / raw)
To: Jan Kiszka, Tom Rini
Cc: U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng,
Simon Glass
Hi Tom,
On 09/06/21 6:47 pm, Jan Kiszka wrote:
> On 07.06.21 13:44, Jan Kiszka wrote:
>> On 07.06.21 13:40, Tom Rini wrote:
>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>> +Tom,
>>>>
>>>> Hi Tom,
>>>>
>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>> properly hashed in case of secure boot.
>>>>>
>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> ---
>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>> drivers/watchdog/Makefile | 5 +++
>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>
>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>> --- a/drivers/watchdog/Kconfig
>>>>> +++ b/drivers/watchdog/Kconfig
>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>
>>>>> +if WDT_K3_RTI
>>>>> +
>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>> + bool "Load watchdog firmware"
>>>>> + depends on REMOTEPROC
>>>>> + help
>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>> + of the watchdog timer, typically by resetting the system.
>>>>> +
>>>>> +config WDT_K3_RTI_FW_FILE
>>>>> + string "Watchdog firmware image file"
>>>>> + default "k3-rti-wdt.fw"
>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>> + help
>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>> + start.
>>>>
>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>> drivers?
>>>
>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>> using binman and including this blob (which I happily see is GPLv2)
>>> similar to how we do things with x86 for one example.
>>>
>>
>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>
>> Jan
>>
>
> Did this help to answer open questions? Otherwise, please let me know.
>
> I'd also like to avoid that his patch alone blocks 1-3 of the series
> needless - but I would also not mind getting everything in at once.
Can you provide your reviewed-by if you are okay with this approach?
Thanks and regards,
Lokesh
>
> Thanks,
> Jan
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-11 13:44 ` Lokesh Vutla
@ 2021-06-11 14:08 ` Tom Rini
2021-06-26 18:29 ` Simon Glass
0 siblings, 1 reply; 28+ messages in thread
From: Tom Rini @ 2021-06-11 14:08 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Jan Kiszka, U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao,
Chao Zeng, Simon Glass
[-- Attachment #1: Type: text/plain, Size: 3887 bytes --]
On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> Hi Tom,
>
> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> > On 07.06.21 13:44, Jan Kiszka wrote:
> >> On 07.06.21 13:40, Tom Rini wrote:
> >>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> >>>> +Tom,
> >>>>
> >>>> Hi Tom,
> >>>>
> >>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> >>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>
> >>>>> To avoid the need of extra boot scripting on AM65x for loading a
> >>>>> watchdog firmware, add the required rproc init and loading logic for the
> >>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> >>>>> in lock-step mode, also initialize the second core. The firmware itself
> >>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> >>>>> properly hashed in case of secure boot.
> >>>>>
> >>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >>>>>
> >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>> ---
> >>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> >>>>> drivers/watchdog/Makefile | 5 +++
> >>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> >>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> >>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> >>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >>>>>
> >>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>>> index f0ff2612a6..1a1fddfe9f 100644
> >>>>> --- a/drivers/watchdog/Kconfig
> >>>>> +++ b/drivers/watchdog/Kconfig
> >>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> >>>>> Say Y here if you want to include support for the K3 watchdog
> >>>>> timer (RTI module) available in the K3 generation of processors.
> >>>>>
> >>>>> +if WDT_K3_RTI
> >>>>> +
> >>>>> +config WDT_K3_RTI_LOAD_FW
> >>>>> + bool "Load watchdog firmware"
> >>>>> + depends on REMOTEPROC
> >>>>> + help
> >>>>> + Automatically load the specified firmware image into the MCU R5F
> >>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> >>>>> + of the watchdog timer, typically by resetting the system.
> >>>>> +
> >>>>> +config WDT_K3_RTI_FW_FILE
> >>>>> + string "Watchdog firmware image file"
> >>>>> + default "k3-rti-wdt.fw"
> >>>>> + depends on WDT_K3_RTI_LOAD_FW
> >>>>> + help
> >>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> >>>>> + start.
> >>>>
> >>>> I need your input on this proach. Is it okay to include the linker file unders
> >>>> drivers?
> >>>
> >>> Maybe? I suppose the first thing that springs to mind is why aren't we
> >>> using binman and including this blob (which I happily see is GPLv2)
> >>> similar to how we do things with x86 for one example.
> >>>
> >>
> >> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> >>
> >> Jan
> >>
> >
> > Did this help to answer open questions? Otherwise, please let me know.
> >
> > I'd also like to avoid that his patch alone blocks 1-3 of the series
> > needless - but I would also not mind getting everything in at once.
>
> Can you provide your reviewed-by if you are okay with this approach?
I was kind of hoping Simon would chime in here on binman usage. So,
re-re-reading the above URL, yes, fsloader wouldn't be the right choice
for watchdog firmware. But I think binman_entry_find() and related
could work, in general, for this case of "need firmware blob embedded in
to image". That said, this isn't just any firmware blob, it's the
watchdog firmware. The less reliance on other things the safer it is.
That means this would be an exception to the general firmware blob
loading rule and yeah, OK, we can do it this way. Sorry for the delay.
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/5] Add SIMATIC IOT2050 board support
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
` (4 preceding siblings ...)
2021-06-02 9:37 ` [PATCH v2 5/5] configs: iot2050: Enable watchdog support, but do not auto-start it Jan Kiszka
@ 2021-06-11 14:30 ` Lokesh Vutla
2021-06-11 14:53 ` Tom Rini
5 siblings, 1 reply; 28+ messages in thread
From: Lokesh Vutla @ 2021-06-11 14:30 UTC (permalink / raw)
To: Jan Kiszka, U-Boot Mailing List; +Cc: Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng
On 02/06/21 3:07 pm, Jan Kiszka wrote:
> This is the baseline support for the SIMATIC IOT2050 devices.
>
> Changes in v2:
> - rebased
> - sync with upstream-accepted DT
> - add boot switch
> - include watchdog support
>
> Allows to boot mainline 5.10 kernels, but not the original BSP-derived
> kernel we currently ship as reference. This is due to the TI sysfw ABI
> breakages between 2.x and 3.x. We will soon provide a transitional
> kernel that allows booting both firmware ABIs - as long as full upstream
> kernel support is work in progress.
>
> Note that this baseline support lacks Ethernet drivers. We are working
> closely with TI to ensure that the to-be-upstreamed icssg-prueth driver
> will work both with new SR2.0 AM65x silicon as well as with SR1.0 which
> is used in the currently shipped IOT2050 devices.
>
> A staging tree for complete IOT2050 support can be found at [1]. Full
> image integration is available via [2].
>
> Regarding patch 4: There has been some doubts on the proposed approach,
> but there has been also no suggestion provided for a similarly
> lightweight and secure embedding method. Therefore, I'm proposing our
> solution once again.
There are multiple checkpatch issues with this series. Can you fix them where
ever possible?
➜ u-boot-ti git:(for-next) ./scripts/checkpatch.pl --strict siemens/*.patch
--------------------------------------------------------
siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch
--------------------------------------------------------
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#50:
new file mode 100644
WARNING: line length of 102 exceeds 100 columns
#103: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:49:
+ filename = "arch/arm/dts/k3-am6528-iot2050-basic.dtb";
WARNING: line length of 105 exceeds 100 columns
#113: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:59:
+ filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
total: 0 errors, 3 warnings, 0 checks, 1025 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch has style problems,
please review.
-----------------------------------------------------------------------
siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch
-----------------------------------------------------------------------
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#53:
new file mode 100644
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#282: FILE: board/siemens/iot2050/board.c:86:
+#ifdef CONFIG_NET
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#338: FILE: board/siemens/iot2050/board.c:142:
+#ifdef CONFIG_SPL_LOAD_FIT
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#361: FILE: board/siemens/iot2050/board.c:165:
+#ifdef CONFIG_IOT2050_BOOT_SWITCH
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#404: FILE: board/siemens/iot2050/board.c:208:
+#ifdef CONFIG_IOT2050_BOOT_SWITCH
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#413: FILE: board/siemens/iot2050/board.c:217:
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#458: FILE: board/siemens/iot2050/board.c:262:
+#if CONFIG_IS_ENABLED(LED)
CHECK: Macro argument reuse 'func' - possible side-effects?
#683: FILE: include/configs/iot2050.h:43:
+#define BOOT_TARGET_DEVICES(func) \
+ func(MMC, mmc, 1) \
+ func(MMC, mmc, 0) \
+ func(USB, usb, 0) \
+ func(USB, usb, 1) \
+ func(USB, usb, 2)
total: 0 errors, 7 warnings, 1 checks, 606 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch has
style problems, please review.
------------------------------------------------------------------
siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch
------------------------------------------------------------------
total: 0 errors, 0 warnings, 0 checks, 13 lines checked
siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch has no
obvious style problems and is ready for submission.
--------------------------------------------------------------------
siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch
--------------------------------------------------------------------
WARNING: externs should be avoided in .c files
#95: FILE: drivers/watchdog/rti_wdt.c:47:
+extern const u32 rti_wdt_fw[];
WARNING: externs should be avoided in .c files
#96: FILE: drivers/watchdog/rti_wdt.c:48:
+extern const int rti_wdt_fw_size;
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#100: FILE: drivers/watchdog/rti_wdt.c:52:
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
#113: FILE: drivers/watchdog/rti_wdt.c:64:
+#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
WARNING: labels should not be indented
#116: FILE: drivers/watchdog/rti_wdt.c:67:
+ dt_error:
WARNING: labels should not be indented
#137: FILE: drivers/watchdog/rti_wdt.c:88:
+ fw_error:
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#163:
new file mode 100644
WARNING: Improper SPDX comment style for 'drivers/watchdog/rti_wdt_fw.S', please
use '/*' instead
#168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
+// SPDX-License-Identifier: GPL-2.0+
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
+// SPDX-License-Identifier: GPL-2.0+
total: 0 errors, 9 warnings, 0 checks, 139 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch has style
problems, please review.
-----------------------------------------------------------------------
siemens/0005-configs-iot2050-Enable-watchdog-support-but-do-not-a.patch
-----------------------------------------------------------------------
Thanks and regards,
Lokesh
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/5] Add SIMATIC IOT2050 board support
2021-06-11 14:30 ` [PATCH v2 0/5] Add SIMATIC IOT2050 board support Lokesh Vutla
@ 2021-06-11 14:53 ` Tom Rini
2021-06-11 18:20 ` Jan Kiszka
0 siblings, 1 reply; 28+ messages in thread
From: Tom Rini @ 2021-06-11 14:53 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Jan Kiszka, U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao,
Chao Zeng
[-- Attachment #1: Type: text/plain, Size: 7409 bytes --]
On Fri, Jun 11, 2021 at 08:00:17PM +0530, Lokesh Vutla wrote:
>
>
> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> > This is the baseline support for the SIMATIC IOT2050 devices.
> >
> > Changes in v2:
> > - rebased
> > - sync with upstream-accepted DT
> > - add boot switch
> > - include watchdog support
> >
> > Allows to boot mainline 5.10 kernels, but not the original BSP-derived
> > kernel we currently ship as reference. This is due to the TI sysfw ABI
> > breakages between 2.x and 3.x. We will soon provide a transitional
> > kernel that allows booting both firmware ABIs - as long as full upstream
> > kernel support is work in progress.
> >
> > Note that this baseline support lacks Ethernet drivers. We are working
> > closely with TI to ensure that the to-be-upstreamed icssg-prueth driver
> > will work both with new SR2.0 AM65x silicon as well as with SR1.0 which
> > is used in the currently shipped IOT2050 devices.
> >
> > A staging tree for complete IOT2050 support can be found at [1]. Full
> > image integration is available via [2].
> >
> > Regarding patch 4: There has been some doubts on the proposed approach,
> > but there has been also no suggestion provided for a similarly
> > lightweight and secure embedding method. Therefore, I'm proposing our
> > solution once again.
>
> There are multiple checkpatch issues with this series. Can you fix them where
> ever possible?
>
> ➜ u-boot-ti git:(for-next) ./scripts/checkpatch.pl --strict siemens/*.patch
> --------------------------------------------------------
> siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch
> --------------------------------------------------------
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #50:
> new file mode 100644
>
> WARNING: line length of 102 exceeds 100 columns
> #103: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:49:
> + filename = "arch/arm/dts/k3-am6528-iot2050-basic.dtb";
>
> WARNING: line length of 105 exceeds 100 columns
> #113: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:59:
> + filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
>
> total: 0 errors, 3 warnings, 0 checks, 1025 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch has style problems,
> please review.
> -----------------------------------------------------------------------
> siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch
> -----------------------------------------------------------------------
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #53:
> new file mode 100644
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #282: FILE: board/siemens/iot2050/board.c:86:
> +#ifdef CONFIG_NET
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #338: FILE: board/siemens/iot2050/board.c:142:
> +#ifdef CONFIG_SPL_LOAD_FIT
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #361: FILE: board/siemens/iot2050/board.c:165:
> +#ifdef CONFIG_IOT2050_BOOT_SWITCH
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #404: FILE: board/siemens/iot2050/board.c:208:
> +#ifdef CONFIG_IOT2050_BOOT_SWITCH
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #413: FILE: board/siemens/iot2050/board.c:217:
> +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #458: FILE: board/siemens/iot2050/board.c:262:
> +#if CONFIG_IS_ENABLED(LED)
>
> CHECK: Macro argument reuse 'func' - possible side-effects?
> #683: FILE: include/configs/iot2050.h:43:
> +#define BOOT_TARGET_DEVICES(func) \
> + func(MMC, mmc, 1) \
> + func(MMC, mmc, 0) \
> + func(USB, usb, 0) \
> + func(USB, usb, 1) \
> + func(USB, usb, 2)
>
> total: 0 errors, 7 warnings, 1 checks, 606 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch has
> style problems, please review.
> ------------------------------------------------------------------
> siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch
> ------------------------------------------------------------------
> total: 0 errors, 0 warnings, 0 checks, 13 lines checked
>
> siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch has no
> obvious style problems and is ready for submission.
> --------------------------------------------------------------------
> siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch
> --------------------------------------------------------------------
> WARNING: externs should be avoided in .c files
> #95: FILE: drivers/watchdog/rti_wdt.c:47:
> +extern const u32 rti_wdt_fw[];
>
> WARNING: externs should be avoided in .c files
> #96: FILE: drivers/watchdog/rti_wdt.c:48:
> +extern const int rti_wdt_fw_size;
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #100: FILE: drivers/watchdog/rti_wdt.c:52:
> +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
>
> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
> #113: FILE: drivers/watchdog/rti_wdt.c:64:
> +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
>
> WARNING: labels should not be indented
> #116: FILE: drivers/watchdog/rti_wdt.c:67:
> + dt_error:
>
> WARNING: labels should not be indented
> #137: FILE: drivers/watchdog/rti_wdt.c:88:
> + fw_error:
>
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #163:
> new file mode 100644
>
> WARNING: Improper SPDX comment style for 'drivers/watchdog/rti_wdt_fw.S', please
> use '/*' instead
> #168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
> +// SPDX-License-Identifier: GPL-2.0+
>
> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
> #168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
> +// SPDX-License-Identifier: GPL-2.0+
>
> total: 0 errors, 9 warnings, 0 checks, 139 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
> mechanically convert to the typical style using --fix or --fix-inplace.
>
> siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch has style
> problems, please review.
> -----------------------------------------------------------------------
> siemens/0005-configs-iot2050-Enable-watchdog-support-but-do-not-a.patch
> -----------------------------------------------------------------------
Since I just pointed out some checkpatch problems to Lokesh in his last
PR, I should note that out of all of this list, I only really care about
the SPDX one. There are plenty of cases where:
#ifdef CONFIG_FOO
...
#endif
is more readable / clear than:
if (IS_ENABLED(CONFIG_FOO)) {
...
}
Warnings are warning and can be ignored for good reason, errors cannot.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 0/5] Add SIMATIC IOT2050 board support
2021-06-11 14:53 ` Tom Rini
@ 2021-06-11 18:20 ` Jan Kiszka
0 siblings, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-06-11 18:20 UTC (permalink / raw)
To: Tom Rini, Lokesh Vutla
Cc: U-Boot Mailing List, Le Jin, Bao Cheng Su, Nian Gao, Chao Zeng
On 11.06.21 16:53, Tom Rini wrote:
> On Fri, Jun 11, 2021 at 08:00:17PM +0530, Lokesh Vutla wrote:
>>
>>
>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>> This is the baseline support for the SIMATIC IOT2050 devices.
>>>
>>> Changes in v2:
>>> - rebased
>>> - sync with upstream-accepted DT
>>> - add boot switch
>>> - include watchdog support
>>>
>>> Allows to boot mainline 5.10 kernels, but not the original BSP-derived
>>> kernel we currently ship as reference. This is due to the TI sysfw ABI
>>> breakages between 2.x and 3.x. We will soon provide a transitional
>>> kernel that allows booting both firmware ABIs - as long as full upstream
>>> kernel support is work in progress.
>>>
>>> Note that this baseline support lacks Ethernet drivers. We are working
>>> closely with TI to ensure that the to-be-upstreamed icssg-prueth driver
>>> will work both with new SR2.0 AM65x silicon as well as with SR1.0 which
>>> is used in the currently shipped IOT2050 devices.
>>>
>>> A staging tree for complete IOT2050 support can be found at [1]. Full
>>> image integration is available via [2].
>>>
>>> Regarding patch 4: There has been some doubts on the proposed approach,
>>> but there has been also no suggestion provided for a similarly
>>> lightweight and secure embedding method. Therefore, I'm proposing our
>>> solution once again.
>>
>> There are multiple checkpatch issues with this series. Can you fix them where
>> ever possible?
>>
>> ➜ u-boot-ti git:(for-next) ./scripts/checkpatch.pl --strict siemens/*.patch
>> --------------------------------------------------------
>> siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch
>> --------------------------------------------------------
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #50:
>> new file mode 100644
>>
>> WARNING: line length of 102 exceeds 100 columns
>> #103: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:49:
>> + filename = "arch/arm/dts/k3-am6528-iot2050-basic.dtb";
>>
>> WARNING: line length of 105 exceeds 100 columns
>> #113: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:59:
>> + filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
>>
>> total: 0 errors, 3 warnings, 0 checks, 1025 lines checked
>>
>> NOTE: For some of the reported defects, checkpatch may be able to
>> mechanically convert to the typical style using --fix or --fix-inplace.
>>
>> siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch has style problems,
>> please review.
>> -----------------------------------------------------------------------
>> siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch
>> -----------------------------------------------------------------------
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #53:
>> new file mode 100644
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #282: FILE: board/siemens/iot2050/board.c:86:
>> +#ifdef CONFIG_NET
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #338: FILE: board/siemens/iot2050/board.c:142:
>> +#ifdef CONFIG_SPL_LOAD_FIT
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #361: FILE: board/siemens/iot2050/board.c:165:
>> +#ifdef CONFIG_IOT2050_BOOT_SWITCH
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #404: FILE: board/siemens/iot2050/board.c:208:
>> +#ifdef CONFIG_IOT2050_BOOT_SWITCH
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #413: FILE: board/siemens/iot2050/board.c:217:
>> +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #458: FILE: board/siemens/iot2050/board.c:262:
>> +#if CONFIG_IS_ENABLED(LED)
>>
>> CHECK: Macro argument reuse 'func' - possible side-effects?
>> #683: FILE: include/configs/iot2050.h:43:
>> +#define BOOT_TARGET_DEVICES(func) \
>> + func(MMC, mmc, 1) \
>> + func(MMC, mmc, 0) \
>> + func(USB, usb, 0) \
>> + func(USB, usb, 1) \
>> + func(USB, usb, 2)
>>
>> total: 0 errors, 7 warnings, 1 checks, 606 lines checked
>>
>> NOTE: For some of the reported defects, checkpatch may be able to
>> mechanically convert to the typical style using --fix or --fix-inplace.
>>
>> siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch has
>> style problems, please review.
>> ------------------------------------------------------------------
>> siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch
>> ------------------------------------------------------------------
>> total: 0 errors, 0 warnings, 0 checks, 13 lines checked
>>
>> siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch has no
>> obvious style problems and is ready for submission.
>> --------------------------------------------------------------------
>> siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch
>> --------------------------------------------------------------------
>> WARNING: externs should be avoided in .c files
>> #95: FILE: drivers/watchdog/rti_wdt.c:47:
>> +extern const u32 rti_wdt_fw[];
>>
>> WARNING: externs should be avoided in .c files
>> #96: FILE: drivers/watchdog/rti_wdt.c:48:
>> +extern const int rti_wdt_fw_size;
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #100: FILE: drivers/watchdog/rti_wdt.c:52:
>> +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
>>
>> WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible
>> #113: FILE: drivers/watchdog/rti_wdt.c:64:
>> +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW
>>
>> WARNING: labels should not be indented
>> #116: FILE: drivers/watchdog/rti_wdt.c:67:
>> + dt_error:
>>
>> WARNING: labels should not be indented
>> #137: FILE: drivers/watchdog/rti_wdt.c:88:
>> + fw_error:
>>
>> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
>> #163:
>> new file mode 100644
>>
>> WARNING: Improper SPDX comment style for 'drivers/watchdog/rti_wdt_fw.S', please
>> use '/*' instead
>> #168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
>> +// SPDX-License-Identifier: GPL-2.0+
>>
>> WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
>> #168: FILE: drivers/watchdog/rti_wdt_fw.S:1:
>> +// SPDX-License-Identifier: GPL-2.0+
>>
>> total: 0 errors, 9 warnings, 0 checks, 139 lines checked
>>
>> NOTE: For some of the reported defects, checkpatch may be able to
>> mechanically convert to the typical style using --fix or --fix-inplace.
>>
>> siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch has style
>> problems, please review.
>> -----------------------------------------------------------------------
>> siemens/0005-configs-iot2050-Enable-watchdog-support-but-do-not-a.patch
>> -----------------------------------------------------------------------
>
> Since I just pointed out some checkpatch problems to Lokesh in his last
> PR, I should note that out of all of this list, I only really care about
> the SPDX one. There are plenty of cases where:
> #ifdef CONFIG_FOO
> ...
> #endif
>
> is more readable / clear than:
> if (IS_ENABLED(CONFIG_FOO)) {
> ...
> }
>
>
> Warnings are warning and can be ignored for good reason, errors cannot.
>
I've done some fixes (including the SPDX one), still need to retest. v3
will follow.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-11 14:08 ` Tom Rini
@ 2021-06-26 18:29 ` Simon Glass
2021-06-27 18:01 ` Jan Kiszka
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2021-06-26 18:29 UTC (permalink / raw)
To: Tom Rini
Cc: Lokesh Vutla, Jan Kiszka, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi,
On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> > Hi Tom,
> >
> > On 09/06/21 6:47 pm, Jan Kiszka wrote:
> > > On 07.06.21 13:44, Jan Kiszka wrote:
> > >> On 07.06.21 13:40, Tom Rini wrote:
> > >>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> > >>>> +Tom,
> > >>>>
> > >>>> Hi Tom,
> > >>>>
> > >>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> > >>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> > >>>>>
> > >>>>> To avoid the need of extra boot scripting on AM65x for loading a
> > >>>>> watchdog firmware, add the required rproc init and loading logic for the
> > >>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> > >>>>> in lock-step mode, also initialize the second core. The firmware itself
> > >>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> > >>>>> properly hashed in case of secure boot.
> > >>>>>
> > >>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> > >>>>>
> > >>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > >>>>> ---
> > >>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> > >>>>> drivers/watchdog/Makefile | 5 +++
> > >>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> > >>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> > >>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> > >>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> > >>>>>
> > >>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > >>>>> index f0ff2612a6..1a1fddfe9f 100644
> > >>>>> --- a/drivers/watchdog/Kconfig
> > >>>>> +++ b/drivers/watchdog/Kconfig
> > >>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> > >>>>> Say Y here if you want to include support for the K3 watchdog
> > >>>>> timer (RTI module) available in the K3 generation of processors.
> > >>>>>
> > >>>>> +if WDT_K3_RTI
> > >>>>> +
> > >>>>> +config WDT_K3_RTI_LOAD_FW
> > >>>>> + bool "Load watchdog firmware"
> > >>>>> + depends on REMOTEPROC
> > >>>>> + help
> > >>>>> + Automatically load the specified firmware image into the MCU R5F
> > >>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> > >>>>> + of the watchdog timer, typically by resetting the system.
> > >>>>> +
> > >>>>> +config WDT_K3_RTI_FW_FILE
> > >>>>> + string "Watchdog firmware image file"
> > >>>>> + default "k3-rti-wdt.fw"
> > >>>>> + depends on WDT_K3_RTI_LOAD_FW
> > >>>>> + help
> > >>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> > >>>>> + start.
> > >>>>
> > >>>> I need your input on this proach. Is it okay to include the linker file unders
> > >>>> drivers?
> > >>>
> > >>> Maybe? I suppose the first thing that springs to mind is why aren't we
> > >>> using binman and including this blob (which I happily see is GPLv2)
> > >>> similar to how we do things with x86 for one example.
> > >>>
> > >>
> > >> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> > >>
> > >> Jan
> > >>
> > >
> > > Did this help to answer open questions? Otherwise, please let me know.
> > >
> > > I'd also like to avoid that his patch alone blocks 1-3 of the series
> > > needless - but I would also not mind getting everything in at once.
> >
> > Can you provide your reviewed-by if you are okay with this approach?
>
> I was kind of hoping Simon would chime in here on binman usage. So,
> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> for watchdog firmware. But I think binman_entry_find() and related
> could work, in general, for this case of "need firmware blob embedded in
> to image". That said, this isn't just any firmware blob, it's the
> watchdog firmware. The less reliance on other things the safer it is.
> That means this would be an exception to the general firmware blob
> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
Yes I've been a little tied up recently. But I think this should use
binman. We really don't want to be building binary firmware into
U-Boot itself!
Also Tom says, see x86 for a load of binaries of different types and
how they are accessed at runttime. This is what binman is for.
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> --
> Tom
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-26 18:29 ` Simon Glass
@ 2021-06-27 18:01 ` Jan Kiszka
2021-06-27 18:18 ` Simon Glass
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-06-27 18:01 UTC (permalink / raw)
To: Simon Glass, Tom Rini
Cc: Lokesh Vutla, U-Boot Mailing List, Le Jin, Bao Cheng Su,
Nian Gao, Chao Zeng
On 26.06.21 20:29, Simon Glass wrote:
> Hi,
>
> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
>>> Hi Tom,
>>>
>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
>>>> On 07.06.21 13:44, Jan Kiszka wrote:
>>>>> On 07.06.21 13:40, Tom Rini wrote:
>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>>>>> +Tom,
>>>>>>>
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>
>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>>>>> properly hashed in case of secure boot.
>>>>>>>>
>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>>>>
>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>> ---
>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>>>>> drivers/watchdog/Makefile | 5 +++
>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>>>>
>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>>>>> --- a/drivers/watchdog/Kconfig
>>>>>>>> +++ b/drivers/watchdog/Kconfig
>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>>>>
>>>>>>>> +if WDT_K3_RTI
>>>>>>>> +
>>>>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>>>>> + bool "Load watchdog firmware"
>>>>>>>> + depends on REMOTEPROC
>>>>>>>> + help
>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>>>>> + of the watchdog timer, typically by resetting the system.
>>>>>>>> +
>>>>>>>> +config WDT_K3_RTI_FW_FILE
>>>>>>>> + string "Watchdog firmware image file"
>>>>>>>> + default "k3-rti-wdt.fw"
>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>>>>> + help
>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>>>>> + start.
>>>>>>>
>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>>>>> drivers?
>>>>>>
>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>>>>> using binman and including this blob (which I happily see is GPLv2)
>>>>>> similar to how we do things with x86 for one example.
>>>>>>
>>>>>
>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>>>>
>>>>> Jan
>>>>>
>>>>
>>>> Did this help to answer open questions? Otherwise, please let me know.
>>>>
>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
>>>> needless - but I would also not mind getting everything in at once.
>>>
>>> Can you provide your reviewed-by if you are okay with this approach?
>>
>> I was kind of hoping Simon would chime in here on binman usage. So,
>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
>> for watchdog firmware. But I think binman_entry_find() and related
>> could work, in general, for this case of "need firmware blob embedded in
>> to image". That said, this isn't just any firmware blob, it's the
>> watchdog firmware. The less reliance on other things the safer it is.
>> That means this would be an exception to the general firmware blob
>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
>
> Yes I've been a little tied up recently. But I think this should use
> binman. We really don't want to be building binary firmware into
> U-Boot itself!
>
> Also Tom says, see x86 for a load of binaries of different types and
> how they are accessed at runttime. This is what binman is for.
>
Please take the time and study my arguments. I'm open for better
proposals, but they need to be concrete and addressing my points.
Thanks,
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-27 18:01 ` Jan Kiszka
@ 2021-06-27 18:18 ` Simon Glass
2021-06-27 19:34 ` Tom Rini
2021-06-28 5:40 ` Jan Kiszka
0 siblings, 2 replies; 28+ messages in thread
From: Simon Glass @ 2021-06-27 18:18 UTC (permalink / raw)
To: Jan Kiszka
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi Jan,
On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 26.06.21 20:29, Simon Glass wrote:
> > Hi,
> >
> > On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> >>> Hi Tom,
> >>>
> >>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> >>>> On 07.06.21 13:44, Jan Kiszka wrote:
> >>>>> On 07.06.21 13:40, Tom Rini wrote:
> >>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> >>>>>>> +Tom,
> >>>>>>>
> >>>>>>> Hi Tom,
> >>>>>>>
> >>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>
> >>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> >>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> >>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> >>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> >>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> >>>>>>>> properly hashed in case of secure boot.
> >>>>>>>>
> >>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>> ---
> >>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> >>>>>>>> drivers/watchdog/Makefile | 5 +++
> >>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> >>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> >>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> >>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> >>>>>>>> --- a/drivers/watchdog/Kconfig
> >>>>>>>> +++ b/drivers/watchdog/Kconfig
> >>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> >>>>>>>> Say Y here if you want to include support for the K3 watchdog
> >>>>>>>> timer (RTI module) available in the K3 generation of processors.
> >>>>>>>>
> >>>>>>>> +if WDT_K3_RTI
> >>>>>>>> +
> >>>>>>>> +config WDT_K3_RTI_LOAD_FW
> >>>>>>>> + bool "Load watchdog firmware"
> >>>>>>>> + depends on REMOTEPROC
> >>>>>>>> + help
> >>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> >>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> >>>>>>>> + of the watchdog timer, typically by resetting the system.
> >>>>>>>> +
> >>>>>>>> +config WDT_K3_RTI_FW_FILE
> >>>>>>>> + string "Watchdog firmware image file"
> >>>>>>>> + default "k3-rti-wdt.fw"
> >>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> >>>>>>>> + help
> >>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> >>>>>>>> + start.
> >>>>>>>
> >>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> >>>>>>> drivers?
> >>>>>>
> >>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> >>>>>> using binman and including this blob (which I happily see is GPLv2)
> >>>>>> similar to how we do things with x86 for one example.
> >>>>>>
> >>>>>
> >>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> >>>>>
> >>>>> Jan
> >>>>>
> >>>>
> >>>> Did this help to answer open questions? Otherwise, please let me know.
> >>>>
> >>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> >>>> needless - but I would also not mind getting everything in at once.
> >>>
> >>> Can you provide your reviewed-by if you are okay with this approach?
> >>
> >> I was kind of hoping Simon would chime in here on binman usage. So,
> >> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> >> for watchdog firmware. But I think binman_entry_find() and related
> >> could work, in general, for this case of "need firmware blob embedded in
> >> to image". That said, this isn't just any firmware blob, it's the
> >> watchdog firmware. The less reliance on other things the safer it is.
> >> That means this would be an exception to the general firmware blob
> >> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> >
> > Yes I've been a little tied up recently. But I think this should use
> > binman. We really don't want to be building binary firmware into
> > U-Boot itself!
> >
> > Also Tom says, see x86 for a load of binaries of different types and
> > how they are accessed at runttime. This is what binman is for.
> >
>
> Please take the time and study my arguments. I'm open for better
> proposals, but they need to be concrete and addressing my points.
Do you mean 'properly hashed' and 'easy access', or something else?
What can binman not do?
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-27 18:18 ` Simon Glass
@ 2021-06-27 19:34 ` Tom Rini
2021-06-27 20:37 ` Simon Glass
2021-06-28 5:40 ` Jan Kiszka
1 sibling, 1 reply; 28+ messages in thread
From: Tom Rini @ 2021-06-27 19:34 UTC (permalink / raw)
To: Simon Glass
Cc: Jan Kiszka, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
[-- Attachment #1: Type: text/plain, Size: 5406 bytes --]
On Sun, Jun 27, 2021 at 12:18:09PM -0600, Simon Glass wrote:
> Hi Jan,
>
> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >
> > On 26.06.21 20:29, Simon Glass wrote:
> > > Hi,
> > >
> > > On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> > >>
> > >> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> > >>> Hi Tom,
> > >>>
> > >>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> > >>>> On 07.06.21 13:44, Jan Kiszka wrote:
> > >>>>> On 07.06.21 13:40, Tom Rini wrote:
> > >>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> > >>>>>>> +Tom,
> > >>>>>>>
> > >>>>>>> Hi Tom,
> > >>>>>>>
> > >>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> > >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> > >>>>>>>>
> > >>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> > >>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> > >>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> > >>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> > >>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> > >>>>>>>> properly hashed in case of secure boot.
> > >>>>>>>>
> > >>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > >>>>>>>> ---
> > >>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> > >>>>>>>> drivers/watchdog/Makefile | 5 +++
> > >>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> > >>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> > >>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> > >>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> > >>>>>>>>
> > >>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > >>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> > >>>>>>>> --- a/drivers/watchdog/Kconfig
> > >>>>>>>> +++ b/drivers/watchdog/Kconfig
> > >>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> > >>>>>>>> Say Y here if you want to include support for the K3 watchdog
> > >>>>>>>> timer (RTI module) available in the K3 generation of processors.
> > >>>>>>>>
> > >>>>>>>> +if WDT_K3_RTI
> > >>>>>>>> +
> > >>>>>>>> +config WDT_K3_RTI_LOAD_FW
> > >>>>>>>> + bool "Load watchdog firmware"
> > >>>>>>>> + depends on REMOTEPROC
> > >>>>>>>> + help
> > >>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> > >>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> > >>>>>>>> + of the watchdog timer, typically by resetting the system.
> > >>>>>>>> +
> > >>>>>>>> +config WDT_K3_RTI_FW_FILE
> > >>>>>>>> + string "Watchdog firmware image file"
> > >>>>>>>> + default "k3-rti-wdt.fw"
> > >>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> > >>>>>>>> + help
> > >>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> > >>>>>>>> + start.
> > >>>>>>>
> > >>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> > >>>>>>> drivers?
> > >>>>>>
> > >>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> > >>>>>> using binman and including this blob (which I happily see is GPLv2)
> > >>>>>> similar to how we do things with x86 for one example.
> > >>>>>>
> > >>>>>
> > >>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> > >>>>>
> > >>>>> Jan
> > >>>>>
> > >>>>
> > >>>> Did this help to answer open questions? Otherwise, please let me know.
> > >>>>
> > >>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> > >>>> needless - but I would also not mind getting everything in at once.
> > >>>
> > >>> Can you provide your reviewed-by if you are okay with this approach?
> > >>
> > >> I was kind of hoping Simon would chime in here on binman usage. So,
> > >> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> > >> for watchdog firmware. But I think binman_entry_find() and related
> > >> could work, in general, for this case of "need firmware blob embedded in
> > >> to image". That said, this isn't just any firmware blob, it's the
> > >> watchdog firmware. The less reliance on other things the safer it is.
> > >> That means this would be an exception to the general firmware blob
> > >> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> > >
> > > Yes I've been a little tied up recently. But I think this should use
> > > binman. We really don't want to be building binary firmware into
> > > U-Boot itself!
> > >
> > > Also Tom says, see x86 for a load of binaries of different types and
> > > how they are accessed at runttime. This is what binman is for.
> > >
> >
> > Please take the time and study my arguments. I'm open for better
> > proposals, but they need to be concrete and addressing my points.
>
> Do you mean 'properly hashed' and 'easy access', or something else?
> What can binman not do?
Well, as I said when ack'ing this, we're also talking about making sure
the system watchdog works. It needs to be as dead simple as possible.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-27 19:34 ` Tom Rini
@ 2021-06-27 20:37 ` Simon Glass
0 siblings, 0 replies; 28+ messages in thread
From: Simon Glass @ 2021-06-27 20:37 UTC (permalink / raw)
To: Tom Rini
Cc: Jan Kiszka, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi Tom,
On Sun, 27 Jun 2021 at 13:34, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jun 27, 2021 at 12:18:09PM -0600, Simon Glass wrote:
> > Hi Jan,
> >
> > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> > >
> > > On 26.06.21 20:29, Simon Glass wrote:
> > > > Hi,
> > > >
> > > > On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> > > >>
> > > >> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> > > >>> Hi Tom,
> > > >>>
> > > >>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> > > >>>> On 07.06.21 13:44, Jan Kiszka wrote:
> > > >>>>> On 07.06.21 13:40, Tom Rini wrote:
> > > >>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> > > >>>>>>> +Tom,
> > > >>>>>>>
> > > >>>>>>> Hi Tom,
> > > >>>>>>>
> > > >>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> > > >>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> > > >>>>>>>>
> > > >>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> > > >>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> > > >>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> > > >>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> > > >>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> > > >>>>>>>> properly hashed in case of secure boot.
> > > >>>>>>>>
> > > >>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> > > >>>>>>>>
> > > >>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > >>>>>>>> ---
> > > >>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> > > >>>>>>>> drivers/watchdog/Makefile | 5 +++
> > > >>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> > > >>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> > > >>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> > > >>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> > > >>>>>>>>
> > > >>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > > >>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> > > >>>>>>>> --- a/drivers/watchdog/Kconfig
> > > >>>>>>>> +++ b/drivers/watchdog/Kconfig
> > > >>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> > > >>>>>>>> Say Y here if you want to include support for the K3 watchdog
> > > >>>>>>>> timer (RTI module) available in the K3 generation of processors.
> > > >>>>>>>>
> > > >>>>>>>> +if WDT_K3_RTI
> > > >>>>>>>> +
> > > >>>>>>>> +config WDT_K3_RTI_LOAD_FW
> > > >>>>>>>> + bool "Load watchdog firmware"
> > > >>>>>>>> + depends on REMOTEPROC
> > > >>>>>>>> + help
> > > >>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> > > >>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> > > >>>>>>>> + of the watchdog timer, typically by resetting the system.
> > > >>>>>>>> +
> > > >>>>>>>> +config WDT_K3_RTI_FW_FILE
> > > >>>>>>>> + string "Watchdog firmware image file"
> > > >>>>>>>> + default "k3-rti-wdt.fw"
> > > >>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> > > >>>>>>>> + help
> > > >>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> > > >>>>>>>> + start.
> > > >>>>>>>
> > > >>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> > > >>>>>>> drivers?
> > > >>>>>>
> > > >>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> > > >>>>>> using binman and including this blob (which I happily see is GPLv2)
> > > >>>>>> similar to how we do things with x86 for one example.
> > > >>>>>>
> > > >>>>>
> > > >>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> > > >>>>>
> > > >>>>> Jan
> > > >>>>>
> > > >>>>
> > > >>>> Did this help to answer open questions? Otherwise, please let me know.
> > > >>>>
> > > >>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> > > >>>> needless - but I would also not mind getting everything in at once.
> > > >>>
> > > >>> Can you provide your reviewed-by if you are okay with this approach?
> > > >>
> > > >> I was kind of hoping Simon would chime in here on binman usage. So,
> > > >> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> > > >> for watchdog firmware. But I think binman_entry_find() and related
> > > >> could work, in general, for this case of "need firmware blob embedded in
> > > >> to image". That said, this isn't just any firmware blob, it's the
> > > >> watchdog firmware. The less reliance on other things the safer it is.
> > > >> That means this would be an exception to the general firmware blob
> > > >> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> > > >
> > > > Yes I've been a little tied up recently. But I think this should use
> > > > binman. We really don't want to be building binary firmware into
> > > > U-Boot itself!
> > > >
> > > > Also Tom says, see x86 for a load of binaries of different types and
> > > > how they are accessed at runttime. This is what binman is for.
> > > >
> > >
> > > Please take the time and study my arguments. I'm open for better
> > > proposals, but they need to be concrete and addressing my points.
> >
> > Do you mean 'properly hashed' and 'easy access', or something else?
> > What can binman not do?
>
> Well, as I said when ack'ing this, we're also talking about making sure
> the system watchdog works. It needs to be as dead simple as possible.
Another option would be for the system to fail to boot if the watchdog
could not be enabled.
Anyway, I'll leave it to you.
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-27 18:18 ` Simon Glass
2021-06-27 19:34 ` Tom Rini
@ 2021-06-28 5:40 ` Jan Kiszka
2021-07-05 15:29 ` Simon Glass
1 sibling, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-06-28 5:40 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
On 27.06.21 20:18, Simon Glass wrote:
> Hi Jan,
>
> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 26.06.21 20:29, Simon Glass wrote:
>>> Hi,
>>>
>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>>>>
>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
>>>>> Hi Tom,
>>>>>
>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>>>>>>> +Tom,
>>>>>>>>>
>>>>>>>>> Hi Tom,
>>>>>>>>>
>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>
>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>>>>>>> properly hashed in case of secure boot.
>>>>>>>>>>
>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>> ---
>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>>>>>>> --- a/drivers/watchdog/Kconfig
>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>>>>>>
>>>>>>>>>> +if WDT_K3_RTI
>>>>>>>>>> +
>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>>>>>>> + bool "Load watchdog firmware"
>>>>>>>>>> + depends on REMOTEPROC
>>>>>>>>>> + help
>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
>>>>>>>>>> +
>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
>>>>>>>>>> + string "Watchdog firmware image file"
>>>>>>>>>> + default "k3-rti-wdt.fw"
>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>>>>>>> + help
>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>>>>>>> + start.
>>>>>>>>>
>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>>>>>>> drivers?
>>>>>>>>
>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
>>>>>>>> similar to how we do things with x86 for one example.
>>>>>>>>
>>>>>>>
>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>>>>>>
>>>>>>> Jan
>>>>>>>
>>>>>>
>>>>>> Did this help to answer open questions? Otherwise, please let me know.
>>>>>>
>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
>>>>>> needless - but I would also not mind getting everything in at once.
>>>>>
>>>>> Can you provide your reviewed-by if you are okay with this approach?
>>>>
>>>> I was kind of hoping Simon would chime in here on binman usage. So,
>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
>>>> for watchdog firmware. But I think binman_entry_find() and related
>>>> could work, in general, for this case of "need firmware blob embedded in
>>>> to image". That said, this isn't just any firmware blob, it's the
>>>> watchdog firmware. The less reliance on other things the safer it is.
>>>> That means this would be an exception to the general firmware blob
>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
>>>
>>> Yes I've been a little tied up recently. But I think this should use
>>> binman. We really don't want to be building binary firmware into
>>> U-Boot itself!
>>>
>>> Also Tom says, see x86 for a load of binaries of different types and
>>> how they are accessed at runttime. This is what binman is for.
>>>
>>
>> Please take the time and study my arguments. I'm open for better
>> proposals, but they need to be concrete and addressing my points.
>
> Do you mean 'properly hashed' and 'easy access', or something else?
> What can binman not do?
Binman itself can stick things into binary images. But that is at most
half of the tasks needed here. I would need concrete guidance how to
- access that binary from u-boot proper in a reasonably simple way
- make sure the binary can be signed and the signature is evaluated
before using it
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-06-28 5:40 ` Jan Kiszka
@ 2021-07-05 15:29 ` Simon Glass
2021-07-14 9:53 ` Jan Kiszka
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2021-07-05 15:29 UTC (permalink / raw)
To: Jan Kiszka
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi Jan,
On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 27.06.21 20:18, Simon Glass wrote:
> > Hi Jan,
> >
> > On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> On 26.06.21 20:29, Simon Glass wrote:
> >>> Hi,
> >>>
> >>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> >>>>
> >>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> >>>>> Hi Tom,
> >>>>>
> >>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> >>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
> >>>>>>> On 07.06.21 13:40, Tom Rini wrote:
> >>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> >>>>>>>>> +Tom,
> >>>>>>>>>
> >>>>>>>>> Hi Tom,
> >>>>>>>>>
> >>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> >>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>
> >>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> >>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> >>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> >>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> >>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> >>>>>>>>>> properly hashed in case of secure boot.
> >>>>>>>>>>
> >>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>> ---
> >>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> >>>>>>>>>> drivers/watchdog/Makefile | 5 +++
> >>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> >>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> >>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> >>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> >>>>>>>>>> --- a/drivers/watchdog/Kconfig
> >>>>>>>>>> +++ b/drivers/watchdog/Kconfig
> >>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> >>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
> >>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
> >>>>>>>>>>
> >>>>>>>>>> +if WDT_K3_RTI
> >>>>>>>>>> +
> >>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
> >>>>>>>>>> + bool "Load watchdog firmware"
> >>>>>>>>>> + depends on REMOTEPROC
> >>>>>>>>>> + help
> >>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> >>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> >>>>>>>>>> + of the watchdog timer, typically by resetting the system.
> >>>>>>>>>> +
> >>>>>>>>>> +config WDT_K3_RTI_FW_FILE
> >>>>>>>>>> + string "Watchdog firmware image file"
> >>>>>>>>>> + default "k3-rti-wdt.fw"
> >>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> >>>>>>>>>> + help
> >>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> >>>>>>>>>> + start.
> >>>>>>>>>
> >>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> >>>>>>>>> drivers?
> >>>>>>>>
> >>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> >>>>>>>> using binman and including this blob (which I happily see is GPLv2)
> >>>>>>>> similar to how we do things with x86 for one example.
> >>>>>>>>
> >>>>>>>
> >>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> >>>>>>>
> >>>>>>> Jan
> >>>>>>>
> >>>>>>
> >>>>>> Did this help to answer open questions? Otherwise, please let me know.
> >>>>>>
> >>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> >>>>>> needless - but I would also not mind getting everything in at once.
> >>>>>
> >>>>> Can you provide your reviewed-by if you are okay with this approach?
> >>>>
> >>>> I was kind of hoping Simon would chime in here on binman usage. So,
> >>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> >>>> for watchdog firmware. But I think binman_entry_find() and related
> >>>> could work, in general, for this case of "need firmware blob embedded in
> >>>> to image". That said, this isn't just any firmware blob, it's the
> >>>> watchdog firmware. The less reliance on other things the safer it is.
> >>>> That means this would be an exception to the general firmware blob
> >>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> >>>
> >>> Yes I've been a little tied up recently. But I think this should use
> >>> binman. We really don't want to be building binary firmware into
> >>> U-Boot itself!
> >>>
> >>> Also Tom says, see x86 for a load of binaries of different types and
> >>> how they are accessed at runttime. This is what binman is for.
> >>>
> >>
> >> Please take the time and study my arguments. I'm open for better
> >> proposals, but they need to be concrete and addressing my points.
> >
> > Do you mean 'properly hashed' and 'easy access', or something else?
> > What can binman not do?
>
> Binman itself can stick things into binary images. But that is at most
> half of the tasks needed here. I would need concrete guidance how to
>
> - access that binary from u-boot proper in a reasonably simple way
I thought you wanted to access it from SPL. For that you would use
linker symbols. See 'Access to binman entry offsets at run time
(symbols)' in the binman docs for that.
But for U-Boot proper, the section is 'Access to binman entry offsets
at run time (fdt)'. There is no mention of the runtime library that
now exists (binman.h) so I will send a patch for that. But basically
you call binman_entry_find("name", &entry) and it returns the offset
and size for you.
> - make sure the binary can be signed and the signature is evaluated
> before using it
Do you mean signed or hashed? I think you mean hashed. If you trust
the U-Boot binary then presumably you can put the firmware in a
similar place do you can trust that as well. After all, you seem happy
enough to link it into U-Boot.
If not, you can ask binman to add a hash:
my-firmware {
type = "blob";
hash {
algo = "sha256";
};
};
Then you can add support for that to some sort of helper function in
binman.c, e.g.:
ofnode node, hashnode;
const u8 *hash;
int size;
node = binman_section_find_node("name");
if (!ofnode_valid(node))
...return error
hashnode = ofnode_read_prop(node, "hash");
if (!ofnode_valid(hashnode))
...return error
hash = ofnode_read_prop(hashnode, "value", &size);
/* Hash the firmware...need to read it from flash into fwdata/fwlen
using binman_entry_find() ...then: */
u8 digest[SHA256_SUM_LEN];
ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
if (ret)
return log_msg_ret("hash", ret);
/* compare the hashes */
if (size != sizeof(digest) || memcmp(hash, digest))
return log_msg_ret("cmp", ret);
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-07-05 15:29 ` Simon Glass
@ 2021-07-14 9:53 ` Jan Kiszka
2021-07-14 14:15 ` Simon Glass
0 siblings, 1 reply; 28+ messages in thread
From: Jan Kiszka @ 2021-07-14 9:53 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
On 05.07.21 17:29, Simon Glass wrote:
> Hi Jan,
>
> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 27.06.21 20:18, Simon Glass wrote:
>>> Hi Jan,
>>>
>>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>
>>>> On 26.06.21 20:29, Simon Glass wrote:
>>>>> Hi,
>>>>>
>>>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>>>>>>
>>>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
>>>>>>> Hi Tom,
>>>>>>>
>>>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
>>>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
>>>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
>>>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>>>>>>>>> +Tom,
>>>>>>>>>>>
>>>>>>>>>>> Hi Tom,
>>>>>>>>>>>
>>>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>>
>>>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>>>>>>>>> properly hashed in case of secure boot.
>>>>>>>>>>>>
>>>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
>>>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>>>>>>>>> --- a/drivers/watchdog/Kconfig
>>>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
>>>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>>>>>>>>
>>>>>>>>>>>> +if WDT_K3_RTI
>>>>>>>>>>>> +
>>>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>> + bool "Load watchdog firmware"
>>>>>>>>>>>> + depends on REMOTEPROC
>>>>>>>>>>>> + help
>>>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
>>>>>>>>>>>> +
>>>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
>>>>>>>>>>>> + string "Watchdog firmware image file"
>>>>>>>>>>>> + default "k3-rti-wdt.fw"
>>>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>> + help
>>>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>>>>>>>>> + start.
>>>>>>>>>>>
>>>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>>>>>>>>> drivers?
>>>>>>>>>>
>>>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
>>>>>>>>>> similar to how we do things with x86 for one example.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>>>>>>>>
>>>>>>>>> Jan
>>>>>>>>>
>>>>>>>>
>>>>>>>> Did this help to answer open questions? Otherwise, please let me know.
>>>>>>>>
>>>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
>>>>>>>> needless - but I would also not mind getting everything in at once.
>>>>>>>
>>>>>>> Can you provide your reviewed-by if you are okay with this approach?
>>>>>>
>>>>>> I was kind of hoping Simon would chime in here on binman usage. So,
>>>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
>>>>>> for watchdog firmware. But I think binman_entry_find() and related
>>>>>> could work, in general, for this case of "need firmware blob embedded in
>>>>>> to image". That said, this isn't just any firmware blob, it's the
>>>>>> watchdog firmware. The less reliance on other things the safer it is.
>>>>>> That means this would be an exception to the general firmware blob
>>>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
>>>>>
>>>>> Yes I've been a little tied up recently. But I think this should use
>>>>> binman. We really don't want to be building binary firmware into
>>>>> U-Boot itself!
>>>>>
>>>>> Also Tom says, see x86 for a load of binaries of different types and
>>>>> how they are accessed at runttime. This is what binman is for.
>>>>>
>>>>
>>>> Please take the time and study my arguments. I'm open for better
>>>> proposals, but they need to be concrete and addressing my points.
>>>
>>> Do you mean 'properly hashed' and 'easy access', or something else?
>>> What can binman not do?
>>
>> Binman itself can stick things into binary images. But that is at most
>> half of the tasks needed here. I would need concrete guidance how to
>>
>> - access that binary from u-boot proper in a reasonably simple way
>
> I thought you wanted to access it from SPL. For that you would use
> linker symbols. See 'Access to binman entry offsets at run time
> (symbols)' in the binman docs for that.
>
> But for U-Boot proper, the section is 'Access to binman entry offsets
> at run time (fdt)'. There is no mention of the runtime library that
> now exists (binman.h) so I will send a patch for that. But basically
> you call binman_entry_find("name", &entry) and it returns the offset
> and size for you.
>
>> - make sure the binary can be signed and the signature is evaluated
>> before using it
>
> Do you mean signed or hashed? I think you mean hashed. If you trust
> the U-Boot binary then presumably you can put the firmware in a
> similar place do you can trust that as well. After all, you seem happy
> enough to link it into U-Boot.
>
> If not, you can ask binman to add a hash:
>
> my-firmware {
> type = "blob";
> hash {
> algo = "sha256";
> };
> };
>
> Then you can add support for that to some sort of helper function in
> binman.c, e.g.:
>
> ofnode node, hashnode;
> const u8 *hash;
> int size;
>
> node = binman_section_find_node("name");
> if (!ofnode_valid(node))
> ...return error
> hashnode = ofnode_read_prop(node, "hash");
> if (!ofnode_valid(hashnode))
> ...return error
> hash = ofnode_read_prop(hashnode, "value", &size);
>
> /* Hash the firmware...need to read it from flash into fwdata/fwlen
> using binman_entry_find() ...then: */
> u8 digest[SHA256_SUM_LEN];
> ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
> if (ret)
> return log_msg_ret("hash", ret);
>
> /* compare the hashes */
> if (size != sizeof(digest) || memcmp(hash, digest))
> return log_msg_ret("cmp", ret);
>
Yes, it will likely be a hash that will be signed, and all that will be
checked by SPL when loading proper. The infrastructure for that should
be there, just not yet plugged for the way of loading things like we do
(one of my todos).
Obviously, what would be simplest for that is to have the watchdog blob
embedded, rather than separately hashed, as that would Just Work. I
didn't fully grasp yet what you propose, but it seems right now it will
complicate things. I'm not saying it will make it impossible, just harder.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-07-14 9:53 ` Jan Kiszka
@ 2021-07-14 14:15 ` Simon Glass
2021-07-20 12:57 ` Jan Kiszka
0 siblings, 1 reply; 28+ messages in thread
From: Simon Glass @ 2021-07-14 14:15 UTC (permalink / raw)
To: Jan Kiszka
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi Jan,
On Wed, 14 Jul 2021 at 03:53, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 05.07.21 17:29, Simon Glass wrote:
> > Hi Jan,
> >
> > On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> On 27.06.21 20:18, Simon Glass wrote:
> >>> Hi Jan,
> >>>
> >>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>>
> >>>> On 26.06.21 20:29, Simon Glass wrote:
> >>>>> Hi,
> >>>>>
> >>>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>
> >>>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> >>>>>>> Hi Tom,
> >>>>>>>
> >>>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> >>>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
> >>>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
> >>>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> >>>>>>>>>>> +Tom,
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Tom,
> >>>>>>>>>>>
> >>>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> >>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>>>
> >>>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> >>>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> >>>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> >>>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> >>>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> >>>>>>>>>>>> properly hashed in case of secure boot.
> >>>>>>>>>>>>
> >>>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> >>>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
> >>>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> >>>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> >>>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> >>>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> >>>>>>>>>>>> --- a/drivers/watchdog/Kconfig
> >>>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
> >>>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> >>>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
> >>>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
> >>>>>>>>>>>>
> >>>>>>>>>>>> +if WDT_K3_RTI
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
> >>>>>>>>>>>> + bool "Load watchdog firmware"
> >>>>>>>>>>>> + depends on REMOTEPROC
> >>>>>>>>>>>> + help
> >>>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> >>>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> >>>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
> >>>>>>>>>>>> +
> >>>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
> >>>>>>>>>>>> + string "Watchdog firmware image file"
> >>>>>>>>>>>> + default "k3-rti-wdt.fw"
> >>>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> >>>>>>>>>>>> + help
> >>>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> >>>>>>>>>>>> + start.
> >>>>>>>>>>>
> >>>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> >>>>>>>>>>> drivers?
> >>>>>>>>>>
> >>>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> >>>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
> >>>>>>>>>> similar to how we do things with x86 for one example.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> >>>>>>>>>
> >>>>>>>>> Jan
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Did this help to answer open questions? Otherwise, please let me know.
> >>>>>>>>
> >>>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> >>>>>>>> needless - but I would also not mind getting everything in at once.
> >>>>>>>
> >>>>>>> Can you provide your reviewed-by if you are okay with this approach?
> >>>>>>
> >>>>>> I was kind of hoping Simon would chime in here on binman usage. So,
> >>>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> >>>>>> for watchdog firmware. But I think binman_entry_find() and related
> >>>>>> could work, in general, for this case of "need firmware blob embedded in
> >>>>>> to image". That said, this isn't just any firmware blob, it's the
> >>>>>> watchdog firmware. The less reliance on other things the safer it is.
> >>>>>> That means this would be an exception to the general firmware blob
> >>>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> >>>>>
> >>>>> Yes I've been a little tied up recently. But I think this should use
> >>>>> binman. We really don't want to be building binary firmware into
> >>>>> U-Boot itself!
> >>>>>
> >>>>> Also Tom says, see x86 for a load of binaries of different types and
> >>>>> how they are accessed at runttime. This is what binman is for.
> >>>>>
> >>>>
> >>>> Please take the time and study my arguments. I'm open for better
> >>>> proposals, but they need to be concrete and addressing my points.
> >>>
> >>> Do you mean 'properly hashed' and 'easy access', or something else?
> >>> What can binman not do?
> >>
> >> Binman itself can stick things into binary images. But that is at most
> >> half of the tasks needed here. I would need concrete guidance how to
> >>
> >> - access that binary from u-boot proper in a reasonably simple way
> >
> > I thought you wanted to access it from SPL. For that you would use
> > linker symbols. See 'Access to binman entry offsets at run time
> > (symbols)' in the binman docs for that.
> >
> > But for U-Boot proper, the section is 'Access to binman entry offsets
> > at run time (fdt)'. There is no mention of the runtime library that
> > now exists (binman.h) so I will send a patch for that. But basically
> > you call binman_entry_find("name", &entry) and it returns the offset
> > and size for you.
> >
> >> - make sure the binary can be signed and the signature is evaluated
> >> before using it
> >
> > Do you mean signed or hashed? I think you mean hashed. If you trust
> > the U-Boot binary then presumably you can put the firmware in a
> > similar place do you can trust that as well. After all, you seem happy
> > enough to link it into U-Boot.
> >
> > If not, you can ask binman to add a hash:
> >
> > my-firmware {
> > type = "blob";
> > hash {
> > algo = "sha256";
> > };
> > };
> >
> > Then you can add support for that to some sort of helper function in
> > binman.c, e.g.:
> >
> > ofnode node, hashnode;
> > const u8 *hash;
> > int size;
> >
> > node = binman_section_find_node("name");
> > if (!ofnode_valid(node))
> > ...return error
> > hashnode = ofnode_read_prop(node, "hash");
> > if (!ofnode_valid(hashnode))
> > ...return error
> > hash = ofnode_read_prop(hashnode, "value", &size);
> >
> > /* Hash the firmware...need to read it from flash into fwdata/fwlen
> > using binman_entry_find() ...then: */
> > u8 digest[SHA256_SUM_LEN];
> > ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
> > if (ret)
> > return log_msg_ret("hash", ret);
> >
> > /* compare the hashes */
> > if (size != sizeof(digest) || memcmp(hash, digest))
> > return log_msg_ret("cmp", ret);
> >
>
> Yes, it will likely be a hash that will be signed, and all that will be
> checked by SPL when loading proper. The infrastructure for that should
> be there, just not yet plugged for the way of loading things like we do
> (one of my todos).
>
> Obviously, what would be simplest for that is to have the watchdog blob
> embedded, rather than separately hashed, as that would Just Work. I
> didn't fully grasp yet what you propose, but it seems right now it will
> complicate things. I'm not saying it will make it impossible, just harder.
I'm not even sure that is true. In binman, add the entry:
watchdog-firmware {
type = "blob";
filename = "...";
};
In the C code, declare a symbol that will get the image position of the entry:
binman_sym_declare(unsigned long, watchdog_firmware, image_pos);
then read that value when you need it:
int check_it(..)
{
ulong pos = binman_sym(ulong, watchdog_firmware, image_pos);
// read from flash offset @pos into a buffer
// check the hash
};
This is one of the key features of binman. It seems a shame to bring
in linker magic instead, when it is already there.
But my point was really that if you combine the watchdog firmware into
the image with binman, you should be able to avoid verifying it, or at
least rely on the same mechanism you use to verify U-Boot.
Feel free to come along and discuss this at one of the contributor
calls if that would be easier.
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-07-14 14:15 ` Simon Glass
@ 2021-07-20 12:57 ` Jan Kiszka
2021-07-20 16:14 ` Jan Kiszka
2021-07-20 17:33 ` Simon Glass
0 siblings, 2 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-07-20 12:57 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
On 14.07.21 16:15, Simon Glass wrote:
> Hi Jan,
>
> On Wed, 14 Jul 2021 at 03:53, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 05.07.21 17:29, Simon Glass wrote:
>>> Hi Jan,
>>>
>>> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>
>>>> On 27.06.21 20:18, Simon Glass wrote:
>>>>> Hi Jan,
>>>>>
>>>>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>
>>>>>> On 26.06.21 20:29, Simon Glass wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>
>>>>>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
>>>>>>>>> Hi Tom,
>>>>>>>>>
>>>>>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
>>>>>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
>>>>>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
>>>>>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>>>>>>>>>>> +Tom,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Tom,
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>>>>>>>>>>> properly hashed in case of secure boot.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
>>>>>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>>>>>>>>>>> --- a/drivers/watchdog/Kconfig
>>>>>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
>>>>>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> +if WDT_K3_RTI
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>>>> + bool "Load watchdog firmware"
>>>>>>>>>>>>>> + depends on REMOTEPROC
>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
>>>>>>>>>>>>>> +
>>>>>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
>>>>>>>>>>>>>> + string "Watchdog firmware image file"
>>>>>>>>>>>>>> + default "k3-rti-wdt.fw"
>>>>>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>>>>>>>>>>> + start.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>>>>>>>>>>> drivers?
>>>>>>>>>>>>
>>>>>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>>>>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
>>>>>>>>>>>> similar to how we do things with x86 for one example.
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>>>>>>>>>>
>>>>>>>>>>> Jan
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Did this help to answer open questions? Otherwise, please let me know.
>>>>>>>>>>
>>>>>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
>>>>>>>>>> needless - but I would also not mind getting everything in at once.
>>>>>>>>>
>>>>>>>>> Can you provide your reviewed-by if you are okay with this approach?
>>>>>>>>
>>>>>>>> I was kind of hoping Simon would chime in here on binman usage. So,
>>>>>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
>>>>>>>> for watchdog firmware. But I think binman_entry_find() and related
>>>>>>>> could work, in general, for this case of "need firmware blob embedded in
>>>>>>>> to image". That said, this isn't just any firmware blob, it's the
>>>>>>>> watchdog firmware. The less reliance on other things the safer it is.
>>>>>>>> That means this would be an exception to the general firmware blob
>>>>>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
>>>>>>>
>>>>>>> Yes I've been a little tied up recently. But I think this should use
>>>>>>> binman. We really don't want to be building binary firmware into
>>>>>>> U-Boot itself!
>>>>>>>
>>>>>>> Also Tom says, see x86 for a load of binaries of different types and
>>>>>>> how they are accessed at runttime. This is what binman is for.
>>>>>>>
>>>>>>
>>>>>> Please take the time and study my arguments. I'm open for better
>>>>>> proposals, but they need to be concrete and addressing my points.
>>>>>
>>>>> Do you mean 'properly hashed' and 'easy access', or something else?
>>>>> What can binman not do?
>>>>
>>>> Binman itself can stick things into binary images. But that is at most
>>>> half of the tasks needed here. I would need concrete guidance how to
>>>>
>>>> - access that binary from u-boot proper in a reasonably simple way
>>>
>>> I thought you wanted to access it from SPL. For that you would use
>>> linker symbols. See 'Access to binman entry offsets at run time
>>> (symbols)' in the binman docs for that.
>>>
>>> But for U-Boot proper, the section is 'Access to binman entry offsets
>>> at run time (fdt)'. There is no mention of the runtime library that
>>> now exists (binman.h) so I will send a patch for that. But basically
>>> you call binman_entry_find("name", &entry) and it returns the offset
>>> and size for you.
>>>
>>>> - make sure the binary can be signed and the signature is evaluated
>>>> before using it
>>>
>>> Do you mean signed or hashed? I think you mean hashed. If you trust
>>> the U-Boot binary then presumably you can put the firmware in a
>>> similar place do you can trust that as well. After all, you seem happy
>>> enough to link it into U-Boot.
>>>
>>> If not, you can ask binman to add a hash:
>>>
>>> my-firmware {
>>> type = "blob";
>>> hash {
>>> algo = "sha256";
>>> };
>>> };
>>>
>>> Then you can add support for that to some sort of helper function in
>>> binman.c, e.g.:
>>>
>>> ofnode node, hashnode;
>>> const u8 *hash;
>>> int size;
>>>
>>> node = binman_section_find_node("name");
>>> if (!ofnode_valid(node))
>>> ...return error
>>> hashnode = ofnode_read_prop(node, "hash");
>>> if (!ofnode_valid(hashnode))
>>> ...return error
>>> hash = ofnode_read_prop(hashnode, "value", &size);
>>>
>>> /* Hash the firmware...need to read it from flash into fwdata/fwlen
>>> using binman_entry_find() ...then: */
>>> u8 digest[SHA256_SUM_LEN];
>>> ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
>>> if (ret)
>>> return log_msg_ret("hash", ret);
>>>
>>> /* compare the hashes */
>>> if (size != sizeof(digest) || memcmp(hash, digest))
>>> return log_msg_ret("cmp", ret);
>>>
>>
>> Yes, it will likely be a hash that will be signed, and all that will be
>> checked by SPL when loading proper. The infrastructure for that should
>> be there, just not yet plugged for the way of loading things like we do
>> (one of my todos).
>>
>> Obviously, what would be simplest for that is to have the watchdog blob
>> embedded, rather than separately hashed, as that would Just Work. I
>> didn't fully grasp yet what you propose, but it seems right now it will
>> complicate things. I'm not saying it will make it impossible, just harder.
>
> I'm not even sure that is true. In binman, add the entry:
>
> watchdog-firmware {
> type = "blob";
> filename = "...";
> };
>
> In the C code, declare a symbol that will get the image position of the entry:
>
> binman_sym_declare(unsigned long, watchdog_firmware, image_pos);
>
> then read that value when you need it:
>
> int check_it(..)
> {
> ulong pos = binman_sym(ulong, watchdog_firmware, image_pos);
>
> // read from flash offset @pos into a buffer
>
> // check the hash
> };
This function is the extra boilerplate code that is not needed when the
blob is simply part of the U-Boot binary that is loaded and checked by
SPL. The worst part of this: It requires special handling of different
storage media. We currently only have OSPI for out board, but you may
also imagine versions that load U-Boot from filesystems (the TI EVM does
that). So this is the extra complexity without extra value I'm always
talking about.
But I'm happy to take concrete suggestions where to add the firmware
into our board and where to add the necessary code in a simple way.
What we do so far: U-Boot proper and DTBs go into a fit image that SPL
will load (and later also validate). It would be simple to do
diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
index 96647719df..d3242af70c 100644
--- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
+++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
@@ -60,6 +60,11 @@
filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
};
};
+
+ k3-rti-firmware {
+ type = "blob";
+ filename = CONFIG_WDT_K3_RTI_FW_FILE;
+ };
};
configurations {
in [1] (used by [2]).
But now: How do I communicate that blob address from SPL to U-Boot
proper so that I can skip the extra medium-specific loading part and
also reuse the fit image validation of SPL? If there is a simple way for
that, I could indeed switch the model.
Jan
[1] https://patchwork.ozlabs.org/project/uboot/patch/f7cf19b1fd2ce52d74c25e590aee452d30a6f1e4.1622626660.git.jan.kiszka@siemens.com/
[2] https://patchwork.ozlabs.org/project/uboot/patch/a83aa9023ea9c49cf1efd4a72d85bedb1e388478.1623440557.git.jan.kiszka@siemens.com/
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-07-20 12:57 ` Jan Kiszka
@ 2021-07-20 16:14 ` Jan Kiszka
2021-07-20 17:33 ` Simon Glass
1 sibling, 0 replies; 28+ messages in thread
From: Jan Kiszka @ 2021-07-20 16:14 UTC (permalink / raw)
To: Simon Glass
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
On 20.07.21 14:57, Jan Kiszka wrote:
> On 14.07.21 16:15, Simon Glass wrote:
>> Hi Jan,
>>
>> On Wed, 14 Jul 2021 at 03:53, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>
>>> On 05.07.21 17:29, Simon Glass wrote:
>>>> Hi Jan,
>>>>
>>>> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>
>>>>> On 27.06.21 20:18, Simon Glass wrote:
>>>>>> Hi Jan,
>>>>>>
>>>>>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>>>>>>
>>>>>>> On 26.06.21 20:29, Simon Glass wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
>>>>>>>>>
>>>>>>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
>>>>>>>>>> Hi Tom,
>>>>>>>>>>
>>>>>>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
>>>>>>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
>>>>>>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
>>>>>>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
>>>>>>>>>>>>>> +Tom,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hi Tom,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
>>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
>>>>>>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
>>>>>>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
>>>>>>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
>>>>>>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
>>>>>>>>>>>>>>> properly hashed in case of secure boot.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
>>>>>>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
>>>>>>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
>>>>>>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
>>>>>>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
>>>>>>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>>>>>>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
>>>>>>>>>>>>>>> --- a/drivers/watchdog/Kconfig
>>>>>>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
>>>>>>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
>>>>>>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
>>>>>>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> +if WDT_K3_RTI
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>>>>> + bool "Load watchdog firmware"
>>>>>>>>>>>>>>> + depends on REMOTEPROC
>>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
>>>>>>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
>>>>>>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
>>>>>>>>>>>>>>> +
>>>>>>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
>>>>>>>>>>>>>>> + string "Watchdog firmware image file"
>>>>>>>>>>>>>>> + default "k3-rti-wdt.fw"
>>>>>>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
>>>>>>>>>>>>>>> + help
>>>>>>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
>>>>>>>>>>>>>>> + start.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
>>>>>>>>>>>>>> drivers?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
>>>>>>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
>>>>>>>>>>>>> similar to how we do things with x86 for one example.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
>>>>>>>>>>>>
>>>>>>>>>>>> Jan
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Did this help to answer open questions? Otherwise, please let me know.
>>>>>>>>>>>
>>>>>>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
>>>>>>>>>>> needless - but I would also not mind getting everything in at once.
>>>>>>>>>>
>>>>>>>>>> Can you provide your reviewed-by if you are okay with this approach?
>>>>>>>>>
>>>>>>>>> I was kind of hoping Simon would chime in here on binman usage. So,
>>>>>>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
>>>>>>>>> for watchdog firmware. But I think binman_entry_find() and related
>>>>>>>>> could work, in general, for this case of "need firmware blob embedded in
>>>>>>>>> to image". That said, this isn't just any firmware blob, it's the
>>>>>>>>> watchdog firmware. The less reliance on other things the safer it is.
>>>>>>>>> That means this would be an exception to the general firmware blob
>>>>>>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
>>>>>>>>
>>>>>>>> Yes I've been a little tied up recently. But I think this should use
>>>>>>>> binman. We really don't want to be building binary firmware into
>>>>>>>> U-Boot itself!
>>>>>>>>
>>>>>>>> Also Tom says, see x86 for a load of binaries of different types and
>>>>>>>> how they are accessed at runttime. This is what binman is for.
>>>>>>>>
>>>>>>>
>>>>>>> Please take the time and study my arguments. I'm open for better
>>>>>>> proposals, but they need to be concrete and addressing my points.
>>>>>>
>>>>>> Do you mean 'properly hashed' and 'easy access', or something else?
>>>>>> What can binman not do?
>>>>>
>>>>> Binman itself can stick things into binary images. But that is at most
>>>>> half of the tasks needed here. I would need concrete guidance how to
>>>>>
>>>>> - access that binary from u-boot proper in a reasonably simple way
>>>>
>>>> I thought you wanted to access it from SPL. For that you would use
>>>> linker symbols. See 'Access to binman entry offsets at run time
>>>> (symbols)' in the binman docs for that.
>>>>
>>>> But for U-Boot proper, the section is 'Access to binman entry offsets
>>>> at run time (fdt)'. There is no mention of the runtime library that
>>>> now exists (binman.h) so I will send a patch for that. But basically
>>>> you call binman_entry_find("name", &entry) and it returns the offset
>>>> and size for you.
>>>>
>>>>> - make sure the binary can be signed and the signature is evaluated
>>>>> before using it
>>>>
>>>> Do you mean signed or hashed? I think you mean hashed. If you trust
>>>> the U-Boot binary then presumably you can put the firmware in a
>>>> similar place do you can trust that as well. After all, you seem happy
>>>> enough to link it into U-Boot.
>>>>
>>>> If not, you can ask binman to add a hash:
>>>>
>>>> my-firmware {
>>>> type = "blob";
>>>> hash {
>>>> algo = "sha256";
>>>> };
>>>> };
>>>>
>>>> Then you can add support for that to some sort of helper function in
>>>> binman.c, e.g.:
>>>>
>>>> ofnode node, hashnode;
>>>> const u8 *hash;
>>>> int size;
>>>>
>>>> node = binman_section_find_node("name");
>>>> if (!ofnode_valid(node))
>>>> ...return error
>>>> hashnode = ofnode_read_prop(node, "hash");
>>>> if (!ofnode_valid(hashnode))
>>>> ...return error
>>>> hash = ofnode_read_prop(hashnode, "value", &size);
>>>>
>>>> /* Hash the firmware...need to read it from flash into fwdata/fwlen
>>>> using binman_entry_find() ...then: */
>>>> u8 digest[SHA256_SUM_LEN];
>>>> ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
>>>> if (ret)
>>>> return log_msg_ret("hash", ret);
>>>>
>>>> /* compare the hashes */
>>>> if (size != sizeof(digest) || memcmp(hash, digest))
>>>> return log_msg_ret("cmp", ret);
>>>>
>>>
>>> Yes, it will likely be a hash that will be signed, and all that will be
>>> checked by SPL when loading proper. The infrastructure for that should
>>> be there, just not yet plugged for the way of loading things like we do
>>> (one of my todos).
>>>
>>> Obviously, what would be simplest for that is to have the watchdog blob
>>> embedded, rather than separately hashed, as that would Just Work. I
>>> didn't fully grasp yet what you propose, but it seems right now it will
>>> complicate things. I'm not saying it will make it impossible, just harder.
>>
>> I'm not even sure that is true. In binman, add the entry:
>>
>> watchdog-firmware {
>> type = "blob";
>> filename = "...";
>> };
>>
>> In the C code, declare a symbol that will get the image position of the entry:
>>
>> binman_sym_declare(unsigned long, watchdog_firmware, image_pos);
>>
>> then read that value when you need it:
>>
>> int check_it(..)
>> {
>> ulong pos = binman_sym(ulong, watchdog_firmware, image_pos);
>>
>> // read from flash offset @pos into a buffer
>>
>> // check the hash
>> };
>
> This function is the extra boilerplate code that is not needed when the
> blob is simply part of the U-Boot binary that is loaded and checked by
> SPL. The worst part of this: It requires special handling of different
> storage media. We currently only have OSPI for out board, but you may
> also imagine versions that load U-Boot from filesystems (the TI EVM does
> that). So this is the extra complexity without extra value I'm always
> talking about.
>
> But I'm happy to take concrete suggestions where to add the firmware
> into our board and where to add the necessary code in a simple way.
>
> What we do so far: U-Boot proper and DTBs go into a fit image that SPL
> will load (and later also validate). It would be simple to do
>
> diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> index 96647719df..d3242af70c 100644
> --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> @@ -60,6 +60,11 @@
> filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
> };
> };
> +
> + k3-rti-firmware {
> + type = "blob";
> + filename = CONFIG_WDT_K3_RTI_FW_FILE;
> + };
> };
>
> configurations {
>
> in [1] (used by [2]).
>
> But now: How do I communicate that blob address from SPL to U-Boot
> proper so that I can skip the extra medium-specific loading part and
> also reuse the fit image validation of SPL? If there is a simple way for
> that, I could indeed switch the model.
>
OK, I think I found a trick (outside of binman): Making the firmware an
additional loadable in the u-boot fit image, and then picking its
location up from /fit-images/k3-rti-wdt-firmware in rti_wdt_load_fw().
That should be nicer then object embedded - without requiring the
complexity of separate image loading and validating.
Jan
--
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware
2021-07-20 12:57 ` Jan Kiszka
2021-07-20 16:14 ` Jan Kiszka
@ 2021-07-20 17:33 ` Simon Glass
1 sibling, 0 replies; 28+ messages in thread
From: Simon Glass @ 2021-07-20 17:33 UTC (permalink / raw)
To: Jan Kiszka
Cc: Tom Rini, Lokesh Vutla, U-Boot Mailing List, Le Jin,
Bao Cheng Su, Nian Gao, Chao Zeng
Hi Jan,
On Tue, 20 Jul 2021 at 06:58, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 14.07.21 16:15, Simon Glass wrote:
> > Hi Jan,
> >
> > On Wed, 14 Jul 2021 at 03:53, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> On 05.07.21 17:29, Simon Glass wrote:
> >>> Hi Jan,
> >>>
> >>> On Sun, 27 Jun 2021 at 23:40, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>>
> >>>> On 27.06.21 20:18, Simon Glass wrote:
> >>>>> Hi Jan,
> >>>>>
> >>>>> On Sun, 27 Jun 2021 at 12:01, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>>>>>
> >>>>>> On 26.06.21 20:29, Simon Glass wrote:
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> On Fri, 11 Jun 2021 at 08:08, Tom Rini <trini@konsulko.com> wrote:
> >>>>>>>>
> >>>>>>>> On Fri, Jun 11, 2021 at 07:14:21PM +0530, Lokesh Vutla wrote:
> >>>>>>>>> Hi Tom,
> >>>>>>>>>
> >>>>>>>>> On 09/06/21 6:47 pm, Jan Kiszka wrote:
> >>>>>>>>>> On 07.06.21 13:44, Jan Kiszka wrote:
> >>>>>>>>>>> On 07.06.21 13:40, Tom Rini wrote:
> >>>>>>>>>>>> On Mon, Jun 07, 2021 at 03:33:52PM +0530, Lokesh Vutla wrote:
> >>>>>>>>>>>>> +Tom,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Hi Tom,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On 02/06/21 3:07 pm, Jan Kiszka wrote:
> >>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> To avoid the need of extra boot scripting on AM65x for loading a
> >>>>>>>>>>>>>> watchdog firmware, add the required rproc init and loading logic for the
> >>>>>>>>>>>>>> first R5F core to the watchdog start handler. In case the R5F cluster is
> >>>>>>>>>>>>>> in lock-step mode, also initialize the second core. The firmware itself
> >>>>>>>>>>>>>> is embedded into U-Boot binary to ease access to it and ensure it is
> >>>>>>>>>>>>>> properly hashed in case of secure boot.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> One possible firmware source is https://github.com/siemens/k3-rti-wdt.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>> drivers/watchdog/Kconfig | 20 ++++++++++++
> >>>>>>>>>>>>>> drivers/watchdog/Makefile | 5 +++
> >>>>>>>>>>>>>> drivers/watchdog/rti_wdt.c | 58 ++++++++++++++++++++++++++++++++++-
> >>>>>>>>>>>>>> drivers/watchdog/rti_wdt_fw.S | 20 ++++++++++++
> >>>>>>>>>>>>>> 4 files changed, 102 insertions(+), 1 deletion(-)
> >>>>>>>>>>>>>> create mode 100644 drivers/watchdog/rti_wdt_fw.S
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>>>>>>>>>>>>> index f0ff2612a6..1a1fddfe9f 100644
> >>>>>>>>>>>>>> --- a/drivers/watchdog/Kconfig
> >>>>>>>>>>>>>> +++ b/drivers/watchdog/Kconfig
> >>>>>>>>>>>>>> @@ -209,6 +209,26 @@ config WDT_K3_RTI
> >>>>>>>>>>>>>> Say Y here if you want to include support for the K3 watchdog
> >>>>>>>>>>>>>> timer (RTI module) available in the K3 generation of processors.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> +if WDT_K3_RTI
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> +config WDT_K3_RTI_LOAD_FW
> >>>>>>>>>>>>>> + bool "Load watchdog firmware"
> >>>>>>>>>>>>>> + depends on REMOTEPROC
> >>>>>>>>>>>>>> + help
> >>>>>>>>>>>>>> + Automatically load the specified firmware image into the MCU R5F
> >>>>>>>>>>>>>> + core 0. On the AM65x, this firmware is supposed to handle the expiry
> >>>>>>>>>>>>>> + of the watchdog timer, typically by resetting the system.
> >>>>>>>>>>>>>> +
> >>>>>>>>>>>>>> +config WDT_K3_RTI_FW_FILE
> >>>>>>>>>>>>>> + string "Watchdog firmware image file"
> >>>>>>>>>>>>>> + default "k3-rti-wdt.fw"
> >>>>>>>>>>>>>> + depends on WDT_K3_RTI_LOAD_FW
> >>>>>>>>>>>>>> + help
> >>>>>>>>>>>>>> + Firmware image to be embedded into U-Boot and loaded on watchdog
> >>>>>>>>>>>>>> + start.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I need your input on this proach. Is it okay to include the linker file unders
> >>>>>>>>>>>>> drivers?
> >>>>>>>>>>>>
> >>>>>>>>>>>> Maybe? I suppose the first thing that springs to mind is why aren't we
> >>>>>>>>>>>> using binman and including this blob (which I happily see is GPLv2)
> >>>>>>>>>>>> similar to how we do things with x86 for one example.
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> See https://www.mail-archive.com/u-boot@lists.denx.de/msg377894.html
> >>>>>>>>>>>
> >>>>>>>>>>> Jan
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Did this help to answer open questions? Otherwise, please let me know.
> >>>>>>>>>>
> >>>>>>>>>> I'd also like to avoid that his patch alone blocks 1-3 of the series
> >>>>>>>>>> needless - but I would also not mind getting everything in at once.
> >>>>>>>>>
> >>>>>>>>> Can you provide your reviewed-by if you are okay with this approach?
> >>>>>>>>
> >>>>>>>> I was kind of hoping Simon would chime in here on binman usage. So,
> >>>>>>>> re-re-reading the above URL, yes, fsloader wouldn't be the right choice
> >>>>>>>> for watchdog firmware. But I think binman_entry_find() and related
> >>>>>>>> could work, in general, for this case of "need firmware blob embedded in
> >>>>>>>> to image". That said, this isn't just any firmware blob, it's the
> >>>>>>>> watchdog firmware. The less reliance on other things the safer it is.
> >>>>>>>> That means this would be an exception to the general firmware blob
> >>>>>>>> loading rule and yeah, OK, we can do it this way. Sorry for the delay.
> >>>>>>>
> >>>>>>> Yes I've been a little tied up recently. But I think this should use
> >>>>>>> binman. We really don't want to be building binary firmware into
> >>>>>>> U-Boot itself!
> >>>>>>>
> >>>>>>> Also Tom says, see x86 for a load of binaries of different types and
> >>>>>>> how they are accessed at runttime. This is what binman is for.
> >>>>>>>
> >>>>>>
> >>>>>> Please take the time and study my arguments. I'm open for better
> >>>>>> proposals, but they need to be concrete and addressing my points.
> >>>>>
> >>>>> Do you mean 'properly hashed' and 'easy access', or something else?
> >>>>> What can binman not do?
> >>>>
> >>>> Binman itself can stick things into binary images. But that is at most
> >>>> half of the tasks needed here. I would need concrete guidance how to
> >>>>
> >>>> - access that binary from u-boot proper in a reasonably simple way
> >>>
> >>> I thought you wanted to access it from SPL. For that you would use
> >>> linker symbols. See 'Access to binman entry offsets at run time
> >>> (symbols)' in the binman docs for that.
> >>>
> >>> But for U-Boot proper, the section is 'Access to binman entry offsets
> >>> at run time (fdt)'. There is no mention of the runtime library that
> >>> now exists (binman.h) so I will send a patch for that. But basically
> >>> you call binman_entry_find("name", &entry) and it returns the offset
> >>> and size for you.
> >>>
> >>>> - make sure the binary can be signed and the signature is evaluated
> >>>> before using it
> >>>
> >>> Do you mean signed or hashed? I think you mean hashed. If you trust
> >>> the U-Boot binary then presumably you can put the firmware in a
> >>> similar place do you can trust that as well. After all, you seem happy
> >>> enough to link it into U-Boot.
> >>>
> >>> If not, you can ask binman to add a hash:
> >>>
> >>> my-firmware {
> >>> type = "blob";
> >>> hash {
> >>> algo = "sha256";
> >>> };
> >>> };
> >>>
> >>> Then you can add support for that to some sort of helper function in
> >>> binman.c, e.g.:
> >>>
> >>> ofnode node, hashnode;
> >>> const u8 *hash;
> >>> int size;
> >>>
> >>> node = binman_section_find_node("name");
> >>> if (!ofnode_valid(node))
> >>> ...return error
> >>> hashnode = ofnode_read_prop(node, "hash");
> >>> if (!ofnode_valid(hashnode))
> >>> ...return error
> >>> hash = ofnode_read_prop(hashnode, "value", &size);
> >>>
> >>> /* Hash the firmware...need to read it from flash into fwdata/fwlen
> >>> using binman_entry_find() ...then: */
> >>> u8 digest[SHA256_SUM_LEN];
> >>> ret = hash_block("sha256", fwdata, fwlen, digest, sizeof(digest);
> >>> if (ret)
> >>> return log_msg_ret("hash", ret);
> >>>
> >>> /* compare the hashes */
> >>> if (size != sizeof(digest) || memcmp(hash, digest))
> >>> return log_msg_ret("cmp", ret);
> >>>
> >>
> >> Yes, it will likely be a hash that will be signed, and all that will be
> >> checked by SPL when loading proper. The infrastructure for that should
> >> be there, just not yet plugged for the way of loading things like we do
> >> (one of my todos).
> >>
> >> Obviously, what would be simplest for that is to have the watchdog blob
> >> embedded, rather than separately hashed, as that would Just Work. I
> >> didn't fully grasp yet what you propose, but it seems right now it will
> >> complicate things. I'm not saying it will make it impossible, just harder.
> >
> > I'm not even sure that is true. In binman, add the entry:
> >
> > watchdog-firmware {
> > type = "blob";
> > filename = "...";
> > };
> >
> > In the C code, declare a symbol that will get the image position of the entry:
> >
> > binman_sym_declare(unsigned long, watchdog_firmware, image_pos);
> >
> > then read that value when you need it:
> >
> > int check_it(..)
> > {
> > ulong pos = binman_sym(ulong, watchdog_firmware, image_pos);
> >
> > // read from flash offset @pos into a buffer
> >
> > // check the hash
> > };
>
> This function is the extra boilerplate code that is not needed when the
> blob is simply part of the U-Boot binary that is loaded and checked by
> SPL. The worst part of this: It requires special handling of different
> storage media. We currently only have OSPI for out board, but you may
> also imagine versions that load U-Boot from filesystems (the TI EVM does
> that). So this is the extra complexity without extra value I'm always
> talking about.
Well you should load the whole thing (including U-Boot and the
watchdog-firmware) in one go, i.e. put everything into a section
called u-boot and SPL should load everything. My 'read from flash
offset' could just be 'locate flash offset and determine where it has
been loaded already'.
But yes if you have to load it separately it is more complicated. Also
that defeats the original goal I think, since the idea was to enable
the watchdog fairly early.
>
> But I'm happy to take concrete suggestions where to add the firmware
> into our board and where to add the necessary code in a simple way.
>
> What we do so far: U-Boot proper and DTBs go into a fit image that SPL
> will load (and later also validate). It would be simple to do
>
> diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> index 96647719df..d3242af70c 100644
> --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> @@ -60,6 +60,11 @@
> filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb";
> };
> };
> +
> + k3-rti-firmware {
> + type = "blob";
> + filename = CONFIG_WDT_K3_RTI_FW_FILE;
> + };
> };
>
> configurations {
>
> in [1] (used by [2]).
>
> But now: How do I communicate that blob address from SPL to U-Boot
> proper so that I can skip the extra medium-specific loading part and
> also reuse the fit image validation of SPL? If there is a simple way for
> that, I could indeed switch the model.
The easiest way might be to put everything in a section:
u-boot { // name this so that SPL locates the image_pos/size symbols
type = "section";
u-boot {
};
k3-rti-firmware {
};
};
But I suppose you could also do things with a special SPL loader if necessary.
Regards,
Simon
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2021-07-20 17:33 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 9:37 [PATCH v2 0/5] Add SIMATIC IOT2050 board support Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 1/5] arm: dts: Add IOT2050 device tree files Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 2/5] board: siemens: Add support for SIMATIC IOT2050 devices Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 3/5] arm64: dts: ti: k3-am65-mcu: Add RTI watchdog entry Jan Kiszka
2021-06-02 9:37 ` [PATCH v2 4/5] watchdog: rti_wdt: Add support for loading firmware Jan Kiszka
2021-06-07 10:03 ` Lokesh Vutla
2021-06-07 10:20 ` Jan Kiszka
2021-06-07 11:40 ` Tom Rini
2021-06-07 11:44 ` Jan Kiszka
2021-06-09 13:17 ` Jan Kiszka
2021-06-11 13:44 ` Lokesh Vutla
2021-06-11 14:08 ` Tom Rini
2021-06-26 18:29 ` Simon Glass
2021-06-27 18:01 ` Jan Kiszka
2021-06-27 18:18 ` Simon Glass
2021-06-27 19:34 ` Tom Rini
2021-06-27 20:37 ` Simon Glass
2021-06-28 5:40 ` Jan Kiszka
2021-07-05 15:29 ` Simon Glass
2021-07-14 9:53 ` Jan Kiszka
2021-07-14 14:15 ` Simon Glass
2021-07-20 12:57 ` Jan Kiszka
2021-07-20 16:14 ` Jan Kiszka
2021-07-20 17:33 ` Simon Glass
2021-06-02 9:37 ` [PATCH v2 5/5] configs: iot2050: Enable watchdog support, but do not auto-start it Jan Kiszka
2021-06-11 14:30 ` [PATCH v2 0/5] Add SIMATIC IOT2050 board support Lokesh Vutla
2021-06-11 14:53 ` Tom Rini
2021-06-11 18:20 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.