On Fri, Jun 11, 2021 at 08:00:17PM +0530, Lokesh Vutla wrote: > > > On 02/06/21 3:07 pm, Jan Kiszka wrote: > > This is the baseline support for the SIMATIC IOT2050 devices. > > > > Changes in v2: > > - rebased > > - sync with upstream-accepted DT > > - add boot switch > > - include watchdog support > > > > Allows to boot mainline 5.10 kernels, but not the original BSP-derived > > kernel we currently ship as reference. This is due to the TI sysfw ABI > > breakages between 2.x and 3.x. We will soon provide a transitional > > kernel that allows booting both firmware ABIs - as long as full upstream > > kernel support is work in progress. > > > > Note that this baseline support lacks Ethernet drivers. We are working > > closely with TI to ensure that the to-be-upstreamed icssg-prueth driver > > will work both with new SR2.0 AM65x silicon as well as with SR1.0 which > > is used in the currently shipped IOT2050 devices. > > > > A staging tree for complete IOT2050 support can be found at [1]. Full > > image integration is available via [2]. > > > > Regarding patch 4: There has been some doubts on the proposed approach, > > but there has been also no suggestion provided for a similarly > > lightweight and secure embedding method. Therefore, I'm proposing our > > solution once again. > > There are multiple checkpatch issues with this series. Can you fix them where > ever possible? > > ➜ u-boot-ti git:(for-next) ./scripts/checkpatch.pl --strict siemens/*.patch > -------------------------------------------------------- > siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch > -------------------------------------------------------- > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #50: > new file mode 100644 > > WARNING: line length of 102 exceeds 100 columns > #103: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:49: > + filename = "arch/arm/dts/k3-am6528-iot2050-basic.dtb"; > > WARNING: line length of 105 exceeds 100 columns > #113: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:59: > + filename = "arch/arm/dts/k3-am6548-iot2050-advanced.dtb"; > > total: 0 errors, 3 warnings, 0 checks, 1025 lines checked > > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inplace. > > siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch has style problems, > please review. > ----------------------------------------------------------------------- > siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch > ----------------------------------------------------------------------- > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #53: > new file mode 100644 > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #282: FILE: board/siemens/iot2050/board.c:86: > +#ifdef CONFIG_NET > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #338: FILE: board/siemens/iot2050/board.c:142: > +#ifdef CONFIG_SPL_LOAD_FIT > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #361: FILE: board/siemens/iot2050/board.c:165: > +#ifdef CONFIG_IOT2050_BOOT_SWITCH > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #404: FILE: board/siemens/iot2050/board.c:208: > +#ifdef CONFIG_IOT2050_BOOT_SWITCH > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #413: FILE: board/siemens/iot2050/board.c:217: > +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #458: FILE: board/siemens/iot2050/board.c:262: > +#if CONFIG_IS_ENABLED(LED) > > CHECK: Macro argument reuse 'func' - possible side-effects? > #683: FILE: include/configs/iot2050.h:43: > +#define BOOT_TARGET_DEVICES(func) \ > + func(MMC, mmc, 1) \ > + func(MMC, mmc, 0) \ > + func(USB, usb, 0) \ > + func(USB, usb, 1) \ > + func(USB, usb, 2) > > total: 0 errors, 7 warnings, 1 checks, 606 lines checked > > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inplace. > > siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch has > style problems, please review. > ------------------------------------------------------------------ > siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch > ------------------------------------------------------------------ > total: 0 errors, 0 warnings, 0 checks, 13 lines checked > > siemens/0003-arm64-dts-ti-k3-am65-mcu-Add-RTI-watchdog-entry.patch has no > obvious style problems and is ready for submission. > -------------------------------------------------------------------- > siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch > -------------------------------------------------------------------- > WARNING: externs should be avoided in .c files > #95: FILE: drivers/watchdog/rti_wdt.c:47: > +extern const u32 rti_wdt_fw[]; > > WARNING: externs should be avoided in .c files > #96: FILE: drivers/watchdog/rti_wdt.c:48: > +extern const int rti_wdt_fw_size; > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #100: FILE: drivers/watchdog/rti_wdt.c:52: > +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW > > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible > #113: FILE: drivers/watchdog/rti_wdt.c:64: > +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW > > WARNING: labels should not be indented > #116: FILE: drivers/watchdog/rti_wdt.c:67: > + dt_error: > > WARNING: labels should not be indented > #137: FILE: drivers/watchdog/rti_wdt.c:88: > + fw_error: > > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #163: > new file mode 100644 > > WARNING: Improper SPDX comment style for 'drivers/watchdog/rti_wdt_fw.S', please > use '/*' instead > #168: FILE: drivers/watchdog/rti_wdt_fw.S:1: > +// SPDX-License-Identifier: GPL-2.0+ > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > #168: FILE: drivers/watchdog/rti_wdt_fw.S:1: > +// SPDX-License-Identifier: GPL-2.0+ > > total: 0 errors, 9 warnings, 0 checks, 139 lines checked > > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inplace. > > siemens/0004-watchdog-rti_wdt-Add-support-for-loading-firmware.patch has style > problems, please review. > ----------------------------------------------------------------------- > siemens/0005-configs-iot2050-Enable-watchdog-support-but-do-not-a.patch > ----------------------------------------------------------------------- Since I just pointed out some checkpatch problems to Lokesh in his last PR, I should note that out of all of this list, I only really care about the SPDX one. There are plenty of cases where: #ifdef CONFIG_FOO ... #endif is more readable / clear than: if (IS_ENABLED(CONFIG_FOO)) { ... } Warnings are warning and can be ignored for good reason, errors cannot. -- Tom