[PATCH 2/4] ARM: dts: imx6ull-dhcom: Add DH electronics DHCOM i.MX6ULL SoM and PDK2 board

Krzysztof Kozlowski krzysztof.kozlowski at linaro.org
Thu Nov 17 05:09:34 PST 2022 

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 = <&reg_panel_3v3>;
> +		pwms = <&pwm1 0 50000 PWM_POLARITY_INVERTED>;
> +		status = "okay";

okay is by default, unless you override a node. Are you overriding?

> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		button-0 {
> +			gpios = <&gpio5 0 GPIO_ACTIVE_LOW>; /* GPIO A */
> +			label = "TA1-GPIO-A";
> +			linux,code = <KEY_A>;
> +			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 at 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.

> +			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 at 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 <dt-bindings/clock/imx6ul-clock.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pwm/pwm.h>
> +#include "imx6ull.dtsi"
> +
> +/ {
> +	memory at 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?

> +};
> +
> +&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 at 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 */
> +};
> +
> +&reg_arm {
> +	vin-supply = <&vdd_soc_in_1v4>;
> +};
> +
> +&reg_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.

> +	#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 at 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

More information about the linux-arm-kernel mailing list