From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 638C6C4332F for ; Thu, 17 Nov 2022 14:12:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:In-Reply-To:References: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=vtUtDVorRZchyWiPJmGDuomj+0a57npR9RDu5Igd2ak=; b=ZAFwsGa1u5En8h PPz5Cp40krDTmA2ZhCMRELu1gxbziWsN5xXw8Hi6q/LZAwjJbu9Mugen1wvZR+VY7I5EtjcsT0+QE rEJvOXxdl5TxHAQe/kRWxoqe2DDCxTVtNrucNcZU5Y2GiHRrXO4doBUSw1FTvLeKsps2VM4yPir+T xjqjX60jqIttO6fCDvi04QHoss4b5a2lKuhVJEempp8Jcb2YPIpAQQd8XJV0BdbwD+g+CjsoYbEk7 iqwumP8m2DZl624V3l8RwmpKqtyCWQT1AV5JFUGuANEYLW65x9VC4PZ7/4Iy1jgWQrnnkaJjoWobX uXIAYTzxsOMpHyqtIg8Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1ovfcB-00EieC-IM; Thu, 17 Nov 2022 14:11:39 +0000 Received: from mx4.securetransport.de ([178.254.6.145]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1ovfc6-00Eian-N0 for linux-arm-kernel@lists.infradead.org; Thu, 17 Nov 2022 14:11:37 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dh-electronics.com; s=dhelectronicscom; t=1668694266; bh=2qoggqxs2xcLz6cybekwF+CcFHeDO5K7AEZjW7tzsJA=; h=From:To:CC:Subject:Date:References:In-Reply-To:From; b=wocJntdUpX60mys6EiT+Pc2hUxkXidt+0SDXiqoo9yfC+SIMQMxUgjf73HRU4WM1Q +FQzFHsjC1vtNfu57RKeJJmRqudB40NQjYyFsFXtRuH+lgoeYdnrfTw96gL4Fm7FR9 PqvZoHe2TFvPAAtfW3XdJHAunpekhgAuFdVNOlBbCl6fycX5ZLpSNotzCFb23dinBH 6fga4NDzqinVYyy2b/aIKn3rt51w+I0Bz3A9SBc5vEIHX1/bcplyeMHOT58Z458j9m iIi7d/S+fClDlpbPo4HCQjQMPd3EiUtHrpeFJQ6bnlYmFSXQPhFBlMJyTUICLvy5Ot NtrO60CB9dTYw== X-secureTransport-forwarded: yes From: Christoph Niedermaier Complaints-To: abuse@cubewerk.de To: Krzysztof Kozlowski CC: Rob Herring , Krzysztof Kozlowski , Peng Fan , Shawn Guo , Marek Vasut , Fabio Estevam , NXP Linux Team , kernel , "linux-arm-kernel@lists.infradead.org" Subject: RE: [PATCH 2/4] ARM: dts: imx6ull-dhcom: Add DH electronics DHCOM i.MX6ULL SoM and PDK2 board Thread-Topic: [PATCH 2/4] ARM: dts: imx6ull-dhcom: Add DH electronics DHCOM i.MX6ULL SoM and PDK2 board Thread-Index: AQHY+m/i8HDH8ZvdakChfjIb9+/Inq5DBgkAgAAbXGA= Date: Thu, 17 Nov 2022 14:11:01 +0000 Message-ID: <31567c6722b34abba179f330cab004db@dh-electronics.com> References: <20221117103134.6452-1-cniedermaier@dh-electronics.com> <20221117103134.6452-3-cniedermaier@dh-electronics.com> <2333f957-5ff8-da99-7862-ba9eb794c20c@linaro.org> In-Reply-To: <2333f957-5ff8-da99-7862-ba9eb794c20c@linaro.org> Accept-Language: de-DE, en-US Content-Language: de-DE X-MS-Has-Attach: X-MS-TNEF-Correlator: MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20221117_061135_257446_E430AE58 X-CRM114-Status: GOOD ( 23.80 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Krzysztof Kozlowski [mailto:krzysztof.kozlowski@linaro.org] Sent: Thursday, November 17, 2022 2:10 PM > On 17/11/2022 11:31, Christoph Niedermaier wrote: >> Add support for DH electronics DHCOM SoM and PDK2 rev. 400 carrier >> board. This is an SoM with i.MX6ULL and an evaluation kit. The >> baseboard provides Ethernet, UART, USB, CAN and optional display. >> > > Thank you for your patch. There is something to discuss/improve. > >> +#include "imx6ull-dhcom-som.dtsi" >> + >> +/ { >> + model = "DH electronics i.MX6ULL DHCOM on Premium Developer Kit (2)"; >> + compatible = "dh,imx6ull-dhcom-pdk2", "dh,imx6ull-dhcom-som", >> + "dh,imx6ull-dhcor-som", "fsl,imx6ull"; >> + >> + clk_ext_audio_codec: clock-codec { >> + #clock-cells = <0>; >> + clock-frequency = <24000000>; >> + compatible = "fixed-clock"; >> + }; >> + >> + display_bl: display-bl { >> + brightness-levels = <0 16 22 30 40 55 75 102 138 188 255>; >> + compatible = "pwm-backlight"; >> + default-brightness-level = <8>; >> + enable-gpios = <&gpio5 8 GPIO_ACTIVE_HIGH>; /* GPIO G */ >> + power-supply = <®_panel_3v3>; >> + pwms = <&pwm1 0 50000 PWM_POLARITY_INVERTED>; >> + status = "okay"; > > okay is by default, unless you override a node. Are you overriding? No, I am not overriding. Will be removed in next version. >> + }; >> + >> + gpio-keys { >> + compatible = "gpio-keys"; >> + >> + button-0 { >> + gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; /* GPIO A */ >> + label = "TA1-GPIO-A"; >> + linux,code = ; >> + wakeup-source; >> + }; >> + > > (....) > >> + >> +&fec2 { /* DHCOM ETH2 */ >> + phy-handle = <&mdio2_phy1>; >> + phy-mode = "rmii"; >> + pinctrl-0 = <&pinctrl_fec2>; >> + pinctrl-names = "default"; >> + status = "okay"; >> + >> + mdio { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + mdio2_phy0: ethernet-phy@0 { /* SMSC LAN8710Ai */ >> + clock-names = "rmii-ref"; >> + clocks = <&clks IMX6UL_CLK_ENET_REF>; >> + compatible = "ethernet-phy-id0007.c0f0", >> + "ethernet-phy-ieee802.3-c22"; >> + interrupt-parent = <&gpio5>; >> + interrupts = <5 IRQ_TYPE_LEVEL_LOW>; >> + pinctrl-0 = <&pinctrl_fec1_phy &pinctrl_snvs_fec1_phy>; >> + pinctrl-names = "default"; >> + reg = <0>; > > compatible first, reg second, then the rest of properties. Same in other > places. Sorry, I have sorted it alphabetically. Should the status property be sorted at the end or also alphabetically? >> + reset-assert-us = <500>; >> + reset-deassert-us = <500>; >> + reset-gpios = <&gpio3 23 GPIO_ACTIVE_LOW>; >> + smsc,disable-energy-detect; /* Make plugin detection reliable */ >> + }; >> + >> + mdio2_phy1: ethernet-phy@1 { /* SMSC LAN8710Ai */ >> + clock-names = "rmii-ref"; >> + clocks = <&clks IMX6UL_CLK_ENET2_REF>; >> + compatible = "ethernet-phy-id0007.c0f0", >> + "ethernet-phy-ieee802.3-c22"; >> + interrupt-parent = <&gpio5>; >> + interrupts = <6 IRQ_TYPE_LEVEL_LOW>; >> + pinctrl-0 = <&pinctrl_fec2_phy &pinctrl_snvs_fec2_phy>; >> + pinctrl-names = "default"; >> + reg = <1>; >> + reset-assert-us = <500>; >> + reset-deassert-us = <500>; >> + reset-gpios = <&gpio3 24 GPIO_ACTIVE_LOW>; >> + smsc,disable-energy-detect; /* Make plugin detection reliable */ >> + }; >> + }; >> +}; >> + > > (...) > >> diff --git a/arch/arm/boot/dts/imx6ull-dhcor-som.dtsi b/arch/arm/boot/dts/imx6ull-dhcor-som.dtsi >> new file mode 100644 >> index 000000000000..4155458897e2 >> --- /dev/null >> +++ b/arch/arm/boot/dts/imx6ull-dhcor-som.dtsi >> @@ -0,0 +1,247 @@ >> +// SPDX-License-Identifier: GPL-2.0+ OR BSD-3-Clause >> +/* >> + * Copyright (C) 2022 DH electronics GmbH >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "imx6ull.dtsi" >> + >> +/ { >> + memory@80000000 { /* Appropriate memory size will be filled by U-Boot */ >> + device_type = "memory"; >> + reg = <0x80000000 0>; >> + }; >> +}; >> + >> +&cpu0 { >> + fsl,soc-operating-points = < >> + /* KHz uV */ >> + 900000 1250000 >> + 792000 1250000 >> + 528000 1175000 >> + 396000 1175000 >> + 198000 1175000 >> + >; >> + operating-points = < >> + /* kHz uV */ >> + 900000 1275000 >> + 792000 1250000 >> + 528000 1175000 >> + 396000 1025000 >> + 198000 950000 >> + >; > > Why two properties? No support for operating-points-v2? Or is it > override of SoC DTSI? Yes, overriding SoC DTSI. > >> +}; >> + >> +&gpio1 { >> + pinctrl-0 = <&pinctrl_spi1_switch>; >> + pinctrl-names = "default"; >> + /* >> + * Pin SPI_BOOT_FLASH_EN (GPIO 1.9) is a switch for either using the >> + * DHCOM SPI1 interface or accessing the SPI bootflash. Both using >> + * ecspi1, but muxed to different pins. The DHCOM SPI1 interface uses >> + * the pins PAD_LCD_DATA21..23 and the SPI bootflash uses the pins >> + * PAD_CSI_DATA04..07. If the SPI bootflash is enabled the pins for >> + * DHCOM GPIOs N/O/P/Q/R/S/T/U aren't usable anymore, because they >> + * are used for the bus interface to the SPI bootflash. The GPIOs are >> + * disconnected by a buffer which is also controlled via the pin >> + * SPI_BOOT_FLASH_EN. Therefore the access to the bootflash is a >> + * special case and is disabled by setting GPIO 1.9 to high. >> + */ >> + spi1-switch-hog { >> + gpio-hog; >> + gpios = <9 0>; >> + output-high; >> + line-name = "spi1-switch"; >> + }; >> +}; >> + >> +&i2c1 { >> + clock-frequency = <100000>; >> + pinctrl-0 = <&pinctrl_i2c1>; >> + pinctrl-1 = <&pinctrl_i2c1_gpio>; >> + pinctrl-names = "default", "gpio"; >> + scl-gpios = <&gpio1 28 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> + sda-gpios = <&gpio1 29 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>; >> + status = "okay"; >> + >> + pmic@58 { >> + compatible = "dlg,da9061"; >> + reg = <0x58>; >> + >> + onkey { >> + compatible = "dlg,da9061-onkey", "dlg,da9062-onkey"; >> + status = "disabled"; >> + }; >> + >> + regulators { >> + vdd_soc_in_1v4: buck1 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <1400000>; >> + regulator-min-microvolt = <1400000>; >> + regulator-name = "vdd_soc_in_1v4"; >> + }; >> + >> + vcc_3v3: buck2 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <3300000>; >> + regulator-min-microvolt = <3300000>; >> + regulator-name = "vcc_3v3"; >> + }; >> + >> + /* >> + * The current DRR3 memory can be supplied with a >> + * voltage of either 1.35V or 1.5V. For reasons of >> + * backward compatibility to only 1.5V DDR3 memory, >> + * the voltage is set to 1.5V. >> + */ >> + vcc_ddr_1v35: buck3 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <1500000>; >> + regulator-min-microvolt = <1500000>; >> + regulator-name = "vcc_ddr_1v35"; >> + }; >> + >> + vcc_2v5: ldo1 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <2500000>; >> + regulator-min-microvolt = <2500000>; >> + regulator-name = "vcc_2v5"; >> + }; >> + >> + vdd_snvs_in_3v3: ldo2 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <3300000>; >> + regulator-min-microvolt = <3300000>; >> + regulator-name = "vdd_snvs_in_3v3"; >> + }; >> + >> + vcc_1v8: ldo3 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <1800000>; >> + regulator-min-microvolt = <1800000>; >> + regulator-name = "vcc_1v8"; >> + }; >> + >> + vcc_1v2: ldo4 { >> + regulator-always-on; >> + regulator-boot-on; >> + regulator-max-microvolt = <1200000>; >> + regulator-min-microvolt = <1200000>; >> + regulator-name = "vcc_1v2"; >> + }; >> + }; >> + >> + thermal { >> + compatible = "dlg,da9061-thermal", "dlg,da9062-thermal"; >> + status = "disabled"; >> + }; >> + >> + watchdog { >> + compatible = "dlg,da9061-watchdog", "dlg,da9062-watchdog"; >> + status = "disabled"; >> + }; >> + }; >> +}; >> + >> +&ocotp { >> + read-only; /* Don't get write access by default */ >> +}; >> + >> +®_arm { >> + vin-supply = <&vdd_soc_in_1v4>; >> +}; >> + >> +®_soc { >> + vin-supply = <&vdd_soc_in_1v4>; >> +}; >> + >> +&uart2 { /* BT on LGA (BT_REG_ON is connected to LGA pin E1) */ >> + pinctrl-0 = <&pinctrl_uart2>; >> + pinctrl-names = "default"; >> + uart-has-rtscts; >> + status = "okay"; >> + >> + /* >> + * Actually, the maximum speed of the chip is 4MBdps, but there are >> + * limitations that prevent this speed. It hasn't yet been figured out >> + * what the reason for this is. Currently, the maximum speed of 3MBdps >> + * can be used without any problems. If the limitation can be overcome, >> + * the speed can be increased accordingly. >> + */ >> + bluetooth: bluetooth { /* muRata 1DX */ >> + compatible = "brcm,bcm43430a1-bt"; >> + max-speed = <3000000>; >> + vbat-supply = <&vcc_3v3>; >> + vddio-supply = <&vcc_3v3>; >> + }; >> +}; >> + >> +&usdhc1 { /* WiFi on LGA (WL_REG_ON is connected to LGA pin E3) */ > > This is weird commenting style. Don't do this. Comments go usually > before the block. Fix it here and in other places. Will be fixed in next version. >> + #address-cells = <1>; >> + #size-cells = <0>; >> + bus-width = <4>; >> + no-1-8-v; >> + non-removable; >> + keep-power-in-suspend; >> + pinctrl-0 = <&pinctrl_usdhc1_wifi>; >> + pinctrl-names = "default"; >> + wakeup-source; >> + status = "okay"; >> + >> + brcmf: wifi@1 { /* muRata 1DX */ >> + compatible = "brcm,bcm4329-fmac"; >> + reg = <1>; >> + }; >> +}; >> + >> +&iomuxc { >> + pinctrl_i2c1: i2c1-grp { >> + fsl,pins = < >> + MX6UL_PAD_UART4_TX_DATA__I2C1_SCL 0x4001b8b0 >> + MX6UL_PAD_UART4_RX_DATA__I2C1_SDA 0x4001b8b0 >> + >; >> + }; >> + >> + pinctrl_i2c1_gpio: i2c1-gpio-grp { >> + fsl,pins = < >> + MX6UL_PAD_UART4_TX_DATA__GPIO1_IO28 0x4001b8b0 >> + MX6UL_PAD_UART4_RX_DATA__GPIO1_IO29 0x4001b8b0 >> + >; >> + }; >> + >> + pinctrl_spi1_switch: spi1-switch-grp { >> + fsl,pins = < >> + MX6UL_PAD_GPIO1_IO09__GPIO1_IO09 0x120b0 /* SPI_BOOT_FLASH_EN */ >> + >; >> + }; >> + >> + pinctrl_uart2: uart2-grp { >> + fsl,pins = < >> + MX6UL_PAD_UART2_TX_DATA__UART2_DCE_TX 0x1b0b1 >> + MX6UL_PAD_UART2_RX_DATA__UART2_DCE_RX 0x1b0b1 >> + MX6UL_PAD_UART3_RX_DATA__UART2_DCE_RTS 0x1b0b1 >> + MX6UL_PAD_UART3_TX_DATA__UART2_DCE_CTS 0x1b0b1 >> + >; >> + }; >> + >> + pinctrl_usdhc1_wifi: usdhc1-wifi-grp { >> + fsl,pins = < >> + MX6UL_PAD_SD1_CMD__USDHC1_CMD 0x1b0b0 >> + MX6UL_PAD_SD1_CLK__USDHC1_CLK 0x10010 >> + MX6UL_PAD_SD1_DATA0__USDHC1_DATA0 0x1b0b0 >> + MX6UL_PAD_SD1_DATA1__USDHC1_DATA1 0x1b0b0 >> + MX6UL_PAD_SD1_DATA2__USDHC1_DATA2 0x1b0b0 >> + MX6UL_PAD_SD1_DATA3__USDHC1_DATA3 0x1b0b0 >> + >; >> + }; >> +}; > > Best regards, > Krzysztof Thanks and regards Christoph _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel