[PATCH v2 0/5] Add SIMATIC IOT2050 board support
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
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"; +};
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.... +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 */
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>; + }; };
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
+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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.
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
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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.
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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
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 R5Fcore 0. On the AM65x, this firmware is supposed to handle the expiryof 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 watchdogstart.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
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
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
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
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.
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
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
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
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
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
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/f7cf19b1fd2ce52d74c25e590ae... [2] https://patchwork.ozlabs.org/project/uboot/patch/a83aa9023ea9c49cf1efd4a72d8...
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
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
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
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
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.
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
participants (4)
-
Jan Kiszka -
Lokesh Vutla -
Simon Glass -
Tom Rini