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

[Josua Mayer]

Am 12.01.24 um 18:50 schrieb Andrew Davis:

> On 1/12/24 11:12 AM, Josua Mayer wrote:
>> +
>> +    /* charger@6a */
>
> ?
The particular chip bq25713 has no mainline support, no dt bindings, and I do not understand it well enough for defining reasonable bindings myself.

Since it is assembly option and not currently available I added just a marker.
Drop it instead?

>
>> +};
>> +
>> +&main_i2c1 {
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&main_i2c1_pins_default>, <&main_i2c1_int_pins_default>;
>
> The interrupt pin belongs to the rtc@69 and should be in the node below.
Ack.
I will rename the interrupt pinctrl node rtc_int_pins_default, accordingly.
>
>> +  
>> +
>> +&pcie0_rc {
>> +    pinctrl-names = "default";
>> +    pinctrl-0 = <&pcie0_pins_default>;
>> +    reset-gpios = <&main_gpio1 15 GPIO_ACTIVE_HIGH>;
>> +    phys = <&serdes0_link>;
>> +    phy-names = "pcie-phy";
>> +    num-lanes = <1>;
>> +    mux-controls = <&serdes_mux>;
>> +    mux-control-names = "serdes";
mux-controls was a downstream workaround to ensure setting the mux before pci probe (downstream patch on linux 5.10 pci driver).
I shall remove it until confirming whether it makes sense / is required upstream.
>> +    status = "disabled";
>
> This node is already default disabled in the parent .dtsi and
> has been for more than a year now, you might need to go recheck
> if these disables are needed. I see several below that also
> are not needed.
Ack.
Turns out very many nodes are without status in k3-am64-main.dtsi.
I went through all of them in som and carrier dts, left with explicit status are:

- cpsw3g_mdio
- cpsw_port2
- icssg1_mdio
- mailbox0_cluster2
- mailbox0_cluster4
- main_i2c0
- main_i2c1
- main_mcan0
- main_mcan1
- main_uart0
- main_uart3
- ospi0

>
>> +};
>> +
>> +&pcie0_ep {
>> +    phys = <&serdes0_link>;
>> +    phy-names = "pcie-phy";
>> +    num-lanes = <1>;
>> +    status = "disabled";
>> +};
>
> Delete this node, added it back if/when you plan to use it
> and only in the dt file that adds the rest of the needed
> properties.
Ack.
>> +
>> +    reserved-memory {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +        ranges;
>> +
>> +        secure_ddr: optee@9e800000 {
>> +            reg = <0x00 0x9e800000 0x00 0x01800000>; /* for OP-TEE */
>> +            alignment = <0x1000>;
>
> Alignment not needed for no-map nodes.
Ack.

Thanks! sincerely,
Josua Mayer