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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id B729FC433F5 for ; Tue, 22 Mar 2022 23:03:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238736AbiCVXEh (ORCPT ); Tue, 22 Mar 2022 19:04:37 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34006 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230303AbiCVXEf (ORCPT ); Tue, 22 Mar 2022 19:04:35 -0400 Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D52813E01; Tue, 22 Mar 2022 16:03:05 -0700 (PDT) Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6FEB06F3; Wed, 23 Mar 2022 00:03:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1647990183; bh=kZZ9bNz45LQ42/X+9rsybz3mUPSLrdhkzdo9QoUWPBM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=As0ijnSxkrHjyraydS/Kgachh4nDbPY7bYDhl+sq9djRCzeEhfUy9Azq1SUHTXQmt eA5KnJw8jj02TqZ4UwFEjKS1Cq3FpgpSwhu8T71DEuosgJIziGKDaWzuNUlLthVYeE czXcIR7tD636B7n4AWW45fjFxVekGjtG/b0yALYY= Date: Wed, 23 Mar 2022 01:02:46 +0200 From: Laurent Pinchart To: Marcel Ziswiler Cc: linux-arm-kernel@lists.infradead.org, Marcel Ziswiler , Alex Marginean , Alexander Stein , Arnd Bergmann , Fabio Estevam , Frank Rowand , NXP Linux Team , Olof Johansson , Pengutronix Kernel Team , Reinhold Mueller , Rob Herring , Sascha Hauer , Shawn Guo , Tim Harvey , Vladimir Oltean , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 3/3] arm64: dts: freescale: add initial support for verdin imx8m plus Message-ID: References: <20220317160122.341484-1-marcel@ziswiler.com> <20220317160122.341484-4-marcel@ziswiler.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20220317160122.341484-4-marcel@ziswiler.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marcel, Thank you for the patch. On Thu, Mar 17, 2022 at 05:01:22PM +0100, Marcel Ziswiler wrote: > From: Marcel Ziswiler > > This patch adds the device tree to support Toradex Verdin iMX8M Plus [1] > a computer on module which can be used on different carrier boards. > > The module consists of an NXP i.MX 8M Plus family SoC (either i.MX 8M > Plus Quad or 8M Plus QuadLite), a PCA9450C PMIC, a Gigabit Ethernet PHY, > 1, 2, 4 or 8 GB of LPDDR4 RAM, an eMMC, a TLA2024 ADC, an I2C EEPROM, an > RX8130 RTC, an optional I2C temperature sensor plus an optional > Bluetooth/Wi-Fi module. > > Anything that is not self-contained on the module is disabled by > default. > > The device tree for the Dahlia includes the module's device tree and > enables the supported peripherals of the carrier board. > > The device tree for the Verdin Development Board includes the module's > device tree as well as the Dahlia one as it is a superset and supports > almost all peripherals available. > > So far there is no display functionality supported at all but basic > console UART, USB host, eMMC and Ethernet functionality work fine. > > [1] https://www.toradex.com/computer-on-modules/verdin-arm-family/nxp-imx-8m-plus > > Signed-off-by: Marcel Ziswiler > > --- > > arch/arm64/boot/dts/freescale/Makefile | 4 + > .../dts/freescale/imx8mp-verdin-dahlia.dtsi | 125 ++ > .../boot/dts/freescale/imx8mp-verdin-dev.dtsi | 44 + > .../imx8mp-verdin-nonwifi-dahlia.dts | 18 + > .../freescale/imx8mp-verdin-nonwifi-dev.dts | 18 + > .../dts/freescale/imx8mp-verdin-nonwifi.dtsi | 54 + > .../freescale/imx8mp-verdin-wifi-dahlia.dts | 18 + > .../dts/freescale/imx8mp-verdin-wifi-dev.dts | 18 + > .../dts/freescale/imx8mp-verdin-wifi.dtsi | 82 + > .../boot/dts/freescale/imx8mp-verdin.dtsi | 1373 +++++++++++++++++ > 10 files changed, 1754 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dahlia.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dev.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dahlia.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dev.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi [snip] > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > new file mode 100644 > index 000000000000..103d97a3b029 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2022 Toradex > + */ > + > +&backlight { > + power-supply = <®_3p3v>; > +}; > + > +/* Verdin SPI_1 */ > +&ecspi1 { > + status = "okay"; > +}; > + > +/* EEPROM on display adapter boards */ > +&eeprom_display_adapter { > + status = "okay"; > +}; > + > +/* EEPROM on Verdin Development board */ > +&eeprom_carrier_board { > + status = "okay"; > +}; > + > +&eqos { > + status = "okay"; > +}; > + > +&flexcan1 { > + status = "okay"; > +}; > + > +&flexcan2 { > + status = "okay"; > +}; > + > +/* Verdin QSPI_1 */ > +&flexspi { > + status = "okay"; > +}; > + > +/* Current measurement into module VCC */ > +&hwmon { > + status = "okay"; > +}; > + > +&hwmon_temp { > + vs-supply = <®_1p8v>; > + status = "okay"; > +}; > + > +/* Verdin I2C_2_DSI */ > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c3 { > + status = "okay"; > +}; > + > +/* Verdin I2C_1 */ > +&i2c4 { > + status = "okay"; > +}; > + > +/* Verdin PCIE_1 */ Is this comment needed ? Is it a placeholder for future PCIe support ? If so I'd write /* TODO: Verdin PCIE_1 */ > + > +/* Verdin PWM_1 */ > +&pwm1 { > + status = "okay"; > +}; > + > +/* Verdin PWM_2 */ > +&pwm2 { > + status = "okay"; > +}; > + > +/* Verdin PWM_3_DSI */ > +&pwm3 { > + status = "okay"; > +}; > + > +®_usdhc2_vmmc { > + vin-supply = <®_3p3v>; > +}; > + > +/* VERDIN I2S_1 */ Same here. By the way, you may want to standardize on Verdin or VERDIN and not mix both. These comments apply to the other files too. > + > +/* Verdin UART_1 */ > +&uart1 { > + status = "okay"; > +}; > + > +/* Verdin UART_2 */ > +&uart2 { > + status = "okay"; > +}; > + > +/* Verdin UART_3, used as the Linux Console */ > +&uart3 { > + status = "okay"; > +}; > + > +/* Verdin USB_1 */ > +&usb3_0 { > + status = "okay"; > +}; > + > +&usb3_phy0 { > + status = "okay"; > +}; > + > +/* Verdin USB_2 */ > +&usb3_1 { > + status = "okay"; > +}; > + > +&usb3_phy1 { > + status = "okay"; > +}; > + > +/* Verdin SD_1 */ > +&usdhc2 { > + status = "okay"; > +}; [snip] > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > new file mode 100644 > index 000000000000..26d6c2819ee8 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > @@ -0,0 +1,1373 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2022 Toradex > + */ > + > +#include "dt-bindings/pwm/pwm.h" > +#include "imx8mp.dtsi" > + > +/ { > + chosen { > + stdout-path = &uart3; > + }; > + > + aliases { > + /* Ethernet aliases to ensure correct MAC addresses */ > + ethernet0 = &eqos; > + ethernet1 = &fec; On Dahlia the ethernet connector is routed to the eqos if I'm not mistaken. On my board U-Boot considers this to be the second ethernet controller, with the fec being the first one. The mismatch results in the MAC addresses being swapped between eth0 and eth1 when comparing U-Boot and Linux. Am I using a too old boot loader, or should the two ethernet controlls be swapped here ? > + rtc0 = &rtc_i2c; > + rtc1 = &snvs_rtc; > + }; [snip] With these issues addressed, Reviewed-by: Laurent Pinchart Tested-by: Laurent Pinchart -- Regards, Laurent Pinchart 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 E09A3C433F5 for ; Tue, 22 Mar 2022 23:04:25 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=UbniMBc8oeKUhR3aBQjrWaroBgeYfHoEpceYs3xzepA=; b=OpNgrf5JyUC90E jxVFSCvzMGyZXrKaqO1WeEZeQPDml1yMJNn/0RFzRMBLRDRwb3KkVEmt0kMkhrwP3XCGh4xxvZkcs B1VoDRFA/0vsYx4d1IjLzKusT7BzExjyjpekQQNIGjKMhGUXaAQVckAk0iJgJpC9hYlRUnjRQPsq5 iPxZr9sE63aSceLRvBZcQ2nQpJdgS4+GWfh+GFzFrRGI2yk5MyoEHZuY9ZguiXP6q1Bm8sIhbDOHL lR5OxIIbwXjWESgRZSZCgH9UdkpHPbIoZgxQ0EOXxv1dHGO3/YceVwGvOWpgx6quf4Sp5FjHGN9k0 HA9IKXzgRlrQtALut/dg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nWnWx-00COis-35; Tue, 22 Mar 2022 23:03:11 +0000 Received: from perceval.ideasonboard.com ([213.167.242.64]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nWnWs-00COiA-S3 for linux-arm-kernel@lists.infradead.org; Tue, 22 Mar 2022 23:03:08 +0000 Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6FEB06F3; Wed, 23 Mar 2022 00:03:03 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1647990183; bh=kZZ9bNz45LQ42/X+9rsybz3mUPSLrdhkzdo9QoUWPBM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=As0ijnSxkrHjyraydS/Kgachh4nDbPY7bYDhl+sq9djRCzeEhfUy9Azq1SUHTXQmt eA5KnJw8jj02TqZ4UwFEjKS1Cq3FpgpSwhu8T71DEuosgJIziGKDaWzuNUlLthVYeE czXcIR7tD636B7n4AWW45fjFxVekGjtG/b0yALYY= Date: Wed, 23 Mar 2022 01:02:46 +0200 From: Laurent Pinchart To: Marcel Ziswiler Cc: linux-arm-kernel@lists.infradead.org, Marcel Ziswiler , Alex Marginean , Alexander Stein , Arnd Bergmann , Fabio Estevam , Frank Rowand , NXP Linux Team , Olof Johansson , Pengutronix Kernel Team , Reinhold Mueller , Rob Herring , Sascha Hauer , Shawn Guo , Tim Harvey , Vladimir Oltean , devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v1 3/3] arm64: dts: freescale: add initial support for verdin imx8m plus Message-ID: References: <20220317160122.341484-1-marcel@ziswiler.com> <20220317160122.341484-4-marcel@ziswiler.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220317160122.341484-4-marcel@ziswiler.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220322_160307_109266_4FF18595 X-CRM114-Status: GOOD ( 30.94 ) 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 Hi Marcel, Thank you for the patch. On Thu, Mar 17, 2022 at 05:01:22PM +0100, Marcel Ziswiler wrote: > From: Marcel Ziswiler > > This patch adds the device tree to support Toradex Verdin iMX8M Plus [1] > a computer on module which can be used on different carrier boards. > > The module consists of an NXP i.MX 8M Plus family SoC (either i.MX 8M > Plus Quad or 8M Plus QuadLite), a PCA9450C PMIC, a Gigabit Ethernet PHY, > 1, 2, 4 or 8 GB of LPDDR4 RAM, an eMMC, a TLA2024 ADC, an I2C EEPROM, an > RX8130 RTC, an optional I2C temperature sensor plus an optional > Bluetooth/Wi-Fi module. > > Anything that is not self-contained on the module is disabled by > default. > > The device tree for the Dahlia includes the module's device tree and > enables the supported peripherals of the carrier board. > > The device tree for the Verdin Development Board includes the module's > device tree as well as the Dahlia one as it is a superset and supports > almost all peripherals available. > > So far there is no display functionality supported at all but basic > console UART, USB host, eMMC and Ethernet functionality work fine. > > [1] https://www.toradex.com/computer-on-modules/verdin-arm-family/nxp-imx-8m-plus > > Signed-off-by: Marcel Ziswiler > > --- > > arch/arm64/boot/dts/freescale/Makefile | 4 + > .../dts/freescale/imx8mp-verdin-dahlia.dtsi | 125 ++ > .../boot/dts/freescale/imx8mp-verdin-dev.dtsi | 44 + > .../imx8mp-verdin-nonwifi-dahlia.dts | 18 + > .../freescale/imx8mp-verdin-nonwifi-dev.dts | 18 + > .../dts/freescale/imx8mp-verdin-nonwifi.dtsi | 54 + > .../freescale/imx8mp-verdin-wifi-dahlia.dts | 18 + > .../dts/freescale/imx8mp-verdin-wifi-dev.dts | 18 + > .../dts/freescale/imx8mp-verdin-wifi.dtsi | 82 + > .../boot/dts/freescale/imx8mp-verdin.dtsi | 1373 +++++++++++++++++ > 10 files changed, 1754 insertions(+) > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-dev.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dahlia.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi-dev.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-nonwifi.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dahlia.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi-dev.dts > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin-wifi.dtsi > create mode 100644 arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi [snip] > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > new file mode 100644 > index 000000000000..103d97a3b029 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin-dahlia.dtsi > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2022 Toradex > + */ > + > +&backlight { > + power-supply = <®_3p3v>; > +}; > + > +/* Verdin SPI_1 */ > +&ecspi1 { > + status = "okay"; > +}; > + > +/* EEPROM on display adapter boards */ > +&eeprom_display_adapter { > + status = "okay"; > +}; > + > +/* EEPROM on Verdin Development board */ > +&eeprom_carrier_board { > + status = "okay"; > +}; > + > +&eqos { > + status = "okay"; > +}; > + > +&flexcan1 { > + status = "okay"; > +}; > + > +&flexcan2 { > + status = "okay"; > +}; > + > +/* Verdin QSPI_1 */ > +&flexspi { > + status = "okay"; > +}; > + > +/* Current measurement into module VCC */ > +&hwmon { > + status = "okay"; > +}; > + > +&hwmon_temp { > + vs-supply = <®_1p8v>; > + status = "okay"; > +}; > + > +/* Verdin I2C_2_DSI */ > +&i2c2 { > + status = "okay"; > +}; > + > +&i2c3 { > + status = "okay"; > +}; > + > +/* Verdin I2C_1 */ > +&i2c4 { > + status = "okay"; > +}; > + > +/* Verdin PCIE_1 */ Is this comment needed ? Is it a placeholder for future PCIe support ? If so I'd write /* TODO: Verdin PCIE_1 */ > + > +/* Verdin PWM_1 */ > +&pwm1 { > + status = "okay"; > +}; > + > +/* Verdin PWM_2 */ > +&pwm2 { > + status = "okay"; > +}; > + > +/* Verdin PWM_3_DSI */ > +&pwm3 { > + status = "okay"; > +}; > + > +®_usdhc2_vmmc { > + vin-supply = <®_3p3v>; > +}; > + > +/* VERDIN I2S_1 */ Same here. By the way, you may want to standardize on Verdin or VERDIN and not mix both. These comments apply to the other files too. > + > +/* Verdin UART_1 */ > +&uart1 { > + status = "okay"; > +}; > + > +/* Verdin UART_2 */ > +&uart2 { > + status = "okay"; > +}; > + > +/* Verdin UART_3, used as the Linux Console */ > +&uart3 { > + status = "okay"; > +}; > + > +/* Verdin USB_1 */ > +&usb3_0 { > + status = "okay"; > +}; > + > +&usb3_phy0 { > + status = "okay"; > +}; > + > +/* Verdin USB_2 */ > +&usb3_1 { > + status = "okay"; > +}; > + > +&usb3_phy1 { > + status = "okay"; > +}; > + > +/* Verdin SD_1 */ > +&usdhc2 { > + status = "okay"; > +}; [snip] > diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > new file mode 100644 > index 000000000000..26d6c2819ee8 > --- /dev/null > +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi > @@ -0,0 +1,1373 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT > +/* > + * Copyright 2022 Toradex > + */ > + > +#include "dt-bindings/pwm/pwm.h" > +#include "imx8mp.dtsi" > + > +/ { > + chosen { > + stdout-path = &uart3; > + }; > + > + aliases { > + /* Ethernet aliases to ensure correct MAC addresses */ > + ethernet0 = &eqos; > + ethernet1 = &fec; On Dahlia the ethernet connector is routed to the eqos if I'm not mistaken. On my board U-Boot considers this to be the second ethernet controller, with the fec being the first one. The mismatch results in the MAC addresses being swapped between eth0 and eth1 when comparing U-Boot and Linux. Am I using a too old boot loader, or should the two ethernet controlls be swapped here ? > + rtc0 = &rtc_i2c; > + rtc1 = &snvs_rtc; > + }; [snip] With these issues addressed, Reviewed-by: Laurent Pinchart Tested-by: Laurent Pinchart -- Regards, Laurent Pinchart _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel