Re: [PATCH 3/3] ARM: dts: imx7: add Toradex Colibri iMX7S/iMX7D support
Sali Stefan On Wed, 2016-06-15 at 22:37 -0700, Stefan Agner wrote: > On 2016-06-11 19:31, Shawn Guo wrote: > > > > On Tue, Jun 07, 2016 at 07:37:09PM -0700, Stefan Agner wrote: > > > > > > > > > > > +/ { > > > + model = "Toradex Colibri iMX7D on Colibri Evaluation > > > Board V3"; > > > + compatible = "toradex,colibri_imx7d-eval", > > > "toradex,colibri_imx7d", \ > > We always use hyphen than underscore in compatible string. > We always used underscores, this goes back to the first Colibri > specific > device tree: tegra20-iris-512.dts. > > I would rather prefer to not change this for the sake of > consistency... > What do you think? I have been there before when submitting a first Apalis TK1 version: https://lkml.org/lkml/2016/5/16/288 Upon Rob Herring's suggestion I changed it now all to dashes: https://lkml.org/lkml/2016/5/27/399 Even in our downstream U-Boot: http://git.toradex.com/cgit/u-boot-toradex.git/commit/?h=2015.04-torade x-next&id=4ae2e657f3350422ad81b1632de156bdd9517206 Of course for Colibri iMX7 one could argue that we already sold a couple hundred pieces still using underscores. However I guess we could still change to dashes now as those were all prototype, sample and/or beta. Cheers Marcel
Re: [PATCH 3/3] ARM: dts: imx7: add Toradex Colibri iMX7S/iMX7D support
On 2016-06-11 19:31, Shawn Guo wrote: > On Tue, Jun 07, 2016 at 07:37:09PM -0700, Stefan Agner wrote: >> +&lcdif { >> +status = "okay"; >> +display = <&display0>; > > Please put 'status' at the bottom of property list. > >> + >> +display0: lcd-display { >> +bits-per-pixel = <16>; >> +bus-width = <18>; >> + >> +display-timings { >> +native-mode = <&timing_vga>; >> + >> +/* Standard VGA timing */ >> +timing_vga: 640x480 { >> +clock-frequency = <25175000>; >> +hactive = <640>; >> +vactive = <480>; >> +hback-porch = <40>; >> +hfront-porch = <24>; >> +vback-porch = <32>; >> +vfront-porch = <11>; >> +hsync-len = <96>; >> +vsync-len = <2>; >> +de-active = <1>; >> +hsync-active = <0>; >> +vsync-active = <0>; >> +pixelclk-active = <0>; >> +}; >> +}; >> +}; >> +}; > > > >> +&usdhc1 { >> +pinctrl-names = "default"; >> +pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>; >> +no-1-8-v; >> +cd-gpios = <&gpio1 0 0>; > > Use the macro in include/dt-bindings/gpio/gpio.h. > >> +enable-sdio-wakeup; > > This is deprecated. Please use wakeup-source instead. > >> +fsl,tuning-step = <2>; >> +keep-power-in-suspend; >> +status = "okay"; >> +}; > > > >> +&fec1 { >> +pinctrl-names = "default"; >> +pinctrl-0 = <&pinctrl_enet1>; >> +clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>, >> +<&clks IMX7D_ENET_AXI_ROOT_CLK>, >> +<&clks IMX7D_ENET1_TIME_ROOT_CLK>, >> +<&clks IMX7D_PLL_ENET_MAIN_50M_CLK>; >> +clock-names = "ipg", "ahb", "ptp", "enet_clk_ref"; >> + > > Drop this newline. > >> +assigned-clocks = <&clks IMX7D_ENET1_TIME_ROOT_SRC>, >> + <&clks IMX7D_ENET1_TIME_ROOT_CLK>; >> +assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>; >> +assigned-clock-rates = <0>, <1>; >> +phy-mode = "rmii"; >> +phy-supply = <®_LDO1>; >> +fsl,magic-packet; >> +}; > > > >> +&uart1 { >> +pinctrl-names = "default"; >> +pinctrl-0 = <&pinctrl_uart1 &pinctrl_uart1_ctrl1 &pinctrl_uart1_ctrl2>; >> +assigned-clocks = <&clks IMX7D_UART1_ROOT_SRC>; >> +assigned-clock-parents = <&clks IMX7D_OSC_24M_CLK>; >> +fsl,uart-has-rtscts; > > Please use generic 'uart-has-rtscts' instead. > >> +fsl,dte-mode; >> +}; >> + >> +&uart2 { >> +pinctrl-names = "default"; >> +pinctrl-0 = <&pinctrl_uart2>; >> +assigned-clocks = <&clks IMX7D_UART2_ROOT_SRC>; >> +assigned-clock-parents = <&clks IMX7D_OSC_24M_CLK>; >> +fsl,uart-has-rtscts; > > Ditto > >> +fsl,dte-mode; >> +}; Agreed on all the stuff above. > > > >> +/ { >> +model = "Toradex Colibri iMX7D on Colibri Evaluation Board V3"; >> +compatible = "toradex,colibri_imx7d-eval", "toradex,colibri_imx7d", \ > > We always use hyphen than underscore in compatible string. We always used underscores, this goes back to the first Colibri specific device tree: tegra20-iris-512.dts. I would rather prefer to not change this for the sake of consistency... What do you think? > >> + "fsl,imx7d"; >> + >> +reg_usb_otg2_vbus: regulator-usb-otg2-vbus { >> +compatible = "regulator-fixed"; >> +pinctrl-names = "default"; >> +pinctrl-0 = <&pinctrl_usbotg2_reg>; >> +regulator-name = "VCC_USB[1-4]"; >> +regulator-min-microvolt = <500>; >> +regulator-max-microvolt = <500>; >> +gpio = <&gpio4 7 GPIO_ACTIVE_LOW>; >> +}; >> +}; >> +/ { >> +model = "Toradex Colibri iMX7S on Colibri Evaluation Board V3"; >> +compatible = "toradex,colibri_imx7s-eval", "toradex,colibri_imx7s", \ >> + "fsl,imx7s", "fsl,imx7d"; > > You may not want to have "fsl,imx7d" here. Otherwise, the detection > between imx7s and imx7d colibri-eval-v3 board will be difficult. Yeah my idea behind this is that "imx7d" is still kind of the base name of that SoC. But I agree for the bindings it does not make that much sense. I have to add imx7s to arch/arm/mach-imx/mach-imx7d.c though. -- Stefan
Re: [PATCH 3/3] ARM: dts: imx7: add Toradex Colibri iMX7S/iMX7D support
On Tue, Jun 07, 2016 at 07:37:09PM -0700, Stefan Agner wrote: > +&lcdif { > + status = "okay"; > + display = <&display0>; Please put 'status' at the bottom of property list. > + > + display0: lcd-display { > + bits-per-pixel = <16>; > + bus-width = <18>; > + > + display-timings { > + native-mode = <&timing_vga>; > + > + /* Standard VGA timing */ > + timing_vga: 640x480 { > + clock-frequency = <25175000>; > + hactive = <640>; > + vactive = <480>; > + hback-porch = <40>; > + hfront-porch = <24>; > + vback-porch = <32>; > + vfront-porch = <11>; > + hsync-len = <96>; > + vsync-len = <2>; > + de-active = <1>; > + hsync-active = <0>; > + vsync-active = <0>; > + pixelclk-active = <0>; > + }; > + }; > + }; > +}; > +&usdhc1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_usdhc1 &pinctrl_cd_usdhc1>; > + no-1-8-v; > + cd-gpios = <&gpio1 0 0>; Use the macro in include/dt-bindings/gpio/gpio.h. > + enable-sdio-wakeup; This is deprecated. Please use wakeup-source instead. > + fsl,tuning-step = <2>; > + keep-power-in-suspend; > + status = "okay"; > +}; > +&fec1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_enet1>; > + clocks = <&clks IMX7D_ENET_AXI_ROOT_CLK>, > + <&clks IMX7D_ENET_AXI_ROOT_CLK>, > + <&clks IMX7D_ENET1_TIME_ROOT_CLK>, > + <&clks IMX7D_PLL_ENET_MAIN_50M_CLK>; > + clock-names = "ipg", "ahb", "ptp", "enet_clk_ref"; > + Drop this newline. > + assigned-clocks = <&clks IMX7D_ENET1_TIME_ROOT_SRC>, > + <&clks IMX7D_ENET1_TIME_ROOT_CLK>; > + assigned-clock-parents = <&clks IMX7D_PLL_ENET_MAIN_100M_CLK>; > + assigned-clock-rates = <0>, <1>; > + phy-mode = "rmii"; > + phy-supply = <®_LDO1>; > + fsl,magic-packet; > +}; > +&uart1 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_uart1 &pinctrl_uart1_ctrl1 &pinctrl_uart1_ctrl2>; > + assigned-clocks = <&clks IMX7D_UART1_ROOT_SRC>; > + assigned-clock-parents = <&clks IMX7D_OSC_24M_CLK>; > + fsl,uart-has-rtscts; Please use generic 'uart-has-rtscts' instead. > + fsl,dte-mode; > +}; > + > +&uart2 { > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_uart2>; > + assigned-clocks = <&clks IMX7D_UART2_ROOT_SRC>; > + assigned-clock-parents = <&clks IMX7D_OSC_24M_CLK>; > + fsl,uart-has-rtscts; Ditto > + fsl,dte-mode; > +}; > +/ { > + model = "Toradex Colibri iMX7D on Colibri Evaluation Board V3"; > + compatible = "toradex,colibri_imx7d-eval", "toradex,colibri_imx7d", \ We always use hyphen than underscore in compatible string. > + "fsl,imx7d"; > + > + reg_usb_otg2_vbus: regulator-usb-otg2-vbus { > + compatible = "regulator-fixed"; > + pinctrl-names = "default"; > + pinctrl-0 = <&pinctrl_usbotg2_reg>; > + regulator-name = "VCC_USB[1-4]"; > + regulator-min-microvolt = <500>; > + regulator-max-microvolt = <500>; > + gpio = <&gpio4 7 GPIO_ACTIVE_LOW>; > + }; > +}; > +/ { > + model = "Toradex Colibri iMX7S on Colibri Evaluation Board V3"; > + compatible = "toradex,colibri_imx7s-eval", "toradex,colibri_imx7s", \ > + "fsl,imx7s", "fsl,imx7d"; You may not want to have "fsl,imx7d" here. Otherwise, the detection between imx7s and imx7d colibri-eval-v3 board will be difficult. Shawn > +};
[PATCH 3/3] ARM: dts: imx7: add Toradex Colibri iMX7S/iMX7D support
Add support for the Computer on Module Colibri iMX7S/iMX7D along with the development/evaluation carrier board device trees. Follow the usual hierarchic include model, maintaining shared configuration in imx7-colibri.dtsi and imx7-colibri-eval-v3.dtsi respectively. Signed-off-by: Stefan Agner --- arch/arm/boot/dts/Makefile | 4 +- arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi | 149 arch/arm/boot/dts/imx7-colibri.dtsi | 568 arch/arm/boot/dts/imx7d-colibri-eval-v3.dts | 66 arch/arm/boot/dts/imx7d-colibri.dtsi| 54 +++ arch/arm/boot/dts/imx7s-colibri-eval-v3.dts | 51 +++ arch/arm/boot/dts/imx7s-colibri.dtsi| 50 +++ 7 files changed, 941 insertions(+), 1 deletion(-) create mode 100644 arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi create mode 100644 arch/arm/boot/dts/imx7-colibri.dtsi create mode 100644 arch/arm/boot/dts/imx7d-colibri-eval-v3.dts create mode 100644 arch/arm/boot/dts/imx7d-colibri.dtsi create mode 100644 arch/arm/boot/dts/imx7s-colibri-eval-v3.dts create mode 100644 arch/arm/boot/dts/imx7s-colibri.dtsi diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 06b6c2d..3bb6c14 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -399,9 +399,11 @@ dtb-$(CONFIG_SOC_IMX6UL) += \ imx6ul-tx6ul-mainboard.dtb dtb-$(CONFIG_SOC_IMX7D) += \ imx7d-cl-som-imx7.dtb \ + imx7d-colibri-eval-v3.dtb \ imx7d-nitrogen7.dtb \ imx7d-sbc-imx7.dtb \ - imx7d-sdb.dtb + imx7d-sdb.dtb \ + imx7s-colibri-eval-v3.dtb dtb-$(CONFIG_SOC_LS1021A) += \ ls1021a-qds.dtb \ ls1021a-twr.dtb diff --git a/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi new file mode 100644 index 000..29744c3 --- /dev/null +++ b/arch/arm/boot/dts/imx7-colibri-eval-v3.dtsi @@ -0,0 +1,149 @@ +/* + * Copyright 2016 Toradex AG + * + * This file is dual-licensed: you can use it either under the terms + * of the GPL or the X11 license, at your option. Note that this dual + * licensing only applies to this file, and not this project as a + * whole. + * + * a) This file is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This file is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * Or, alternatively, + * + * b) Permission is hereby granted, free of charge, to any person + * obtaining a copy of this software and associated documentation + * files (the "Software"), to deal in the Software without + * restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or + * sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following + * conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + */ + +/ { + chosen { + stdout-path = "serial0:115200n8"; + }; +}; + +&bl { + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <6>; + status = "okay"; +}; + +&adc1 { + status = "okay"; +}; + +&adc2 { + status = "okay"; +}; + +&fec1 { + status = "okay"; +}; + +&i2c4 { + status = "okay"; + + /* M41T0M6 real time clock on carrier board */ + rtc: m41t0m6@68 { + compatible = "st,m41t00"; + reg = <0x68>; + }; +}; + +&lcdif { + status = "okay"; + display = <&display0>; + + display0: lcd-display { + bits-per-pixel = <16>; + bus-width = <18>; + + display-timings { + native-mode = <&timing_vga>; + + /* Standard VGA timing */ + timing_vga: 640x480 { + clock-frequency = <25175000>; + hactive = <640>; + vactive = <4