Re: [PATCH 1/2] arm64: dts: Add a device tree for the Librem5 phone

From: Marco Felsch
Date: Wed May 27 2020 - 05:35:59 EST

Hi Martin,

On 20-05-14 17:57, Martin Kepplinger wrote:
> From: "Angus Ainslie (Purism)" <angus@xxxxxxxx>
>
> Add a devicetree description for the Librem 5 phone. The early batches
> that have been sold are supported as well as the mass-produced device
> available later this year, see https://puri.sm/products/librem-5/
>
> This boots to a working console with working WWAN modem, wifi usdhc,
> IMU sensor device, proximity sensor, haptic motor, gpio keys, GNSS and LEDs.
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@xxxxxxx>
> Signed-off-by: Angus Ainslie (Purism) <angus@xxxxxxxx>
> Signed-off-by: Guido Günther <agx@xxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> .../boot/dts/freescale/imx8mq-librem5.dts | 1174 +++++++++++++++++
> 2 files changed, 1175 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index cd38d04da5a7..342579121f98 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -34,6 +34,7 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mn-ddr4-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mp-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-evk.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-hummingboard-pulse.dtb
> +dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-nitrogen.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-phanbell.dtb
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> new file mode 100644
> index 000000000000..95c105b4c120
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dts
> @@ -0,0 +1,1174 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018-2020 Purism SPC
> + */
> +
> +/dts-v1/;
> +
> +#include "dt-bindings/input/input.h"
> +#include "dt-bindings/pwm/pwm.h"
> +#include "dt-bindings/usb/pd.h"
> +#include "imx8mq.dtsi"
> +
> +/ {
> + model = "Purism Librem 5";
> + compatible = "purism,librem5", "fsl,imx8mq";
> +
> + backlight_dsi: backlight-dsi {
> + compatible = "led-backlight";
> + leds = <&led_backlight>;
> + brightness-levels = <255>;
> + default-brightness-level = <100>;
> + };
> +
> + bm818_codec: sound-wwan-codec {
> + compatible = "broadmobi,bm818", "option,gtm601";
> + #sound-dai-cells = <0>;
> + };

Please sort the node names alpabetical.

> +
> + chosen {
> + stdout-path = &uart1;
> + };
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_keys>, <&pinctrl_hp>;
> +
> + hp-det {
> + label = "HP_DET";
> + gpios = <&gpio3 9 GPIO_ACTIVE_HIGH>;
> + wakeup-source;
> + linux,code = <KEY_HP>;

Nit: I would add the wakeup-source behind the linux,code.

> + };
> +
> + vol-down {
> + label = "VOL_DOWN";
> + gpios = <&gpio1 17 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_VOLUMEDOWN>;
> + };
> +
> + vol-up {
> + label = "VOL_UP";
> + gpios = <&gpio1 16 GPIO_ACTIVE_LOW>;
> + linux,code = <KEY_VOLUMEUP>;
> + };
> + };
> +
> + pwmleds {
> + compatible = "pwm-leds";
> +
> + blue {
> + label = "phone:blue:front";
> + max-brightness = <248>;
> + pwms = <&pwm2 0 50000>;
> + };
> +
> + green {
> + label = "phone:green:front";
> + max-brightness = <248>;
> + pwms = <&pwm4 0 50000>;
> + };
> +
> + red {
> + label = "phone:red:front";
> + max-brightness = <248>;
> + pwms = <&pwm3 0 50000>;
> + };
> + };
> +
> + pmic_osc: clock-pmic {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <32768>;
> + clock-output-names = "pmic_osc";
> + };

Please sort nodes alphabetical.

> +
> + reg_audio_pwr_en: regulator-audio-pwr-en {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_audiopwr>;
> + regulator-name = "AUDIO_PWR_EN";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + gpio = <&gpio1 4 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + regulator-always-on;

Why should this regulator be always on? The wm8962.c driver can handle
the regualtor enable/disable.

> + };
> +
> + reg_aud_1v8: regulator-audio-v1v8 {
^
regulator-audio-1v8?

> + compatible = "regulator-fixed";
> + regulator-name = "aud_1v8";

Is it intended to use capitalized and no-capitalized regulator-name's?

> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + vin-supply = <&reg_audio_pwr_en>;
> + };

Can we squash regulator-audio-pwr-en and regulator-audio-v1v8?

> +
> + reg_gnss: regulator-gnss {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gnsspwr>;
> + regulator-name = "GNSS";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio3 12 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reg_hub: regulator-hub {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_hub_pwr>;
> + regulator-name = "HUB";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpio = <&gpio1 14 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> + };
> +
> + reg_lcd_1v8: regulator-lcd-1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "lcd_1v8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dsien>;
> + vin-supply = <&reg_vdd_1v8>;
> + enable-active-high;
> + gpio = <&gpio1 5 GPIO_ACTIVE_HIGH>;
> + };

This regulator is never used.

> +
> + reg_lcd_3v4: regulator-lcd-3v4 {
> + compatible = "regulator-fixed";
> + regulator-name = "lcd_3v4";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dsibiasen>;
> + vin-supply = <&reg_vsys_3v4>;
> + enable-active-high;
> + gpio = <&gpio1 20 GPIO_ACTIVE_HIGH>;
> + };
> +
> + reg_vdd_sen: regulator-vdd-sen {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd_sen";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + reg_vdd_3v3: regulator-vdd-3v3 {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd_3v3";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + reg_vdd_1v8: regulator-vdd-1v8 {
> + compatible = "regulator-fixed";
> + regulator-name = "vdd_1v8";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <1800000>;
> + };
> +
> + reg_vsys_3v4: regulator-vsys-3v4 {
> + compatible = "regulator-fixed";
> + regulator-name = "vsys_3v4";
> + regulator-min-microvolt = <3400000>;
> + regulator-max-microvolt = <3400000>;
> + regulator-always-on;
> + };
> +
> + reg_3v3_wifi: regulator-3v3-wifi {
^
reg_wifi_3v3: regulator-wifi-3v3?

> + compatible = "regulator-fixed";
> + regulator-name = "3v3_wifi";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + };
> +
> + sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "wm8962";
> + simple-audio-card,format = "i2s";
> + simple-audio-card,widgets =
> + "Headphone", "Headphone",
> + "Microphone", "Headset Mic",
> + "Microphone", "Digital Mic",
> + "Speaker", "Speaker";
> + simple-audio-card,routing =
> + "Headphone", "HPOUTL",
> + "Headphone", "HPOUTR",
> + "Speaker", "SPKOUTL",
> + "Speaker", "SPKOUTR",
> + "Headset Mic", "MICBIAS",
> + "IN3R", "Headset Mic",
> + "DMICDAT", "Digital Mic";
> + simple-audio-card,cpu {
> + sound-dai = <&sai2>;
> + };
> + simple-audio-card,codec {
> + sound-dai = <&codec>;
> + clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
> + frame-master;
> + bitclock-master;
> + };
> + };
> +
> + sound-wwan {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "MODEM";
> + simple-audio-card,format = "i2s";
> +
> + simple-audio-card,cpu {
> + sound-dai = <&sai6>;
> + frame-inversion;
> + };
> +
> + telephony_link_master: simple-audio-card,codec {
^
useless phandle?
> + sound-dai = <&bm818_codec>;
> + frame-master;
> + bitclock-master;
> + };
> + };
> +
> + vibrator {
> + compatible = "pwm-vibrator";
> + pwms = <&pwm1 0 1000000000 0>;
> + pwm-names = "enable";
> + vcc-supply = <&reg_vdd_3v3>;
> + };
> +};
> +
> +&A53_0 {
> + cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_1 {
> + cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_2 {
> + cpu-supply = <&buck2_reg>;
> +};
> +
> +&A53_3 {
> + cpu-supply = <&buck2_reg>;
> +};
> +
> +&clk {
> + assigned-clocks = <&clk IMX8MQ_AUDIO_PLL1>, <&clk IMX8MQ_AUDIO_PLL2>;
> + assigned-clock-rates = <786432000>, <722534400>;
> +};

Either I would bundle all clock settings here or within the sai nodes.

> +
> +&ddrc {
> + operating-points-v2 = <&ddrc_opp_table>;
> +
> + ddrc_opp_table: ddrc-opp-table {
> + compatible = "operating-points-v2";
> +
> + opp-25M {
> + opp-hz = /bits/ 64 <25000000>;
> + };
> + opp-100M {
> + opp-hz = /bits/ 64 <100000000>;
> + };
> + opp-800M {
> + opp-hz = /bits/ 64 <800000000>;
> + };
> + };
> +};
> +
> +&dphy {
> + status = "okay";
> +};
> +
> +&ecspi1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_ecspi1>;
> + cs-gpios = <&gpio5 9 GPIO_ACTIVE_HIGH>;

Missmatch with the pinctrl_ecspi1?

> + status = "okay";

Status is always the last property.

> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + nor_flash: flash@0 {
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <1000000>;
> + reg = <0>;
> + };
> +};
> +
> +&gpio1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pmic_5v>;
> +
> + pmic-5v {
> + gpio-hog;
> + gpio = <&gpio1 1 GPIO_ACTIVE_HIGH>;
> + input;
> + };
> +};
> +
> +&iomuxc {
> + pinctrl_audiopwr: audiopwrgrp {
> + fsl,pins = <
> + /* AUDIO_POWER_EN_3V3 */
> + MX8MQ_IOMUXC_GPIO1_IO04_GPIO1_IO4 0x83
> + >;
> + };
> +
> + pinctrl_bl: blgrp {
> + fsl,pins = <
> + /* BACKLINGE_EN */
> + MX8MQ_IOMUXC_NAND_DQS_GPIO3_IO14 0x83
> + >;
> + };
> +
> + pinctrl_charger_in: chargeringrp {
> + fsl,pins = <
> + /* CHRG_INT */
> + MX8MQ_IOMUXC_NAND_CE2_B_GPIO3_IO3 0x80
> + /* CHG_STATUS_B */
> + MX8MQ_IOMUXC_NAND_ALE_GPIO3_IO0 0x80
> + >;
> + };
> +
> + pinctrl_dsibiasen: dsibiasengrp {
> + fsl,pins = <
> + /* DSI_BIAS_EN */
> + MX8MQ_IOMUXC_ENET_TD1_GPIO1_IO20 0x83
> + >;
> + };
> +
> + pinctrl_dsien: dsiengrp {
> + fsl,pins = <
> + /* DSI_EN_3V3 */
> + MX8MQ_IOMUXC_GPIO1_IO05_GPIO1_IO5 0x83
> + >;
> + };
> +
> + pinctrl_ecspi1: spi1grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_ECSPI1_MOSI_ECSPI1_MOSI 0x83
> + MX8MQ_IOMUXC_ECSPI1_MISO_ECSPI1_MISO 0x83
> + MX8MQ_IOMUXC_ECSPI1_SS0_GPIO5_IO9 0x19
> + MX8MQ_IOMUXC_ECSPI1_SCLK_ECSPI1_SCLK 0x83
> + /* SPI_SS1 */
> + MX8MQ_IOMUXC_UART4_RXD_GPIO5_IO28 0x19
> + >;
> + };
> +
> + pinctrl_gauge: gaugegrp {
> + fsl,pins = <
> + /* BAT_LOW */
> + MX8MQ_IOMUXC_SAI5_RXC_GPIO3_IO20 0x80
> + >;
> + };
> +
> + pinctrl_gnsspwr: gnsspwrgrp {
> + fsl,pins = <
> + /* GPS3V3_EN */
> + MX8MQ_IOMUXC_NAND_DATA06_GPIO3_IO12 0x83
> + >;
> + };
> +
> + pinctrl_haptic: hapticgrp {
> + fsl,pins = <
> + /* MOTO */
> + MX8MQ_IOMUXC_SPDIF_EXT_CLK_PWM1_OUT 0x83
> + >;
> + };
> +
> + pinctrl_hp: hpgrp {
> + fsl,pins = <
> + /* HEADPHONE_DET_1V8 */
> + MX8MQ_IOMUXC_NAND_DATA03_GPIO3_IO9 0x180
> + >;
> + };
> +
> + pinctrl_hub_pwr: hubpwrgrp {
> + fsl,pins = <
> + /* HUB_PWR_3V3_EN */
> + MX8MQ_IOMUXC_GPIO1_IO14_GPIO1_IO14 0x83
> + >;
> + };
> +
> + pinctrl_i2c1: i2c1grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_I2C1_SCL_I2C1_SCL 0x40000026
> + MX8MQ_IOMUXC_I2C1_SDA_I2C1_SDA 0x40000026
> + >;
> + };
> +
> + pinctrl_i2c2: i2c2grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_I2C2_SCL_I2C2_SCL 0x40000026
> + MX8MQ_IOMUXC_I2C2_SDA_I2C2_SDA 0x40000026
> + >;
> + };
> +
> + pinctrl_i2c3: i2c3grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_I2C3_SCL_I2C3_SCL 0x40000026
> + MX8MQ_IOMUXC_I2C3_SDA_I2C3_SDA 0x40000026
> + >;
> + };
> +
> + pinctrl_i2c4: i2c4grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL 0x40000026
> + MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA 0x40000026
> + >;
> + };
> +
> + pinctrl_keys: keysgrp {
> + fsl,pins = <
> + /* 4G_WAKE */
> + MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15 0x80
> + /* PWR_KEY */
> + MX8MQ_IOMUXC_NAND_CLE_GPIO3_IO5 0x01C0

gpio3 5/15 are never used was this intended?

> + /* VOL- */
> + MX8MQ_IOMUXC_ENET_MDIO_GPIO1_IO17 0x01C0
> + /* VOL+ */
> + MX8MQ_IOMUXC_ENET_MDC_GPIO1_IO16 0x01C0
> + >;
> + };
> +
> + pinctrl_led_b: ledbgrp {
> + fsl,pins = <
> + /* LED_B */
> + MX8MQ_IOMUXC_GPIO1_IO13_PWM2_OUT 0x06
> + >;
> + };
> +
> + pinctrl_led_g: ledggrp {
> + fsl,pins = <
> + /* LED_G */
> + MX8MQ_IOMUXC_SAI3_MCLK_PWM4_OUT 0x06
> + >;
> + };
> +
> + pinctrl_led_r: ledrgrp {
> + fsl,pins = <
> + /* LED_R */
> + MX8MQ_IOMUXC_SPDIF_TX_PWM3_OUT 0x06
> + >;
> + };
> +
> + pinctrl_mag: maggrp {
> + fsl,pins = <
> + /* INT_MAG */
> + MX8MQ_IOMUXC_SAI5_RXD1_GPIO3_IO22 0x80
> + >;
> + };
> +
> + pinctrl_pmic: pmicgrp {
> + fsl,pins = <
> + /* PMIC_NINT */
> + MX8MQ_IOMUXC_GPIO1_IO07_GPIO1_IO7 0x80
> + >;
> + };
> +
> + pinctrl_pmic_5v: pmic5vgrp {
> + fsl,pins = <
> + /* PMIC_5V */
> + MX8MQ_IOMUXC_GPIO1_IO01_GPIO1_IO1 0x80
> + >;
> + };
> +
> + pinctrl_prox: proxgrp {
> + fsl,pins = <
> + /* INT_LIGHT */
> + MX8MQ_IOMUXC_NAND_DATA01_GPIO3_IO7 0x80
> + >;
> + };
> +
> + pinctrl_rtc: rtcgrp {
> + fsl,pins = <
> + /* RTC_INT */
> + MX8MQ_IOMUXC_GPIO1_IO09_GPIO1_IO9 0x80
> + >;
> + };
> +
> + pinctrl_sai2: sai2grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SAI2_TXD0_SAI2_TX_DATA0 0xd6
> + MX8MQ_IOMUXC_SAI2_TXFS_SAI2_TX_SYNC 0xd6
> + MX8MQ_IOMUXC_SAI2_MCLK_SAI2_MCLK 0xd6
> + MX8MQ_IOMUXC_SAI2_RXD0_SAI2_RX_DATA0 0xd6
> + MX8MQ_IOMUXC_SAI2_TXC_SAI2_TX_BCLK 0xd6
> + >;
> + };
> +
> + pinctrl_sai6: sai6grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SAI1_RXD5_SAI6_RX_DATA0 0xd6
> + MX8MQ_IOMUXC_SAI1_RXD6_SAI6_RX_SYNC 0xd6
> + MX8MQ_IOMUXC_SAI1_TXD4_SAI6_RX_BCLK 0xd6
> + MX8MQ_IOMUXC_SAI1_TXD5_SAI6_TX_DATA0 0xd6
> + >;
> + };
> +
> + pinctrl_tcpc: tcpcgrp {
> + fsl,pins = <
> + /* TCPC_INT */
> + MX8MQ_IOMUXC_GPIO1_IO10_GPIO1_IO10 0x01C0
> + >;
> + };
> +
> + pinctrl_typec: typecgrp {
> + fsl,pins = <
> + /* TYPEC_MUX_EN */
> + MX8MQ_IOMUXC_GPIO1_IO11_GPIO1_IO11 0x83
> + >;
> + };
> +
> + pinctrl_uart1: uart1grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_UART1_RXD_UART1_DCE_RX 0x49
> + MX8MQ_IOMUXC_UART1_TXD_UART1_DCE_TX 0x49
> + >;
> + };
> +
> + pinctrl_uart2: uart2grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_UART2_TXD_UART2_DCE_TX 0x49
> + MX8MQ_IOMUXC_UART2_RXD_UART2_DCE_RX 0x49
> + >;
> + };
> +
> + pinctrl_uart3: uart3grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_UART3_RXD_UART3_DCE_RX 0x49
> + MX8MQ_IOMUXC_UART3_TXD_UART3_DCE_TX 0x49
> + >;
> + };
> +
> + pinctrl_uart4: uart4grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_ECSPI2_SCLK_UART4_DCE_RX 0x49
> + MX8MQ_IOMUXC_ECSPI2_MOSI_UART4_DCE_TX 0x49
> + MX8MQ_IOMUXC_ECSPI2_MISO_UART4_DCE_CTS_B 0x49
> + MX8MQ_IOMUXC_ECSPI2_SS0_UART4_DCE_RTS_B 0x49
> + >;
> + };
> +
> + pinctrl_usdhc1: usdhc1grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x83
> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xc3
> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xc3
> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xc3
> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xc3
> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xc3
> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xc3
> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xc3
> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xc3
> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xc3
> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x83
> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_usdhc1_100mhz: usdhc1grp100mhz {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x8d
> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xcd
> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xcd
> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xcd
> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xcd
> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xcd
> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xcd
> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xcd
> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xcd
> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xcd
> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x8d
> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_usdhc1_200mhz: usdhc1grp200mhz {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD1_CLK_USDHC1_CLK 0x9f
> + MX8MQ_IOMUXC_SD1_CMD_USDHC1_CMD 0xdf
> + MX8MQ_IOMUXC_SD1_DATA0_USDHC1_DATA0 0xdf
> + MX8MQ_IOMUXC_SD1_DATA1_USDHC1_DATA1 0xdf
> + MX8MQ_IOMUXC_SD1_DATA2_USDHC1_DATA2 0xdf
> + MX8MQ_IOMUXC_SD1_DATA3_USDHC1_DATA3 0xdf
> + MX8MQ_IOMUXC_SD1_DATA4_USDHC1_DATA4 0xdf
> + MX8MQ_IOMUXC_SD1_DATA5_USDHC1_DATA5 0xdf
> + MX8MQ_IOMUXC_SD1_DATA6_USDHC1_DATA6 0xdf
> + MX8MQ_IOMUXC_SD1_DATA7_USDHC1_DATA7 0xdf
> + MX8MQ_IOMUXC_SD1_STROBE_USDHC1_STROBE 0x9f
> + MX8MQ_IOMUXC_SD1_RESET_B_USDHC1_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_usdhc2: usdhc2grp {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x83
> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xc3
> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xc3
> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xc3
> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xc3
> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xc3
> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_usdhc2_100mhz: usdhc2grp100mhz {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x8d
> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xcd
> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xcd
> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xcd
> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xcd
> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xcd
> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_usdhc2_200mhz: usdhc2grp200mhz {
> + fsl,pins = <
> + MX8MQ_IOMUXC_SD2_CD_B_USDHC2_CD_B 0x80
> + MX8MQ_IOMUXC_SD2_CLK_USDHC2_CLK 0x9f
> + MX8MQ_IOMUXC_SD2_CMD_USDHC2_CMD 0xcf
> + MX8MQ_IOMUXC_SD2_DATA0_USDHC2_DATA0 0xcf
> + MX8MQ_IOMUXC_SD2_DATA1_USDHC2_DATA1 0xcf
> + MX8MQ_IOMUXC_SD2_DATA2_USDHC2_DATA2 0xcf
> + MX8MQ_IOMUXC_SD2_DATA3_USDHC2_DATA3 0xcf
> + MX8MQ_IOMUXC_SD2_RESET_B_USDHC2_RESET_B 0xc1
> + >;
> + };
> +
> + pinctrl_wdog: wdoggrp {
> + fsl,pins = <
> + /* nWDOG */
> + MX8MQ_IOMUXC_GPIO1_IO02_WDOG1_WDOG_B 0x1f
> + >;
> + };
> +};
> +
> +&i2c1 {
> + clock-frequency = <387000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c1>;
> + status = "okay";
> +
> + typec_pd: usb-pd@3f {
> + compatible = "ti,tps6598x";
> + reg = <0x3f>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_typec>, <&pinctrl_tcpc>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <10 IRQ_TYPE_LEVEL_LOW>;
> +
> + connector {
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + usb_con_hs: endpoint {
> + remote-endpoint = <&typec_hs>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + usb_con_ss: endpoint {
> + remote-endpoint = <&typec_ss>;
> + };
> + };
> + };
> + };
> + };
> +
> + pmic: pmic@4b {
> + compatible = "rohm,bd71837";
> + reg = <0x4b>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_pmic>;
> + clocks = <&pmic_osc>;
> + clock-names = "osc";
> + clock-output-names = "pmic_clk";
> + interrupt-parent = <&gpio1>;
> + interrupts = <7 GPIO_ACTIVE_LOW>;
> + interrupt-names = "irq";
> + rohm,reset-snvs-powered;
> +
> + regulators {
> + buck1_reg: BUCK1 {
> + regulator-name = "buck1";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-ramp-delay = <1250>;
> + rohm,dvs-run-voltage = <900000>;
> + rohm,dvs-idle-voltage = <850000>;
> + rohm,dvs-suspend-voltage = <800000>;
> + regulator-always-on;
> + };
> +
> + buck2_reg: BUCK2 {
> + regulator-name = "buck2";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1300000>;
> + regulator-ramp-delay = <1250>;
> + rohm,dvs-run-voltage = <1000000>;
> + rohm,dvs-idle-voltage = <900000>;
> + regulator-always-on;
> + };
> +
> + buck3_reg: BUCK3 {
> + regulator-name = "buck3";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1300000>;
> + rohm,dvs-run-voltage = <900000>;
> + regulator-always-on;
> + };
> +
> + buck4_reg: BUCK4 {
> + regulator-name = "buck4";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1300000>;
> + rohm,dvs-run-voltage = <1000000>;
> + };
> +
> + buck5_reg: BUCK5 {
> + regulator-name = "buck5";
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <1350000>;
> + regulator-always-on;
> + };
> +
> + buck6_reg: BUCK6 {
> + regulator-name = "buck6";
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> + buck7_reg: BUCK7 {
> + regulator-name = "buck7";
> + regulator-min-microvolt = <1605000>;
> + regulator-max-microvolt = <1995000>;
> + regulator-always-on;
> + };
> +
> + buck8_reg: BUCK8 {
> + regulator-name = "buck8";
> + regulator-min-microvolt = <800000>;
> + regulator-max-microvolt = <1400000>;
> + regulator-always-on;
> + };
> +
> + ldo1_reg: LDO1 {
> + regulator-name = "ldo1";
> + regulator-min-microvolt = <3000000>;
> + regulator-max-microvolt = <3300000>;
> + /* leave on for snvs power button */
> + regulator-always-on;
> + };
> +
> + ldo2_reg: LDO2 {
> + regulator-name = "ldo2";
> + regulator-min-microvolt = <900000>;
> + regulator-max-microvolt = <900000>;
> + /* leave on for snvs power button */
> + regulator-always-on;
> + };
> +
> + ldo3_reg: LDO3 {
> + regulator-name = "ldo3";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> + ldo4_reg: LDO4 {
> + regulator-name = "ldo4";
> + regulator-min-microvolt = <900000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + ldo5_reg: LDO5 {
> + /* VDD_PHY_0V9 - MIPI and HDMI domains */
> + regulator-name = "ldo5";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };
> +
> + ldo6_reg: LDO6 {
> + /* VDD_PHY_0V9 - MIPI, HDMI and USB domains */
> + regulator-name = "ldo6";
> + regulator-min-microvolt = <900000>;
> + regulator-max-microvolt = <1800000>;
> + regulator-always-on;
> + };
> +
> + ldo7_reg: LDO7 {
> + /* VDD_PHY_3V3 - USB domain */
> + regulator-name = "ldo7";
> + regulator-min-microvolt = <1800000>;
> + regulator-max-microvolt = <3300000>;
> + regulator-always-on;
> + };

Out of curiosity, why did you marked all regulators as
regulator-always-on? I thought the librem5 is a smartphone.

> + };
> + };
> +
> + rtc@68 {
> + compatible = "microcrystal,rv4162";
> + reg = <0x68>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_rtc>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <9 IRQ_TYPE_LEVEL_LOW>;
> + };
> +};
> +
> +&i2c2 {
> + clock-frequency = <387000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c2>;
> + status = "okay";
> +
> + magnetometer@1e {
> + compatible = "st,lsm9ds1-magn";
> + reg = <0x1e>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_mag>;
> + interrupt-parent = <&gpio3>;
> + interrupts = <22 IRQ_TYPE_LEVEL_HIGH>;
> + vdd-supply = <&reg_vdd_sen>;
> + vddio-supply = <&reg_vdd_1v8>;
> + };
> +
> + regulator@3e {
> + compatible = "tps65132";
> + reg = <0x3e>;
> + reg_lcd_avdd: outp {
> + regulator-name = "lcd_avdd";
> + vin-supply = <&reg_lcd_3v4>;
> + };
> +
> + reg_lcd_avee: outn {
> + regulator-name = "lcd_avee";
> + vin-supply = <&reg_lcd_3v4>;
> + };
both phandles are not used.
> + };
> +
> + flash@53 {
> + compatible = "lm3560";
> + reg = <0x53>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + flash@0 {
> + reg = <0x0>;
> + flash-timeout-us = <150000>;
> + flash-max-microamp = <320000>;
> + led-max-microamp = <60000>;
> + label = "lm3560:flash";
> + };
> +
> + torch@1 {
> + reg = <0x1>;
> + led-max-microamp = <10000>;
> + label = "lm3560:torch";
> + };
> +
> + };
> +
> + prox@60 {
> + compatible = "vishay,vcnl4040";
> + reg = <0x60>;
> + pinctrl-0 = <&pinctrl_prox>;
> + interrupt-parent = <&gpio3>;
> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> + };
> +
> + accel-gyro@6a {
> + compatible = "st,lsm9ds1-imu";
> + reg = <0x6a>;
> + vdd-supply = <&reg_vdd_sen>;
> + vddio-supply = <&reg_vdd_1v8>;
> + mount-matrix = "1", "0", "0",
> + "0", "1", "0",
> + "0", "0", "-1";
> + };
> +};
> +
> +&i2c3 {
> + clock-frequency = <387000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c3>;
> + status = "okay";
> +
> + codec: wm8962@1a {

Please use generic names.

> + compatible = "wlf,wm8962";
> + reg = <0x1a>; // 0x4a is the test address
> + clocks = <&clk IMX8MQ_CLK_SAI2_ROOT>;
> + assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
> + assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> + assigned-clock-rates = <24576000>;
> + #sound-dai-cells = <0>;
> + mic-cfg = <0x200>;
> + DCVDD-supply = <&reg_aud_1v8>;
> + DBVDD-supply = <&reg_aud_1v8>;
> + AVDD-supply = <&reg_aud_1v8>;
> + CPVDD-supply = <&reg_aud_1v8>;
> + MICVDD-supply = <&reg_aud_1v8>;
> + PLLVDD-supply = <&reg_aud_1v8>;
> + SPKVDD1-supply = <&reg_vsys_3v4>;
> + SPKVDD2-supply = <&reg_vsys_3v4>;
> + gpio-cfg = <
> + 0x0000 /* n/c */
> + 0x0001 /* gpio2, 1: default */
> + 0x0013 /* gpio3, 2: dmicclk */
> + 0x0000 /* n/c, 3: default */
> + 0x8014 /* gpio5, 4: dmic_dat */
> + 0x0000 /* gpio6, 5: default */
> + >;
> + status = "okay";

status can be dropped

> + };
> +
> + backlight@36 {
> + compatible = "ti,lm36922";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_bl>;
> + reg = <0x36>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + enable-gpios = <&gpio3 14 GPIO_ACTIVE_HIGH>;
> + vled-supply = <&reg_vsys_3v4>;
> + ti,ovp-microvolt = <25000000>;
> +
> + led_backlight: led@0 {
> + reg = <0>;
> + label = "white:backlight_cluster";
> + linux,default-trigger = "backlight";
> + led-max-microamp = <20000>;
> + };
> + };
> +
> + touchscreen@38 {
> + compatible = "edt,edt-ft5506";
> + reg = <0x38>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <27 IRQ_TYPE_EDGE_FALLING>;

You need to mux the irq gpio.

> + irq-gpios = <&gpio1 27 GPIO_ACTIVE_HIGH>;

irq-gpios is not supported by the driver. We only have a
wake/reset-gpio.

> + touchscreen-size-x = <720>;
> + touchscreen-size-y = <1440>;
> + };
> +};
> +
> +&i2c4 {
> + clock-frequency = <387000>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2c4>;
> + status = "okay";
> +
> + bat: fuel-gauge@36 {
> + compatible = "maxim,max17055";
> + interrupt-parent = <&gpio3>;
> + interrupts = <20 IRQ_TYPE_LEVEL_LOW>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_gauge>;
> + reg = <0x36>;

Please check that "reg" is always the 2nd property after the
"compatible".

> + maxim,over-heat-temp = <700>;
> + maxim,over-volt = <4500>;
> + maxim,rsns-microohm = <5000>;
> + };
> +
> + charger@6a { /* bq25895 */
> + compatible = "ti,bq25890";

The compatible should be "ti,bq25895" if it is a bq25895. So we can drop
the comment too.

> + reg = <0x6a>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_charger_in>;
> + interrupt-parent = <&gpio3>;
> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + phys = <&usb3_phy0>;
> + ti,battery-regulation-voltage = <4192000>; /* 4.192V */
> + ti,charge-current = <1600000>; /* 1.6A */
> + ti,termination-current = <66000>; /* 66mA */
> + ti,precharge-current = <130000>; /* 130mA */
> + ti,minimum-sys-voltage = <3700000>; /* 3.7V */
> + ti,boost-voltage = <5000000>; /* 5V */
> + ti,boost-max-current = <50000>; /* 50mA */
> + ti,use-vinmin-threshold = <1>; /* enable VINDPM */
> + ti,vinmin-threshold = <3900000>; /* 3.9V */

I would only mention the units within a comment because comments like
this begin to divergence after you fix something.

Regards,
Marco

> + monitored-battery = <&bat>;
> + };
> +};
> +
> +&pgc_gpu {
> + power-supply = <&buck3_reg>;
> +};
> +
> +&pgc_mipi {
> + power-supply = <&ldo5_reg>;
> +};
> +
> +&pgc_vpu {
> + power-supply = <&buck4_reg>;
> +};
> +
> +&pwm1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_haptic>;
> + status = "okay";
> +};
> +
> +&pwm2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_led_b>;
> + status = "okay";
> +};
> +
> +&pwm3 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_led_g>;
> + status = "okay";
> +};
> +
> +&pwm4 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_led_r>;
> + status = "okay";
> +};
> +
> +&sai2 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sai2>;
> + assigned-clocks = <&clk IMX8MQ_CLK_SAI2>;
> + assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> + assigned-clock-rates = <24576000>;
> + status = "okay";
> +};
> +
> +&sai6 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_sai6>;
> + assigned-clocks = <&clk IMX8MQ_CLK_SAI6>;
> + assigned-clock-parents = <&clk IMX8MQ_AUDIO_PLL1_OUT>;
> + assigned-clock-rates = <24576000>;
> + fsl,sai-synchronous-rx;
> + status = "okay";
> +};
> +
> +&snvs_pwrkey {
> + status = "okay";
> +};
> +
> +&snvs_rtc {
> + status = "disabled";
> +};
> +
> +&uart1 { /* console */
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart1>;
> + status = "okay";
> +};
> +
> +&uart2 { /* TPS - GPS - DEBUG */
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart2>;
> + status = "okay";
> +
> + gnss {
> + compatible = "globaltop,pa6h";
> + vcc-supply = <&reg_gnss>;
> + current-speed = <9600>;
> + };
> +};
> +
> +&uart3 { /* SMC */
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart3>;
> + status = "okay";
> +};
> +
> +&uart4 { /* BT */
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_uart4>;
> + uart-has-rtscts;
> + status = "okay";
> +};
> +
> +&usb3_phy0 {
> + status = "okay";
> +};
> +
> +&usb3_phy1 {
> + vbus-supply = <&reg_hub>;
> + status = "okay";
> +};
> +
> +&usb_dwc3_0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + dr_mode = "otg";
> + snps,dis_u3_susphy_quirk;
> + status = "okay";
> +
> + port@0 {
> + reg = <0>;
> +
> + typec_hs: endpoint {
> + remote-endpoint = <&usb_con_hs>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + typec_ss: endpoint {
> + remote-endpoint = <&usb_con_ss>;
> + };
> + };
> +};
> +
> +&usb_dwc3_1 {
> + dr_mode = "host";
> + snps,dis_u2_susphy_quirk;
> + snps,dis_u3_susphy_quirk;
> + status = "okay";
> +};
> +
> +&usdhc1 {
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + pinctrl-0 = <&pinctrl_usdhc1>;
> + pinctrl-1 = <&pinctrl_usdhc1_100mhz>;
> + pinctrl-2 = <&pinctrl_usdhc1_200mhz>;
> + bus-width = <8>;
> + vmmc-supply = <&reg_vdd_3v3>;
> + power-supply = <&reg_vdd_1v8>;
> + non-removable;
> + status = "okay";
> +};
> +
> +&usdhc2 {
> + pinctrl-names = "default", "state_100mhz", "state_200mhz";
> + pinctrl-0 = <&pinctrl_usdhc2>;
> + pinctrl-1 = <&pinctrl_usdhc2_100mhz>;
> + pinctrl-2 = <&pinctrl_usdhc2_200mhz>;
> + bus-width = <4>;
> + vmmc-supply = <&reg_3v3_wifi>;
> + broken-cd;
> + disable-wp;
> + cap-sdio-irq;
> + keep-power-in-suspend;
> + wakeup-source;
> + status = "okay";
> +};
> +
> +&wdog1 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_wdog>;
> + fsl,ext-reset-output;
> + status = "okay";
> +};
> --
> 2.20.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel