| 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: <da859ac9-8f87-43b1-88ba-0357fa9b7be3@linaro.org>
Date: Mon, 19 Feb 2024 09:21:24 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: "Peng Fan (OSS)" <peng.fan@....nxp.com>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Li Yang <leoyang.li@....com>, Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, NXP Linux Team <linux-imx@....com>
Cc: devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, Peng Fan <peng.fan@....com>
Subject: Re: [PATCH 2/3] arm64: dts: freescale: add i.MX95 basic dtsi
On 18/02/2024 07:38, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@....com>
>
> i.MX95 features 6 A55 Cores, ARM Mali GPU, ISP, ML acceleration NPU,
> and Edgelock secure enclave security. This patch is to add a minimal
> dtsi, with cpu cores, coresight, scmi, gic, uart, mu, sdhc, lpi2c added.
>
> Signed-off-by: Peng Fan <peng.fan@....com>
> ---
> arch/arm64/boot/dts/freescale/imx95-clock.h | 187 +++++
> arch/arm64/boot/dts/freescale/imx95-power.h | 55 ++
> arch/arm64/boot/dts/freescale/imx95.dtsi | 1215 +++++++++++++++++++++++++++
> 3 files changed, 1457 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx95-clock.h b/arch/arm64/boot/dts/freescale/imx95-clock.h
> new file mode 100644
> index 000000000000..939655f0414e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-clock.h
What is this?
> @@ -0,0 +1,187 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#ifndef __DT_BINDINGS_CLOCK_IMX95_H
NAK, bindings do not go to DTS!
> +#endif /* __DT_BINDINGS_CLOCK_IMX95_H */
> diff --git a/arch/arm64/boot/dts/freescale/imx95-power.h b/arch/arm64/boot/dts/freescale/imx95-power.h
> new file mode 100644
> index 000000000000..17be8a4cb96b
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95-power.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#ifndef __DT_BINDINGS_IMX95_POWER_H__
> +#define __DT_BINDINGS_IMX95_POWER_H__
NAK, bindings do not go to DTS.
> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> new file mode 100644
> index 000000000000..5b5b29a9aff5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> @@ -0,0 +1,1215 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
Weird license. We do not want GPL v3 or v4.
> +/*
> + * Copyright 2024 NXP
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/thermal/thermal.h>
> +
> +#include "imx95-clock.h"
> +#include "imx95-power.h"
> +
> +/ {
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + aliases {
> + gpio0 = &gpio1;
> + gpio1 = &gpio2;
> + gpio2 = &gpio3;
> + gpio3 = &gpio4;
> + gpio4 = &gpio5;
> + i2c0 = &lpi2c1;
> + i2c1 = &lpi2c2;
> + i2c2 = &lpi2c3;
> + i2c3 = &lpi2c4;
> + i2c4 = &lpi2c5;
> + i2c5 = &lpi2c6;
> + i2c6 = &lpi2c7;
> + i2c7 = &lpi2c8;
None of these are properties of the SoC.
> + mmc0 = &usdhc1;
> + mmc1 = &usdhc2;
> + mmc2 = &usdhc3;
> + serial0 = &lpuart1;
> + serial1 = &lpuart2;
> + serial2 = &lpuart3;
> + serial3 = &lpuart4;
> + serial4 = &lpuart5;
> + serial5 = &lpuart6;
> + serial6 = &lpuart7;
> + serial7 = &lpuart8;
Neither these.
> + };
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + idle-states {
> + entry-method = "psci";
> +
> + cpu_pd_wait: cpu-pd-wait {
> + compatible = "arm,idle-state";
> + arm,psci-suspend-param = <0x0010033>;
> + local-timer-stop;
> + entry-latency-us = <10000>;
> + exit-latency-us = <7000>;
> + min-residency-us = <27000>;
> + wakeup-latency-us = <15000>;
> + status = "disabled";
> + };
> + };
> +
> + A55_0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x0>;
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + cpu-idle-states = <&cpu_pd_wait>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l0>;
> + };
> +
> + A55_1: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x100>;
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + cpu-idle-states = <&cpu_pd_wait>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l1>;
> + };
> +
> + A55_2: cpu@200 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x200>;
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + cpu-idle-states = <&cpu_pd_wait>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l2>;
> + };
> +
> + A55_3: cpu@300 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x300>;
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l3>;
> + };
> +
> + A55_4: cpu@400 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x400>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + cpu-idle-states = <&cpu_pd_wait>;
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l4>;
> + };
> +
> + A55_5: cpu@500 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a55";
> + reg = <0x500>;
> + power-domains = <&scmi_devpd IMX95_PERF_A55>;
> + power-domain-names = "perf";
> + enable-method = "psci";
> + #cooling-cells = <2>;
> + cpu-idle-states = <&cpu_pd_wait>;
> + i-cache-size = <32768>;
> + i-cache-line-size = <64>;
> + i-cache-sets = <128>;
> + d-cache-size = <32768>;
> + d-cache-line-size = <64>;
> + d-cache-sets = <128>;
> + next-level-cache = <&l2_cache_l5>;
> + };
> +
> + l2_cache_l0: l2-cache-l0 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l1: l2-cache-l1 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l2: l2-cache-l2 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l3: l2-cache-l3 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l4: l2-cache-l4 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l2_cache_l5: l2-cache-l5 {
> + compatible = "cache";
> + cache-size = <65536>;
> + cache-line-size = <64>;
> + cache-sets = <256>;
> + cache-level = <2>;
> + cache-unified;
> + next-level-cache = <&l3_cache>;
> + };
> +
> + l3_cache: l3-cache {
> + compatible = "cache";
> + cache-size = <524288>;
> + cache-line-size = <64>;
> + cache-sets = <1024>;
> + cache-level = <3>;
> + cache-unified;
> + };
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&A55_0>;
> + };
> +
> + core1 {
> + cpu = <&A55_1>;
> + };
> +
> + core2 {
> + cpu = <&A55_2>;
> + };
> +
> + core3 {
> + cpu = <&A55_3>;
> + };
> +
> + core4 {
> + cpu = <&A55_4>;
> + };
> +
> + core5 {
> + cpu = <&A55_5>;
> + };
> + };
> + };
> + };
> +
> + clk_ext1: clock-ext1 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <133000000>;
> + clock-output-names = "clk_ext1";
> + };
> +
> + dummy: clk-dummy {
Nothing explains why do you add dummy clocks. Dummy is fake, so don't
add fake clocks to DTS.
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <0>;
> + clock-output-names = "dummy";
> + };
> +
> + sai1_mclk: sai-mclk1 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency= <0>;
> + clock-output-names = "sai1_mclk";
> + };
> +
> + sai2_mclk: sai-mclk2 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency= <0>;
> + clock-output-names = "sai2_mclk";
> + };
> +
> + sai3_mclk: sai-mclk3 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency= <0>;
> + clock-output-names = "sai3_mclk";
> + };
> +
> + sai4_mclk: sai-mclk4 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency= <0>;
> + clock-output-names = "sai4_mclk";
> + };
> +
> + sai5_mclk: sai-mclk5 {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency= <0>;
> + clock-output-names = "sai5_mclk";
> + };
> +
> + osc_24m: osc-24m {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24000000>;
> + clock-output-names = "osc_24m";
> + };
> +
> + sram1: sram@...80000 {
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 = "mmio-sram";
> + reg = <0x0 0x204c0000 0x0 0x18000>;
> + };
> +
> + firmware {
> + scmi {
> + compatible = "arm,scmi";
> + mboxes = <&mu2 5 0>, <&mu2 3 0>, <&mu2 3 1>;
> + shmem = <&scmi_buf0>, <&scmi_buf1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + scmi_devpd: protocol@11 {
> + reg = <0x11>;
> + #power-domain-cells = <1>;
> + };
> +
> + scmi_sys_power: protocol@12 {
> + reg = <0x12>;
> + };
> +
> + scmi_perf: protocol@13 {
> + reg = <0x13>;
> + #clock-cells = <1>;
> + #power-domain-cells = <1>;
> + };
> +
> + scmi_clk: protocol@14 {
> + reg = <0x14>;
> + #clock-cells = <1>;
> + };
> +
> + scmi_sensor: protocol@15 {
> + reg = <0x15>;
> + #thermal-sensor-cells = <1>;
> + };
> + };
> + };
> +
> + pmu {
> + compatible = "arm,cortex-a55-pmu";
> + interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_HIGH)>;
> + };
> +
> + thermal-zones {
> + a55 {
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).
> + polling-delay-passive = <250>;
> + polling-delay = <2000>;
> + thermal-sensors = <&scmi_sensor 1>;
> +
> + trips {
> + cpu_alert0: trip0 {
> + temperature = <85000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> +
> + cpu_crit0: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&cpu_alert0>;
> + cooling-device =
> + <&A55_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A55_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A55_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A55_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A55_4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&A55_5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> + };
> + };
> + };
> + };
> +
> + etm0: etm@...40000 {
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).
I see you did not test it at all. No need to review. First fix all the
issues found by tooling.
This applies to all your future submissions as well. You must first run
your patches through tooling before asking people to review. Otherwise
it is huge waste of our time and quite disrespectful to that time...
> + compatible = "arm,coresight-etm4x", "arm,primecell";
> + reg = <0x0 0x40840000 0x0 0x10000>;
> + arm,primecell-periphid = <0xbb95d>;
> + cpu = <&A55_0>;
> + clocks = <&scmi_clk IMX95_CLK_A55PERIPH>;
> + clock-names = "apb_pclk";
> + status = "disabled";
> +
..
> +
> + elemu5: mailbox@...70000 {
> + compatible = "fsl,imx95-mu-ele";
> + reg = <0x0 0x47570000 0x0 0x10000>;
> + interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> + #mbox-cells = <2>;
> + status = "disabled";
> + };
> +
> + v2x_mu: v2x-mu@...50000 {
> + compatible = "fsl,imx95-mu-v2x";
> + reg = <0x0 0x47350000 0x0 0x10000>;
> + interrupts = <GIC_SPI 255 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "tx", "rx";
> + #mbox-cells = <2>;
> + status = "okay";
Why? Drop. This applies everywhere.
Best regards,
Krzysztof
Powered by blists - more mailing lists