Re: [PATCH 6/6] arm64: dts: cix: add initial CIX P1(SKY1) dts support

[Peter Chen <peter.chen@cixtech.com>]

On 25-02-22 06:46:51, Rob Herring wrote:

Hi Rob,

Thanks for your reviewing.

> 
> > +
> > +     pmu: pmu {
> > +             compatible = "arm,armv8-pmuv3";
> 
> Also needs the CPU model specific compatible string.

For CIX Sky1 SoC, it is Armv9 big-LITTLE architecture, if we
add two compatibles for both A720 and A520, there will be two
Arm PMU devices, could it work well for user tool like perf?

> 
> > +             interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> > +             interrupt-parent = <&gic>;
> > +             status = "okay";
> 
> okay is the default, don't need status.

Will change.

> 
> > +     };
> > +
> > +     pmu_spe: pmu_spe {
> > +             compatible = "arm,statistical-profiling-extension-v1";
> > +             interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_LOW>;
> > +             interrupt-parent = <&gic>;
> > +             status = "okay";
> > +     };
> > +
> > +     reserved-memory {
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             ranges;
> > +
> > +             linux,cma {
> > +                     compatible = "shared-dma-pool";
> > +                     reusable;
> > +                     size = <0x0 0x28000000>;
> > +                     linux,cma-default;
> > +             };
> > +
> > +     };
> > +
> > +     sky1_fixed_clocks: fixed-clocks {
> 
> Drop this container node.

Okay, I will delete above line.

> 
> > +             uartclk: uartclk {
> 
> clock-100000000 for the node name.

Will change.
> 
> > +                     compatible = "fixed-clock";
> > +                     #clock-cells = <0>;
> > +                     clock-frequency = <100000000>;
> > +                     clock-output-names = "uartclk";
> > +             };
> > +
> > +             uart_apb_pclk: uart_apb_pclk {
> 
> Similar here.
> 
> > +                     compatible = "fixed-clock";
> > +                     #clock-cells = <0>;
> > +                     clock-frequency = <200000000>;
> > +                     clock-output-names = "apb_pclk";
> > +             };
> > +     };
> > +
> > +     soc@0 {
> > +             compatible = "simple-bus";
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             ranges;
> > +             dma-ranges;
> > +
> > +             uart2: uart@040d0000 {
> 
> serial@...

Will change

> 
> > +                     compatible = "arm,pl011", "arm,primecell";
> > +                     reg = <0x0 0x040d0000 0x0 0x1000>;
> > +                     interrupts = <GIC_SPI 298 IRQ_TYPE_LEVEL_HIGH>;
> > +                     clock-names = "uartclk", "apb_pclk";
> > +                     clocks = <&uartclk>, <&uart_apb_pclk>;
> > +                     status = "disabled";
> > +             };
> > +
> > +             gic: interrupt-controller@0e001000 {
> > +                     compatible = "arm,gic-v3";
> > +                     #address-cells = <2>;
> > +                     #interrupt-cells = <3>;
> > +                     #size-cells = <2>;
> > +                     ranges;
> > +                     interrupt-controller;
> > +                     #redistributor-regions = <1>;
> > +                     reg = <0x0 0x0e010000 0 0x10000>,       /* GICD */
> > +                           <0x0 0x0e090000 0 0x300000>;       /* GICR * 12 */
> > +                     redistributor-stride = <0x40000>;
> > +                     interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_LOW>;
> > +                     interrupt-parent = <&gic>;
> > +
> > +                     its_pcie: its@e050000 {
> 
> msi-controller@...

Will change

-- 

Best regards,
Peter