On 16/07/2022 01:12, Shenwei Wang wrote: > i.MX8DXL is a device targeting the automotive and industrial market > segments. The chip is designed to achieve both high performance and > low power consumption. It has a dual (2x) Cortex-A35 processor. > > This patch adds the imx8dxl soc and EVK board support. I saw this patch and I was already commenting it: https://lore.kernel.org/all/20220404134609.2676793-2-abel.vesa@xxxxxxx/ Doing things twice, reviewing twice is waste of time. I actually spotted this duplication after I perform a review, but this patch should be abandoned and Abel's patches should rather go. > (...) > + > +/ { > + model = "Freescale i.MX8DXL EVK"; > + compatible = "fsl,imx8dxl-evk", "fsl,imx8dxl"; > + > + chosen { > + stdout-path = &lpuart0; > + }; > + > + memory@80000000 { > + device_type = "memory"; > + reg = <0x00000000 0x80000000 0 0x40000000>; > + }; > + > + reserved-memory { > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + /* > + *Memory reserved for optee usage. Please do not use. > + *This will be automaticky added to dtb if OP-TEE is installed. > + *optee@96000000 { > + * reg = <0 0x96000000 0 0x2000000>; > + * no-map; > + *}; > + */ Now it is not a proper Linux coding style comment. See Coding Style. > + > + /* global autoconfigured region for contiguous allocations */ > + linux,cma { > + compatible = "shared-dma-pool"; > + reusable; > + size = <0 0x14000000>; > + alloc-ranges = <0 0x98000000 0 0x14000000>; > + linux,cma-default; > + }; > + }; > + > + mux3_en: regulator-0 { > + compatible = "regulator-fixed"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + regulator-name = "mux3_en"; > + gpio = <&pca6416_2 8 GPIO_ACTIVE_LOW>; > + regulator-always-on; > + }; > + > + reg_fec1_sel: regulator-1 { > + compatible = "regulator-fixed"; > + regulator-name = "fec1_supply"; > + regulator-min-microvolt = <3300000>; > + regulator-max-microvolt = <3300000>; > + gpio = <&pca6416_1 11 GPIO_ACTIVE_HIGH>; > + regulator-always-on; > + status = "disabled"; > + }; > + > + reg_fec1_io: regulator-2 { > + compatible = "regulator-fixed"; > + regulator-name = "fec1_io_supply"; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + gpio = <&max7322 0 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + regulator-always-on; > + status = "disabled"; > + }; > + > + reg_usdhc2_vmmc: regulator-3 { > + compatible = "regulator-fixed"; > + regulator-name = "SD1_SPWR"; > + regulator-min-microvolt = <3000000>; > + regulator-max-microvolt = <3000000>; > + gpio = <&lsio_gpio4 30 GPIO_ACTIVE_HIGH>; > + enable-active-high; > + off-on-delay-us = <3480>; > + }; > +}; > + > +&eqos { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_eqos>; > + phy-mode = "rgmii-id"; > + phy-handle = <ðphy0>; > + nvmem-cells = <&fec_mac1>; > + nvmem-cell-names = "mac-address"; > + snps,reset-gpios = <&pca6416_1 2 GPIO_ACTIVE_LOW>; > + snps,reset-delays-us = <10 20 200000>; > + status = "okay"; > + > + mdio { > + compatible = "snps,dwmac-mdio"; > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethphy0: ethernet-phy@0 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <0>; > + eee-broken-1000t; > + qca,disable-smarteee; > + vddio-supply = <&vddio0>; > + > + vddio0: vddio-regulator { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + }; > + }; > + }; > +}; > + > +&fec1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_fec1>; > + phy-mode = "rgmii-txid"; > + phy-handle = <ðphy1>; > + fsl,magic-packet; > + rx-internal-delay-ps = <2000>; > + nvmem-cells = <&fec_mac0>; > + nvmem-cell-names = "mac-address"; > + phy-reset-gpios = <&pca6416_1 0 GPIO_ACTIVE_LOW>; > + phy-reset-duration = <10>; > + phy-reset-post-delay = <150>; > + status = "disabled"; > + > + mdio { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethphy1: ethernet-phy@1 { > + compatible = "ethernet-phy-ieee802.3-c22"; > + reg = <1>; > + qca,disable-smarteee; > + vddio-supply = <&vddio1>; > + > + vddio1: vddio-regulator { > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + }; > + }; > + }; > +}; > + > +&flexspi0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_flexspi0>; > + nxp,fspi-dll-slvdly = <4>; > + status = "okay"; > + > + mt35xu512aba0: flash@0 { > + compatible = "jedec,spi-nor"; > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <1>; > + spi-max-frequency = <133000000>; > + spi-tx-bus-width = <8>; > + spi-rx-bus-width = <8>; > + }; > +}; > + > +&i2c2 { > + #address-cells = <1>; > + #size-cells = <0>; > + clock-frequency = <100000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c2>; > + status = "okay"; > + > + pca6416_1: gpio@20 { > + compatible = "ti,tca6416"; > + reg = <0x20>; > + gpio-controller; > + #gpio-cells = <2>; > + }; > + > + pca6416_2: gpio@21 { > + compatible = "ti,tca6416"; > + reg = <0x21>; > + gpio-controller; > + #gpio-cells = <2>; > + }; > + > + pca9548_1: gpio@70 { > + compatible = "nxp,pca9548"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x70>; > + > + i2c@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x0>; > + > + max7322: gpio@68 { > + compatible = "maxim,max7322"; > + reg = <0x68>; > + gpio-controller; > + #gpio-cells = <2>; > + status = "disabled"; > + }; > + }; > + > + i2c@4 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x4>; > + }; > + > + i2c@5 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x5>; > + }; > + > + i2c@6 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x6>; > + }; > + }; > +}; > + > +&i2c3 { > + #address-cells = <1>; > + #size-cells = <0>; > + clock-frequency = <100000>; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_i2c3>; > + status = "okay"; > + > + pca6416_3: gpio@20 { > + compatible = "ti,tca6416"; > + reg = <0x20>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-parent = <&lsio_gpio2>; > + interrupts = <5 IRQ_TYPE_EDGE_RISING>; > + }; > + > + pca9548_2: gpio@70 { > + compatible = "nxp,pca9548"; > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x70>; > + > + i2c@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x0>; > + }; > + > + i2c@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x1>; > + }; > + > + i2c@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x2>; > + }; > + > + i2c@3 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x3>; > + }; > + > + i2c@4 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0x4>; > + }; > + }; > +}; > + > +&lpuart0 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_lpuart0>; > + status = "okay"; > +}; > + > +&lsio_gpio4 { > + status = "okay"; > +}; > + > +&lsio_gpio5 { > + status = "okay"; > +}; > + > +&thermal_zones { > + pmic-thermal0 { > + polling-delay-passive = <250>; > + polling-delay = <2000>; > + thermal-sensors = <&tsens IMX_SC_R_PMIC_0>; > + > + trips { > + pmic_alert0: trip0 { > + temperature = <110000>; > + hysteresis = <2000>; > + type = "passive"; > + }; > + pmic_crit0: trip1 { > + temperature = <125000>; > + hysteresis = <2000>; > + type = "critical"; > + }; > + }; > + > + cooling-maps { > + map0 { > + trip = <&pmic_alert0>; > + cooling-device = > + <&A35_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>, > + <&A35_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>; > + }; > + }; > + }; > +}; > + > +&usdhc1 { > + pinctrl-names = "default", "state_100mhz", "state_200mhz"; > + pinctrl-0 = <&pinctrl_usdhc1>; > + pinctrl-1 = <&pinctrl_usdhc1_100mhz>; > + pinctrl-2 = <&pinctrl_usdhc1_200mhz>; > + bus-width = <8>; > + no-sd; > + no-sdio; > + non-removable; > + status = "okay"; > +}; > + > +&usdhc2 { > + pinctrl-names = "default", "state_100mhz", "state_200mhz"; > + pinctrl-0 = <&pinctrl_usdhc2>, <&pinctrl_usdhc2_gpio>; > + pinctrl-1 = <&pinctrl_usdhc2_100mhz>, <&pinctrl_usdhc2_gpio>; > + pinctrl-2 = <&pinctrl_usdhc2_200mhz>, <&pinctrl_usdhc2_gpio>; > + bus-width = <4>; > + vmmc-supply = <®_usdhc2_vmmc>; > + cd-gpios = <&lsio_gpio5 1 GPIO_ACTIVE_LOW>; > + wp-gpios = <&lsio_gpio5 0 GPIO_ACTIVE_HIGH>; > + max-frequency = <100000000>; > + status = "okay"; > +}; > + > +&iomuxc { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_hog>; > + > + pinctrl_hog: hoggrp { > + fsl,pins = < > + IMX8DXL_COMP_CTL_GPIO_1V8_3V3_GPIORHB_PAD 0x000514a0 > + IMX8DXL_COMP_CTL_GPIO_1V8_3V3_GPIORHK_PAD 0x000014a0 > + IMX8DXL_SPI3_CS0_ADMA_ACM_MCLK_OUT1 0x0600004c > + IMX8DXL_SNVS_TAMPER_OUT1_LSIO_GPIO2_IO05_IN 0x0600004c > + >; > + }; > + > + pinctrl_usbotg1: otg1 { All pinctrl groups must end with "grp". Just open the bindings for several iMX pin controllers. I pointed it out and you still did not fix it. > + fsl,pins = < > + IMX8DXL_USB_SS3_TC0_CONN_USB_OTG1_PWR 0x00000021 > + >; > + }; > + > + pinctrl_usbotg2: otg2 { > + fsl,pins = < > + IMX8DXL_USB_SS3_TC1_CONN_USB_OTG2_PWR 0x00000021 > + >; > + }; (...) > diff --git a/arch/arm64/boot/dts/freescale/imx8dxl.dtsi b/arch/arm64/boot/dts/freescale/imx8dxl.dtsi > new file mode 100644 > index 0000000000000..2de56dfa16d6b > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8dxl.dtsi > @@ -0,0 +1,245 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Copyright 2019~2020 NXP > + */ > + > +#include <dt-bindings/clock/imx8-clock.h> > +#include <dt-bindings/firmware/imx/rsrc.h> > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/interrupt-controller/arm-gic.h> > +#include <dt-bindings/input/input.h> > +#include <dt-bindings/pinctrl/pads-imx8dxl.h> > +#include <dt-bindings/thermal/thermal.h> > + > +/ { > + interrupt-parent = <&gic>; > + #address-cells = <2>; > + #size-cells = <2>; > + > + aliases { > + ethernet0 = &fec1; > + ethernet1 = &eqos; > + gpio0 = &lsio_gpio0; > + gpio1 = &lsio_gpio1; > + gpio2 = &lsio_gpio2; > + gpio3 = &lsio_gpio3; > + gpio4 = &lsio_gpio4; > + gpio5 = &lsio_gpio5; > + gpio6 = &lsio_gpio6; > + gpio7 = &lsio_gpio7; > + i2c2 = &i2c2; > + i2c3 = &i2c3; Still not a SoC aliases but rather board. > + mmc0 = &usdhc1; > + mmc1 = &usdhc2; > + mu1 = &lsio_mu1; > + serial0 = &lpuart0; > + serial1 = &lpuart1; > + serial2 = &lpuart2; > + serial3 = &lpuart3; > + }; (...) > + > + xtal32k: clock-xtal32k { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <32768>; > + clock-output-names = "xtal_32KHz"; > + }; > + > + xtal24m: clock-xtal24m { > + compatible = "fixed-clock"; > + #clock-cells = <0>; > + clock-frequency = <24000000>; Didn't you ignore (again) comments? So let me again clarify - either you implement the comment or you keep discussion going. You keep not implementing the comments and you decided not discussing them, so it's still a NAK. Best regards, Krzysztof
