Re: [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard

From: Krzysztof Kozlowski
Date: Thu Sep 22 2022 - 08:45:17 EST

Next message: xinlei.lee: "[PATCH v8,0/3] Add dpi output format control for MT8186"
Previous message: Chen Zhongjin: "[PATCH -next v2] kcov: Switch to use list_for_each_entry() helper"
In reply to: Quentin Schulz: "Re: [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard"
Next in thread: Quentin Schulz: "Re: [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 22/09/2022 12:12, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
>
> The PX30-uQ7 (Ringneck) SoM is a µQseven-compatible (40mmx70mm, MXM-230
> connector) system-on-module from Theobroma Systems[1], featuring the
> Rockchip PX30.
>
> It provides the following feature set:
> * up to 4GB DDR4
> * up to 128GB on-module eMMC (with 8-bit 1.8V interface)
> * SD card (on a baseboard) via edge connector
> * Fast Ethernet with on-module TI DP83825I PHY
> * MIPI-DSI/LVDS
> * MIPI-CSI
> * USB
> - 1x USB 2.0 dual-role
> - 3x USB 2.0 host
> * on-module STM32 Cortex-M0 companion controller, implementing:
> - low-power RTC functionality (ISL1208 emulation)
> - fan controller (AMC6821 emulation)
> - USB<->CAN bridge controller
> * on-module Espressif ESP32 for Bluetooth + 2.4GHz WiFi
> * on-module NXP SE05x Secure Element
>
> [1] https://www.theobroma-systems.com/som-product/px30-%C2%B5q7/
>
> Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/rockchip/Makefile | 1 +
> .../dts/rockchip/px30-ringneck-haikou.dts | 239 ++++++++++++
> .../boot/dts/rockchip/px30-ringneck.dtsi | 347 ++++++++++++++++++
> 3 files changed, 587 insertions(+)
> create mode 100644 arch/arm64/boot/dts/rockchip/px30-ringneck-haikou.dts
> create mode 100644 arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
>
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index ef79a672804a..1e3b9d7c1a8b 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -3,6 +3,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += px30-evb.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += px30-engicam-px30-core-ctouch2.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += px30-engicam-px30-core-ctouch2-of10.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += px30-engicam-px30-core-edimm2.2.dtb
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += px30-ringneck-haikou.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-evb.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-roc-cc.dtb
> dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3308-rock-pi-s.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck-haikou.dts b/arch/arm64/boot/dts/rockchip/px30-ringneck-haikou.dts
> new file mode 100644
> index 000000000000..6304d6f7baa3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/px30-ringneck-haikou.dts
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
> + */
> +
> +/dts-v1/;
> +#include "px30-ringneck.dtsi"
> +#include <dt-bindings/input/input.h>
> +
> +/ {
> + model = "Theobroma Systems PX30-µQ7 SoM on Haikou devkit";
> + compatible = "tsd,px30-ringneck-haikou", "rockchip,px30";
> +
> + aliases {
> + mmc2 = &sdmmc;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +
> + gpio_keys: haikou-keys {

By convention node name is gpio-keys

> + compatible = "gpio-keys";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + pinctrl-0 = <&haikou_keys_pin>;
> + pinctrl-names = "default";
> +
> + BATLOWn {

Does not look like you tested the DTS against bindings. Please run `make
dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst
for instructions).

Missing generic node name, only lowercase characters.

> + label = "BATLOW#";
> + linux,code = <KEY_BATTERY>;
> + gpios = <&gpio3 RK_PA7 GPIO_ACTIVE_LOW>;
> + };
> +
> + BIOS_DISABLEn {
> + label = "BIOS_DISABLE#";
> + linux,code = <SW_BOOT_ALT>;
> + linux,input-type = <EV_SW>;
> + gpios = <&gpio2 RK_PC2 GPIO_ACTIVE_HIGH>;
> + };
> +
> + LID_BTNn {
> + label = "LID_BTN#";
> + linux,code = <SW_LID>;
> + linux,input-type = <EV_SW>;
> + gpios = <&gpio3 RK_PA6 GPIO_ACTIVE_LOW>;
> + };
> +
> + SLP_BTNn {
> + label = "SLP_BTN#";
> + linux,code = <KEY_SLEEP>;
> + gpios = <&gpio3 RK_PA5 GPIO_ACTIVE_LOW>;
> + };
> +
> + WAKEn {
> + label = "WAKE#";
> + linux,code = <KEY_WAKEUP>;
> + gpios = <&gpio3 RK_PA4 GPIO_ACTIVE_LOW>;
> + wakeup-source;
> + };
> + };
> +
> + leds {
> + pinctrl-0 = <&module_led_pin>, <&sd_card_led_pin>;
> +
> + sd_card_led: led-1 {
> + label = "sd_card_led";
> + gpios = <&gpio3 RK_PB3 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "mmc2";
> + };
> + };
> +
> + i2s0-sound {
> + compatible = "simple-audio-card";
> + simple-audio-card,format = "i2s";
> + simple-audio-card,name = "Haikou,I2S-codec";
> + simple-audio-card,mclk-fs = <512>;
> +
> + simple-audio-card,codec {
> + clocks = <&sgtl5000_clk>;
> + sound-dai = <&sgtl5000>;
> + };
> +
> + simple-audio-card,cpu {
> + bitclock-master;
> + frame-master;
> + sound-dai = <&i2s0_8ch>;
> + };
> + };
> +
> + sgtl5000_clk: sgtl5000-oscillator {
> + compatible = "fixed-clock";
> + #clock-cells = <0>;
> + clock-frequency = <24576000>;
> + };
> +
> + dc_12v: dc-12v {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

So at least generic ("regulator") prefix or suffix.

> + compatible = "regulator-fixed";
> + regulator-name = "dc_12v";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <12000000>;
> + regulator-max-microvolt = <12000000>;
> + };
> +
> + vcc3v3_baseboard: vcc3v3-baseboard {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc3v3_baseboard";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <&dc_12v>;
> + };
> +
> + vcc5v0_baseboard: vcc5v0-baseboard {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc5v0_baseboard";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + vin-supply = <&dc_12v>;
> + };
> +
> + vdda_codec: vdda-codec {
> + compatible = "regulator-fixed";
> + regulator-name = "vdda_codec";
> + regulator-boot-on;
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + vin-supply = <&vcc5v0_baseboard>;
> + };
> +
> + vddd_codec: vddd-codec {
> + compatible = "regulator-fixed";
> + regulator-name = "vddd_codec";
> + regulator-boot-on;
> + regulator-min-microvolt = <1600000>;
> + regulator-max-microvolt = <1600000>;
> + vin-supply = <&vcc5v0_baseboard>;
> + };
> +};
> +
> +&i2c2 {
> + status = "okay";
> + clock-frequency = <400000>;
> +
> + sgtl5000: codec@a {
> + compatible = "fsl,sgtl5000";
> + reg = <0x0a>;
> + clocks = <&sgtl5000_clk>;
> + #sound-dai-cells = <0>;
> + VDDA-supply = <&vdda_codec>;
> + VDDIO-supply = <&vcc3v3_baseboard>;
> + VDDD-supply = <&vddd_codec>;
> + status = "okay";

Okay is by default.

> + };
> +};
> +
> +&i2c3 {
> + status = "okay";
> +
> + eeprom@50 {
> + reg = <0x50>;
> + compatible = "atmel,24c01";
> + pagesize = <8>;
> + size = <128>;
> + vcc-supply = <&vcc3v3_baseboard>;
> + };
> +};
> +
> +&i2s0_8ch {
> + status = "okay";
> +};
> +
> +&gmac {
> + status = "okay";
> +};
> +
> +&pinctrl {
> + haikou {
> + haikou_keys_pin: haikou-keys-pin {
> + rockchip,pins =
> + /* WAKE# */
> + <3 RK_PA4 RK_FUNC_GPIO &pcfg_pull_up>,
> + /* SLP_BTN# */
> + <3 RK_PA5 RK_FUNC_GPIO &pcfg_pull_up>,
> + /* LID_BTN */
> + <3 RK_PA6 RK_FUNC_GPIO &pcfg_pull_up>,
> + /* BATLOW# */
> + <3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>,
> + /* BIOS_DISABLE# */
> + <2 RK_PC2 RK_FUNC_GPIO &pcfg_pull_up>;
> + };
> + };
> +
> + leds {
> + sd_card_led_pin: sd-card-led-pin {
> + rockchip,pins =
> + <3 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> +};
> +
> +&pwm0 {
> + status = "okay";
> +};
> +
> +&sdmmc {
> + bus-width = <4>;
> + cap-mmc-highspeed;
> + cap-sd-highspeed;
> + cd-gpios = <&gpio0 RK_PA3 GPIO_ACTIVE_LOW>;
> + disable-wp;
> + vmmc-supply = <&vcc3v3_baseboard>;
> + status = "okay";
> +};
> +
> +&spi1 {
> + status = "okay";
> +};
> +
> +&u2phy_otg {
> + status = "okay";
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&uart5 {
> + pinctrl-0 = <&uart5_xfer>;
> + status = "okay";
> +};
> +
> +&usb20_otg {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> new file mode 100644
> index 000000000000..74a7f1182c21
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/px30-ringneck.dtsi
> @@ -0,0 +1,347 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
> + */
> +
> +/dts-v1/;
> +#include "px30.dtsi"
> +
> +/ {
> + aliases {
> + mmc0 = &emmc;
> + mmc1 = &sdio;
> + rtc0 = &rtc_twi;
> + rtc1 = &rk809;
> + };
> +
> + emmc_pwrseq: emmc-pwrseq {
> + compatible = "mmc-pwrseq-emmc";
> + pinctrl-0 = <&emmc_reset>;
> + pinctrl-names = "default";
> + reset-gpios = <&gpio1 RK_PB3 GPIO_ACTIVE_HIGH>;
> + };
> +
> + leds {
> + compatible = "gpio-leds";

This belongs to DTSI.

> + pinctrl-names = "default";
> + pinctrl-0 = <&module_led_pin>;
> + status = "okay";

No need.

> +
> + module_led: led-0 {

Why DTSI - which in logic comes first - starts with 1 and this is 0?\

> + label = "module_led";
> + gpios = <&gpio1 RK_PB0 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "heartbeat";
> + };
> + };
> +
> + vcc5v0_sys: vccsys {

Node names should be generic.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation

> + compatible = "regulator-fixed";
> + regulator-name = "vcc5v0_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +};
> +
> +&cpu0 {
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu1 {
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu2 {
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&cpu3 {
> + cpu-supply = <&vdd_arm>;
> +};
> +
> +&emmc {
> + bus-width = <8>;
> + cap-mmc-highspeed;
> + mmc-hs200-1_8v;
> + supports-emmc;
> + mmc-pwrseq = <&emmc_pwrseq>;
> + non-removable;
> + vmmc-supply = <&vcc_3v3>;
> + vqmmc-supply = <&vcc_emmc>;
> +
> + status = "okay";
> +};
> +
> +/* On-module TI DP83825I PHY but no connector, enable in carrierboard */
> +&gmac {
> + snps,reset-gpio = <&gpio3 RK_PB0 GPIO_ACTIVE_LOW>;
> + snps,reset-active-low;
> + snps,reset-delays-us = <0 50000 50000>;
> + phy-supply = <&vcc_3v3>;
> + clock_in_out = "output";
> +};
> +
> +&gpu {
> + status = "okay";
> +};
> +
> +&i2c0 {
> + status = "okay";
> +
> + rk809: pmic@20 {
> + compatible = "rockchip,rk809";
> + reg = <0x20>;
> + interrupt-parent = <&gpio0>;
> + interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> + pinctrl-0 = <&pmic_int>;
> + pinctrl-names = "default";
> + #clock-cells = <0>;
> + clock-output-names = "xin32k";
> + rockchip,system-power-controller;
> + wakeup-source;
> + status = "okay";

No need.

> +
> + vcc1-supply = <&vcc5v0_sys>;
> + vcc2-supply = <&vcc5v0_sys>;
> + vcc3-supply = <&vcc5v0_sys>;
> + vcc4-supply = <&vcc5v0_sys>;
> + vcc5-supply = <&vcc_3v3>;
> + vcc6-supply = <&vcc_3v3>;
> + vcc7-supply = <&vcc_3v3>;
> + vcc9-supply = <&vcc5v0_sys>;
> +

(...)

> +&pinctrl {
> + emmc {
> + emmc_reset: emmc-reset {
> + rockchip,pins = <1 RK_PB3 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> +
> + leds {
> + module_led_pin: module-led-pin {
> + rockchip,pins = <1 RK_PB0 RK_FUNC_GPIO &pcfg_pull_none>;
> + };
> + };
> +
> + pmic {
> + pmic_int: pmic_int {

No underscores in node names.

> + rockchip,pins =
> + <0 RK_PA7 RK_FUNC_GPIO &pcfg_pull_up>;
> + };
> + };
> +
> +

Skip unneeded blank lines.

> +};
> +
> +&tsadc {
> + status = "okay";
> +};
> +
> +&u2phy {
> + status = "okay";
> +};
> +
> +&u2phy_host {
> + status = "okay";
> +};
> +
> +/* Mule UCAN */
> +&usb_host0_ehci {
> + status = "okay";
> +};
> +
> +&usb_host0_ohci {
> + status = "okay";
> +};
> +
> +&wdt {
> + status = "okay";
> +};

Best regards,
Krzysztof

Next message: xinlei.lee: "[PATCH v8,0/3] Add dpi output format control for MT8186"
Previous message: Chen Zhongjin: "[PATCH -next v2] kcov: Switch to use list_for_each_entry() helper"
In reply to: Quentin Schulz: "Re: [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard"
Next in thread: Quentin Schulz: "Re: [PATCH 2/3] arm64: dts: rockchip: add PX30-µQ7 (Ringneck) SoM with Haikou baseboard"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]