Re: [PATCH 2/5] arm64: dts: add description for solidrun am642 som and evaluation board

From: Josua Mayer
Date: Thu Jan 04 2024 - 06:03:46 EST

resend because HTML :( sorry ...

Am 04.01.24 um 08:54 schrieb Krzysztof Kozlowski:
> On 03/01/2024 12:27, Josua Mayer wrote:
>> Add description for the SolidRun AM642 SoM, and HummingBoard-T
>> evaluation board.
>>
>> The SoM features:
>> - 1x cpsw ethernet with phy
>> - 2x pru ethernet with phy
>> - eMMC
>> - spi flash (assembly option)
>>
>> Additionally microSD and usb-2.0 otg are included in the SoM
>> description as they are supported boot sources for the SOC boot-rom.
>>
>> The Carrier provides:
>> - 3x RJ45 connector
>> - 2x M.2 connector
>> - USB-2.0 Hub
>> - USB-A Connector
>> - LEDs
>> - 2x CAN transceiver
>> - 1x RS485 transceiver
>> - sensors
>>
>> The M.2 connectors support either USB-3.1 or PCI-E depending on status
>> of a mux. By default the mux is switched off.
>>
>> RFC: dtbs_check reports:
>>
>> - error in pru ethernet:
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: icssg1-eth: dmas: [[12, 49664, 15], [12, 49665, 15], [12, 49666, 15], [12, 49667, 15], [12, 49668, 15], [12, 49669, 15], [12, 49670, 15], [12, 49671, 15], [12, 16896, 15], [12, 16897, 15], [12, 16898, 0], [12, 16899, 0]] is too long
>> from schema $id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: icssg1-eth: Unevaluated properties are not allowed ('dmas' was unexpected)
>> from schema $id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
>>
>> It is caused by definint 12 dmas, when ti,icssg-prueth.yaml specifies a
>> maximum of 10. The pru ethernet on am64 mostly identical to am65 - see
>> e.g. arch/arm64/boot/dts/ti/k3-am654-idk.dtso which also defines 12 dma.
>>
>> At least for this revision I am skipping fixing the bindings, because
>> aside from raising the maximum to 12, dma-names also has just 10 entries
>> - and don't know which names would be correct to add.
>>
>> - undocumented compatible ti,bq25713 (battery charger)
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-pcie.dtb: /bus@f4000/i2c@20000000/battery-charger@6a: failed to match any schema with compatible: ['ti,bq25713']
>>
>> This specific charger has no linux support yet, I am not sure where
>> (or whether) to document the new compatible.
>> The reference could also be dropped completely, since the charger is
>> not assebled by default.
>>
>> - undocumented compatible for rtc: "abracon,abx80x"
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t-pcie.dtb: /bus@f4000/i2c@20010000/am1805aq@69: failed to match any schema with compatible: ['abracon,abx80x']
>>
>> It is actually documented in text format:
>> Documentation/devicetree/bindings/rtc/abracon,abx80x.txt
>>
>> - phy@0:cdns,phy-type:0:0: 0 is less than the minimum of 1
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: serdes@f000000: phy@0:cdns,phy-type:0:0: 0 is less than the minimum of 1
>> from schema $id: http://devicetree.org/schemas/phy/phy-cadence-torrent.yaml#
>>
>> I used value 0 here on purpose (phy.h: #define PHY_NONE 0), however
>> maybe better to choose a specific protocol?
>> Or better to update binding and allow 0?
>>
>> - interrupt properties not allowed for spi flash
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: flash@0: Unevaluated properties are not allowed ('interrupt-parent', 'interrupts' were unexpected)
>> from schema $id: http://devicetree.org/schemas/mtd/jedec,spi-nor.yaml#
>>
>> The assembled flash memory "sh28hs512t" definitely has an interrupt
>> pin wired to a cpu gpio. Should interrupts be added to spi flash
>> binding?
>>
>> - wrong names for pinctrl nodes
>>
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dtb: pinctrl@f4000: 'ethernet-phy-pins-default', 'ethernet-phy0-pins-default', 'ethernet-phy1-pins-default', 'ethernet-phy2-pins-default', 'leds-pins-default', 'main-i2c0-pins-default', 'main-i2c0-pins-int-default', 'main-i2c1-int-pins-default', 'main-i2c1-pins-default', 'main-mcan0-pins-default', 'main-mcan1-pins-default', 'main-mmc1-pins-default', 'main-uart0-pins-default', 'main-uart3-pins-default', 'mdio0-pins-default', 'ospi0-flash0-pins-default', 'ospi0-pins-default', 'pcie0-pins-default', 'pru-rgmii1-pins-default', 'pru-rgmii2-pins-default', 'pru1-mdio0-pins-default', 'regulator-pcie-3v3-pins-default', 'regulator-vpp-1v8-pins-default', 'rgmii1-pins-default', 'serdes-mux-pins-default', 'usb0-pins-default' do not match any of the regexes: '-pins(-[0-9]+)?$|-pin$', 'pinctrl-[0-9]+'
>>
>> Other TI DTSs consistently end with *-pins-default. Should a different
>> naming convention be used?
>>
>> Signed-off-by: Josua Mayer <josua@xxxxxxxxxxxxx>
>> ---
>> arch/arm64/boot/dts/ti/Makefile | 1 +
>> arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts | 333 +++++++++++
>> arch/arm64/boot/dts/ti/k3-am642-sr-som.dtsi | 638 +++++++++++++++++++++
>> 3 files changed, 972 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
>> index 77a347f9f47d..041c3b71155e 100644
>> --- a/arch/arm64/boot/dts/ti/Makefile
>> +++ b/arch/arm64/boot/dts/ti/Makefile
>> @@ -32,6 +32,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-am62p5-sk.dtb
>>
>> # Boards with AM64x SoC
>> dtb-$(CONFIG_ARCH_K3) += k3-am642-evm.dtb
>> +dtb-$(CONFIG_ARCH_K3) += k3-am642-hummingboard-t.dtb
>> dtb-$(CONFIG_ARCH_K3) += k3-am642-phyboard-electra-rdk.dtb
>> dtb-$(CONFIG_ARCH_K3) += k3-am642-sk.dtb
>> dtb-$(CONFIG_ARCH_K3) += k3-am642-tqma64xxl-mbax4xxl.dtb
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts
>> new file mode 100644
>> index 000000000000..f7b48ada8ef3
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-hummingboard-t.dts
>> @@ -0,0 +1,333 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2023 Josua Mayer <josua@xxxxxxxxxxxxx>
>> + *
>> + * DTS for SolidRun AM642 HummingBoard-T,
>> + * running on Cortex A53.
>> + *
>> + */
>> +
>> +/dts-v1/;
>> +
>> +#include <dt-bindings/phy/phy.h>
>> +
>> +#include "k3-am642.dtsi"
>> +#include "k3-am642-sr-som.dtsi"
>> +
>> +/ {
>> + model = "SolidRun AM642 HummingBoard-T";
>> + compatible = "solidrun,am642-hummingboard-t", "solidrun,am642-sr-som", "ti,am642";
>> +
>> + aliases {
>> + serial5 = &main_uart3;
>> + };
>> +
>> + leds {
>> + compatible = "gpio-leds";
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&leds_pins_default>;
>> + status = "okay";
> Where was it disabled?
Nowhere. Better to omit status on new nodes added by new dts file?
>> +
>> + /* D24 */
>> + led1: led-1 {
>> + label = "led1:green";
> Use function
This board has no default function defined by labels or enclosure.
Not sure what to pick for "function" property. Can I omit it and set only color?

If so - should I drop label completely - or keep the "led1"  part?

> and color instead.
Ack
>> + gpios = <&main_gpio0 29 GPIO_ACTIVE_HIGH>;
>> + };
>> +
> ...
>
>> +
>> +&main_i2c0 {
>> + pinctrl-0 = <&main_i2c0_pins_default>, <&main_i2c0_int_pins_default>;
>> +
>> + humidity-sensor@41 {
>> + compatible = "ti,hdc2010";
>> + reg = <0x41>;
>> + interrupt-parent = <&main_gpio0>;
>> + interrupts = <37 IRQ_TYPE_EDGE_FALLING>;
>> + status = "okay";
> Where was it disabled?
Nowhere.
>> + };
>> +
>> + light-sensor@44 {
>> + compatible = "ti,opt3001";
>> + reg = <0x44>;
>> + interrupt-parent = <&main_gpio0>;
>> + interrupts = <37 IRQ_TYPE_EDGE_FALLING>;
>> + status = "okay";
> Where was it disabled?
Nowhere.
>> + };
>> +
>> + battery-charger@6a {
> charger@
>
>> + compatible = "ti,bq25713";
>> + reg = <0x6a>;
>> + status = "okay";
> Where was it disabled?
Nowhere.
>> + };
>> +};
>> +
>> +&main_i2c1 {
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&main_i2c1_pins_default>, <&main_i2c1_int_pins_default>;
>> + status = "okay";
>> +
>> + rtc: am1805aq@69 {
> 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
Ack.
>> + compatible = "abracon,abx80x";
>> + reg = <0x69>;
>> + abracon,tc-diode = "schottky";
>> + abracon,tc-resistor = <3>;
>> + interrupt-parent = <&main_gpio0>;
>> + interrupts = <44 IRQ_TYPE_EDGE_FALLING>;
>> + status = "okay";
> Where was it disabled?
Nowhere.
>> + };
>> +};
>> +
> ...
>
>> +
>> +&serdes0 {
>> + /*
>> + * Serdes Signals are routed via mux to either m.2 connectors:
>> + * - M1: USB-3.1
>> + * - M2: PCI-E
>> + */
>> + status = "okay";
I set status here because I expect nodes inherited from other dtsi to be maybe disabled.
In this instance, k3-am64-main.dtsi however sets no status for "serdes0: serdes@f000000" node.
>> +
>> + serdes0_link: phy@0 {
>> + reg = <0>;
>> + cdns,num-lanes = <1>;
>> + #phy-cells = <0>;
>> + cdns,phy-type = <PHY_NONE>;
>> + resets = <&serdes_wiz0 1>;
>> + status = "okay";
> Where was it disabled?
Nowhere.
>> + };
>> +};
>> +
>> +&usb0 {
>> + dr_mode = "host";
>> +};
>> diff --git a/arch/arm64/boot/dts/ti/k3-am642-sr-som.dtsi b/arch/arm64/boot/dts/ti/k3-am642-sr-som.dtsi
>> new file mode 100644
>> index 000000000000..952a262d6874
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am642-sr-som.dtsi
>> @@ -0,0 +1,638 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (C) 2023 Josua Mayer <josua@xxxxxxxxxxxxx>
>> + *
>> + */
>> +
>> +#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";
>> + bootargs = "earlycon=ns16550a,mmio32,0x02800000";
> Drop bootargs. earlycon is for debugging.

Ack.

Sincerely Josua Mayer