Re: [PATCH v2 0/5] Add SIMATIC IOT2050 board support

[Lokesh Vutla <lokeshvutla@ti.com>]

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