On 12/01/2024 18:12, Josua Mayer wrote:
> Add description for the SolidRun AM642 SoM, and HummingBoard-T
> evaluation board.
>
...
> +&main_gpio0 {
> + m2-reset-hog {
> + gpio-hog;
> + gpios = <12 GPIO_ACTIVE_LOW>;
> + output-low; /* deasserted */
> + line-name = "m2-reset";
> + };
> +
> + m1-m2-w-disable1-hog {
> + gpio-hog;
> + gpios = <32 GPIO_ACTIVE_LOW>;
> + output-low; /* deasserted */
> + line-name = "m1-m2-pcie-w-disable1";
> + };
> +
> + m1-m2-w_disable2-hog {
No, underscores are not allowed.
...
> +
> +#include <dt-bindings/net/ti-dp83869.h>
> +
> +/ {
> + model = "SolidRun AM642 SoM";
> + compatible = "solidrun,am642-sr-som", "ti,am642";
> +
> + aliases {
> + ethernet0 = &cpsw_port1;
> + ethernet1 = &icssg1_emac0;
> + ethernet2 = &icssg1_emac1;
> + mmc0 = &sdhci0;
> + mmc1 = &sdhci1;
> + serial2 = &main_uart0;
> + };
> +
> + chosen {
> + /* SoC default UART console */
> + stdout-path = "serial2:115200n8";
> + };
> +
> + /* PRU Ethernet Controller */
> + icssg1_eth: icssg1-eth {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,am642-icssg-prueth";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pru_rgmii1_pins_default>, <&pru_rgmii2_pins_default>;
> +
> + sram = <&oc_sram>;
> + ti,prus = <&pru1_0>, <&rtu1_0>, <&tx_pru1_0>, <&pru1_1>, <&rtu1_1>, <&tx_pru1_1>;
> + firmware-name = "ti-pruss/am65x-sr2-pru0-prueth-fw.elf",
> + "ti-pruss/am65x-sr2-rtu0-prueth-fw.elf",
> + "ti-pruss/am65x-sr2-txpru0-prueth-fw.elf",
> + "ti-pruss/am65x-sr2-pru1-prueth-fw.elf",
> + "ti-pruss/am65x-sr2-rtu1-prueth-fw.elf",
> + "ti-pruss/am65x-sr2-txpru1-prueth-fw.elf";
> +
> + ti,pruss-gp-mux-sel = <2>, /* MII mode */
> + <2>,
> + <2>,
> + <2>, /* MII mode */
> + <2>,
> + <2>;
> +
> + ti,mii-g-rt = <&icssg1_mii_g_rt>;
> + ti,mii-rt = <&icssg1_mii_rt>;
> + ti,iep = <&icssg1_iep0>, <&icssg1_iep1>;
> +
> + interrupt-parent = <&icssg1_intc>;
> + interrupts = <24 0 2>, <25 1 3>;
None of these are typical interrupt constants/flags?
> + interrupt-names = "tx_ts0", "tx_ts1";
> +
> + dmas = <&main_pktdma 0xc200 15>, /* egress slice 0 */
> + <&main_pktdma 0xc201 15>, /* egress slice 0 */
> + <&main_pktdma 0xc202 15>, /* egress slice 0 */
> + <&main_pktdma 0xc203 15>, /* egress slice 0 */
> + <&main_pktdma 0xc204 15>, /* egress slice 1 */
> + <&main_pktdma 0xc205 15>, /* egress slice 1 */
> + <&main_pktdma 0xc206 15>, /* egress slice 1 */
> + <&main_pktdma 0xc207 15>, /* egress slice 1 */
> + <&main_pktdma 0x4200 15>, /* ingress slice 0 */
> + <&main_pktdma 0x4201 15>, /* ingress slice 1 */
> + <&main_pktdma 0x4202 0>, /* mgmnt rsp slice 0 */
> + <&main_pktdma 0x4203 0>; /* mgmnt rsp slice 1 */
> + dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
> + "tx1-0", "tx1-1", "tx1-2", "tx1-3",
> + "rx0", "rx1";
> +
> + status = "okay";
Drop. Didn't you get such comments before?
> +
> + ethernet-ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + icssg1_emac0: port@0 {
> + reg = <0>;
> + ti,syscon-rgmii-delay = <&main_conf 0x4110>;
> + /* Filled in by bootloader */
> + local-mac-address = [00 00 00 00 00 00];
> + phy-handle = <ðernet_phy2>;
> + phy-mode = "rgmii-id";
> + status = "okay";
?
> + };
> +
> + icssg1_emac1: port@1 {
> + reg = <1>;
> + ti,syscon-rgmii-delay = <&main_conf 0x4114>;
> + /* Filled in by bootloader */
> + local-mac-address = [00 00 00 00 00 00];
> + phy-handle = <ðernet_phy1>;
> + phy-mode = "rgmii-id";
> + status = "okay";
?
....
> + ethernet_phy0: ethernet-phy@0 {
> + compatible = "ethernet-phy-id2000.a0f1";
> + reg = <0>;
> + pinctrl-names = "default";
> + pinctrl-0 = <ðernet_phy0_pins_default>;
> + ti,clk-output-sel = <DP83869_CLK_O_SEL_REF_CLK>;
> + ti,op-mode = <DP83869_RGMII_COPPER_ETHERNET>;
> + /*
> + * Disable interrupts because ISR never clears 0x0040
> + *
> + * interrupt-parent = <&main_gpio1>;
> + * interrupts = <70 IRQ_TYPE_LEVEL_LOW>;
> + */
> + /*
> + * Disable HW Reset because clock signal is daisy-chained
> + *
> + * reset-gpios = <&main_gpio0 84 GPIO_ACTIVE_LOW>;
> + * reset-assert-us = <1>;
> + * reset-deassert-us = <30>;
> + */
> + status = "okay";
Drop, this applies everywhere where not needed. You have this in
multiple places...
Best regards,
Krzysztof
