Re: [PATCH 3/4] arm64: dts: ti: Introduce AM62A7 family of SoCs

From: Vignesh Raghavendra
Date: Thu Sep 01 2022 - 01:40:09 EST




On 30/08/22 15:26, Krzysztof Kozlowski wrote:
> On 29/08/2022 11:21, Vignesh Raghavendra wrote:
>
> (...)
>
>> + /*
>> + * vcpumntirq:
>> + * virtual CPU interface maintenance interrupt
>> + */
>> + interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> + gic_its: msi-controller@1820000 {
>> + compatible = "arm,gic-v3-its";
>> + reg = <0x00 0x01820000 0x00 0x10000>;
>> + socionext,synquacer-pre-its = <0x1000000 0x400000>;
>> + msi-controller;
>> + #msi-cells = <1>;
>> + };
>> + };
>> +
>> + main_conf: syscon@100000 {
>> + compatible = "syscon", "simple-mfd";
>
> No, these are not allowed alone.

Will add device specific compatible

compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";

>
>> + reg = <0x00 0x00100000 0x00 0x20000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x00 0x00 0x00100000 0x20000>;
>> + };
>> +
>> + dmss: bus@48000000 {
>> + compatible = "simple-mfd";
>
> No. Not allowed alone.

This can be "simple-bus". Will change accordingly

>
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + dma-ranges;
>> + ranges = <0x00 0x48000000 0x00 0x48000000 0x00 0x06000000>;
>> +
>> + ti,sci-dev-id = <25>;
>> +
>> + secure_proxy_main: mailbox@4d000000 {
>> + compatible = "ti,am654-secure-proxy";
>> + #mbox-cells = <1>;
>> + reg-names = "target_data", "rt", "scfg";
>> + reg = <0x00 0x4d000000 0x00 0x80000>,
>> + <0x00 0x4a600000 0x00 0x80000>,
>> + <0x00 0x4a400000 0x00 0x80000>;
>> + interrupt-names = "rx_012";
>> + interrupts = <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> + };
>> +
>> + dmsc: system-controller@44043000 {
>> + compatible = "ti,k2g-sci";
>> + ti,host-id = <12>;
>> + mbox-names = "rx", "tx";
>> + mboxes= <&secure_proxy_main 12>,
>> + <&secure_proxy_main 13>;
>> + reg-names = "debug_messages";
>> + reg = <0x00 0x44043000 0x00 0xfe0>;
>
> First compatible, then reg, then the reset of properties. This applies
> everywhere.

Will fix

>
>> +
>> + k3_pds: power-controller {
>> + compatible = "ti,sci-pm-domain";
>> + #power-domain-cells = <2>;
>> + };
>> +
>> + k3_clks: clock-controller {
>> + compatible = "ti,k2g-sci-clk";
>> + #clock-cells = <2>;
>> + };
>> +
>> + k3_reset: reset-controller {
>> + compatible = "ti,sci-reset";
>> + #reset-cells = <2>;
>> + };
>> + };
>> +
[...]
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
>> new file mode 100644
>> index 000000000000..fe6d682a0f33
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a-wakeup.dtsi
>> @@ -0,0 +1,54 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Device Tree Source for AM62A SoC Family Wakeup Domain peripherals
>> + *
>> + * Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
>> + */
>> +
>> +&cbass_wakeup {
>> + wkup_conf: syscon@43000000 {
>> + compatible = "syscon", "simple-mfd";
>
> No. Not allowed alone.

Will change to

compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";

>
>> + reg = <0x00 0x43000000 0x00 0x20000>;
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + ranges = <0x00 0x00 0x43000000 0x20000>;
>> +
>> + chipid: chipid@14 {
>> + compatible = "ti,am654-chipid";
>> + reg = <0x14 0x4>;
>> + };
>> + };
>> +
>
>

Thanks for the review!
--
Regards
Vignesh