| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <cc55daa1-0256-48d1-97a4-0f755fb4951b@linaro.org>
Date: Sun, 17 Mar 2024 15:50:55 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Wadim Mueller <wafgo01@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, NXP Linux Team <linux-imx@....com>,
Chester Lin <chester62515@...il.com>, Andreas Färber
<afaerber@...e.de>, Matthias Brugger <mbrugger@...e.com>,
NXP S32 Linux Team <s32@....com>,
Alexandre Torgue <alexandre.torgue@...s.st.com>,
Jose Abreu <joabreu@...opsys.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd
<sboyd@...nel.org>, Richard Cochran <richardcochran@...il.com>,
Simon Horman <horms@...nel.org>, Andrew Halaney <ahalaney@...hat.com>,
Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
Johannes Zink <j.zink@...gutronix.de>, Shenwei Wang <shenwei.wang@....com>,
"Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
Swee Leong Ching <leong.ching.swee@...el.com>,
Giuseppe Cavallaro <peppe.cavallaro@...com>, netdev@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-stm32@...md-mailman.stormreply.com, linux-clk@...r.kernel.org
Subject: Re: [PATCH 1/3] arm64: dts: S32G3: Introduce device tree for
S32G-VNP-RDB3
On 15/03/2024 23:27, Wadim Mueller wrote:
> This commit adds device tree support for the NXP S32G3-based
> S32G-VNP-RDB3 Board (Vehicle Networking Platform - Reference Design Board) [1].
>
> The S32G3 features an 8-core ARM Cortex-A53 based SoC developed by NXP.
>
> The device tree files are derived from the official NXP downstream Linux tree [2].
>
> This addition encompasses a limited selection of peripherals that are upstream-supported. Apart from the ARM System Modules (GIC, Generic Timer, etc.), the following IPs have been validated:
>
> UART: fsl-linflexuart
> SDHC: fsl-imx-esdhc
> Ethernet: synopsys gmac/stmac
>
> Clock settings for the chip rely on ATF Firmware [3]. Pin control integration into the device tree is pending and currently relies on Firmware/U-Boot settings [4].
>
> These changes were validated using the latest BSP39 Firmware/U-Boot from NXP [5].
>
> The modifications enable booting the official Ubuntu 22.04 from NXP on
> the RDB3 with default settings from the SD card and eMMC.
>
> [1] https://www.nxp.com/design/design-center/designs/s32g3-vehicle-networking-reference-design:S32G-VNP-RDB3
> [2] https://github.com/nxp-auto-linux/linux
> [3] https://github.com/nxp-auto-linux/arm-trusted-firmware
> [4] https://github.com/nxp-auto-linux/u-boot
> [5] https://github.com/nxp-auto-linux/auto_yocto_bsp
>
> Signed-off-by: Wadim Mueller <wafgo01@...il.com>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> arch/arm64/boot/dts/freescale/s32g3.dtsi | 352 ++++++++++++++++++
> .../boot/dts/freescale/s32g399a-rdb3.dts | 57 +++
> .../dt-bindings/clock/nxp,s32-scmi-clock.h | 158 ++++++++
Please run scripts/checkpatch.pl and fix reported warnings. Some
warnings can be ignored, but the code here looks like it needs a fix.
Feel free to get in touch if the warning is not clear.
Bindings are not DTS.
> 4 files changed, 568 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/s32g3.dtsi
> create mode 100644 arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
> create mode 100644 include/dt-bindings/clock/nxp,s32-scmi-clock.h
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 2cb0212b63c6..e701008dbc7b 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -252,3 +252,4 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mp-venice-gw74xx-rpidsi.dtb
> dtb-$(CONFIG_ARCH_S32) += s32g274a-evb.dtb
> dtb-$(CONFIG_ARCH_S32) += s32g274a-rdb2.dtb
> dtb-$(CONFIG_ARCH_S32) += s32v234-evb.dtb
> +dtb-$(CONFIG_ARCH_S32) += s32g399a-rdb3.dtb
> diff --git a/arch/arm64/boot/dts/freescale/s32g3.dtsi b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> new file mode 100644
> index 000000000000..481ddcfd3a6d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g3.dtsi
> @@ -0,0 +1,352 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/clock/nxp,s32-scmi-clock.h>
> +/ {
> + compatible = "nxp,s32g3";
Order your patches correctly. Bindings come before users.
> + interrupt-parent = <&gic>;
> + #address-cells = <0x02>;
> + #size-cells = <0x02>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> +
> + core1 {
> + cpu = <&cpu1>;
> + };
> +
> + core2 {
> + cpu = <&cpu2>;
> + };
> +
> + core3 {
> + cpu = <&cpu3>;
> + };
> + };
> +
> + cluster1 {
> + core0 {
> + cpu = <&cpu4>;
> + };
> +
> + core1 {
> + cpu = <&cpu5>;
> + };
> +
> + core2 {
> + cpu = <&cpu6>;
> + };
> +
> + core3 {
> + cpu = <&cpu7>;
> + };
> + };
> + };
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster0_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x1>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster0_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x2>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster0_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x3>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster0_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu4: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x100>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster1_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu5: cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x101>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster1_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu6: cpu@102 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x102>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster1_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cpu7: cpu@103 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x103>;
> + enable-method = "psci";
> + clocks = <&dfs S32_SCMI_CLK_A53>;
> + next-level-cache = <&cluster1_l2_cache>;
> + #cooling-cells = <2>;
> + };
> +
> + cluster0_l2_cache: l2-cache0 {
l2-cache-0
> + compatible = "cache";
> + status = "okay";
Why do you need it? It's enabled by default.
Anyway it incomplete:
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> + };
> +
> + cluster1_l2_cache: l2-cache1 {
l2-cache-1
> + compatible = "cache";
> + status = "okay";
> + };
> + };
> +
> + pmu {
> + compatible = "arm,cortex-a53-pmu";
> + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>, /* sec-phys */
> + <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>, /* virt */
> + <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>, /* hyp-virt */
> + <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>, /* sec-phys */
> + <GIC_PPI 12 IRQ_TYPE_LEVEL_LOW>; /* phys */
> + always-on;
> + };
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + scmi_tx_buf: shm@...00000 {
> + compatible = "arm,scmi-shmem";
> + reg = <0x0 0xd0000000 0x0 0x80>;
> + no-map;
> + };
> +
> + scmi_rx_buf: shm@...00080 {
> + compatible = "arm,scmi-shmem";
> + reg = <0x0 0xd0000080 0x0 0x80>;
> + no-map;
> + };
> + };
> +
> + firmware {
> + scmi: scmi {
> + compatible = "arm,scmi-smc";
> + mbox-names = "tx", "rx";
> + shmem = <&scmi_tx_buf>, <&scmi_rx_buf>;
> + arm,smc-id = <0xc20000fe>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + status = "okay";
Drop or explain why is it needed.
> + interrupts = <GIC_SPI 300 IRQ_TYPE_EDGE_RISING>;
> + interrupt-names = "p2a_notif";
> +
> + dfs: protocol@13 {
> + reg = <0x13>;
> + #clock-cells = <1>;
> + };
> +
> + clks: protocol@14 {
> + reg = <0x14>;
> + #clock-cells = <1>;
> + };
> +
> + reset: protocol@16 {
> + reg = <0x16>;
> + #reset-cells = <1>;
> + };
> +
> + pinctrl_scmi: protocol@80 {
> + reg = <0x80>;
> + #pinctrl-cells = <2>;
> +
> + status = "disabled";
> + };
> + };
> +
> + psci: psci {
> + compatible = "arm,psci-1.0";
> + method = "smc";
> + };
> + };
> +
> + soc@0 {
> + compatible = "simple-bus";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0 0 0 0x80000000>;
> +
> + uart0: serial@...c8000 {
> + compatible = "nxp,s32g3-linflexuart",
> + "fsl,s32v234-linflexuart";
> + reg = <0x401c8000 0x3000>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };
> +
> + uart1: serial@...cc000 {
> + compatible = "nxp,s32g3-linflexuart",
> + "fsl,s32v234-linflexuart";
> + reg = <0x401cc000 0x3000>;
> + interrupts = <GIC_SPI 83 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };
> +
> + uart2: serial@...bc000 {
> + compatible = "nxp,s32g3-linflexuart",
> + "fsl,s32v234-linflexuart";
> + reg = <0x402bc000 0x3000>;
> + interrupts = <GIC_SPI 84 IRQ_TYPE_EDGE_RISING>;
> + status = "disabled";
> + };
> +
> + gic: interrupt-controller@...00000 {
> + compatible = "arm,gic-v3";
> + #interrupt-cells = <3>;
> + interrupt-controller;
> + reg = <0x50800000 0x10000>,
> + <0x50900000 0x200000>,
> + <0x50400000 0x2000>,
> + <0x50410000 0x2000>,
> + <0x50420000 0x2000>;
> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> + mbi-ranges = <167 16>;
> + };
> +
> + qspi: spi@...34000 {
Keep order by unit address.
> + compatible = "nxp,s32g3-qspi";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x0 0x00000000 0x0 0x20000000>,
> + <0x0 0x40134000 0x0 0x1000>;
> + reg-names = "QuadSPI-memory", "QuadSPI";
> + interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "qspi_en", "qspi";
> + clocks = <&clks S32_SCMI_CLK_QSPI_FLASH1X>,
> + <&clks S32_SCMI_CLK_QSPI_FLASH1X>;
> + spi-max-frequency = <200000000>;
> + spi-num-chipselects = <2>;
> + status = "disabled";
> + };
> +
> + usdhc0: mmc@...f0000 {
> + compatible = "nxp,s32g3-usdhc",
> + "nxp,s32g2-usdhc";
> + reg = <0x402f0000 0x1000>;
> + interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clks S32_SCMI_CLK_USDHC_MODULE>,
> + <&clks S32_SCMI_CLK_USDHC_AHB>,
> + <&clks S32_SCMI_CLK_USDHC_CORE>;
> + clock-names = "ipg", "ahb", "per";
> + status = "disabled";
> + };
> +
> + gmac0: ethernet@...3c000 {
> + status = "disabled";
Random code... sorry, but status does not come first. Put it last and
please read carefully DTS coding style.
> + compatible = "nxp,s32-dwmac";
> + reg = <0x4033c000 0x2000>, /* gmac IP */
> + <0x4007c004 0x4>; /* S32 CTRL_STS reg */
> + interrupt-parent = <&gic>;
> + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + tx-fifo-depth = <20480>;
> + rx-fifo-depth = <20480>;
> + dma-coherent;
> + snps,mtl-rx-config = <&mtl_rx_setup_gmac0>;
> + snps,mtl-tx-config = <&mtl_tx_setup_gmac0>;
> + clocks = <&clks S32_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32_SCMI_CLK_GMAC0_AXI>,
> + <&clks S32_SCMI_CLK_GMAC0_TX_SGMII>,
> + <&clks S32_SCMI_CLK_GMAC0_TX_RGMII>,
> + <&clks S32_SCMI_CLK_GMAC0_TX_RMII>,
> + <&clks S32_SCMI_CLK_GMAC0_TX_MII>,
> + <&clks S32_SCMI_CLK_GMAC0_RX_SGMII>,
> + <&clks S32_SCMI_CLK_GMAC0_RX_RGMII>,
> + <&clks S32_SCMI_CLK_GMAC0_RX_RMII>,
> + <&clks S32_SCMI_CLK_GMAC0_RX_MII>,
> + <&clks S32_SCMI_CLK_GMAC0_TS>;
> + clock-names = "stmmaceth", "pclk",
> + "tx_sgmii", "tx_rgmii",
> + "tx_rmii", "tx_mii",
> + "rx_sgmii", "rx_rgmii",
> + "rx_rmii", "rx_mii",
> + "ptp_ref";
> +
> + mtl_rx_setup_gmac0: rx-queues-config {
> + snps,rx-queues-to-use = <5>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + queue@0 {};
> + queue@1 {};
> + queue@2 {};
> + queue@3 {};
> + queue@4 {};
> + };
> +
> + mtl_tx_setup_gmac0: tx-queues-config {
> + snps,tx-queues-to-use = <5>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + queue@0 {};
> + queue@1 {};
> + queue@2 {};
> + queue@3 {};
> + queue@4 {};
> + };
> +
> + gmac0_mdio: mdio0 {
mdio?
It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
> new file mode 100644
> index 000000000000..707b503c0165
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/s32g399a-rdb3.dts
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)
> + */
> +
> +/dts-v1/;
> +
> +#include "s32g3.dtsi"
> +
> +/ {
> + model = "NXP S32G3 Reference Design Board 3 (S32G-VNP-RDB3)";
> + compatible = "nxp,s32g399a-rdb3", "nxp,s32g3";
Missing bindings.
Best regards,
Krzysztof
Powered by blists - more mailing lists