Thanks Andrew, sorry for the delay! On Wed, Nov 2, 2022 at 11:16 AM Andrew Davis <afd@xxxxxx> wrote: > > On 10/31/22 3:01 PM, Robert Nelson wrote: > > BeagleBoard.org BeagleBone AI-64 is an open source hardware single > > board computer based on the Texas Instruments TDA4VM SoC featuring > > dual-core 2.0GHz Arm Cortex-A72 processor, C7x+MMA and 2 C66x > > floating-point VLIW DSPs, 3x dual Arm Cortex-R5 co-processors, > > 2x 6-core Programmable Real-Time Unit and Industrial Communication > > SubSystem, PowerVR Rogue 8XE GE8430 3D GPU. The board features 4GB > > DDR4, USB3.0 Type-C, 2x USB SS Type-A, miniDisplayPort, 2x 4-lane > > CSI, DSI, 16GB eMMC flash, 1G Ethernet, M.2 E-key for WiFi/BT, and > > BeagleBone expansion headers. > > > > This board family can be indentified by the BBONEAI-64-B0 in the > > at24 eeprom: > > > > [aa 55 33 ee 01 37 00 10 2e 00 42 42 4f 4e 45 41 |.U3..7....BBONEA|] > > [49 2d 36 34 2d 42 30 2d 00 00 42 30 30 30 37 38 |I-64-B0-..B00078|] > > > > https://beagleboard.org/ai-64 > > https://git.beagleboard.org/beagleboard/beaglebone-ai-64 > > + > > + reserved_memory: reserved-memory { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + ranges; > > + > > + secure_ddr: optee@9e800000 { > > + reg = <0x00 0x9e800000 0x00 0x01800000>; > > + alignment = <0x1000>; > > "alignment" property should not be needed here since you cannot > allocate from this region anyway. I removed alignment, wondering if these all should be eventually moved into the common file, with custom applications just updating the offset's. > > + > > + gpio_keys: gpio-keys { > > + compatible = "gpio-keys"; > > + autorepeat; > > Do you need "autorepeat" on these? > Removed, no idea where that came from... > > + pinctrl-names = "default"; > > + pinctrl-0 = <&sw_pwr_pins_default>; > > + > > + sw_boot: switch-1 { > > I don't see anyone referencing these nodes, the "sw_boot" shouldn't be needed. > > > + label = "BOOT"; > > + linux,code = <BTN_0>; > > + gpios = <&wkup_gpio0 0 GPIO_ACTIVE_LOW>; > > + }; > > + > > + sw_pwr: switch-2 { > > NIT (no need to actually change this), > Would "button"-1/2 be better here, I see on the silkscreen has them as "sw" > but most would call these buttons if they saw them. I do like the 'button' label much better, nothing uses these, so i removed the label name. > > > + label = "POWER"; > > + linux,code = <KEY_POWER>; > > + gpios = <&wkup_gpio0 4 GPIO_ACTIVE_LOW>; > > + }; > > + }; > > [...] > > > + > > +&main_sdhci2 { > > + /* Unused */ > > + status = "disabled"; > > +}; > > + > > For J7x I did not "disable by default" several classes of device > like this one, since the default pinmux may allow their function. > Once that is sorted out I'll fix up this DT here and in the spots > below for you. > > BTW, thanks for taking the time to rebase on my series for the > devices I did disable. Hope that didn't cause too much churn > on your side :) I love it! I prefer everything disabled, and just enable what nodes we need. > > + > > +&main_r5fss0_core0 { > > + firmware-name = "pdk-ipc/ipc_echo_test_mcu2_0_release_strip.xer5f"; > > +}; > > + > > What is this crazy firmware name? These are not in linux-firmware, might > be better to leave these out until we get these names sorted out and > upstreamed. (yes I know the same snuck into k3-j721e-sk.dtb but > it probably isn't correct there either). Yeap, direct copy from k3-j721e-sk, I'll remove it till we get everything sorted out.. > > > + > > +&mcu_cpsw { > > + pinctrl-names = "default"; > > + pinctrl-0 = <&mcu_cpsw_pins_default &mcu_mdio_pins_default>; > > mcu_mdio_pins_default should belong to the MDIO node below. > > > +}; > > + > > +&davinci_mdio { > > > Right here. > > pinctrl-names = "default"; > pinctrl-0 = <&mcu_mdio_pins_default>; and moved! > Everything else looks sane enough to me, > > Reviewed-by: Andrew Davis <afd@xxxxxx> Thanks! -- Robert Nelson https://rcn-ee.com/
