From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E13B3C48BD1 for ; Fri, 11 Jun 2021 14:53:54 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B3902611CD for ; Fri, 11 Jun 2021 14:53:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B3902611CD Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 933AA8031F; Fri, 11 Jun 2021 16:53:51 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="IZ0QBIzx"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 5EE3C801DE; Fri, 11 Jun 2021 16:53:49 +0200 (CEST) Received: from mail-qk1-x72d.google.com (mail-qk1-x72d.google.com [IPv6:2607:f8b0:4864:20::72d]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 1BA69801DE for ; Fri, 11 Jun 2021 16:53:46 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x72d.google.com with SMTP id j62so17410366qke.10 for ; Fri, 11 Jun 2021 07:53:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=SFHgU9QswRvjRU5XW0MOj7cE1OJvNuFsI90Hcku4Zew=; b=IZ0QBIzxRyQ5it1pcFUQdjpwS3xwO792PzsdtpybmMk7Ivk8N6Y9xjcLA74ij+17Nt 4WO/qptW9Puo0PdjMFyUHSNLgqQsbJzHyOwiLJWGERH7KVL8eL9YHQGFSdTMX/la59aZ TJdWkqPSdJhexasnr5tJMeSnu/sRDtGPFA/Dg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=SFHgU9QswRvjRU5XW0MOj7cE1OJvNuFsI90Hcku4Zew=; b=oop4ONQicmmkMNZ0C0GxpQi3i55abwJQBmCUaUkIjM72xt/6PeWi4b4HvaSQp4vp/L 6gomt3MJmztpu24E85rDmdsQ1HQYn2fdpM5shx+3tbh1iXPWbLmZJlmpE5XIxkutvf7g MimYl84G9IRqfjXy/gTIp9X8tUSJjiQpM4QzbhSusU6C5gc3pf6d14o7ktanaeZdeyS6 9b0d7/2sCXbv8fqgc6ZQbOd6LxbeiP39WTb/bOaDA9OoL0DFxhzWR8n3+mkMtBpE65ET zJtYVyxqahoY5PGtcvaccKqRsBFaP8bK+h/+C34lp9Xdtyj3ZTxoAfD3bBqEyeTnIo+q waSQ== X-Gm-Message-State: AOAM531q71dFab/V6Ndj1U5qhLRcIvcgQteG0EnUQfOI2h7/VRKthFhr txlmWV+oZPUPcNZOYvHRLZvJ3A== X-Google-Smtp-Source: ABdhPJxMJfMEG3YeWURogU1vkvuNhlGqflSEYQviQCla3460VsL0WMhxZ2ocAnju/DG+gomxZTy40Q== X-Received: by 2002:a37:642:: with SMTP id 63mr4086259qkg.400.1623423224805; Fri, 11 Jun 2021 07:53:44 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-a919-72df-8fd4-9925.res6.spectrum.com. [2603:6081:7b01:cbda:a919:72df:8fd4:9925]) by smtp.gmail.com with ESMTPSA id s133sm4543439qke.97.2021.06.11.07.53.43 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Fri, 11 Jun 2021 07:53:44 -0700 (PDT) Date: Fri, 11 Jun 2021 10:53:41 -0400 From: Tom Rini To: Lokesh Vutla Cc: Jan Kiszka , U-Boot Mailing List , Le Jin , Bao Cheng Su , Nian Gao , Chao Zeng Subject: Re: [PATCH v2 0/5] Add SIMATIC IOT2050 board support Message-ID: <20210611145341.GX9516@bill-the-cat> References: <989cfad9-769d-01dc-ef36-4092da8b0683@ti.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="7aepRUwN+BJ3ry8D" Content-Disposition: inline In-Reply-To: <989cfad9-769d-01dc-ef36-4092da8b0683@ti.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean --7aepRUwN+BJ3ry8D Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jun 11, 2021 at 08:00:17PM +0530, Lokesh Vutla wrote: >=20 >=20 > On 02/06/21 3:07 pm, Jan Kiszka wrote: > > This is the baseline support for the SIMATIC IOT2050 devices. > >=20 > > Changes in v2: > > - rebased > > - sync with upstream-accepted DT > > - add boot switch > > - include watchdog support > >=20 > > 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. > >=20 > > 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. > >=20 > > A staging tree for complete IOT2050 support can be found at [1]. Full > > image integration is available via [2]. > >=20 > > 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. >=20 > There are multiple checkpatch issues with this series. Can you fix them w= here > ever possible? >=20 > =E2=9E=9C u-boot-ti git:(for-next) ./scripts/checkpatch.pl --strict siem= ens/*.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 >=20 > WARNING: line length of 102 exceeds 100 columns > #103: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:49: > + filename =3D "arch/arm/dts/k3-am6528-iot2050-basic.dtb"; >=20 > WARNING: line length of 105 exceeds 100 columns > #113: FILE: arch/arm/dts/k3-am65-iot2050-boot-image.dtsi:59: > + filename =3D "arch/arm/dts/k3-am6548-iot2050-advanced.dtb"; >=20 > total: 0 errors, 3 warnings, 0 checks, 1025 lines checked >=20 > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inpl= ace. >=20 > siemens/0001-arm-dts-Add-IOT2050-device-tree-files.patch has style proble= ms, > 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 >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #282: FILE: board/siemens/iot2050/board.c:86: > +#ifdef CONFIG_NET >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #338: FILE: board/siemens/iot2050/board.c:142: > +#ifdef CONFIG_SPL_LOAD_FIT >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #361: FILE: board/siemens/iot2050/board.c:165: > +#ifdef CONFIG_IOT2050_BOOT_SWITCH >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #404: FILE: board/siemens/iot2050/board.c:208: > +#ifdef CONFIG_IOT2050_BOOT_SWITCH >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #413: FILE: board/siemens/iot2050/board.c:217: > +#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP) >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #458: FILE: board/siemens/iot2050/board.c:262: > +#if CONFIG_IS_ENABLED(LED) >=20 > 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) >=20 > total: 0 errors, 7 warnings, 1 checks, 606 lines checked >=20 > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inpl= ace. >=20 > siemens/0002-board-siemens-Add-support-for-SIMATIC-IOT2050-device.patch h= as > 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 >=20 > 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[]; >=20 > WARNING: externs should be avoided in .c files > #96: FILE: drivers/watchdog/rti_wdt.c:48: > +extern const int rti_wdt_fw_size; >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #100: FILE: drivers/watchdog/rti_wdt.c:52: > +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW >=20 > WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' wher= e possible > #113: FILE: drivers/watchdog/rti_wdt.c:64: > +#ifdef CONFIG_WDT_K3_RTI_LOAD_FW >=20 > WARNING: labels should not be indented > #116: FILE: drivers/watchdog/rti_wdt.c:67: > + dt_error: >=20 > WARNING: labels should not be indented > #137: FILE: drivers/watchdog/rti_wdt.c:88: > + fw_error: >=20 > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #163: > new file mode 100644 >=20 > 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+ >=20 > 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+ >=20 > total: 0 errors, 9 warnings, 0 checks, 139 lines checked >=20 > NOTE: For some of the reported defects, checkpatch may be able to > mechanically convert to the typical style using --fix or --fix-inpl= ace. >=20 > 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 =2E.. #endif is more readable / clear than: if (IS_ENABLED(CONFIG_FOO)) { ... } Warnings are warning and can be ignored for good reason, errors cannot. --=20 Tom --7aepRUwN+BJ3ry8D Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmDDePIACgkQFHw5/5Y0 tyyzpQv+OcZje/bP3cNmpeYQxh76PU1/sxDd5yOJD1elAsH5/Bvm0IS7pv2dxpU2 TZvyeKB0+pzBUZN5hLlGXKnK/Ezs4FbIzS5kE3YVUTvS210m0w5xNi7m0tPGriz2 yyCESfC6D7uYHPRZv0EdeZ+zdzuW42/vN9uaqgobJBEK+L6vgWQj7FWVjITMkUCs qPB3VkvcRdNJRLCU434GRbEUIxKkcjwfz/fNEg5KTlcMnpE3HpgqnSTrGPbkeQcD uyPzWvLsWC0xXuJu+5l8NclvFq0zxYtxATT/j3z9DS7gbDa0erXN14CHWWbAxCmx HomBeQBYw3IcFZf6QzgMdpOT/Yba5HIIVTtPievRlyE67zbn58iWMrFHzUr3WggD FS5B4knbYhYqEGN355XI9VANGcqb3jgJPJO/WolFvnyxqw4O6c1yYdCEGJmQ5MSb tjuQeuz5n2+TDkCDeUA3xntSIgFVgbQfk/QM9aRbzCg6tGF5nGddI1cK0qAJFWKa L5cc1BCS =hPnH -----END PGP SIGNATURE----- --7aepRUwN+BJ3ry8D--