| 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: <e1a2c92a-1d60-4251-9133-5fc238866336@gmx.net>
Date: Fri, 13 Sep 2024 14:36:03 +0200
From: Stefan Wahren <wahrenst@....net>
To: Lukasz Majewski <lukma@...x.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>
Cc: Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>, devicetree@...r.kernel.org,
imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 2/2] dts: nxp: mxs: Add descriptions for imx287 based
btt3-[012] devices
Hi Lukasz,
please adjust the subject of your patch accordingly to the subsystem.
Suggestion
ARM: dts: mxs: Add descriptions for imx287 based btt3-[012] devices
Am 12.09.24 um 14:48 schrieb Lukasz Majewski:
> The btt3 device' HW revisions from 0 to 2 use imx287 SoC and are to
> some extend similar to already upstreamed XEA devices, hence are
> using common imx28-lwe.dtsi file.
>
> New, imx28-btt3.dtsi has been added to embrace common DTS
> properties for different HW revisions for this device.
>
> As a result - changes introduced in imx28-btt3-[012].dts are
> minimal.
>
> Signed-off-by: Lukasz Majewski <lukma@...x.de>
>
> ---
> Changes for v2:
> - Rename dts file from btt3-[012] to imx28-btt3-[012] to match current
> linux kernel naming convention
> - Remove 'wlf,wm8974' from compatible for codec@1a
>
> Changes for v3:
> - Keep alphabethical order for Makefile entries
>
> Changes for v4:
> - Change compatible for btt3 board (to 'lwn,imx28-btt3')
>
> Changes for v5:
> - Combine patch, which adds btt3-[012] with one adding board entry to
> fsl.yaml
>
> Changes for v6:
> - Make the patch series for adding entry in fsl.yaml and btt3
> ---
> arch/arm/boot/dts/nxp/mxs/Makefile | 3 +
> arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts | 12 +
> arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts | 8 +
> arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts | 12 +
> arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi | 320 +++++++++++++++++++++
> 5 files changed, 355 insertions(+)
> create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
> create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
> create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
> create mode 100644 arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi
>
> diff --git a/arch/arm/boot/dts/nxp/mxs/Makefile b/arch/arm/boot/dts/nxp/mxs/Makefile
> index a430d04f9c69..96dd31ea19ba 100644
> --- a/arch/arm/boot/dts/nxp/mxs/Makefile
> +++ b/arch/arm/boot/dts/nxp/mxs/Makefile
> @@ -8,6 +8,9 @@ dtb-$(CONFIG_ARCH_MXS) += \
> imx28-apf28.dtb \
> imx28-apf28dev.dtb \
> imx28-apx4devkit.dtb \
> + imx28-btt3-0.dtb \
> + imx28-btt3-1.dtb \
> + imx28-btt3-2.dtb \
> imx28-cfa10036.dtb \
> imx28-cfa10037.dtb \
> imx28-cfa10049.dtb \
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
> new file mode 100644
> index 000000000000..6ac46e4b21bb
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-0.dts
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright 2024
> + * Lukasz Majewski, DENX Software Engineering, lukma@...x.de
> + */
> +
> +/dts-v1/;
> +#include "imx28-btt3.dtsi"
> +
> +&hog_pins_rev {
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> +};
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
> new file mode 100644
> index 000000000000..213fe931c58b
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-1.dts
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright 2024
> + * Lukasz Majewski, DENX Software Engineering, lukma@...x.de
> + */
> +
> +/dts-v1/;
> +#include "imx28-btt3.dtsi"
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
> new file mode 100644
> index 000000000000..c787c2d03463
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3-2.dts
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright 2024
> + * Lukasz Majewski, DENX Software Engineering, lukma@...x.de
> + */
> +
> +/dts-v1/;
> +#include "imx28-btt3.dtsi"
> +
> +&lcdif {
> + display = <&display_te_b>;
The reason why you don't move the second display into this file is
because you expect a new hardware revision in the future?
> +};
> diff --git a/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi b/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi
> new file mode 100644
> index 000000000000..94a21ea8d5d2
> --- /dev/null
> +++ b/arch/arm/boot/dts/nxp/mxs/imx28-btt3.dtsi
> @@ -0,0 +1,320 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
> +/*
> + * Copyright 2024
> + * Lukasz Majewski, DENX Software Engineering, lukma@...x.de
> + */
> +/dts-v1/;
> +#include "imx28-lwe.dtsi"
> +
> +/ {
> + model = "BTT3";
> +
> + compatible = "lwn,imx28-btt3", "fsl,imx28";
> +
> + chosen {
> + bootargs = "root=/dev/mmcblk0p2 rootfstype=ext4 ro rootwait console=ttyAMA0,115200 panic=1 quiet";
> + };
It's a little bit unusual to place so many Linux specific stuff into the
device tree bootargs.
> +
> + memory@...00000 {
> + reg = <0x40000000 0x10000000>;
> + device_type = "memory";
> + };
> +
> + poweroff {
> + compatible = "gpio-poweroff";
> + gpios = <&gpio0 24 0>;
Please use the GPIO polarity defines.
> + };
> +
> + sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "BTTC Audio";
> + simple-audio-card,widgets = "Speaker", "BTTC Speaker";
> + simple-audio-card,routing = "BTTC Speaker", "SPKOUTN", "BTTC Speaker", "SPKOUTP";
> + simple-audio-card,dai-link@0 {
> + format = "left_j";
> + bitclock-master = <&dai0_master>;
> + frame-master = <&dai0_master>;
> + mclk-fs = <256>;
> + dai0_master: cpu {
> + sound-dai = <&saif0>;
> + };
> + codec {
> + sound-dai = <&wm89xx>;
> + clocks = <&saif0>;
> + };
> + };
> + };
> +
> + wifi_pwrseq: sdio-pwrseq {
> + compatible = "mmc-pwrseq-simple";
> + pinctrl-names = "default";
> + pinctrl-0 = <&wifi_en_pin_bttc>;
> + reset-gpios = <&gpio0 27 GPIO_ACTIVE_LOW>;
> + /* W1-163 needs 60us for WL_EN to be low and */
> + /* 150ms after high before downloading FW is possible */
> + post-power-on-delay-ms = <200>;
> + power-off-delay-us = <100>;
> + };
> +};
> +
> +&auart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&auart0_2pins_a>;
> + status = "okay";
> +};
> +
> +&auart3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&auart3_pins_a>;
> + uart-has-rtscts;
> + status = "okay";
> +};
> +
> +&i2c0 {
> + wm89xx: codec@1a {
> + compatible = "wlf,wm8940";
> + reg = <0x1a>;
> + #sound-dai-cells = <0>;
> + };
> +};
> +
> +&lcdif {
> + pinctrl-names = "default";
> + pinctrl-0 = <&lcdif_24bit_pins_a>, <&lcdif_sync_pins_bttc>,
> + <&lcdif_reset_pins_bttc>;
> + lcd-supply = <®_3v3>;
> + display = <&display0>;
> + status = "okay";
> + display0: display0 {
> + bits-per-pixel = <32>;
> + bus-width = <24>;
> + display-timings {
> + native-mode = <&timing0>;
> + timing0: timing0 {
> + clock-frequency = <6500000>;
> + hactive = <320>;
> + vactive = <240>;
> + hfront-porch = <20>;
> + hback-porch = <38>;
> + hsync-len = <30>;
> + vfront-porch = <4>;
> + vback-porch = <14>;
> + vsync-len = <4>;
> + hsync-active = <0>;
> + vsync-active = <0>;
> + de-active = <0>;
> + pixelclk-active = <1>;
> + };
> + };
> + };
> + display_te_b: display1 {
> + bits-per-pixel = <32>;
> + bus-width = <24>;
> + display-timings {
> + native-mode = <&timing0>;
> + timing_te_b: timing0 {
> + clock-frequency = <6500000>;
> + hactive = <320>;
> + vactive = <240>;
> + hfront-porch = <20>;
> + hback-porch = <68>;
> + hsync-len = <30>;
> + vfront-porch = <4>;
> + vback-porch = <14>;
> + vsync-len = <4>;
> + hsync-active = <0>;
> + vsync-active = <0>;
> + de-active = <1>;
> + pixelclk-active = <1>;
> + };
> + };
> + };
> +
> +};
> +
> +&mac0 {
> + clocks = <&clks 57>, <&clks 57>, <&clks 64>;
> + clock-names = "ipg", "ahb", "enet_out";
> + phy-handle = <&mac0_phy>;
> + phy-mode = "rmii";
> + phy-supply = <®_3v3>;
> + local-mac-address = [ 00 11 B8 00 BF 8A ];
Is this replaced dynamically by the bootloader? Otherwise this suggests
all boards use the same MAC address.
> + status = "okay";
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + mac0_phy: ethernet-phy@0 {
> + /* LAN8720Ai - PHY ID */
> + compatible = "ethernet-phy-id0007.c0f0","ethernet-phy-ieee802.3-c22";
> + reg = <0>;
> + smsc,disable-energy-detect;
> + max-speed = <100>;
> +
> + reset-gpios = <&gpio4 12 GPIO_ACTIVE_LOW>; /* GPIO4_12 */
I think the comment only repeat what is already defined here.
> + reset-assert-us = <1000>;
> + reset-deassert-us = <1000>;
> + };
> + };
> +};
> +
> +&pinctrl {
> + pinctrl-names = "default";
> + pinctrl-0 = <&hog_pins_a>, <&hog_pins_rev>;
> +
> + hog_pins_a: hog@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_GPMI_RDY2__GPIO_0_22
> + MX28_PAD_GPMI_RDY3__GPIO_0_23
> + MX28_PAD_GPMI_RDN__GPIO_0_24
> + MX28_PAD_LCD_VSYNC__GPIO_1_28
> + MX28_PAD_SSP2_SS1__GPIO_2_20
> + MX28_PAD_SSP2_SS2__GPIO_2_21
> + MX28_PAD_AUART2_CTS__GPIO_3_10
> + MX28_PAD_AUART2_RTS__GPIO_3_11
> + MX28_PAD_GPMI_WRN__GPIO_0_25
> + MX28_PAD_ENET0_RXD2__GPIO_4_9
> + MX28_PAD_ENET0_TXD2__GPIO_4_11
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_DISABLE>;
> + };
> +
> + hog_pins_rev: hog@1 {
> + reg = <1>;
> + fsl,pinmux-ids = <
> + MX28_PAD_ENET0_RXD3__GPIO_4_10
> + MX28_PAD_ENET0_TX_CLK__GPIO_4_5
> + MX28_PAD_ENET0_COL__GPIO_4_14
> + MX28_PAD_ENET0_CRS__GPIO_4_15
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_DISABLE>;
> + };
> +
> + keypad_pins_bttc: keypad-bttc@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_GPMI_D00__GPIO_0_0
> + MX28_PAD_AUART0_CTS__GPIO_3_2
> + MX28_PAD_AUART0_RTS__GPIO_3_3
> + MX28_PAD_GPMI_D03__GPIO_0_3
> + MX28_PAD_GPMI_D04__GPIO_0_4
> + MX28_PAD_GPMI_D05__GPIO_0_5
> + MX28_PAD_GPMI_D06__GPIO_0_6
> + MX28_PAD_GPMI_D07__GPIO_0_7
> + MX28_PAD_GPMI_CE1N__GPIO_0_17
> + MX28_PAD_GPMI_CE2N__GPIO_0_18
> + MX28_PAD_GPMI_CE3N__GPIO_0_19
> + MX28_PAD_GPMI_RDY0__GPIO_0_20
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_DISABLE>;
> + };
> +
> + lcdif_sync_pins_bttc: lcdif-bttc@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_LCD_DOTCLK__LCD_DOTCLK
> + MX28_PAD_LCD_ENABLE__LCD_ENABLE
> + MX28_PAD_LCD_HSYNC__LCD_HSYNC
> + MX28_PAD_LCD_RD_E__LCD_VSYNC
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_DISABLE>;
> + };
> +
> + lcdif_reset_pins_bttc: lcdif-bttc@1 {
> + reg = <1>;
> + fsl,pinmux-ids = <
> + MX28_PAD_LCD_RESET__GPIO_3_30
> + >;
> + fsl,drive-strength = <MXS_DRIVE_4mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };
> +
> + ssp1_sdio_pins_a: ssp1-sdio@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_SSP1_DATA0__SSP1_D0
> + MX28_PAD_GPMI_D01__SSP1_D1
> + MX28_PAD_GPMI_D02__SSP1_D2
> + MX28_PAD_SSP1_DATA3__SSP1_D3
> + MX28_PAD_SSP1_CMD__SSP1_CMD
> + MX28_PAD_SSP1_SCK__SSP1_SCK
> + >;
> + fsl,drive-strength = <MXS_DRIVE_8mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };
> +
> + wifi_en_pin_bttc: wifi_en_pin@0 {
This should trigger a schema warning. The node name should use dashes
instead of underscore.
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX28_PAD_GPMI_CLE__GPIO_0_27
> + >;
> + fsl,drive-strength = <MXS_DRIVE_8mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };
> +};
> +
> +&pwm {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pwm3_pins_a>;
> + status = "okay";
> +};
> +
> +®_usb_5v {
> + gpio = <&gpio1 28 0>;
Please use polarity define.
> +};
> +
> +&saif0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&saif0_pins_a>;
> + #sound-dai-cells = <0>;
> + assigned-clocks = <&clks 53>;
> + assigned-clock-rates = <12000000>;
> + status = "okay";
> +};
> +
> +&saif1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&saif1_pins_a>;
> + fsl,saif-master = <&saif0>;
> + #sound-dai-cells = <0>;
> + status = "okay";
> +};
> +
> +&ssp1 {
> + compatible = "fsl,imx28-mmc";
> + pinctrl-names = "default";
> + pinctrl-0 = <&ssp1_sdio_pins_a>;
> + bus-width = <4>;
> + no-1-8-v; /* force 3.3V VIO */
> + pm-ignore-notify;
This seems to be undocumented.
> + non-removable;
> + vmmc-supply = <®_3v3>;
> + mmc-pwrseq = <&wifi_pwrseq>;
> + keep-power-in-suspend;
> + status = "okay";
> +
> + wlan@1 {
> + reg = <1>;
> + compatible = "brcm,bcm4329-fmac";
> + };
> +};
> +
> +&ssp2 {
> + compatible = "fsl,imx28-spi";
> + pinctrl-names = "default";
> + pinctrl-0 = <&spi2_pins_a>;
> + status = "okay";
> +};
Powered by blists - more mailing lists