Re: [PATCH v3 5/7] arm64: dts: axiado: Add initial support for AX3000 SoC and eval board

[Harshit Shah <hshah@axiado.com>]

On 6/23/2025 11:45 PM, Krzysztof Kozlowski wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On 23/06/2025 19:28, Harshit Shah wrote:
>> +     memory@0 {
>> +             device_type = "memory";
>> +             /* Cortex-A53 will use following memory map */
>> +             reg = <0x00000000 0x3D000000 0x00000000 0x23000000>,
> Lowercase hex, see DTS coding style.

I missed it. I will update it to lower case.


>
>> +     cpus {
>> +             #address-cells = <2>;
>> +             #size-cells = <0>;
>> +
>> +             cpu0: cpu@0 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a53";
>> +                     reg = <0x0 0x0>;
>> +                     enable-method = "spin-table";
>> +                     cpu-release-addr = <0x0 0x3c0013a0>;
>> +                     d-cache-size = <0x8000>;
>> +                     d-cache-line-size = <64>;
>> +                     d-cache-sets = <128>;
>> +                     i-cache-size = <0x8000>;
>> +                     i-cache-line-size = <64>;
>> +                     i-cache-sets = <256>;
>> +                     next-level-cache = <&l2>;
>> +             };
> Missing blank line between each new node. See DTS coding style.

Noted, I will update between each nodes.


>
>> +             cpu1: cpu@1 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a53";
>> +                     reg = <0x0 0x1>;
>> +                     enable-method = "spin-table";
>> +                     cpu-release-addr = <0x0 0x3c0013a0>;
>> +                     d-cache-size = <0x8000>;
>> +                     d-cache-line-size = <64>;
>> +                     d-cache-sets = <128>;
>> +                     i-cache-size = <0x8000>;
>> +                     i-cache-line-size = <64>;
>> +                     i-cache-sets = <256>;
>> +                     next-level-cache = <&l2>;
>> +             };
>> +             cpu2: cpu@2 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a53";
>> +                     reg = <0x0 0x2>;
>> +                     enable-method = "spin-table";
>> +                     cpu-release-addr = <0x0 0x3c0013a0>;
>> +                     d-cache-size = <0x8000>;
>> +                     d-cache-line-size = <64>;
>> +                     d-cache-sets = <128>;
>> +                     i-cache-size = <0x8000>;
>> +                     i-cache-line-size = <64>;
>> +                     i-cache-sets = <256>;
>> +                     next-level-cache = <&l2>;
>> +             };
>> +             cpu3: cpu@3 {
>> +                     device_type = "cpu";
>> +                     compatible = "arm,cortex-a53";
>> +                     reg = <0x0 0x3>;
>> +                     enable-method = "spin-table";
>> +                     cpu-release-addr = <0x0 0x3c0013a0>;
>> +                     d-cache-size = <0x8000>;
>> +                     d-cache-line-size = <64>;
>> +                     d-cache-sets = <128>;
>> +                     i-cache-size = <0x8000>;
>> +                     i-cache-line-size = <64>;
>> +                     i-cache-sets = <256>;
>> +                     next-level-cache = <&l2>;
>> +             };
>> +             l2: l2-cache0 {
>> +                     compatible = "cache";
>> +                     cache-size = <0x100000>;
>> +                     cache-unified;
>> +                     cache-line-size = <64>;
>> +                     cache-sets = <1024>;
>> +                     cache-level = <2>;
>> +             };
>> +     };
>> +
>> +     clocks {
>> +             clk_xin: clock-200000000 {
>> +                     compatible = "fixed-clock";
>> +                     #clock-cells = <0>;
>> +                     clock-frequency = <200000000>;
>> +                     clock-output-names = "clk_xin";
>> +             };
>> +             refclk: clock-125000000 {
>> +                     compatible = "fixed-clock";
>> +                     #clock-cells = <0>;
>> +                     clock-frequency = <125000000>;
>> +             };
>> +     };
>> +
>> +     soc {
>> +             compatible = "simple-bus";
>> +             ranges;
>> +             #address-cells = <2>;
>> +             #size-cells = <2>;
>> +             interrupt-parent = <&gic500>;
>> +
>> +             gic500: interrupt-controller@80300000 {
>> +                     compatible = "arm,gic-v3";
>> +                     reg = <0x00 0x80300000 0x00 0x10000>,
>> +                               <0x00 0x80380000 0x00 0x80000>;
> Does not look aligned.

Agreed. I will update the alignment.


>
>> +                     ranges;
>> +                     #interrupt-cells = <3>;
>> +                     #address-cells = <2>;
>> +                     #size-cells = <2>;
>> +                     interrupt-controller;
>> +                     #redistributor-regions = <1>;
>> +                     interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> +             };
>> +
>> +             /* GPIO Controller banks 0 - 7 */
>> +             gpio0: gpio-controller@80500000 {
>> +                     compatible = "cdns,gpio-r1p02";
>> +                     reg = <0x00 0x80500000 0x00  0x400>;
> Only one space, not double space.

Agreed. There is double space in every GPIO nodes, I will update the same.


Regards,

Harshit.