dt-bindings: arm64: mediatek: add Kontron 3.5"-SBC-i1200
Subsystems (2)
PATCH v2 1/2 dt-bindings: arm64: mediatek: add Kontron 3.5"-SBC-i1200
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
---
v2:
- convert enum to const as there is only one specific board
Documentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Documentation/devicetree/bindings/arm/mediatek.yaml +5 -0
@@ -357,6 +357,11 @@ properties: - radxa,nio-12l - const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200 + - const: mediatek,mt8395 + - const: mediatek,mt8195 - items: - enum: - mediatek,mt8516-pumpkin
Signature · 1 line
2.39.2
PATCH v2 2/2 arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Add basic support for the Kontron 3.5" single board computer featuring a Mediatek i1200 SoC (MT8395/MT8195).
---
v2:
- none
arch/arm64/boot/dts/mediatek/Makefile | 1 + .../mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1091 +++++++++++++++++ 2 files changed, 1092 insertions(+) create mode 100644 arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts
arch/arm64/boot/dts/mediatek/Makefile +1 -0
@@ -76,5 +76,6 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-demo.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-genio-1200-evk.dtb +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-kontron-3-5-sbc-i1200.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-radxa-nio-12l.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtb
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +1091 -0
1,091 lines not on this page · raw message
Signature · 1 line
2.39.2
AngeloGioacchino Del Regno
maintainerRe: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Il 19/02/24 09:44, Michael Walle ha scritto:
Quoted 283 lines
Add basic support for the Kontron 3.5" single board computer featuring a Mediatek i1200 SoC (MT8395/MT8195).
Signed-off-by Michael Walle <[email protected]>---
v2:
- nonearch/arm64/boot/dts/mediatek/Makefile | 1 + .../mediatek/mt8395-kontron-3-5-sbc-i1200.dts | 1091 +++++++++++++++++ 2 files changed, 1092 insertions(+)create mode 100644
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dtsarch/arm64/boot/dts/mediatek/Makefile +1
-0@@ -76,5 +76,6 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-demo.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8195-evb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8365-evk.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-genio-1200-evk.dtb +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-kontron-3-5-sbc-i1200.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8395-radxa-nio-12l.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt8516-pumpkin.dtbarch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +253
-0@@ -0,0 +1,1091 @@ +// SPDX-License-Identifier: (GPL-2.0 OR MIT) +/* + * Copyright (C) 2024 Kontron Europe GmbH + * + * Author: Michael Walle <[email protected]> + */ +/dts-v1/; + +#include "mt8195.dtsi" +#include "mt6359.dtsi" + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/input/input.h> +#include <dt-bindings/leds/common.h> +#include <dt-bindings/pinctrl/mt8195-pinfunc.h> +#include <dt-bindings/regulator/mediatek,mt6360-regulator.h> +#include <dt-bindings/spmi/spmi.h> + +/ { + model = "Kontron 3.5\"-SBC-i1200"; + compatible = "kontron,3-5-sbc-i1200", "mediatek,mt8395", "mediatek,mt8195"; + + aliases { + mmc0 = &mmc0; + mmc1 = &mmc1; + serial0 = &uart1; + serial1 = &uart2; + serial2 = &uart3; + serial3 = &uart4; + serial4 = &uart0; + }; + + chosen { + stdout-path = "serial0:115200n8"; + }; + + firmware { + optee { + compatible = "linaro,optee-tz"; + method = "smc"; + }; + }; + + gpio-keys { + compatible = "gpio-keys"; + pinctrl-names = "default"; + pinctrl-0 = <&gpio_keys_pins>; + + key-0 { + gpios = <&pio 106 GPIO_ACTIVE_LOW>; + label = "volume_up"; + linux,code = <KEY_VOLUMEUP>; + wakeup-source; + debounce-interval = <15>; + }; + }; + + leds { + compatible = "gpio-leds"; + pinctrl-names = "default"; + pinctrl-0 = <&led_pins>; + + led-0 { + gpios = <&pio 107 GPIO_ACTIVE_HIGH>; + default-state = "keep"; + function = LED_FUNCTION_POWER; + color = <LED_COLOR_ID_GREEN>; + }; + }; + + memory@40000000 { + device_type = "memory"; + reg = <0 0x40000000 0x0 0x80000000>; + }; + + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + /* + * 12 MiB reserved for OP-TEE (BL32) + * +-----------------------+ 0x43e0_0000 + * | SHMEM 2MiB | + * +-----------------------+ 0x43c0_0000 + * | | TA_RAM 8MiB | + * + TZDRAM +--------------+ 0x4340_0000 + * | | TEE_RAM 2MiB | + * +-----------------------+ 0x4320_0000 + */ + optee_reserved: optee@43200000 { + no-map; + reg = <0 0x43200000 0 0x00c00000>; + }; + + scp_mem: memory@50000000 { + compatible = "shared-dma-pool"; + reg = <0 0x50000000 0 0x2900000>; + no-map; + }; + + vpu_mem: memory@53000000 { + compatible = "shared-dma-pool"; + reg = <0 0x53000000 0 0x1400000>; /* 20 MB */ + }; + + /* 2 MiB reserved for ARM Trusted Firmware (BL31) */ + bl31_secmon_mem: memory@54600000 { + no-map; + reg = <0 0x54600000 0x0 0x200000>; + }; + + snd_dma_mem: memory@60000000 { + compatible = "shared-dma-pool"; + reg = <0 0x60000000 0 0x1100000>; + no-map; + }; + + apu_mem: memory@62000000 { + compatible = "shared-dma-pool"; + reg = <0 0x62000000 0 0x1400000>; /* 20 MB */ + }; + }; + + thermal_sensor0: thermal-sensor-0 { + compatible = "generic-adc-thermal"; + #thermal-sensor-cells = <0>; + io-channels = <&auxadc 0>; + io-channel-names = "sensor-channel"; + temperature-lookup-table = <(-25000) 1474 + (-20000) 1374 + (-15000) 1260 + (-10000) 1134 + (-5000) 1004 + 0 874 + 5000 750 + 10000 635 + 15000 532 + 20000 443 + 25000 367 + 30000 303 + 35000 250 + 40000 206 + 45000 170 + 50000 141 + 55000 117 + 60000 97 + 65000 81 + 70000 68 + 75000 57 + 80000 48 + 85000 41 + 90000 35 + 95000 30 + 100000 25 + 105000 22 + 110000 19 + 115000 16 + 120000 14 + 125000 12 + 130000 10 + 135000 9 + 140000 8 + 145000 7 + 150000 6>; + }; + + thermal_sensor1: thermal-sensor-1 { + compatible = "generic-adc-thermal"; + #thermal-sensor-cells = <0>; + io-channels = <&auxadc 1>; + io-channel-names = "sensor-channel"; + temperature-lookup-table = <(-25000) 1474 + (-20000) 1374 + (-15000) 1260 + (-10000) 1134 + (-5000) 1004 + 0 874 + 5000 750 + 10000 635 + 15000 532 + 20000 443 + 25000 367 + 30000 303 + 35000 250 + 40000 206 + 45000 170 + 50000 141 + 55000 117 + 60000 97 + 65000 81 + 70000 68 + 75000 57 + 80000 48 + 85000 41 + 90000 35 + 95000 30 + 100000 25 + 105000 22 + 110000 19 + 115000 16 + 120000 14 + 125000 12 + 130000 10 + 135000 9 + 140000 8 + 145000 7 + 150000 6>; + }; + + thermal_sensor2: thermal-sensor-2 { + compatible = "generic-adc-thermal"; + #thermal-sensor-cells = <0>; + io-channels = <&auxadc 2>; + io-channel-names = "sensor-channel"; + temperature-lookup-table = <(-25000) 1474 + (-20000) 1374 + (-15000) 1260 + (-10000) 1134 + (-5000) 1004 + 0 874 + 5000 750 + 10000 635 + 15000 532 + 20000 443 + 25000 367 + 30000 303 + 35000 250 + 40000 206 + 45000 170 + 50000 141 + 55000 117 + 60000 97 + 65000 81 + 70000 68 + 75000 57 + 80000 48 + 85000 41 + 90000 35 + 95000 30 + 100000 25 + 105000 22 + 110000 19 + 115000 16 + 120000 14 + 125000 12 + 130000 10 + 135000 9 + 140000 8 + 145000 7 + 150000 6>; + }; +};
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 263
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ +&auxadc { + status = "okay"; +}; + +ð { + phy-mode ="rgmii-id"; + phy-handle = <ðernet_phy0>; + snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>; + snps,reset-delays-us = <0 10000 80000>;
snps,reset-delays-us and snps,reset-gpio are deprecated.
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 270
Quoted 7 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +7
-0+ pinctrl-names = "default", "sleep"; + pinctrl-0 = <ð_default_pins>; + pinctrl-1 = <ð_sleep_pins>; + status = "okay"; + + mdio { + ethernet_phy0: ethernet-phy@1 {
compatible = "is there any applicable compatible?"P.S.: if you've got the usual rtl8211f, should be "ethernet-phy-id001c.c916"
reg = <0x1>;
interrupts-extended = <&pio 94 IRQ_TYPE_LEVEL_LOW>; reset-assert-us = <10000>; reset-deassert-us = <80000>;
reset-gpios = <&pio 93 GPIO_ACTIVE_HIGH>;
Quoted 6 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +6
-0+ reg = <0x1>; + interrupts-extended = <&pio 94 IRQ_TYPE_LEVEL_LOW>; + }; + }; +}; +
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 286
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+&gpu { + status = "okay"; + mali-supply = <&mt6315_7_vbuck1>; +}; + +&i2c2 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c2_pins>; + clock-frequency = <400000>; + status = "okay";
Are i2c2,3,4 exposed as pins somewhere? If they are, can you please put a comment saying so?
This is not mandatory, but IMO it's nice to have as a kind of documentation.
Quoted 18 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +18
-0+}; + +&i2c3 { + pinctrl-names = "default"; + pinctrl-0 = <&i2c3_pins>; + clock-frequency = <100000>; + status = "okay"; +}; + +&i2c4 { + clock-frequency = <100000>; + pinctrl-0 = <&i2c4_pins>; + pinctrl-names = "default"; + status = "okay"; +}; + +&i2c6 { + clock-frequency = <400000>;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 314
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ pinctrl-0 = <&i2c6_pins>; + pinctrl-names = "default"; + status = "okay"; + + mt6360: pmic@34 { + compatible = "mediatek,mt6360"; + reg = <0x34>; + interrupt-controller; + interrupts-extended = <&pio 101 IRQ_TYPE_EDGE_FALLING>; + interrupt-names = "IRQB";
#interrupt-cells = <1>;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 321
Quoted 7 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +7
-0+ + charger { + compatible = "mediatek,mt6360-chg"; + richtek,vinovp-microvolt = <14500000>; + + otg_vbus_regulator: usb-otg-vbus-regulator { + regulator-compatible = "usb-otg-vbus";
The regulator-compatible property is deprecated. Please don't use it.
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 333
Quoted 12 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +12
-0+ regulator-name = "usb-otg-vbus"; + regulator-min-microvolt = <4425000>; + regulator-max-microvolt = <5825000>; + }; + }; + + regulator { + compatible = "mediatek,mt6360-regulator"; + LDO_VIN3-supply = <&mt6360_buck2>; + + mt6360_buck1: buck1 { + regulator-compatible = "BUCK1";
same here and everywhere else.
Quoted 69 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +69
-0+ regulator-name = "mt6360,buck1"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <1300000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP + MT6360_OPMODE_ULP>; + regulator-always-on; + }; + + mt6360_buck2: buck2 { + regulator-compatible = "BUCK2"; + regulator-name = "mt6360,buck2"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <1300000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP + MT6360_OPMODE_ULP>; + regulator-always-on; + }; + + mt6360_ldo1: ldo1 { + regulator-compatible = "LDO1"; + regulator-name = "mt6360,ldo1"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo2: ldo2 { + regulator-compatible = "LDO2"; + regulator-name = "mt6360,ldo2"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo3: ldo3 { + regulator-compatible = "LDO3"; + regulator-name = "mt6360,ldo3"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo5: ldo5 { + regulator-compatible = "LDO5"; + regulator-name = "mt6360,ldo5"; + regulator-min-microvolt = <2700000>; + regulator-max-microvolt = <3600000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo6: ldo6 { + regulator-compatible = "LDO6"; + regulator-name = "mt6360,ldo6"; + regulator-min-microvolt = <500000>; + regulator-max-microvolt = <2100000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + }; + + mt6360_ldo7: ldo7 { + regulator-compatible = "LDO7"; + regulator-name = "mt6360,ldo7"; + regulator-min-microvolt = <500000>;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 412
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ regulator-max-microvolt = <2100000>; + regulator-allowed-modes = <MT6360_OPMODE_NORMAL + MT6360_OPMODE_LP>; + regulator-always-on; + }; + }; + }; +}; + +&mmc0 {
This is not wrong, but can we please keep the ordering consistent between boards?
pinctrl-names = "default", "state_uhs"; pinctrl-0 = <&mmc0_default_pins>; pinctrl-1 = <&mmc0_uhs_pins>; bus-width = <8>; max-frequency = <200000000>; hs400-ds-delay = <0x14c11>; cap-mmc-highspeed; cap-mmc-hw-reset; mmc-hs200-1_8v; mmc-hs400-1_8v; no-sdio; no-sd; non-removable; vmmc-supply = <&mt6359_vemc_1_ldo_reg>; vqmmc-supply = <&mt6359_vufs_ldo_reg>;
Quoted 20 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +20
-0+ pinctrl-names = "default", "state_uhs"; + pinctrl-0 = <&mmc0_default_pins>; + pinctrl-1 = <&mmc0_uhs_pins>; + bus-width = <8>; + max-frequency = <200000000>; + cap-mmc-highspeed; + mmc-hs200-1_8v; + mmc-hs400-1_8v; + cap-mmc-hw-reset; + no-sdio; + no-sd; + hs400-ds-delay = <0x14c11>; + vmmc-supply = <&mt6359_vemc_1_ldo_reg>; + vqmmc-supply = <&mt6359_vufs_ldo_reg>; + non-removable; + status = "okay"; +}; + +&mmc1 { + pinctrl-names = "default", "state_uhs";
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 442
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ pinctrl-0 = <&mmc1_default_pins>; + pinctrl-1 = <&mmc1_uhs_pins>; + cd-gpios = <&pio 129 GPIO_ACTIVE_LOW>; + bus-width = <4>; + max-frequency = <200000000>; + cap-sd-highspeed; + sd-uhs-sdr50; + sd-uhs-sdr104; + vmmc-supply = <&mt6360_ldo5>; + vqmmc-supply = <&mt6360_ldo3>;
Does mmc1 support eMMC and SDIO?
If not, no-mmc; no-sdio;
Quoted 82 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +82
-0+ status = "okay"; +}; + +&mt6359_vbbck_ldo_reg { + regulator-always-on; +}; + +&mt6359_vcore_buck_reg { + regulator-always-on; +}; + +&mt6359_vgpu11_buck_reg { + regulator-always-on; +}; + +&mt6359_vproc1_buck_reg { + regulator-always-on; +}; + +&mt6359_vproc2_buck_reg { + regulator-always-on; +}; + +&mt6359_vpu_buck_reg { + regulator-always-on; +}; + +&mt6359_vrf12_ldo_reg { + regulator-always-on; +}; + +&mt6359_vsram_md_ldo_reg { + regulator-always-on; +}; + +&mt6359_vsram_others_ldo_reg { + regulator-always-on; +}; + +&nor_flash { + pinctrl-names = "default"; + pinctrl-0 = <&nor_pins_default>; + status = "okay"; + + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <52000000>; + spi-rx-bus-width = <2>; + spi-tx-bus-width = <2>; + }; +}; + +&pcie0 { + pinctrl-names = "default"; + pinctrl-0 = <&pcie0_pins_default>; + status = "okay"; +}; + +&pcie1 { + pinctrl-names = "default"; + pinctrl-0 = <&pcie1_pins_default>; + status = "okay"; +}; + +&pciephy { + status = "okay"; +}; + +&pio { + eth_default_pins: eth-default-pins { + pins-txd { + pinmux = <PINMUX_GPIO77__FUNC_GBE_TXD3>, + <PINMUX_GPIO78__FUNC_GBE_TXD2>, + <PINMUX_GPIO79__FUNC_GBE_TXD1>, + <PINMUX_GPIO80__FUNC_GBE_TXD0>; + drive-strength = <MTK_DRIVE_8mA>; + }; + + pins-rxd { + pinmux = <PINMUX_GPIO81__FUNC_GBE_RXD3>, + <PINMUX_GPIO82__FUNC_GBE_RXD2>,
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 534
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ <PINMUX_GPIO83__FUNC_GBE_RXD1>, + <PINMUX_GPIO84__FUNC_GBE_RXD0>; + }; + + pins-cc { + pinmux = <PINMUX_GPIO85__FUNC_GBE_TXC>, + <PINMUX_GPIO86__FUNC_GBE_RXC>, + <PINMUX_GPIO87__FUNC_GBE_RXDV>, + <PINMUX_GPIO88__FUNC_GBE_TXEN>; + drive-strength = <MTK_DRIVE_8mA>;
s/MTK_DRIVE//g
s/mA//g
drive-strength = <8>;
Please, here and everywhere else, for all values - let's stop using those MTK_DRIVE_(x)mA definitions, they're just defined as (x), where anyway the drive-strength property is in milliamps by default.
We don't need these definitions.
Quoted 85 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +85
-0+ }; + + pins-mdio { + pinmux = <PINMUX_GPIO89__FUNC_GBE_MDC>, + <PINMUX_GPIO90__FUNC_GBE_MDIO>; + input-enable; + }; + + pins-power { + pinmux = <PINMUX_GPIO91__FUNC_GPIO91>, + <PINMUX_GPIO92__FUNC_GPIO92>; + output-high; + }; + + pins-reset { + pinmux = <PINMUX_GPIO93__FUNC_GPIO93>; + output-high; + }; + + pins-interrupt { + pinmux = <PINMUX_GPIO94__FUNC_GPIO94>; + input-enable; + }; + }; + + eth_sleep_pins: eth-sleep-pins { + pins-txd { + pinmux = <PINMUX_GPIO77__FUNC_GPIO77>, + <PINMUX_GPIO78__FUNC_GPIO78>, + <PINMUX_GPIO79__FUNC_GPIO79>, + <PINMUX_GPIO80__FUNC_GPIO80>; + }; + pins-cc { + pinmux = <PINMUX_GPIO85__FUNC_GPIO85>, + <PINMUX_GPIO88__FUNC_GPIO88>, + <PINMUX_GPIO87__FUNC_GPIO87>, + <PINMUX_GPIO86__FUNC_GPIO86>; + }; + pins-rxd { + pinmux = <PINMUX_GPIO81__FUNC_GPIO81>, + <PINMUX_GPIO82__FUNC_GPIO82>, + <PINMUX_GPIO83__FUNC_GPIO83>, + <PINMUX_GPIO84__FUNC_GPIO84>; + }; + pins-mdio { + pinmux = <PINMUX_GPIO89__FUNC_GPIO89>, + <PINMUX_GPIO90__FUNC_GPIO90>; + input-disable; + bias-disable; + }; + }; + + gpio_keys_pins: gpio-keys-pins { + pins { + pinmux = <PINMUX_GPIO106__FUNC_GPIO106>; + input-enable; + }; + }; + + i2c2_pins: i2c2-default-pins { + pins-bus { + pinmux = <PINMUX_GPIO12__FUNC_SDA2>, + <PINMUX_GPIO13__FUNC_SCL2>; + bias-pull-up; + }; + }; + + i2c3_pins: i2c3-pins { + pins { + pinmux = <PINMUX_GPIO14__FUNC_SDA3>, + <PINMUX_GPIO15__FUNC_SCL3>; + bias-pull-up; + }; + }; + + i2c4_pins: i2c4-pins { + pins { + pinmux = <PINMUX_GPIO16__FUNC_SDA4>, + <PINMUX_GPIO17__FUNC_SCL4>; + bias-pull-up; + }; + }; + + i2c6_pins: i2c6-pins { + pins {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 629
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ pinmux = <PINMUX_GPIO25__FUNC_SDA6>, + <PINMUX_GPIO26__FUNC_SCL6>; + bias-pull-up; + }; + }; + + mmc0_default_pins: mmc0-default-pins { + pins-clk { + pinmux = <PINMUX_GPIO122__FUNC_MSDC0_CLK>; + drive-strength = <MTK_DRIVE_6mA>;
drive-strength = <6>; etc :-)
Quoted 73 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +73
-0+ bias-pull-down = <MTK_PUPD_SET_R1R0_10>; + }; + + pins-cmd-dat { + pinmux = <PINMUX_GPIO126__FUNC_MSDC0_DAT0>, + <PINMUX_GPIO125__FUNC_MSDC0_DAT1>, + <PINMUX_GPIO124__FUNC_MSDC0_DAT2>, + <PINMUX_GPIO123__FUNC_MSDC0_DAT3>, + <PINMUX_GPIO119__FUNC_MSDC0_DAT4>, + <PINMUX_GPIO118__FUNC_MSDC0_DAT5>, + <PINMUX_GPIO117__FUNC_MSDC0_DAT6>, + <PINMUX_GPIO116__FUNC_MSDC0_DAT7>, + <PINMUX_GPIO121__FUNC_MSDC0_CMD>; + input-enable; + drive-strength = <MTK_DRIVE_6mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + + pins-rst { + pinmux = <PINMUX_GPIO120__FUNC_MSDC0_RSTB>; + drive-strength = <MTK_DRIVE_6mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + }; + + mmc0_uhs_pins: mmc0-uhs-pins { + pins-clk { + pinmux = <PINMUX_GPIO122__FUNC_MSDC0_CLK>; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-down = <MTK_PUPD_SET_R1R0_10>; + }; + + pins-cmd-dat { + pinmux = <PINMUX_GPIO126__FUNC_MSDC0_DAT0>, + <PINMUX_GPIO125__FUNC_MSDC0_DAT1>, + <PINMUX_GPIO124__FUNC_MSDC0_DAT2>, + <PINMUX_GPIO123__FUNC_MSDC0_DAT3>, + <PINMUX_GPIO119__FUNC_MSDC0_DAT4>, + <PINMUX_GPIO118__FUNC_MSDC0_DAT5>, + <PINMUX_GPIO117__FUNC_MSDC0_DAT6>, + <PINMUX_GPIO116__FUNC_MSDC0_DAT7>, + <PINMUX_GPIO121__FUNC_MSDC0_CMD>; + input-enable; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + + pins-ds { + pinmux = <PINMUX_GPIO127__FUNC_MSDC0_DSL>; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-down = <MTK_PUPD_SET_R1R0_10>; + }; + + pins-rst { + pinmux = <PINMUX_GPIO120__FUNC_MSDC0_RSTB>; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + }; + + mmc1_default_pins: mmc1-default-pins { + pins-clk { + pinmux = <PINMUX_GPIO111__FUNC_MSDC1_CLK>; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-down = <MTK_PUPD_SET_R1R0_10>; + }; + + pins-cmd-dat { + pinmux = <PINMUX_GPIO110__FUNC_MSDC1_CMD>, + <PINMUX_GPIO112__FUNC_MSDC1_DAT0>, + <PINMUX_GPIO113__FUNC_MSDC1_DAT1>, + <PINMUX_GPIO114__FUNC_MSDC1_DAT2>, + <PINMUX_GPIO115__FUNC_MSDC1_DAT3>;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 712
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ input-enable; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + + pins-insert { + pinmux = <PINMUX_GPIO129__FUNC_GPIO129>; + bias-pull-up; + }; + };
MMC1 UHS and DEFAULT are exactly the same, you don't need to define those twice. Here's how we can clean up this block:
mmc1_default_pins: mmc1-default-pins {
pins-clk {
pinmux = <PINMUX_GPIO111__FUNC_MSDC1_CLK>;
bias-pull-down = <MTK_PUPD_SET_R1R0_10>;
drive-strength = <8>;
};
pins-cmd-dat {
pinmux = <PINMUX_GPIO110__FUNC_MSDC1_CMD>,
<PINMUX_GPIO112__FUNC_MSDC1_DAT0>,
<PINMUX_GPIO113__FUNC_MSDC1_DAT1>,
<PINMUX_GPIO114__FUNC_MSDC1_DAT2>,
<PINMUX_GPIO115__FUNC_MSDC1_DAT3>;
bias-pull-up = <MTK_PUPD_SET_R1R0_01>;
drive-strength = <8>;
input-enable;
};
};
mmc1_pins_detect: mmc1-detect-pins {
pins-insert {
pinmux = <PINMUX_GPIO129__FUNC_GPIO129>;
bias-pull-up;
};
};&mmc1 {
pinctrl-names = "default", "state_uhs";
pinctrl-0 = <&mmc1_default_pins>, <&mmc1_pins_detect>;
pinctrl-1 = <&mmc1_default_pins>;
....
}Quoted 81 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +81
-0+ + mmc1_uhs_pins: mmc1-uhs-pins { + pins-clk { + pinmux = <PINMUX_GPIO111__FUNC_MSDC1_CLK>; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-down = <MTK_PUPD_SET_R1R0_10>; + }; + + pins-cmd-dat { + pinmux = <PINMUX_GPIO110__FUNC_MSDC1_CMD>, + <PINMUX_GPIO112__FUNC_MSDC1_DAT0>, + <PINMUX_GPIO113__FUNC_MSDC1_DAT1>, + <PINMUX_GPIO114__FUNC_MSDC1_DAT2>, + <PINMUX_GPIO115__FUNC_MSDC1_DAT3>; + input-enable; + drive-strength = <MTK_DRIVE_8mA>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + }; + + nor_pins_default: nor-default-pins { + pins-ck-io { + pinmux = <PINMUX_GPIO142__FUNC_SPINOR_IO0>, + <PINMUX_GPIO141__FUNC_SPINOR_CK>, + <PINMUX_GPIO143__FUNC_SPINOR_IO1>; + drive-strength = <6>; + bias-pull-down; + }; + + pins-cs { + pinmux = <PINMUX_GPIO140__FUNC_SPINOR_CS>; + drive-strength = <6>; + bias-pull-up; + }; + }; + + pcie0_pins_default: pcie0-default-pins { + pins-bus { + pinmux = <PINMUX_GPIO19__FUNC_WAKEN>, + <PINMUX_GPIO20__FUNC_PERSTN>, + <PINMUX_GPIO21__FUNC_CLKREQN>; + bias-pull-up; + }; + }; + + pcie1_pins_default: pcie1-default-pins { + pins-bus { + pinmux = <PINMUX_GPIO0__FUNC_PERSTN_1>, + <PINMUX_GPIO1__FUNC_CLKREQN_1>, + <PINMUX_GPIO2__FUNC_WAKEN_1>; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + }; + + led_pins: led-pins { + pins-power-en { + pinmux = <PINMUX_GPIO107__FUNC_GPIO107>; + output-high; + }; + }; + + spi0_pins: spi0-default-pins { + pins-cs-mosi-clk { + pinmux = <PINMUX_GPIO132__FUNC_SPIM0_CSB>, + <PINMUX_GPIO134__FUNC_SPIM0_MO>, + <PINMUX_GPIO133__FUNC_SPIM0_CLK>; + bias-disable; + }; + + pins-miso { + pinmux = <PINMUX_GPIO135__FUNC_SPIM0_MI>; + bias-pull-down; + }; + }; + + spi1_pins: spi1-default-pins { + pins-cs-mosi-clk { + pinmux = <PINMUX_GPIO136__FUNC_SPIM1_CSB>, + <PINMUX_GPIO138__FUNC_SPIM1_MO>, + <PINMUX_GPIO137__FUNC_SPIM1_CLK>; + bias-disable;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 803
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ }; + + pins-miso { + pinmux = <PINMUX_GPIO139__FUNC_SPIM1_MI>; + bias-pull-down; + }; + }; + + uart0_pins: uart0-pins { + pins_rx {
No underscores, please.
Quoted 14 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +14
-0+ pinmux = <PINMUX_GPIO99__FUNC_URXD0>; + input-enable; + bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO98__FUNC_UTXD0>; + }; + }; + + uart1_pins: uart1-pins { + pins_rx { + pinmux = <PINMUX_GPIO103__FUNC_URXD1>; + input-enable;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 827
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO102__FUNC_UTXD1>; + }; + + pins_rts { + pinmux = <PINMUX_GPIO100__FUNC_URTS1>; + output-enable;
Are you really sure that you need output-enable here?! RTS is not an output buffer....
I don't think you do. Please double check.
Quoted 65 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +65
-0+ }; + + pins_cts { + pinmux = <PINMUX_GPIO101__FUNC_UCTS1>; + input-enable; + }; + }; + + uart2_pins: uart2-pins { + pins_rx { + pinmux = <PINMUX_GPIO68__FUNC_URXD2>; + input-enable; + bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO67__FUNC_UTXD2>; + }; + + pins_rts { + pinmux = <PINMUX_GPIO66__FUNC_URTS2>; + output-enable; + }; + + pins_cts { + pinmux = <PINMUX_GPIO65__FUNC_UCTS2>; + input-enable; + }; + }; + + uart3_pins: uart3-pins { + pins_rx { + pinmux = <PINMUX_GPIO5__FUNC_URXD3>; + input-enable; + bias-pull-up = <MTK_PUPD_SET_R1R0_01>; + }; + + pins_tx { + pinmux = <PINMUX_GPIO4__FUNC_UTXD3>; + }; + }; + + uart4_pins: uart4-pins { + pins_rx { + pinmux = <PINMUX_GPIO7__FUNC_URXD4>; + input-enable; + bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO6__FUNC_UTXD4>; + }; + }; +}; + +&pmic { + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>; +}; + +&scp { + memory-region = <&scp_mem>; + status = "okay"; +}; + +&spmi {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 902
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ #address-cells = <2>; + #size-cells = <0>; + + mt6315@6 { + compatible = "mediatek,mt6315-regulator"; + reg = <0x6 SPMI_USID>; + + regulators { + mt6315_6_vbuck1: vbuck1 { + regulator-compatible = "vbuck1";
We should check if we can avoid using regulator-compatible on mt6315 as well... I honestly never checked if this is possible, but I think it is.
Eventually, if you can check and verify that it's not needed, we should even remove that from all devicetrees, but that's not entirely relevant for this patch anyway.
I'm just saying my plans out loud :-)
Quoted 34 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +34
-0+ regulator-name = "Vbcpu"; + regulator-min-microvolt = <300000>; + regulator-max-microvolt = <1193750>; + regulator-enable-ramp-delay = <256>; + regulator-ramp-delay = <6250>; + regulator-allowed-modes = <0 1 2>; + regulator-always-on; + }; + }; + }; + + mt6315@7 { + compatible = "mediatek,mt6315-regulator"; + reg = <0x7 SPMI_USID>; + + regulators { + mt6315_7_vbuck1: vbuck1 { + regulator-compatible = "vbuck1"; + regulator-name = "Vgpu"; + regulator-min-microvolt = <625000>; + regulator-max-microvolt = <1193750>; + regulator-enable-ramp-delay = <256>; + regulator-ramp-delay = <6250>; + regulator-allowed-modes = <0 1 2>; + regulator-always-on; + }; + }; + }; +}; + +&spi0 { + pinctrl-names = "default"; + pinctrl-0 = <&spi0_pins>; + mediatek,pad-select = <0>;
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 946
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ status = "okay"; + + tpm: tpm@0 { + compatible = "infineon,slb9670"; + reg = <0>; + spi-max-frequency = <18500000>; + }; +}; + +&spi1 {
Same as before, is this exposed somewhere? If it is, can we please get a comment in here advertising so?
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 954
Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +8
-0+ pinctrl-names = "default"; + pinctrl-0 = <&spi1_pins>; + mediatek,pad-select = <0>; + status = "okay"; +}; + +&thermal_zones { + board0-thermal {
"board0" is a bit generic, is that on purpose?
If this is a fixed placement sensor, you could name this something like
pcb-bottom-thermal pcb-left-thermal skin-thermal board-soc-thermal
etc etc etc :-)
Quoted 104 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +104
-0+ polling-delay = <1000>; /* milliseconds */ + polling-delay-passive = <0>; /* milliseconds */ + thermal-sensors = <&thermal_sensor0>; + + trips { + trip-alert { + temperature = <85000>; + hysteresis = <2000>; + type = "passive"; + }; + + trip-crit { + temperature = <95000>; + hysteresis = <2000>; + type = "critical"; + }; + }; + }; + + board1-thermal { + polling-delay = <1000>; /* milliseconds */ + polling-delay-passive = <0>; /* milliseconds */ + thermal-sensors = <&thermal_sensor1>; + + trips { + trip-alert { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + trip-crit { + temperature = <85000>; + hysteresis = <2000>; + type = "critical"; + }; + }; + }; + + board2-thermal { + polling-delay = <1000>; /* milliseconds */ + polling-delay-passive = <0>; /* milliseconds */ + thermal-sensors = <&thermal_sensor2>; + + trips { + trip-alert { + temperature = <75000>; + hysteresis = <2000>; + type = "passive"; + }; + + trip-crit { + temperature = <85000>; + hysteresis = <2000>; + type = "critical"; + }; + }; + }; +}; + +&uart0 { + pinctrl-names = "default"; + pinctrl-0 = <&uart0_pins>; + status = "okay"; +}; + +&uart1 { + pinctrl-names = "default"; + pinctrl-0 = <&uart1_pins>; + uart-has-rtscts; + status = "okay"; +}; + +&uart2 { + pinctrl-names = "default"; + pinctrl-0 = <&uart2_pins>; + uart-has-rtscts; + status = "okay"; +}; + +&uart3 { + pinctrl-names = "default"; + pinctrl-0 = <&uart3_pins>; + status = "okay"; +}; + +&uart4 { + pinctrl-names = "default"; + pinctrl-0 = <&uart4_pins>; + status = "okay"; +}; + +/* USB3 */ +&u3phy0 { + status = "okay"; +}; + +/* PCIe1/USB2 */ +&u3phy1 { + status = "okay"; +}; + +/* USB2 */ +&u3phy2 {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 1068
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ status = "okay"; +}; + +/* USB2 */ +&u3phy3 { + status = "okay"; +}; + +/* USB3 front port */ +&xhci0 {
It's not gonna work like this. I recently fixed the USB nodes in MT8195 by adding MTU3 where necessary...
Check mt8195.dtsi - only one XHCI controller isn't placed behind MTU3, and that is XHCI1 (11290000), while the others are MTU3.
As far as I can see from this DT, it should now instead look like..
&ssusb0 {
dr_mode = "host";
vusb33-supply = <&mt6359_vusb_ldo_reg>;
status = "okay";
};&ssusb2 {
dr_mode = "host";
vusb33-supply = <&mt6359_vusb_ldo_reg>;
status = "okay";
};&ssusb3 {
dr_mode = "host";
vusb33-supply = <&mt6359_vusb_ldo_reg>;
status = "okay";
};&xhci0 {
vbus-supply = <&otg_vbus_regulator>;
status = "okay";
};&xhci1 {
vusb33-supply = <&mt6359_vusb_ldo_reg>;vbus is always supplied by something, as otherwise USB won't work - whether this is an always-on regulator or a passthrough from external supply this doesn't really matter - you should model a regulator-fixed that provides the 5V VBUS line.
For example:
vbus_fixed: regulator-vbus {
compatible = "regulator-fixed";
regulator-name = "usb-vbus";
regulator-always-on;
regulator-boot-on;
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
};P.S.: If the rail has a different name, please use that different name. Obviously that requires you to have schematics at hand, and I don't know if you do: if you don't, then that regulator-vbus name is just fine.
vbus-supply = <&vbus_fixed>; mediatek,u3p-dis-msk = <0x1>; status = "okay"; };
&xhci2 {
vbus-supply = <&vbus_fixed>;
status = "okay";
};&xhci3 {
vbus-supply = <&vbus_fixed>;
status = "okay";
};Cheers,
Angelo
Quoted 23 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +23
-0+ vusb33-supply = <&mt6359_vusb_ldo_reg>; + vbus-supply = <&otg_vbus_regulator>; + status = "okay"; +}; + +/* USB2 M.2 Key-B */ +&xhci1 { + vusb33-supply = <&mt6359_vusb_ldo_reg>; + mediatek,u3p-dis-msk = <0x01>; + status = "okay"; +}; + +/* USB2 M.2 Key-E */ +&xhci2 { + vusb33-supply = <&mt6359_vusb_ldo_reg>; + status = "okay"; +}; + +/* USB2 to on-board usb hub */ +&xhci3 { + vusb33-supply = <&mt6359_vusb_ldo_reg>; + status = "okay"; +};
Re: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Hi,
thanks for the extensive review!
Quoted 8 lines · AngeloGioacchino Del Regno
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 263
Quoted 5 lines
+ð {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ phy-mode ="rgmii-id"; + phy-handle = <ðernet_phy0>; + snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>; + snps,reset-delays-us = <0 10000 80000>;snps,reset-delays-us and snps,reset-gpio are deprecated.
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 270
Quoted 7 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +7
-0+ pinctrl-names = "default", "sleep"; + pinctrl-0 = <ð_default_pins>; + pinctrl-1 = <ð_sleep_pins>; + status = "okay"; + + mdio { + ethernet_phy0: ethernet-phy@1 {compatible = "is there any applicable compatible?"
P.S.: if you've got the usual rtl8211f, should be "ethernet-phy-id001c.c916"
I'd rather not have a compatible here. First, it's auto discoverable and IIRC it's frowned upon adding any compatible if you ask the PHY maintainers. And second, if we change the PHY (maybe due to a second chip shortage or whatever), there is a chance you don't have to update this in the DT.
Quoted 18 lines
reg = <
0x1>;
interrupts-extended = <&pio 94IRQ_TYPE_LEVEL_LOW>; reset-assert-us = <10000>; reset-deassert-us = <80000>;
reset-gpios = <&pio 93GPIO_ACTIVE_HIGH>;Quoted 11 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +11
-0+ reg = <0x1>; + interrupts-extended = <&pio 94 IRQ_TYPE_LEVEL_LOW>; + }; + }; +}; + +&gpu { + status = "okay"; + mali-supply = <&mt6315_7_vbuck1>; +}; +
Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 286
Quoted 5 lines
+&i2c2 {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ pinctrl-names = "default"; + pinctrl-0 = <&i2c2_pins>; + clock-frequency = <400000>; + status = "okay";Are i2c2,3,4 exposed as pins somewhere? If they are, can you please put a comment saying so?
This is only a basic device tree. On one i2c controller, there is the LVDS bridge for example. My plan is to get the support for this bridge upstream first and then adding the appropriate device nodes here.
That being said, some are exposed to connectors. I'll add a comment then.
Quoted 6 lines
Quoted 6 lines
+&mmc1 {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +5
-0+ pinctrl-names = "default", "state_uhs"; + pinctrl-0 = <&mmc1_default_pins>; + pinctrl-1 = <&mmc1_uhs_pins>; + cd-gpios = <&pio 129 GPIO_ACTIVE_LOW>; + bus-width = <4>;
Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 442
Quoted 6 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +6
-0+ max-frequency = <200000000>; + cap-sd-highspeed; + sd-uhs-sdr50; + sd-uhs-sdr104; + vmmc-supply = <&mt6360_ldo5>; + vqmmc-supply = <&mt6360_ldo3>;Does mmc1 support eMMC and SDIO?
No eMMC, but I'd guess it will support SDIO as in you can just plug an SDIO card in the SD slot, right? Oh, it's a micro SD socket. So uhm, I'm not sure if we should restrict it, though. Someone might come up with a microsd to sd card adapter. I have one right in front of me ;)
Quoted 1 line
If not, no-mmc; no-sdio;
So no-mmc;
Quoted 5 lines
Review
Quoted 1 line
(quoted diff) +1
-0+ drive-strength = <MTK_DRIVE_8mA>;s/MTK_DRIVE//g
s/mA//g
Quoted 7 lines
drive-strength = <8>;
Please, here and everywhere else, for all values - let's stop using those MTK_DRIVE_(x)mA definitions, they're just defined as (x), where anyway the drive-strength property is in milliamps by default.
We don't need these definitions.
Sure, the mt8195-demo was the blueprint for this. So maybe you should get rid of it there to prevent any copying ;) (btw the same goes for the regulator-compatible property).
Speaking of pinctrl, I find the R0R1 bias-pull-down values really hard to grasp. The DT binding documentation didn't really help here. What is R0 and R1, I presume some resistors which can be enabled. Also are they in parallel or in series. I'd have assumed, the DT binding should have hid this by giving the user a choice for the resistance instead. Also I had a quick search in the RM and couldn't find anything, I probably looked at the wrong place ;)
Quoted 18 lines
Quoted 4 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ uart1_pins: uart1-pins { + pins_rx { + pinmux = <PINMUX_GPIO103__FUNC_URXD1>; + input-enable;arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 827
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO102__FUNC_UTXD1>; + }; + + pins_rts { + pinmux = <PINMUX_GPIO100__FUNC_URTS1>; + output-enable;Are you really sure that you need output-enable here?! RTS is not an output buffer....
Quoted 1 line
I don't think you do. Please double check.
Ahh, good catch, it's a leftover from mt8183-kukui.dts. There is probably wrong, too.
Quoted 10 lines
Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +8
-0+ }; + + pins_cts { + pinmux = <PINMUX_GPIO101__FUNC_UCTS1>; + input-enable; + }; + }; +
Quoted 5 lines
Quoted 2 lines
+/* USB3 front port */
+&xhci0 {It's not gonna work like this. I recently fixed the USB nodes in MT8195 by adding MTU3 where necessary...
Uhm, seems like I've missed that.
Quoted 28 lines
Check
mt8195.dtsi- only one XHCI controller isn't placed behind MTU3, and that is XHCI1 (11290000), while the others are MTU3.As far as I can see from this DT, it should now instead look like..
&ssusb0 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&ssusb2 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&ssusb3 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&xhci0 { vbus-supply = <&otg_vbus_regulator>; status = "okay"; };
Quoted 6 lines
&xhci1 { vusb33-supply = <&mt6359_vusb_ldo_reg>;vbus is always supplied by something, as otherwise USB won't work - whether this is an always-on regulator or a passthrough from external supply this doesn't really matter - you should model a regulator-fixed that provides the 5V VBUS line.
I don't think this is correct, though. Think of an on-board USB hub. There only D+/D- are connected (and maybe the USB3.2 SerDes lanes). Or have a look at the M.2 pinout. There is no Vbus.
Also it seems I need the "mediatek,u3p-dis-msk = <0x01>;". At least the last time I've tested it. I'll test it again, with and without. The SerDes Line of the corresponding USB3.2 port is used for PCIe in this case.
Quoted 9 lines
For example:
vbus_fixed: regulator-vbus { compatible = "regulator-fixed"; regulator-name = "usb-vbus"; regulator-always-on; regulator-boot-on; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; };
As mentioned above, I don't think this will make sense in my case.
Quoted 3 lines
P.S.: If the rail has a different name, please use that different name. Obviously that requires you to have schematics at hand, and I don't know if you do: if you don't, then that regulator-vbus name is just fine.
I do have the schematics.
Thanks,
-michael
AngeloGioacchino Del Regno
maintainerRe: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Il 19/02/24 14:09, Michael Walle ha scritto:
Quoted 25 lines
Hi,
thanks for the extensive review!
Quoted 8 lines · AngeloGioacchino Del Regno
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 263
Quoted 5 lines
+ð {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ phy-mode ="rgmii-id"; + phy-handle = <ðernet_phy0>; + snps,reset-gpio = <&pio 93 GPIO_ACTIVE_HIGH>; + snps,reset-delays-us = <0 10000 80000>;snps,reset-delays-us and snps,reset-gpio are deprecated.
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 270
Quoted 7 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +7
-0+ pinctrl-names = "default", "sleep"; + pinctrl-0 = <ð_default_pins>; + pinctrl-1 = <ð_sleep_pins>; + status = "okay"; + + mdio { + ethernet_phy0: ethernet-phy@1 {compatible = "is there any applicable compatible?"
P.S.: if you've got the usual rtl8211f, should be "ethernet-phy-id001c.c916"
Quoted 6 lines
I'd rather not have a compatible here. First, it's auto discoverable and IIRC it's frowned upon adding any compatible if you ask the PHY maintainers. And second, if we change the PHY (maybe due to a second chip shortage or whatever), there is a chance you don't have to update this in the DT.
Okay then, I'm fine with leaving the compatible out.
Quoted 27 lines
Quoted 18 lines
reg = <
0x1>;
interrupts-extended = <&pio 94IRQ_TYPE_LEVEL_LOW>; reset-assert-us = <10000>; reset-deassert-us = <80000>;
reset-gpios = <&pio 93GPIO_ACTIVE_HIGH>;Quoted 11 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +11
-0+ reg = <0x1>; + interrupts-extended = <&pio 94 IRQ_TYPE_LEVEL_LOW>; + }; + }; +}; + +&gpu { + status = "okay"; + mali-supply = <&mt6315_7_vbuck1>; +}; +Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 286
Quoted 5 lines
+&i2c2 {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ pinctrl-names = "default"; + pinctrl-0 = <&i2c2_pins>; + clock-frequency = <400000>; + status = "okay";Are i2c2,3,4 exposed as pins somewhere? If they are, can you please put a comment saying so?
Quoted 7 lines
This is only a basic device tree. On one i2c controller, there is the LVDS bridge for example. My plan is to get the support for this bridge upstream first and then adding the appropriate device nodes here.
That being said, some are exposed to connectors. I'll add a comment then.
In that case, could be nice to read something like
&i2c(x) {
properties
blahblah
status
/* (model, if available) LVDS bridge at 0x10 */
}it's again not mandatory, but I like seeing clear messages implying "this should be there" as those implicitly mean "...yeah but it's not supported yet for reasons".
It's down to preferences though, and this is not a *strong* opinion, nor a strong suggestion - your call here.
Quoted 16 lines
Quoted 15 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 442
Quoted 13 lines
+&mmc1 {
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +11
-0+ pinctrl-names = "default", "state_uhs"; + pinctrl-0 = <&mmc1_default_pins>; + pinctrl-1 = <&mmc1_uhs_pins>; + cd-gpios = <&pio 129 GPIO_ACTIVE_LOW>; + bus-width = <4>; + max-frequency = <200000000>; + cap-sd-highspeed; + sd-uhs-sdr50; + sd-uhs-sdr104; + vmmc-supply = <&mt6360_ldo5>; + vqmmc-supply = <&mt6360_ldo3>;Does mmc1 support eMMC and SDIO?
Quoted 6 lines
No eMMC, but I'd guess it will support SDIO as in you can just plug an SDIO card in the SD slot, right? Oh, it's a micro SD socket. So uhm, I'm not sure if we should restrict it, though. Someone might come up with a microsd to sd card adapter. I have one right in front of me ;)
Honestly ... I even forgot the existance of those adapters!!!
In that case, yes, since the controller should support SDIO on that slot, and since there effectively are ways to add a SDIO card on there, obviously no-sdio shall be omitted.
I agree.
Quoted 3 lines
Quoted 1 line
If not, no-mmc; no-sdio;
So no-mmc;
Yes, agreed.
Quoted 12 lines
Quoted 12 lines
Review
Quoted 2 lines
(quoted diff) +1
-0+ drive-strength = <MTK_DRIVE_8mA>;s/MTK_DRIVE//g
s/mA//gdrive-strength = <8>;
Please, here and everywhere else, for all values - let's stop using those MTK_DRIVE_(x)mA definitions, they're just defined as (x), where anyway the drive-strength property is in milliamps by default.
Quoted 6 lines
Quoted 1 line
We don't need these definitions.
Sure, the mt8195-demo was the blueprint for this. So maybe you should get rid of it there to prevent any copying ;) (btw the same goes for the regulator-compatible property).
Yeah, that's right. You can imagine that my backlog is rather huge... :-)
Quoted 1 line
Speaking of pinctrl, I find the R0R1 bias-pull-down values really
If it was only pull-down it would be one problem, but it's also pull-up so we can sum that up to *two* problems :-P
Quoted 2 lines
hard to grasp. The DT binding documentation didn't really help here. What is R0 and R1, I presume some resistors which can be enabled.
You got it right
Quoted 1 line
Also are they in parallel or in series. I'd have assumed, the DT
I'm not sure, and it depends on the SoC most probably... but does that really matter?
I mean, on the practical side, imo, it doesn't, but I am also a curious person so I can understand why you're eager to know :-)
Quoted 4 lines
binding should have hid this by giving the user a choice for the resistance instead. Also I had a quick search in the RM and couldn't find anything, I probably looked at the wrong place ;)
I'm not sure you looked at mediatek,mt8195-pinctrl.yaml, but anyway, as you can read in there, we're deprecating the MTK_PULL_SET_RSEL_xxx in favor of...
.... the right thing to do :-)
Look for "mediatek,rsel-resistance-in-si-unit": that'll allow you to specify the PU/PD values in ohms, and that's what should be used.
Those RSEL definitions in the devicetree should disappear. Forever.
Quoted 18 lines
Quoted 18 lines
Quoted 4 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +4
-0+ uart1_pins: uart1-pins { + pins_rx { + pinmux = <PINMUX_GPIO103__FUNC_URXD1>; + input-enable;arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts line 827
Quoted 10 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +10
-0+ bias-pull-up; + }; + + pins_tx { + pinmux = <PINMUX_GPIO102__FUNC_UTXD1>; + }; + + pins_rts { + pinmux = <PINMUX_GPIO100__FUNC_URTS1>; + output-enable;Are you really sure that you need output-enable here?! RTS is not an output buffer....
Quoted 5 lines
Quoted 1 line
I don't think you do. Please double check.
Ahh, good catch, it's a leftover from
mt8183-kukui.dts. There is probably wrong, too.
Probably. I don't really know either.
Quoted 10 lines
Quoted 8 lines
Quoted 8 lines
arch/arm64/boot/dts/mediatek/mt8395-kontron-3-5-sbc-i1200.dts new file +8
-0+ }; + + pins_cts { + pinmux = <PINMUX_GPIO101__FUNC_UCTS1>; + input-enable; + }; + }; +
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
+/* USB3 front port */
+&xhci0 {It's not gonna work like this. I recently fixed the USB nodes in MT8195 by adding MTU3 where necessary...
Uhm, seems like I've missed that.
No worries!
Quoted 31 lines
Quoted 31 lines
Check
mt8195.dtsi- only one XHCI controller isn't placed behind MTU3, and that is XHCI1 (11290000), while the others are MTU3.As far as I can see from this DT, it should now instead look like..
&ssusb0 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&ssusb2 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&ssusb3 { dr_mode = "host"; vusb33-supply = <&mt6359_vusb_ldo_reg>; status = "okay"; };&xhci0 { vbus-supply = <&otg_vbus_regulator>; status = "okay"; };&xhci1 { vusb33-supply = <&mt6359_vusb_ldo_reg>;
Quoted 8 lines
Quoted 3 lines
vbus is always supplied by something, as otherwise USB won't work - whether this is an always-on regulator or a passthrough from external supply this doesn't really matter - you should model a regulator-fixed that provides the 5V VBUS line.
I don't think this is correct, though. Think of an on-board USB hub. There only D+/D- are connected (and maybe the USB3.2 SerDes lanes). Or have a look at the M.2 pinout. There is no Vbus.
Yes but the MediaTek MTU3 and/or controllers do have it ;-)
Quoted 5 lines
Also it seems I need the "mediatek,u3p-dis-msk = <
0x01>;". At least the last time I've tested it. I'll test it again, with and without. The SerDes Line of the corresponding USB3.2 port is used for PCIe in this case.
Have I missed it in my example? If I missed it, that was unintentional.
Anyway, for the u3p-dis-msk, I'll spare you the time to check:
- If the controller lies behind MTU3, that property goes to &ssusb(x)
- If it is a standalone XHCI controller, it goes to &xhci(x)
- The property never goes to both, and always goes to the *outer* node (this is why it goes to mtu3 if there's a mtu3 behind).
Quoted 10 lines
Quoted 9 lines
For example:
vbus_fixed: regulator-vbus { compatible = "regulator-fixed"; regulator-name = "usb-vbus"; regulator-always-on; regulator-boot-on; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; };
Quoted 6 lines
As mentioned above, I don't think this will make sense in my case.
Quoted 3 lines
Quoted 1 line
P.S.: If the rail has a different name, please use that different name. Obviouslythat requires you to have schematics at hand, and I don't know if you do: if you don't, then that regulator-vbus name is just fine.
I do have the schematics.
In that case, you should model the power tree with the fixed power lines, check mt8195-cherry (and/or cherry-tomato) and radxa-nio-12l; even though those are technically "doing nothing", this is device tree, so it should provide a description of the hardware ... and the board does have fixed power lines.
It has at least one: DC-IN (typec, barrel jack or whatever, the board needs power, doesn't it?!).
Cheers,
Angelo
Re: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Quoted 10 lines · AngeloGioacchino Del Regno
Quoted 8 lines
Quoted 3 lines
vbus is always supplied by something, as otherwise USB won't work - whether this is an always-on regulator or a passthrough from external supply this doesn't really matter - you should model a regulator-fixed that provides the 5V VBUS line.
I don't think this is correct, though. Think of an on-board USB hub. There only D+/D- are connected (and maybe the USB3.2 SerDes lanes). Or have a look at the M.2 pinout. There is no Vbus.
Yes but the MediaTek MTU3 and/or controllers do have it ;-)
.. and ..
Quoted 17 lines
Quoted 10 lines
Quoted 9 lines
For example:
vbus_fixed: regulator-vbus { compatible = "regulator-fixed"; regulator-name = "usb-vbus"; regulator-always-on; regulator-boot-on; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; };Quoted 6 lines
As mentioned above, I don't think this will make sense in my case.
Quoted 3 lines
Quoted 1 line
P.S.: If the rail has a different name, please use that different name. Obviouslythat requires you to have schematics at hand, and I don't know if you do: if you don't, then that regulator-vbus name is just fine.
I do have the schematics.
Quoted 7 lines
In that case, you should model the power tree with the fixed power lines, check mt8195-cherry (and/or cherry-tomato) and radxa-nio-12l; even though those are technically "doing nothing", this is device tree, so it should provide a description of the hardware ... and the board does have fixed power lines.
It has at least one: DC-IN (typec, barrel jack or whatever, the board needs power, doesn't it?!).
Mh, maybe I don't get it. But within the hardware there is simply no Vbus. Thus I'd argue it doesn't make sense to have a vbus-supply property. Besides, the mediatek,mtu3.yaml binding lists it as deprecated anyway and it should rather be on the connector. There, it makes perfectly sense (at least if it's a USB connector).
Thus in my case, the xhci for the front port has a vbus-supply property (but it should rather have a connector node, as I've just learned). But the internal port which connects to the USB hub shouldn't have one.
+-----+ +-----+ +------+
| |<--Dp/Dn-->| |<------Dp/Dn----->| USB |
| SoC | | | | Conn |
| | | | +-----+ | |
+-----+ | USB | | PWR |--Vbus->| |
| Hub | | SW | +------+
| | +-----+
| | ^
| | | PRTPWR
| |-----'
+-----+"PWR SW" is a power switch, the input (+5V) isn't shown here. The power will be enabled by the USB Hub.
-michael
AngeloGioacchino Del Regno
maintainerRe: [PATCH v2 2/2] arm64: dts: mediatek: add Kontron 3.5"-SBC-i1200
Il 19/02/24 14:59, Michael Walle ha scritto:
Quoted 63 lines · AngeloGioacchino Del Regno
Quoted 10 lines
Quoted 8 lines
Quoted 3 lines
vbus is always supplied by something, as otherwise USB won't work - whether this is an always-on regulator or a passthrough from external supply this doesn't really matter - you should model a regulator-fixed that provides the 5V VBUS line.
I don't think this is correct, though. Think of an on-board USB hub. There only D+/D- are connected (and maybe the USB3.2 SerDes lanes). Or have a look at the M.2 pinout. There is no Vbus.
Yes but the MediaTek MTU3 and/or controllers do have it ;-)
.. and ..
Quoted 17 lines
Quoted 10 lines
Quoted 9 lines
For example:
vbus_fixed: regulator-vbus { compatible = "regulator-fixed"; regulator-name = "usb-vbus"; regulator-always-on; regulator-boot-on; regulator-min-microvolt = <5000000>; regulator-max-microvolt = <5000000>; };Quoted 6 lines
As mentioned above, I don't think this will make sense in my case.
Quoted 3 lines
Quoted 1 line
P.S.: If the rail has a different name, please use that different name. Obviouslythat requires you to have schematics at hand, and I don't know if you do: if you don't, then that regulator-vbus name is just fine.
I do have the schematics.
Quoted 7 lines
In that case, you should model the power tree with the fixed power lines, check mt8195-cherry (and/or cherry-tomato) and radxa-nio-12l; even though those are technically "doing nothing", this is device tree, so it should provide a description of the hardware ... and the board does have fixed power lines.
It has at least one: DC-IN (typec, barrel jack or whatever, the board needs power, doesn't it?!).Mh, maybe I don't get it. But within the hardware there is simply no Vbus. Thus I'd argue it doesn't make sense to have a vbus-supply property. Besides, the mediatek,mtu3.yaml binding lists it as deprecated anyway and it should rather be on the connector. There, it makes perfectly sense (at least if it's a USB connector).
Thus in my case, the xhci for the front port has a vbus-supply property (but it should rather have a connector node, as I've just learned). But the internal port which connects to the USB hub shouldn't have one.
+-----+ +-----+ +------+ | |<--Dp/Dn-->| |<------Dp/Dn----->| USB | | SoC | | | | Conn | | | | | +-----+ | | +-----+ | USB | | PWR |--Vbus->| | | Hub | | SW | +------+ | | +-----+ | | ^ | | | PRTPWR | |-----' +-----+
Quoted 3 lines
"PWR SW" is a power switch, the input (+5V) isn't shown here. The power will be enabled by the USB Hub.
Ok then, put the vbus on the connector, where applicable.
If PWR_SW is a physical switch (not detectable), then you can still model it as an always-on regulator-fixed, as there's no other clean way of doing that.
As for modeling the power tree with the fixed power lines, I mean that you should add the power rails as described in your schematics.
You will surely have one DC-IN supply, and others. That should be modeled as a fixed regulator, which outputs to (probably) the PMIC, using it as an input for its buck regulators.
Again, your PMIC must be getting a voltage from somewhere in order to regulate that to a different voltage for a different power rail, right? :-)
Regards,
Angelo
Quoted 16 lines · Michael Walle
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
Signed-off-by Michael Walle <[email protected]>---
v2:
- convert enum to const as there is only one specific boardDocumentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)Documentation/devicetree/bindings/arm/mediatek.yaml +0
-0
Documentation/devicetree/bindings/arm/mediatek.yaml properties: · line 362
Quoted 6 lines
Documentation/devicetree/bindings/arm/mediatek.yaml +3
-0@@ -357,6 +357,11 @@ properties: - radxa,nio-12l - const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200
And then it should be part of earlier entry. Just take a look there in the file...
Best regards,
Krzysztof
Quoted 12 lines · Krzysztof Kozlowski
Quoted 11 lines · Michael Walle
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
Signed-off-by Michael Walle <[email protected]>---
v2:
- convert enum to const as there is only one specific boardDocumentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)
Quoted 14 lines
Documentation/devicetree/bindings/arm/mediatek.yaml properties: · line 362
Quoted 11 lines
Documentation/devicetree/bindings/arm/mediatek.yaml +3
-0@@ -357,6 +357,11 @@ properties: - radxa,nio-12l - const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200And then it should be part of earlier entry. Just take a look there in the file...
FWIW I did and all the entries with descriptions are by there own (that's also true for fsl.yaml and sunxi.yaml).
mediatek.yaml seems to have both, either boards without description like the "radxa,nio-12l" which you are probably referring to. Or boards which has the same sub compatibles but still have individual entries, like google,burnet, google,cozmo, google,damu.
Therefore, what is the rule of thumb here? Drop the description? If so, why it it there in the first place?
-michael
AngeloGioacchino Del Regno
maintainerIl 19/02/24 09:44, Michael Walle ha scritto:
Quoted 17 lines
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
Signed-off-by Michael Walle <[email protected]>---
v2:
- convert enum to const as there is only one specific boardDocumentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)Documentation/devicetree/bindings/arm/mediatek.yaml +0
-0@@ -357,6 +357,11 @@ properties: - radxa,nio-12l
Documentation/devicetree/bindings/arm/mediatek.yaml properties: · line 367
Quoted 10 lines
Documentation/devicetree/bindings/arm/mediatek.yaml +5
-0- const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200 + - const: mediatek,mt8395 + - const: mediatek,mt8195 - items: - enum: - mediatek,mt8516-pumpkin
I understand that you took inspiration from the Google Chromebooks entries, but those are separated only because they've got "a bunch of revisions".
For machines that don't have a billion compatible strings, I would suggest to add the compatible under a single big entry: like this, the binding is (imo) a bit more readable .. and cleaner.
Eventually, the machine name is in the devicetree so we're not losing any kind of information anyway... :-)
- items:
- enum:
- kontron,3-5-sbc-i1200
- mediatek,mt8395-evk
- radxa,nio-12l
- const: mediatek,mt8395
- const: mediatek,mt8195
Cheers,
Angelo
Quoted 23 lines · AngeloGioacchino Del Regno
Il 19/02/24 09:44, Michael Walle ha scritto:
Quoted 22 lines
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
Signed-off-by Michael Walle <[email protected]>---
v2:
- convert enum to const as there is only one specific boardDocumentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)Documentation/devicetree/bindings/arm/mediatek.yaml +3
-0@@ -357,6 +357,11 @@ properties: - radxa,nio-12l - const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200
Quoted 9 lines
Documentation/devicetree/bindings/arm/mediatek.yaml properties: · line 367
Quoted 5 lines
Documentation/devicetree/bindings/arm/mediatek.yaml +2
-0+ - const: mediatek,mt8395 + - const: mediatek,mt8195 - items: - enum: - mediatek,mt8516-pumpkinI understand that you took inspiration from the Google Chromebooks entries, but those are separated only because they've got "a bunch of revisions".
I don't really care about the description. It's just the way I've done it in the past. And in this file, there is also google,burnet, google,cozmo, google,damu, without any further revs.
Quoted 7 lines
For machines that don't have a billion compatible strings, I would suggest to add the compatible under a single big entry: like this, the binding is (imo) a bit more readable .. and cleaner.
Eventually, the machine name is in the devicetree so we're not losing any kind of information anyway... :-)
Quoted 7 lines
- items:
- enum:
- kontron,3-5-sbc-i1200
- mediatek,mt8395-evk
- radxa,nio-12l
- const: mediatek,mt8395
- const: mediatek,mt8195
Ack. I'll give you some time to look at the actual DTS before posting a new version.
-michael
AngeloGioacchino Del Regno
maintainerIl 19/02/24 10:36, Michael Walle ha scritto:
Quoted 34 lines · AngeloGioacchino Del Regno
Quoted 23 lines
Il 19/02/24 09:44, Michael Walle ha scritto:
Quoted 22 lines
Add the compatible string for the Kontron 3.5"-SBC-i1200 single board computer.
Signed-off-by Michael Walle <[email protected]>---
v2:
- convert enum to const as there is only one specific boardDocumentation/devicetree/bindings/arm/mediatek.yaml | 5 +++++ 1 file changed, 5 insertions(+)Documentation/devicetree/bindings/arm/mediatek.yaml +3
-0@@ -357,6 +357,11 @@ properties: - radxa,nio-12l - const: mediatek,mt8395 - const: mediatek,mt8195 + - description: Kontron 3.5"-SBC-i1200 + items: + - const: kontron,3-5-sbc-i1200Quoted 9 lines
Documentation/devicetree/bindings/arm/mediatek.yaml properties: · line 367
Quoted 5 lines
Documentation/devicetree/bindings/arm/mediatek.yaml +2
-0+ - const: mediatek,mt8395 + - const: mediatek,mt8195 - items: - enum: - mediatek,mt8516-pumpkinI understand that you took inspiration from the Google Chromebooks entries, but those are separated only because they've got "a bunch of revisions".
Quoted 4 lines
I don't really care about the description. It's just the way I've done it in the past. And in this file, there is also google,burnet, google,cozmo, google,damu, without any further revs.
Yeah, I have to check why it is like this - as in - if there's any real reason for those to be like that... otherwise I wouldn't mind cleaning 'em all up.
Whatever - that's not on you, and completely irrelevant to this specific patch, so don't worry about the cleanup of the other entries.
Quoted 19 lines
Quoted 7 lines
For machines that don't have a billion compatible strings, I would suggest to add the compatible under a single big entry: like this, the binding is (imo) a bit more readable .. and cleaner.
Eventually, the machine name is in the devicetree so we're not losing any kind of information anyway... :-)
Quoted 7 lines
- items:
- enum:
- kontron,3-5-sbc-i1200
- mediatek,mt8395-evk
- radxa,nio-12l
- const: mediatek,mt8395
- const: mediatek,mt8195Ack. I'll give you some time to look at the actual DTS before posting a new version.
-michael