Re: [PATCH v2 2/9] arm: dts: ls1028a: move devices into /soc

On Wed, Sep 01, 2021 at 10:55:15AM +0200, Michael Walle wrote:

• fspi: flexspi@20c0000 {

• compatible = "nxp,lx2160a-fspi";
  

• #address-cells = <1>;
  

• #size-cells = <0>;
  

• reg = <0x0 0x20c0000 0x0 0x10000>,
  

•       <0x0 0x20000000 0x0 0x10000000>;
  

• reg-names = "fspi_base", "fspi_mmap";
  

• clocks = <&clockgen 4 3>, <&clockgen 4 3>;
  

• clock-names = "fspi_en", "fspi";
  

• interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
  

• status = "disabled";
  

• };
• serial0: serial@21c0500 {

• device_type = "serial";
  

• compatible = "fsl,ns16550", "ns16550a";
  

• reg = <0x0 0x21c0500 0x0 0x100>;
  

• interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
  

• status = "disabled";
  

• };
• serial1: serial@21c0600 {

• device_type = "serial";
  

• compatible = "fsl,ns16550", "ns16550a";
  

• reg = <0x0 0x21c0600 0x0 0x100>;
  

• interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
  

• status = "disabled";
  

• };
• pcie1: pcie@3400000 {

•       compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
  

•       reg = <0x00 0x03400000 0x0 0x80000
  

•        0x00 0x03480000 0x0 0x40000   /* lut registers */
  

•        0x00 0x034c0000 0x0 0x40000  /* pf controls registers */
  

•        0x80 0x00000000 0x0 0x20000>; /* configuration space */
  

•       reg-names = "dbi", "lut", "ctrl", "config";
  

•       #address-cells = <3>;
  

•       #size-cells = <2>;
  

•       device_type = "pci";
  

•       num-lanes = <4>;
  

•       bus-range = <0x0 0xff>;
  

•       ranges = <0x81000000 0x0 0x00000000 0x80 0x00020000 0x0 0x00010000   /* downstream I/O */
  

•        0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
  

• };

• soc: soc {

• compatible = "simple-bus";
  

• #address-cells = <2>;
  

• #size-cells = <2>;
  

• ranges;
  
  

• pcie2: pcie@3500000 {

•       compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
  

•       reg = <0x00 0x03500000 0x0 0x80000
  

•        0x00 0x03580000 0x0 0x40000   /* lut registers */
  

•        0x00 0x035c0000 0x0 0x40000  /* pf controls registers */
  

•        0x88 0x00000000 0x0 0x20000>; /* configuration space */
  

•       reg-names = "dbi", "lut", "ctrl", "config";
  

•       #address-cells = <3>;
  

•       #size-cells = <2>;
  

•       device_type = "pci";
  

•       num-lanes = <4>;
  

•       bus-range = <0x0 0xff>;
  

•       ranges = <0x81000000 0x0 0x00000000 0x88 0x00020000 0x0 0x00010000   /* downstream I/O */
  

•        0x82000000 0x0 0x40000000 0x88 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
  

• };

• clockgen: clocking@1300000 {
  

• 	compatible = "fsl,ls1028a-clockgen";
  

• 	reg = <0x0 0x1300000 0x0 0xa0000>;
  

• 	#clock-cells = <2>;
  

• 	clocks = <&sysclk>;
  

• };
  
  

• pcie@1f0000000 {

• compatible = "pci-host-ecam-generic";
  

• /* ECAM bus 0, HW has more space reserved but not populated */
  

• bus-range = <0x0 0x0>;
  

• reg = <0x01 0xf0000000 0x0 0x100000>;
  

• #address-cells = <3>;
  

• #size-cells = <2>;
  

• device_type = "pci";
  

• ranges= <0x82000000 0x0 0x00000000 0x1 0xf8000000 0x0 0x160000>;
  

• enetc0: pci@0,0 {
  

• 	reg = <0x000000 0 0 0 0>;
  

• fspi: flexspi@20c0000 {
  

• 	compatible = "nxp,lx2160a-fspi";
  

• 	#address-cells = <1>;
  

• 	#size-cells = <0>;
  

• 	reg = <0x0 0x20c0000 0x0 0x10000>,
  

• 		  <0x0 0x20000000 0x0 0x10000000>;
  

This is indented worse than where you took it from.

• 	reg-names = "fspi_base", "fspi_mmap";
  

• 	clocks = <&clockgen 4 3>, <&clockgen 4 3>;
  

• 	clock-names = "fspi_en", "fspi";
  

• 	interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
  status = "disabled";
  

  };

• enetc1: pci@0,1 {
  

• 	reg = <0x000100 0 0 0 0>;
  

• serial0: serial@21c0500 {
  

• 	device_type = "serial";
  

• 	compatible = "fsl,ns16550", "ns16550a";
  

• 	reg = <0x0 0x21c0500 0x0 0x100>;
  

• 	interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
  status = "disabled";
  

  };

• enetc2: pci@0,2 {
  

• 	reg = <0x000200 0 0 0 0>;
  

• 	status = "disabled";
  

• 	phy-mode = "internal";
  
  

• 	fixed-link {
  

• 		speed = <2500>;
  

• 		full-duplex;
  

• 	};
  

• };
  

• mdio0: pci@0,3 {
  

• 	#address-cells=<0>;
  

• 	#size-cells=<1>;
  

• 	reg = <0x000300 0 0 0 0>;
  

• serial1: serial@21c0600 {
  

• 	device_type = "serial";
  

• 	compatible = "fsl,ns16550", "ns16550a";
  

• 	reg = <0x0 0x21c0600 0x0 0x100>;
  

• 	interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>;
  status = "disabled";
  

• 	fixed-link {
  

• 		speed = <1000>;
  

• 		full-duplex;
  

• 	};
  

  };

• mscc_felix: pci@0,5 {
  

• 	reg = <0x000500 0 0 0 0>;
  

• 	status = "disabled";
  

• pcie1: pcie@3400000 {
  

• 	   compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
  

• 	   reg = <0x00 0x03400000 0x0 0x80000
  

• 		   0x00 0x03480000 0x0 0x40000   /* lut registers */
  

• 		   0x00 0x034c0000 0x0 0x40000  /* pf controls registers */
  

• 		   0x80 0x00000000 0x0 0x20000>; /* configuration space */
  

You could take the opportunity to reindent this badly indented thing right now.

• 	   reg-names = "dbi", "lut", "ctrl", "config";
  

• 	   #address-cells = <3>;
  

• 	   #size-cells = <2>;
  

• 	   device_type = "pci";
  

• 	   num-lanes = <4>;
  

• 	   bus-range = <0x0 0xff>;
  

• 	   ranges = <0x81000000 0x0 0x00000000 0x80 0x00020000 0x0 0x00010000   /* downstream I/O */
  

• 		   0x82000000 0x0 0x40000000 0x80 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
  

• };
  
  

• 	ports {
  

• 		#address-cells = <1>;
  

• 		#size-cells = <0>;
  

• pcie2: pcie@3500000 {
  

• 	   compatible = "fsl,ls-pcie", "fsl,ls1028-pcie", "snps,dw-pcie";
  

• 	   reg = <0x00 0x03500000 0x0 0x80000
  

• 		   0x00 0x03580000 0x0 0x40000   /* lut registers */
  

• 		   0x00 0x035c0000 0x0 0x40000  /* pf controls registers */
  

• 		   0x88 0x00000000 0x0 0x20000>; /* configuration space */
  

• 	   reg-names = "dbi", "lut", "ctrl", "config";
  

• 	   #address-cells = <3>;
  

• 	   #size-cells = <2>;
  

• 	   device_type = "pci";
  

• 	   num-lanes = <4>;
  

• 	   bus-range = <0x0 0xff>;
  

• 	   ranges = <0x81000000 0x0 0x00000000 0x88 0x00020000 0x0 0x00010000   /* downstream I/O */
  

• 		   0x82000000 0x0 0x40000000 0x88 0x40000000 0x0 0x40000000>; /* non-prefetchable memory */
  

• };
  
  

• 		mscc_felix_port0: port@0 {
  

• 			reg = <0>;
  

• 			status = "disabled";
  

• pcie@1f0000000 {
  

If you keep the nodes sorted by reg value, which is the norm anyway, I think there should be less churn, both in this patch as well as when you import the Linux device tree.

The ENETC PCIe ECAM is at an address higher than a lot of other peripherals, it shouldn't come immediately after the external PCIe controllers.

I won't comment on all the nodes that are not sorted by address, but you get the point.