| 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
| ||
|
Date: Tue, 19 Jul 2022 12:01:34 +0200
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Martyn Welch <martyn.welch@...labora.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH] arm64: dts: Add device trees for MSC SM2S-IMX8PLUS SoM
and carrier board
On 18/07/2022 17:23, Martyn Welch wrote:
> Add device trees for one of a number of MSCs variants of the SM2S-IMX8PLUS
> system on module along with the compatible SM2S-SK-AL-EP1 carrier board.
> As the name suggests, this family of SoMs use the NXP i.MX8MP SoC and
> provide the SMARC module interface.
>
> Signed-off-by: Martyn Welch <martyn.welch@...labora.com>
Use subject prefix matching subsystem. I expect other folks in Collabora
help you in that, so you do not need our advices for such trivial stuff. :)
>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> .../freescale/imx8mp-msc-sm2s-14N0600E.dts | 69 ++
> .../dts/freescale/imx8mp-msc-sm2s-ep1.dts | 65 ++
> .../boot/dts/freescale/imx8mp-msc-sm2s.dtsi | 869 ++++++++++++++++++
> 4 files changed, 1004 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-14N0600E.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 238a83e5b8c6..2484d646c6da 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -81,6 +81,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-var-som-symphony.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mn-venice-gw7902.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-icore-mx8mp-edimm2.2.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mp-msc-sm2s-ep1.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-phyboard-pollux-rdk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-venice-gw74xx.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-verdin-nonwifi-dahlia.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-14N0600E.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-14N0600E.dts
> new file mode 100644
> index 000000000000..fb19f2da5087
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-14N0600E.dts
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Avnet Embedded GmbH
> + */
> +
> +#include "imx8mp-msc-sm2s.dtsi"
> +
> +/ {
> + model = "MSC SM2S-IMX8PLUS-QC6-14N0600E SoM";
Missing compatibles, missing bindings.
> +
> + memory@...00000 {
> + device_type = "memory";
> + reg = <0x0 0x40000000 0 0x80000000>; /* bank0, 2GiB */
> + };
> +};
> +
> +&cpu_alert0 {
> + temperature = <95000>;
> +};
> +
> +&cpu_crit0 {
> + temperature = <105000>;
> +};
> +
> +&soc_alert0 {
> + temperature = <95000>;
> +};
> +
> +&soc_crit0 {
> + temperature = <105000>;
> +};
> +
> +&tca6424 {
> + gbe0_int {
> + gpio-hog;
> + input;
> + gpios = <3 GPIO_ACTIVE_LOW>;
> + };
> +
> + gbe1_int {
> + gpio-hog;
> + input;
> + gpios = <4 GPIO_ACTIVE_LOW>;
> + };
> +
> + cam2_rst {
> + gpio-hog;
> + output-high;
> + gpios = <9 GPIO_ACTIVE_LOW>;
> + };
> +
> + cam2_pwr {
> + gpio-hog;
> + output-high;
> + gpios = <10 GPIO_ACTIVE_LOW>;
> + };
> +
> + tpm_int {
> + gpio-hog;
> + input;
> + gpios = <13 GPIO_ACTIVE_LOW>;
> + };
> +
> + wifi_int {
> + gpio-hog;
> + input;
> + gpios = <14 GPIO_ACTIVE_LOW>;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> new file mode 100644
> index 000000000000..c915e2370733
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s-ep1.dts
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Avnet Embedded GmbH
> + */
> +
> +/dts-v1/;
> +
> +#include "imx8mp-msc-sm2s-14N0600E.dts"
> +#include <dt-bindings/clock/imx8mp-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +
> +/ {
> + model = "MSC SM2-MB-EP1 Carrier Board with SM2S-IMX8PLUS-QC6-14N0600E SoM";
Missing compatibles and bindings.
> +};
> +
> +&flexcan1 {
> + status = "okay";
> +};
> +
> +&flexcan2 {
> + status = "okay";
> +};
> +
> +&usdhc2 {
> + no-1-8-v;
> +};
> +
> +&iomuxc {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_smarc_gpio>;
> +
> + pinctrl_smarc_gpio: smarcgpiosgrp {
> + fsl,pins = <
> + /* GPIO0 */
> + MX8MP_IOMUXC_GPIO1_IO11__GPIO1_IO11 0x19
> + /* GPIO1 */
> + MX8MP_IOMUXC_SAI1_RXC__GPIO4_IO01 0x19
> + /* GPIO2 */
> + MX8MP_IOMUXC_SAI1_RXD0__GPIO4_IO02 0x19
> + /* GPIO3 */
> + MX8MP_IOMUXC_SAI1_RXD1__GPIO4_IO03 0x19
> + /* GPIO4 */
> + MX8MP_IOMUXC_SAI3_RXC__GPIO4_IO29 0x19
> + /* GPIO5 */
> + MX8MP_IOMUXC_SAI3_MCLK__GPIO5_IO02 0x19
> + /* GPIO6 */
> + MX8MP_IOMUXC_SAI1_TXD6__GPIO4_IO18 0x19
> + /* GPIO7 */
> + MX8MP_IOMUXC_GPIO1_IO10__GPIO1_IO10 0x19
> + /* GPIO8 */
> + MX8MP_IOMUXC_SAI1_MCLK__GPIO4_IO20 0x19
> + /* GPIO9 */
> + MX8MP_IOMUXC_SAI2_RXFS__GPIO4_IO21 0x19
> + /* GPIO10 */
> + MX8MP_IOMUXC_SAI2_RXC__GPIO4_IO22 0x19
> + /* GPIO11 */
> + MX8MP_IOMUXC_SAI3_RXFS__GPIO4_IO28 0x19
> + /* GPIO12 */
> + MX8MP_IOMUXC_SAI1_TXD7__GPIO4_IO19 0x19
> + /* GPIO13 */
> + MX8MP_IOMUXC_SAI1_RXFS__GPIO4_IO00 0x19
> + >;
> + };
> +};
> +
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
> new file mode 100644
> index 000000000000..7d958cfb4d55
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
> @@ -0,0 +1,869 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2022 Avnet Embedded GmbH
> + */
> +
> +/dts-v1/;
> +
> +#include "imx8mp.dtsi"
> +#include <dt-bindings/net/ti-dp83867.h>
> +
> +/ {
> + model = "MSC SM2S-IMX8MPLUS";
Ditto
> +
> + aliases {
> + rtc0 = &sys_rtc;
> + rtc1 = &snvs_rtc;
> + };
> +
> + chosen {
> + stdout-path = &uart2;
> + };
> +
> + reg_usb0_host_vbus: regulator-usb0-vbus {
> + compatible = "regulator-fixed";
> + regulator-name = "usb0_host_vbus";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usb0_vbus>;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio1 12 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reg_usb1_host_vbus: regulator-usb1-vbus {
> + compatible = "regulator-fixed";
> + regulator-name = "usb1_host_vbus";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usb1_vbus>;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reg_usdhc2_vmmc: regulator-usdhc2 {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usdhc2_vmmc>;
> + regulator-name = "VSD_3V3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio2 19 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + startup-delay-us = <100>;
> + off-on-delay-us = <12000>;
> + };
> +
> + reg_flexcan1_xceiver: regulator-flexcan1-xceiver {
> + compatible = "regulator-fixed";
> + regulator-name = "flexcan1-xceiver";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + enable-active-high;
> + };
> +
> + reg_flexcan2_xceiver: regulator-flexcan2-xceiver {
> + compatible = "regulator-fixed";
> + regulator-name = "flexcan2-xceiver";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + enable-active-high;
> + };
> +
> + lcd0_backlight: lcd0_backlight {
No underscores in node names. Generic node names so backlight-0.
> + compatible = "pwm-backlight";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd0_backlight>;
> + pwms = <&pwm1 0 100000 0>;
> + brightness-levels = <0 255>;
> + num-interpolated-steps = <255>;
> + default-brightness-level = <255>;
> + enable-gpios = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> + status = "disabled";
> + };
> +
> + lcd1_backlight: lcd1_backlight {
> + compatible = "pwm-backlight";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lcd1_backlight>;
> + pwms = <&pwm2 0 100000 0>;
> + brightness-levels = <0 255>;
> + num-interpolated-steps = <255>;
> + default-brightness-level = <255>;
> + enable-gpios = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> + status = "disabled";
> + };
> +
> + leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_leds>;
> + status = "okay";
> +
> + sw-led {
> + label = "sw-led";
> + gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
> + default-state = "off";
> + linux,default-trigger = "heartbeat";
> + };
> + };
> +
> + extcon_usb0: extcon_usb0 {
No underscores, extcon is Linux term, so use node name describing device.
> + compatible = "linux,extcon-usb-gpio";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_usb0_extcon>;
> + id-gpio = <&gpio1 3 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +&A53_0 {
> + cpu-supply = <&vcc_arm>;
> +};
> +
> +&A53_1 {
> + cpu-supply = <&vcc_arm>;
> +};
> +
> +&A53_2 {
> + cpu-supply = <&vcc_arm>;
> +};
> +
> +&A53_3 {
> + cpu-supply = <&vcc_arm>;
> +};
> +
> +&dsp {
> + status = "okay";
> +};
> +
> +&ecspi1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ecspi1>;
> + cs-gpios = <0>, <&gpio2 8 GPIO_ACTIVE_LOW>;
> +};
> +
> +&ecspi2 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ecspi2>;
> + cs-gpios = <0>, <&gpio2 9 GPIO_ACTIVE_LOW>;
> +};
> +
> +&eqos {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_eqos>;
> + phy-mode = "rgmii-id";
> + phy-handle = <ðphy0>;
> + phy-reset-gpios = <&tca6424 16 GPIO_ACTIVE_LOW>;
> + phy-reset-duration = <1>;
> + phy-reset-post-delay = <1>;
> + status = "okay";
> +
> + mdio {
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy0: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + eee-broken-1000t;
> + phy-reset-gpios = <&tca6424 16 GPIO_ACTIVE_LOW>;
> +
> + ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> + ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> + };
> + };
> +};
> +
> +&fec {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_fec>;
> + phy-mode = "rgmii-id";
> + phy-handle = <ðphy1>;
> + phy-reset-gpios = <&tca6424 17 GPIO_ACTIVE_LOW>;
> + phy-reset-duration = <1>;
> + phy-reset-post-delay = <1>;
> + fsl,magic-packet;
> + status = "okay";
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy1: ethernet-phy@1 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <1>;
> + eee-broken-1000t;
> + phy-reset-gpios = <&tca6424 17 GPIO_ACTIVE_LOW>;
> +
> + ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,tx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
> + ti,clk-output-sel = <DP83867_CLK_O_SEL_OFF>;
> + };
> + };
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c1>;
> + clock-frequency = <400000>;
> + status = "okay";
> +
> + id_eeprom: eeprom@50 {
> + compatible = "atmel,24c64";
> + reg = <0x50>;
> + pagesize = <32>;
> + };
> +};
> +
> +&i2c2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c2>;
> + clock-frequency = <400000>;
> + status = "disabled";
> +};
> +
> +&i2c3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c3>;
> + clock-frequency = <400000>;
> + status = "disabled";
> +};
> +
> +&i2c4 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c4>;
> + clock-frequency = <400000>;
> + status = "disabled";
> +};
> +
> +&i2c5 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c5>;
> + clock-frequency = <400000>;
> + status = "disabled";
> +};
> +
> +&i2c6 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c6>;
> + clock-frequency = <400000>;
> + status = "okay";
> +
> + tca6424: gpio@22 {
> + compatible = "ti,tca6424";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tca6424>;
> + reg = <0x22>;
compatible, then reg, then all other properties. This applies everywhere.
> + gpio-controller;
> + #gpio-cells = <2>;
> + gpio-line-names = "BOOT_SEL0#", "BOOT_SEL1#", "BOOT_SEL2#",
> + "gbe0_int", "gbe1_int", "pmic_int", "rtc_int", "lvds_int",
> + "PCIE_WAKE#", "cam2_rst", "cam2_pwr", "SLEEP#",
> + "wifi_pd", "tpm_int", "wifi_int", "PCIE_A_RST#",
> + "gbe0_rst", "gbe1_rst", "LID#", "BATLOW#", "CHARGING#",
> + "CHARGER_PRSNT#";
> + interrupt-parent = <&gpio1>;
> + interrupts = <9 IRQ_TYPE_EDGE_RISING>;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> + };
> +
> + dsi_lvds_bridge: sn65dsi84@2d {
Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,sn65dsi83";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_lvds_bridge>;
> + reg = <0x2d>;
> + enable-gpios = <&gpio1 7 GPIO_ACTIVE_HIGH>;
> + status = "disabled";
> + };
> +
> + pmic: pmic@30 {
> + compatible = "ricoh,rn5t567";
> + interrupt-parent = <&tca6424>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> + reg = <0x30>;
compatible, reg then the rest.
> +
> + regulators {
> + DCDC1 {
> + regulator-name = "VCC_SOC";
> + regulator-always-on;
> + regulator-min-microvolt = <950000>;
> + regulator-max-microvolt = <950000>;
> + };
> + DCDC2 {
> + regulator-name = "VCC_DRAM";
> + regulator-always-on;
> + regulator-min-microvolt = <1100000>;
> + regulator-max-microvolt = <1100000>;
> + };
> + vcc_arm: DCDC3 {
> + regulator-name = "VCC_ARM";
> + regulator-always-on;
> + regulator-min-microvolt = <950000>;
> + regulator-max-microvolt = <950000>;
> + };
> + DCDC4 {
> + regulator-name = "VCC_1V8";
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> + LDO1 {
> + regulator-name = "VCC_LDO1_2V5";
> + regulator-always-on;
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + };
> + LDO2 {
> + regulator-name = "VCC_LDO2_1V8";
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> + LDO3 {
> + regulator-name = "VCC_ETH_2V5";
> + regulator-always-on;
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + };
> + LDO4 {
> + regulator-name = "VCC_DDR4_2V5";
> + regulator-always-on;
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + };
> + LDO5 {
> + regulator-name = "VCC_LDO5_1V8";
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> + LDORTC1 {
> + regulator-name = "VCC_SNVS_1V8";
> + regulator-always-on;
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> + LDORTC2 {
> + regulator-name = "VCC_SNVS_3V3";
> + regulator-always-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> + };
> + };
> +
> + sys_rtc: rtc@32 {
> + compatible = "ricoh,r2221tl";
> + reg = <0x32>;
> + interrupt-parent = <&tca6424>;
> + interrupts = <6 IRQ_TYPE_EDGE_FALLING>;
> + };
> +
> + tmp_sensor: tmp103@71 {
Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,tmp103";
> + reg = <0x71>;
> + label = "BOARD_TEMP";
> + };
> +};
> +
> +&flexcan1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_flexcan1>;
> + xceiver-supply = <®_flexcan1_xceiver>;
> + status = "disabled";
> +};
> +
> +&flexcan2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_flexcan2>;
> + xceiver-supply = <®_flexcan2_xceiver>;
> + status = "disabled";
> +};
> +
> +&flexspi {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_flexspi0>;
> + status = "okay";
> +
> + qspi_flash: qspi_flash@0 {
You didn't test the bindings (dtbs_check), did you? There is no way this
passess...
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "jedec,spi-nor";
Eh? Now compatible in the middle? Sorry, this are trivial code quality
comments. Please use internal review or base your work on some
upstreamed existing board.
Best regards,
Krzysztof
Powered by blists - more mailing lists