[PATCH] arm64: dts: ti: add k3-j721e-beagleboneai64
Nishanth Menon
nm at ti.com
Tue Aug 16 09:31:16 PDT 2022
On 16:37-20220815, Robert Nelson wrote:
[...]
> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
> index 02e5d80344d0..4c88e87b42a6 100644
> --- a/arch/arm64/boot/dts/ti/Makefile
> +++ b/arch/arm64/boot/dts/ti/Makefile
> @@ -12,6 +12,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-am6528-iot2050-basic-pg2.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-am6548-iot2050-advanced-pg2.dtb
>
> +dtb-$(CONFIG_ARCH_K3) += k3-j721e-beagleboneai64.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-j721e-common-proc-board.dtb
> dtb-$(CONFIG_ARCH_K3) += k3-j721e-sk.dtb
>
> diff --git a/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts b/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts
> new file mode 100644
> index 000000000000..4748fe492357
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts
> @@ -0,0 +1,1143 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
Please add a link here: https://beagleboard.org/ai-64
> + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
> + * Copyright (C) 2022 Jason Kridner, BeagleBoard.org Foundation
> + * Copyright (C) 2022 Robert Nelson, BeagleBoard.org Foundation
> + */
> +
> +/dts-v1/;
> +
> +#include "k3-j721e.dtsi"
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/net/ti-dp83867.h>
> +
> +/ {
> + compatible = "beagle,j721e-beagleboneai64", "ti,j721e";
> + model = "BeagleBoard.org BeagleBone AI-64";
> +
> + chosen {
> + stdout-path = "serial2:115200n8";
> + bootargs = "console=ttyS2,115200n8 earlycon=ns16550a,mmio32,0x02800000";
> + };
we should do aliases - the aliases in k3-j721e.dtsi must move
out into the board files.
> +
> + memory at 80000000 {
> + device_type = "memory";
> + /* 4G RAM */
> + reg = <0x00000000 0x80000000 0x00000000 0x80000000>,
> + <0x00000008 0x80000000 0x00000000 0x80000000>;
> + };
> +
> + reserved_memory: reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + secure_ddr: optee at 9e800000 {
> + reg = <0x00 0x9e800000 0x00 0x01800000>;
> + alignment = <0x1000>;
> + no-map;
> + };
> +
> + mcu_r5fss0_core0_dma_memory_region: r5f-dma-memory at a0000000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0xa0000000 0x00 0x100000>;
> + no-map;
> + };
> +
> + mcu_r5fss0_core0_memory_region: r5f-memory at a0100000 {
> + compatible = "shared-dma-pool";
> + reg = <0x00 0xa0100000 0x00 0xf00000>;
> + no-map;
> + };
[...]
> + };
> +
> + gpio_keys: gpio-keys {
> + compatible = "gpio-keys";
> + autorepeat;
> + pinctrl-names = "default";
> + pinctrl-0 = <&sw_pwr_pins_default>;
> +
> + sw_boot: sw1 {
> + status = "disabled";
Please document why disabled?
> + label = "BOOT";
> + linux,code = <BTN_0>;
> + gpios = <&wkup_gpio0 0 GPIO_ACTIVE_LOW>;
> + };
> +
> + sw_pwr: sw2 {
> + label = "POWER";
> + linux,code = <KEY_POWER>;
> + gpios = <&wkup_gpio0 4 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> + gpio_leds: gpio-leds {
> + compatible = "gpio-leds";
> + pinctrl-names = "default";
> + pinctrl-0 = <&led_pins_default>;
> +
> + usr_led0: usr_led0 {
what is wrong with led-0,1,2,3,4 ?
arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts: /gpio-leds/usr_led0: Character '_' not recommended in node name
> + label = "beaglebone:green:usr0";
please see Documentation/devicetree/bindings/leds/common.yaml
label has been deprecated, instead use function and color.
> + default-state = "keep";
> + linux,default-trigger = "heartbeat";
is'nt function a better option?
> + gpios = <&main_gpio0 96 GPIO_ACTIVE_HIGH>;
> + };
> +
> + usr_led1: usr_led1 {
> + label = "beaglebone:green:usr1";
> + default-state = "keep";
> + linux,default-trigger = "mmc0";
here and others, please check against Documentation/devicetree/bindings/leds/common.yaml
enum list ?
> + gpios = <&main_gpio0 95 GPIO_ACTIVE_HIGH>;
> + };
> +
> + usr_led2: usr_led2 {
> + label = "beaglebone:green:usr2";
[...]
> + dp_pwr_3v3: fixedregulator-dp-prw {
> + compatible = "regulator-fixed";
> + pinctrl-names = "default";
> + pinctrl-0 = <&dp0_3v3_en_pins_default>;
> + regulator-name = "dp-pwr";
> + regulator-min-microvolt = <3300000>;
> + regulator-max-microvolt = <3300000>;
> + gpios = <&main_gpio0 49 GPIO_ACTIVE_HIGH>; /* DP0_PWR_SW_EN */
> + enable-active-high;
> +
> + /* Always on for now, until dp-connector driver can handle this */
Hmm... I did notice no implementation in drivers for dp-pwr-supply
> + regulator-always-on;
> + };
> +
[...]
> +
> +&wkup_pmx0 {
> + eeprom_wp_pins_default: eeprom-wp-pins-default {
> + pinctrl-single,pins = <
> + J721E_WKUP_IOPAD(0xc4, PIN_OUTPUT_PULLUP, 7) /* (G24) WKUP_GPIO0_5 */
> + >;
> + };
> +
> + mcu_adc0_pins_default: mcu_adc0_pins_default {
NAK.
dtbs_check should have told you:
arch/arm64/boot/dts/ti/k3-j721e-beagleboneai64.dts: /bus at 100000/bus at 28380000/pinctrl at 4301c000/mcu_adc0_pins_default: Character '_' not recommended in node name
> + pinctrl-single,pins = <
> + J721E_WKUP_IOPAD(0x130, PIN_INPUT, 0) /* (K25) MCU_ADC0_AIN0 */
> + J721E_WKUP_IOPAD(0x134, PIN_INPUT, 0) /* (K26) MCU_ADC0_AIN1 */
> + J721E_WKUP_IOPAD(0x138, PIN_INPUT, 0) /* (K28) MCU_ADC0_AIN2 */
> + J721E_WKUP_IOPAD(0x13c, PIN_INPUT, 0) /* (L28) MCU_ADC0_AIN3 */
> + J721E_WKUP_IOPAD(0x140, PIN_INPUT, 0) /* (K24) MCU_ADC0_AIN4 */
> + J721E_WKUP_IOPAD(0x144, PIN_INPUT, 0) /* (K27) MCU_ADC0_AIN5 */
> + J721E_WKUP_IOPAD(0x148, PIN_INPUT, 0) /* (K29) MCU_ADC0_AIN6 */
> + >;
> + };
> +
> + mcu_adc1_pins_default: mcu_adc1_pins_default {
Same '_' issue
> + pinctrl-single,pins = <
> + J721E_WKUP_IOPAD(0x150, PIN_INPUT, 0) /* (N23) MCU_ADC1_AIN0 */
> + >;
> + };
> +
> + mikro_bus_pins_default: mikro-bus-pins-default {
> + pinctrl-single,pins = <
> + J721E_WKUP_IOPAD(0x108, PIN_INPUT, 7) /* SDAPULLEN (E26) PMIC_POWER_EN0.WKUP_GPIO0_66 */
> + J721E_WKUP_IOPAD(0xd4, PIN_INPUT, 7) /* SDA (G26) WKUP_GPIO0_9.MCU_I2C1_SDA */
> + J721E_WKUP_IOPAD(0xf4, PIN_INPUT, 7) /* SDA (D25) MCU_I3C0_SDA.WKUP_GPIO0_61 */
> +
[...]
> +&wkup_uart0 {
> + /* Wakeup UART is used by System firmware */
More accurate terminology will be TIFS firmware.
> + status = "reserved";
> +};
> +
> +&main_uart0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&main_uart0_pins_default>;
> + /* Shared with ATF on this platform */
> + power-domains = <&k3_pds 146 TI_SCI_PD_SHARED>;
> +};
> +
[...]
> +
> +&main_gpio0 {
> + status = "okay";
I don't think we need a status = "okay" - that has been the default.
> + gpio-line-names = "", "P9_11", "P9_13", "P8_17", "P8_18", /* 0-4 */
> + "P8_22", "P8_24", "P8_34", "P8_36", "P8_38A", /* 5-9 */
> + "P9_23", "P8_37B", "P9_26B", "P9_24B", "P8_08", /* 10-14 */
> + "P8_07", "P8_10", "P8_09", "P9_42B", "", /* 15-19 */
> + "P8_03", "TYPEC_PWR_ST", "M2_RSTz", "M2_I2C_ALERT#", "P8_35A", /* 20-24 */
> + "P8_33A", "P8_32A", "", "P9_17A", "", /* 25-29 */
> + "P8_21", "P8_23", "P8_31A", "P8_05", "P8_06", /* 30-34 */
> + "P8_25", "M2_W_DISABLE1#", "M2_W_DISABLE2#", "P9_22A (BOOTMODE1)", "P9_21A", /* 35-39 */
> + "P9_18A", "DSI_I2C_SCL", "DSI_I2C_SDA", "P9_28B", "P9_30B", /* 40-44 */
> + "P9_12", "P9_27A", "P9_15", "P8_04 (BOOTMODE2)", "VCC_DP_EN", /* 45-49 */
> + "P9_33B", "P8_26", "P9_31B", "P9_29B", "P9_39B", /* 50-54 */
> + "P9_35B", "P9_36B", "P9_37B", "P9_38B", "P8_12", /* 55-59 */
> + "P8_11 (BOOTMODE7)", "P8_15", "P8_16", "", "", /* 60-64 */
> + "P8_43", "P8_44", "P8_41", "P8_42 (BOOTMODE6)", "P8_39", /* 65-69 */
> + "P8_40", "P8_27", "P8_28", "P8_29", "P8_30", /* 70-74 */
> + "P8_14", "P8_20", "P9_20B", "P9_19B", "P8_45", /* 75-79 */
> + "P8_46 (BOOTMODE3)", "P9_40B", "VDD_SD_EN", "CSI_I2C_SCL", "CSI_I2C_SDA", /* 80-84 */
> + "M2_I2S_SCK", "M2_I2S_WS", "M2_I2S_IN", "P8_19", "P8_13", /* 85-89 */
> + "P9_21B", "P9_22B", "M2_I2S_OUT", "P9_14", "P9_16", /* 90-94 */
> + "USR1", "USR0", "USR2", "DSI_GPIO1", "FAN_PWM", /* 95-99 */
> + "FAN_TACH", "CSI1_GPIO1", "CSI0_GPIO2", "CSI0_GPIO1", "P9_25B", /* 100-104 */
> + "P8_38B", "P8_37A", "CSI1_GPIO2", "DSI_GPIO2", "USR4", /* 105-109 */
> + "USR3", "P8_33B", "DP_HPD", "M2_UART_CTSn", "M2_UART_RTSn", /* 110-114 */
> + "P9_17B", "P8_35B", "VDD_SD_SEL", "P9_26A", "P9_24A", /* 115-119 */
> + "P9_18B", "CONSOLE_RX", "CONSOLE_TX", "P9_42A", "P9_27B", /* 120-124 */
> + "M2_UART_RX", "M2_UART_TX", "P9_25A"; /* 125-127 */
> +};
> +
> +&main_gpio1 {
> + status = "okay";
I don't think we need a status = "okay" - that has been the default.
> + gpio-line-names = "P9_41", "P9_19A", "P9_20A", "TYPEC_DIR", "TYPEC_INT", /* 0-4 */
> + "M2_PCIE_WAKE#", "M2_PCIE_CLKREQ#", "M2_I2C_SCL", "M2_I2C_SDA", "TYPEC/CSI1_I2C_SCL", /* 5-9 */
> + "TYPEC/CSI1_I2C_SDA", "P9_28A", "P9_31A", "P9_30A", "P9_29A", /* 10-14 */
> + "uSD_DAT3", "uSD_DAT2", "uSD_DAT1", "uSD_DAT0", "uSD_CLK", /* 15-19 */
> + "uSD_CMD", "uSD_SDCD", "", "M2_SDIO_DAT3", "M2_SDIO_DAT2", /* 20-24 */
> + "M2_SDIO_DAT1", "M2_SDIO_DAT0", "M2_SDIO_CLK", "M2_SDIO_CMD", "USB1_DRVVBUS", /* 25-29 */
> + "", "", "", "", "", /* 30-34 */
> + "", ""; /* 35-36 */
> +};
> +
> +&main_gpio2 {
> + status = "disabled";
Here and elsewhere, when disabled, please document why.
> +};
> +
> +&main_gpio3 {
> + status = "disabled";
> +};
> +
> +&main_gpio4 {
> + status = "disabled";
> +};
> +
> +&main_gpio5 {
> + status = "disabled";
> +};
> +
> +&main_gpio6 {
> + status = "disabled";
> +};
> +
> +&main_gpio7 {
> + status = "disabled";
> +};
> +
> +&wkup_gpio0 {
> + pinctrl-names = "default";
> + pinctrl-0 = <&mcu_adc0_pins_default &mcu_adc1_pins_default &mikro_bus_pins_default>;
> + gpio-line-names = "MB_CLK/BOOT_BTN", "MB_MISO", "MB_MOSI", "MB_CS", "SOC_WAKE", /* 0-4 */
> + "EEPROM_WP", "SOC_INT2z", "H_MCU_INT#", "MB_SCLA", "MB_SDAA", /* 5-9 */
> + "MCU_RGMII_RST#", "MB_PWM", "MCU_BOOTMODE8", "MCU_BOOTMODE9", "MCU_BOOTMODE6", /* 10-14 */
> + "MCU_BOOTMODE7", "", "", "", "", /* 15-19 */
> + "", "", "", "", "", /* 20-24 */
> + "", "", "", "", "", /* 24-29 */
> + "USB_HUB_RST", "", "", "MB_RX", "MB_TX", /* 30-34 */
> + "MB_INT", "", "MB_RST", "MII_TX_CTL", "MII_RX_CTL", /* 35-39 */
> + "MII_TD3", "MII_TD2", "MII_TD1", "MII_TD0", "MII_TXC", /* 40-44 */
> + "MII_RXC", "MII_RD3", "MII_RD2", "MII_RD1", "MII_RD0", /* 45-49 */
> + "MDIO", "MDC", "MCU_BOOTMODE0", "MCU_BOOTMODE1", "MCU_BOOTMODE2", /* 50-54 */
> + "SYS_MCU_PWRDN", "WKUP_UART_RX", "WKUP_UART_TX", "MII_RST#", "MB_AN", /* 55-59 */
> + "MB_SCLB", "MB_SDAB", "WKUP_I2C0_SCL", "WKUP_I2C0_SDA", "MCU_I2C0_SCL", /* 60-64 */
> + "MCU_I2C0_SDA", "MB_SDAPULLEN", "PMIC_POWER_EN1"; /* 65-67 */
> +};
> +
> +&wkup_gpio1 {
> + status = "disabled";
Please document why disabled.
> +};
> +
> +&main_r5fss0_core0{
space before { please.
> + firmware-name = "pdk-ipc/ipc_echo_test_mcu2_0_release_strip.xer5f";
> +};
> +
> +&usb_serdes_mux {
> + idle-states = <1>, <1>; /* USB0 to SERDES3, USB1 to SERDES2 */
> +};
> +
> +&serdes_ln_ctrl {
> + idle-states = <J721E_SERDES0_LANE0_IP4_UNUSED>, <J721E_SERDES0_LANE1_IP4_UNUSED>,
> + <J721E_SERDES1_LANE0_PCIE1_LANE0>, <J721E_SERDES1_LANE1_PCIE1_LANE1>,
> + <J721E_SERDES2_LANE0_IP1_UNUSED>, <J721E_SERDES2_LANE1_USB3_1>,
> + <J721E_SERDES3_LANE0_USB3_0_SWAP>, <J721E_SERDES3_LANE1_USB3_0>,
> + <J721E_SERDES4_LANE0_EDP_LANE0>, <J721E_SERDES4_LANE1_EDP_LANE1>,
> + <J721E_SERDES4_LANE2_EDP_LANE2>, <J721E_SERDES4_LANE3_EDP_LANE3>;
> +};
> +
[...]
--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D
More information about the linux-arm-kernel
mailing list