Re: [PATCH v4 23/29] pci: layerscape: add official ls1028a binding support

[Michael Walle]

Hi Zhiqiang,

thanks for looking at this patch.
Am 2021-10-13 03:46, schrieb Z.Q. Hou:
-----Original Message-----
From: Michael Walle <mich...@walle.cc>
Sent: 2021年10月5日 16:38
To: u-boot@lists.denx.de
Cc: Jagan Teki <ja...@amarulasolutions.com>; Priyanka Jain
<priyanka.j...@nxp.com>; Vladimir Oltean <vladimir.olt...@nxp.com>;
Tom Rini <tr...@konsulko.com>; Peter Griffin 
<peter.grif...@linaro.org>;
Manivannan Sadhasivam <manivannan.sadhasi...@linaro.org>; Michael
Walle <mich...@walle.cc>; Z.Q. Hou <zhiqiang....@nxp.com>
Subject: [PATCH v4 23/29] pci: layerscape: add official ls1028a 
binding
support

The official bindind of the PCIe controller of the ls1028a has the 
following
compatible string:
  compatible = "fsl,ls1028a-pcie";

Additionally, the resource names and count are different. Update the 
driver
to support this binding and change the entry in the ls1028a device 
tree.

Cc: Hou Zhiqiang <zhiqiang....@nxp.com>
Signed-off-by: Michael Walle <mich...@walle.cc>
---
 arch/arm/dts/fsl-ls1028a.dtsi    | 20 +++++------
 drivers/pci/pcie_layerscape_rc.c | 61 
+++++++++++++++++++++++---------
 2 files changed, 53 insertions(+), 28 deletions(-)

diff --git a/arch/arm/dts/fsl-ls1028a.dtsi 
b/arch/arm/dts/fsl-ls1028a.dtsi
index cc055e65e5..435b965d00 100644
--- a/arch/arm/dts/fsl-ls1028a.dtsi
+++ b/arch/arm/dts/fsl-ls1028a.dtsi
@@ -344,12 +344,10 @@
                };

                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";
+                       compatible = "fsl,ls1028a-pcie";
+                       reg = <0x00 0x03400000 0x0 0x00100000>, /* controller
registers */
+                             <0x80 0x00000000 0x0 0x00002000>; /* configuration
space */
+                       reg-names = "regs", "config";
                        #address-cells = <3>;
                        #size-cells = <2>;
                        device_type = "pci";
@@ -360,12 +358,10 @@
                };

                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";
+                       compatible = "fsl,ls1028a-pcie";
+                       reg = <0x00 0x03500000 0x0 0x00100000>, /* controller
registers */
+                             <0x88 0x00000000 0x0 0x00002000>; /* configuration
space */
+                       reg-names = "regs", "config";
                        #address-cells = <3>;
                        #size-cells = <2>;
                        device_type = "pci";
diff --git a/drivers/pci/pcie_layerscape_rc.c
b/drivers/pci/pcie_layerscape_rc.c
index f50d6ef653..217b420076 100644
--- a/drivers/pci/pcie_layerscape_rc.c
+++ b/drivers/pci/pcie_layerscape_rc.c
@@ -21,6 +21,12 @@

 DECLARE_GLOBAL_DATA_PTR;

+struct ls_pcie_drvdata {
+       u32 lut_offset;
+       u32 ctrl_offset;
+       bool big_endian;

The endianness property is better only put in the DT nodes.

The whole point of this series is to align the u-boot binding with
the (official) found in linux. There is no endianness property in
the binding:
  
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/snps%2Cdw-pcie.yaml
  
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pci/layerscape-pci.txt

Also from a technical point of view, this property seems redundant
with the compatible string which names a specific CPU.

-michael

The others looks good for me.

Thanks,
Zhiqiang